diff --git a/CHANGELOG.md b/CHANGELOG.md index 760c3671474..908ef20f5c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 7.8-SNAPSHOT #### Bugs +* Fix #7946: (kubernetes-client) encode API query values built from user-supplied patch, log, and OpenShift binary-build options so `&` and `=` stay inside the intended parameter instead of smuggling extra Kubernetes API options * Fix #7953: (httpclient-jdk) bodyless requests now preserve the requested HTTP method instead of silently defaulting to `GET`. `JdkHttpClientImpl.requestBuilder` only called `HttpRequest.Builder.method(...)` inside the `body != null` branch, so a bodyless `DELETE`/`POST`/`PUT`/`PATCH` (such as `client.raw(uri, "DELETE", null)`) was sent as `GET` on the JDK backend; the method is now set with `BodyPublishers.noBody()` when there is no body, matching the OkHttp, Jetty and Vert.x backends * Fix #7435: (kubernetes-client) A `SharedIndexInformer`'s periodic resync no longer stops permanently and silently when a single resync cycle throws. `DefaultSharedIndexInformer.scheduleResync` runs the resync through `Utils.scheduleAtFixedRate`, whose self-rescheduling chain re-arms the next cycle only when the previous one completes normally; an uncaught exception completed the (unobserved) `resyncFuture` exceptionally and the resync was never scheduled again, with no log, while the independent watch kept `isWatching()` reporting `true` (a restart was required to recover). The resync command now catches and `WARN`-logs the failure so the schedule fires again at the next interval * Fix #7933: (kubernetes-client-api) Deterministic TLS trust failures (untrusted cert, expired cert, hostname mismatch) are now classified as terminal and fail fast instead of being retried by the shared `StandardHttpClient.shouldRetry` backoff loop (~19 s drain). The classifier walks both `getCause()` and `getSuppressed()` trees for `CertificateException`, `CertPathValidatorException`, `CertPathBuilderException`, and `SSLPeerUnverifiedException`. Affects all five HTTP client modules (jdk, jetty, okhttp, vertx-4, vertx-5) on both the HTTP request and WebSocket connect paths diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java index 8ebfd00a19c..a037d23bbdf 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java @@ -61,7 +61,6 @@ public class OperationSupport { - private static final String FIELD_MANAGER_PARAM = "?fieldManager="; public static final String JSON = "application/json"; public static final String JSON_PATCH = "application/json-patch+json"; public static final String STRATEGIC_MERGE_JSON_PATCH = "application/strategic-merge-patch+json"; @@ -197,30 +196,24 @@ public URL getResourceUrl() throws MalformedURLException { } public URL getResourceURLForWriteOperation(URL resourceURL) throws MalformedURLException { - if (dryRun) { - resourceURL = new URL(URLUtils.join(resourceURL.toString(), "?dryRun=All")); - } + URLUtils.URLBuilder urlBuilder = new URLUtils.URLBuilder(resourceURL); if (context.fieldValidation != null) { - resourceURL = new URL( - URLUtils.join(resourceURL.toString(), "?fieldValidation=" + context.fieldValidation.parameterValue())); + urlBuilder.addQueryParameter("fieldValidation", context.fieldValidation.parameterValue()); + } + if (dryRun) { + urlBuilder.addQueryParameter("dryRun", "All"); } - return resourceURL; + return urlBuilder.build(); } public URL getResourceURLForPatchOperation(URL resourceUrl, PatchContext patchContext) throws MalformedURLException { if (patchContext != null) { - String url = resourceUrl.toString(); + URLUtils.URLBuilder urlBuilder = new URLUtils.URLBuilder(resourceUrl); Boolean forceConflicts = patchContext.getForce(); if (forceConflicts == null) { forceConflicts = this.context.forceConflicts; } - if (forceConflicts != null) { - url = URLUtils.join(url, "?force=" + forceConflicts); - } - if ((patchContext.getDryRun() != null && !patchContext.getDryRun().isEmpty()) || dryRun) { - url = URLUtils.join(url, "?dryRun=All"); - } String fieldManager = patchContext.getFieldManager(); if (fieldManager == null) { fieldManager = this.context.fieldManager; @@ -228,17 +221,23 @@ public URL getResourceURLForPatchOperation(URL resourceUrl, PatchContext patchCo if (fieldManager == null && patchContext.getPatchType() == PatchType.SERVER_SIDE_APPLY) { fieldManager = "fabric8"; } - if (fieldManager != null) { - url = URLUtils.join(url, FIELD_MANAGER_PARAM + fieldManager); - } String fieldValidation = patchContext.getFieldValidation(); if (fieldValidation == null && this.context.fieldValidation != null) { fieldValidation = this.context.fieldValidation.parameterValue(); } if (fieldValidation != null) { - url = URLUtils.join(url, "?fieldValidation=" + fieldValidation); + urlBuilder.addQueryParameter("fieldValidation", fieldValidation); + } + if (fieldManager != null) { + urlBuilder.addQueryParameter("fieldManager", fieldManager); + } + if ((patchContext.getDryRun() != null && !patchContext.getDryRun().isEmpty()) || dryRun) { + urlBuilder.addQueryParameter("dryRun", "All"); + } + if (forceConflicts != null) { + urlBuilder.addQueryParameter("force", forceConflicts.toString()); } - return new URL(url); + return urlBuilder.build(); } return resourceUrl; } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java index 78b7c362f04..d9c6c3ef96e 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContext.java @@ -132,31 +132,38 @@ public PodOperationContext withReadyWaitTimeout(Integer readyWaitTimeout) { } public String getLogParameters() { - StringBuilder sb = new StringBuilder(); - sb.append("log?pretty=").append(prettyOutput); + URLBuilder urlBuilder = new URLBuilder("log"); + urlBuilder.addQueryParameter("pretty", Boolean.toString(prettyOutput)); if (containerId != null && !containerId.isEmpty()) { - sb.append("&container=").append(containerId); + urlBuilder.addQueryParameter("container", containerId); } if (terminatedStatus) { - sb.append("&previous=true"); + urlBuilder.addQueryParameter("previous", "true"); } if (sinceSeconds != null) { - sb.append("&sinceSeconds=").append(sinceSeconds); + urlBuilder.addQueryParameter("sinceSeconds", sinceSeconds.toString()); } else if (sinceTimestamp != null) { // https://github.com/fabric8io/kubernetes-client/issues/6459 - sb.append("&sinceTime=").append(URLUtils.encodeToUTF(sinceTimestamp).replace("%3A", ":")); + urlBuilder.addQueryParameter("sinceTime", sinceTimestamp); } if (tailingLines != null) { - sb.append("&tailLines=").append(tailingLines); + urlBuilder.addQueryParameter("tailLines", tailingLines.toString()); } if (limitBytes != null) { - sb.append("&limitBytes=").append(limitBytes); + urlBuilder.addQueryParameter("limitBytes", limitBytes.toString()); } if (timestamps) { - sb.append("×tamps=true"); + urlBuilder.addQueryParameter("timestamps", "true"); } - return sb.toString(); + String logParameters = urlBuilder.toString(); + if (sinceTimestamp != null) { + String encodedSinceTime = URLUtils.encodeToUTF(sinceTimestamp).replace("+", "%20"); + logParameters = logParameters.replace( + "sinceTime=" + encodedSinceTime, + "sinceTime=" + encodedSinceTime.replace("%3A", ":")); + } + return logParameters; } public void addQueryParameters(URLBuilder httpUrlBuilder) { diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContextTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContextTest.java new file mode 100644 index 00000000000..d417b99dfa7 --- /dev/null +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/PodOperationContextTest.java @@ -0,0 +1,33 @@ +/* + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.kubernetes.client.dsl.internal; + +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +class PodOperationContextTest { + + @Test + void getLogParametersEncodesContainerValue() { + String query = new PodOperationContext() + .withContainerId("app&previous=true") + .getLogParameters(); + + assertThat(query) + .isEqualTo("log?pretty=false&container=app%26previous%3Dtrue"); + } +} diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/impl/PatchTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/impl/PatchTest.java index 6913c11bc38..8b538170739 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/impl/PatchTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/impl/PatchTest.java @@ -195,6 +195,22 @@ void testServerSideApplyWithPatchOptions() { PatchType.SERVER_SIDE_APPLY.getContentType()); } + @Test + void testServerSideApplyEncodesFieldManager() { + // Given + + // When + kubernetesClient.pods().inNamespace("ns1") + .resource(new PodBuilder().withNewMetadata().withName("pod1").endMetadata().build()) + .fieldManager("tenant&force=false").forceConflicts().serverSideApply(); + + // Then + verify(mockClient, times(1)).sendAsync(any(), any()); + assertRequest(0, "PATCH", "/api/v1/namespaces/ns1/pods/pod1", + "fieldManager=tenant%26force%3Dfalse&force=true", + PatchType.SERVER_SIDE_APPLY.getContentType()); + } + @Test void testResourceListServerSideApply() { // Given diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/JobTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/JobTest.java index 7273ef3a583..0e64a83dfa9 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/JobTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/JobTest.java @@ -338,6 +338,36 @@ void testJobGetLogMultiContainer() { assertEquals("hello", log); } + @Test + @DisplayName("Should encode container while getting logs for a multi-container job") + void testJobGetLogEncodesContainer() { + // Given + Pod jobPod = createJobPod(); + + server.expect().get().withPath("/apis/batch/v1/namespaces/ns1/jobs/job1") + .andReturn(HttpURLConnection.HTTP_OK, createJobBuilder().build()) + .always(); + + server.expect().get() + .withPath("/api/v1/namespaces/ns1/pods?labelSelector=controller-uid%3D3Dc4c8746c-94fd-47a7-ac01-11047c0323b4") + .andReturn(HttpURLConnection.HTTP_OK, new PodListBuilder().withItems(jobPod).build()) + .once(); + server.expect().get() + .withPath("/api/v1/namespaces/ns1/pods/job1-hk9nf/log?pretty=false" + + "&container=c1%26previous%3Dtrue%26tailLines%3D100000") + .andReturn(HttpURLConnection.HTTP_OK, "hello") + .once(); + + // When + String log = client.batch().v1().jobs().inNamespace("ns1").withName("job1") + .inContainer("c1&previous=true&tailLines=100000") + .getLog(); + + // Then + assertNotNull(log); + assertEquals("hello", log); + } + private Pod createJobPod() { return new PodBuilder() .withNewMetadata() diff --git a/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/BuildConfigTest.java b/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/BuildConfigTest.java index a0c2818d2ee..020bdebb543 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/BuildConfigTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/openshift/client/server/mock/BuildConfigTest.java @@ -141,12 +141,12 @@ void testBinaryBuildFromInputStream() { @Test void testBinaryBuildFromFile() throws IOException { - File warFile = new File("target/test.war"); + File warFile = new File("target/test.war&commit=owned"); warFile.createNewFile(); server.expect().post() - .withPath("/apis/build.openshift.io/v1/namespaces/ns1/buildconfigs/bc2/instantiatebinary?name=bc2&namespace=ns1&asFile=" - + warFile.getName()) + .withPath("/apis/build.openshift.io/v1/namespaces/ns1/buildconfigs/bc2/instantiatebinary" + + "?name=bc2&namespace=ns1&asFile=test.war%26commit%3Downed") .andReturn(201, new BuildBuilder() .withNewMetadata().withName("bc2").endMetadata().build()) .once(); diff --git a/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java b/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java index 29747c3b8a4..9109bd512cb 100644 --- a/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java +++ b/openshift-client/src/main/java/io/fabric8/openshift/client/dsl/internal/build/BuildConfigOperationsImpl.java @@ -165,44 +165,44 @@ public Build fromFile(final File file) { } private String getQueryParameters() throws MalformedURLException { - StringBuilder sb = new StringBuilder(); - sb.append(URLUtils.join(getResourceUrl().toString(), "instantiatebinary")); + URLUtils.URLBuilder urlBuilder = new URLUtils.URLBuilder( + URLUtils.join(getResourceUrl().toString(), "instantiatebinary")); if (Utils.isNotNullOrEmpty(name)) { - sb.append("?name=").append(name); + urlBuilder.addQueryParameter("name", name); } if (Utils.isNotNullOrEmpty(namespace)) { - sb.append("&namespace=").append(namespace); + urlBuilder.addQueryParameter("namespace", namespace); } if (Utils.isNotNullOrEmpty(message)) { - sb.append("&commit=").append(message); + urlBuilder.addQueryParameter("commit", message); } if (!Utils.isNullOrEmpty(authorName)) { - sb.append("&revision.authorName=").append(authorName); + urlBuilder.addQueryParameter("revision.authorName", authorName); } if (!Utils.isNullOrEmpty(authorEmail)) { - sb.append("&revision.authorEmail=").append(authorEmail); + urlBuilder.addQueryParameter("revision.authorEmail", authorEmail); } if (!Utils.isNullOrEmpty(committerName)) { - sb.append("&revision.committerName=").append(committerName); + urlBuilder.addQueryParameter("revision.committerName", committerName); } if (!Utils.isNullOrEmpty(committerEmail)) { - sb.append("&revision.committerEmail=").append(committerEmail); + urlBuilder.addQueryParameter("revision.committerEmail", committerEmail); } if (!Utils.isNullOrEmpty(commit)) { - sb.append("&revision.commit=").append(commit); + urlBuilder.addQueryParameter("revision.commit", commit); } if (!Utils.isNullOrEmpty(asFile)) { - sb.append("&asFile=").append(asFile); + urlBuilder.addQueryParameter("asFile", asFile); } - return sb.toString(); + return urlBuilder.toString(); } @Override