diff --git a/CHANGELOG.md b/CHANGELOG.md index 610f51321cf..c1a0ebbafdb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 5.2-SNAPSHOT #### Bugs +* Fix #2802: NullPointerException in HasMetadataOperation patch/replace when using KubernetesMockServer * Fix #2828: Remove automatic instantiation of CustomResource spec and status as this feature was causing more issues than it was solving * Fix #2857: Fix the log of an unexpected error from an Informer's EventHandler * Fix #2853: Cannot change the type of the Service from ClusterIP to ExternalName with PATCH diff --git a/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesAttributesExtractor.java b/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesAttributesExtractor.java index 9af6c4ae576..dcc167ad29c 100644 --- a/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesAttributesExtractor.java +++ b/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesAttributesExtractor.java @@ -266,7 +266,7 @@ private static Attribute parseLabel(String label) { return null; } - private static HasMetadata toKubernetesResource(String s) { + static HasMetadata toKubernetesResource(String s) { try (InputStream stream = new ByteArrayInputStream(s.getBytes(StandardCharsets.UTF_8.name()))) { return Serialization.unmarshal(stream); } catch (Exception e) { diff --git a/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesCrudDispatcher.java b/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesCrudDispatcher.java index 645cbd3cbcf..74804b58949 100644 --- a/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesCrudDispatcher.java +++ b/kubernetes-server-mock/src/main/java/io/fabric8/kubernetes/client/server/mock/KubernetesCrudDispatcher.java @@ -17,8 +17,14 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.Status; +import io.fabric8.kubernetes.api.model.StatusBuilder; +import io.fabric8.kubernetes.api.model.StatusCause; +import io.fabric8.kubernetes.api.model.StatusCauseBuilder; import io.fabric8.kubernetes.client.dsl.base.CustomResourceDefinitionContext; import io.fabric8.kubernetes.client.utils.Serialization; +import io.fabric8.kubernetes.client.utils.Utils; import io.fabric8.mockwebserver.Context; import io.fabric8.mockwebserver.crud.Attribute; import io.fabric8.mockwebserver.crud.AttributeSet; @@ -27,6 +33,7 @@ import io.fabric8.zjsonpatch.JsonPatch; import okhttp3.mockwebserver.MockResponse; +import java.io.IOException; import java.net.HttpURLConnection; import java.net.URI; import java.net.URISyntaxException; @@ -36,12 +43,15 @@ import java.util.Set; import java.util.concurrent.CopyOnWriteArraySet; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; import okhttp3.mockwebserver.RecordedRequest; import okhttp3.mockwebserver.SocketPolicy; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static io.fabric8.kubernetes.client.server.mock.KubernetesAttributesExtractor.toKubernetesResource; + public class KubernetesCrudDispatcher extends CrudDispatcher { private static final String POST = "POST"; @@ -51,6 +61,7 @@ public class KubernetesCrudDispatcher extends CrudDispatcher { private static final String DELETE = "DELETE"; private static final Logger LOGGER = LoggerFactory.getLogger(KubernetesCrudDispatcher.class); + public static final int HTTP_UNPROCESSABLE_ENTITY = 422; private final Set watchEventListeners = new CopyOnWriteArraySet<>(); public KubernetesCrudDispatcher() { @@ -94,7 +105,7 @@ public synchronized MockResponse dispatch(RecordedRequest request) { */ @Override public MockResponse handleCreate(String path, String s) { - return new MockResponse().setResponseCode(doCreate(path, s, "ADDED")).setBody(s); + return validateRequestBodyAndHandleRequest(s, () -> new MockResponse().setResponseCode(doCreate(path, s, "ADDED")).setBody(s)); } /** @@ -107,7 +118,7 @@ public MockResponse handleReplace(String path, String s) { if (doDelete(path, null) == 404) { return new MockResponse().setResponseCode(404); } - return new MockResponse().setResponseCode(doCreate(path, s, "MODIFIED")).setBody(s); + return validateRequestBodyAndHandleRequest(s, () -> new MockResponse().setResponseCode(doCreate(path, s, "MODIFIED")).setBody(s)); } /** @@ -316,4 +327,61 @@ private int doCreate(String path, String s, String event) { } return HttpURLConnection.HTTP_OK; } + + private MockResponse validateRequestBodyAndHandleRequest(String s, Supplier mockResponseSupplier) { + HasMetadata h = toKubernetesResource(s); + if (h != null) { + try { + validateResource(h); + } catch (IllegalArgumentException illegalArgumentException) { + return getUnprocessableEntityMockResponse(s, h, illegalArgumentException); + } + } + return mockResponseSupplier.get(); + } + + private MockResponse getUnprocessableEntityMockResponse(String s, HasMetadata h, IllegalArgumentException illegalArgumentException) { + String statusBody = getStatusBody(h, HTTP_UNPROCESSABLE_ENTITY, illegalArgumentException); + if (statusBody == null) { + statusBody = s; + } + return new MockResponse().setResponseCode(HTTP_UNPROCESSABLE_ENTITY).setBody(statusBody); + } + + private String getStatusBody(HasMetadata h, int code, IllegalArgumentException illegalArgumentException) { + String kind = Utils.getNonNullOrElse(h.getKind(), "Unknown"); + Status status = new StatusBuilder().withStatus("Failure") + .withReason("Invalid") + .withMessage(kind + " is invalid") + .withNewDetails() + .withKind(h.getKind()) + .withCauses(getFailureStatusCause(illegalArgumentException)) + .endDetails() + .withCode(code) + .build(); + try { + return Serialization.jsonMapper().writeValueAsString(status); + } catch (IOException ioException) { + return null; + } + } + + private StatusCause getFailureStatusCause(IllegalArgumentException illegalArgumentException) { + return new StatusCauseBuilder() + .withMessage(illegalArgumentException.getMessage()) + .withReason("ValueRequired") + .build(); + } + + private void validateResource(HasMetadata item) { + if (item == null) { + throw new IllegalArgumentException("No item provided"); + } + if (item.getMetadata() == null) { + throw new IllegalArgumentException("Required value: metadata is required"); + } + if (Utils.isNullOrEmpty(item.getMetadata().getName()) && Utils.isNullOrEmpty(item.getMetadata().getGenerateName())) { + throw new IllegalArgumentException("Required value: name or generateName is required"); + } + } } diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/SecretCrudTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/SecretCrudTest.java index c3ec51e4492..d82e15cc734 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/SecretCrudTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/SecretCrudTest.java @@ -20,6 +20,9 @@ import io.fabric8.kubernetes.api.model.SecretBuilder; import io.fabric8.kubernetes.api.model.SecretList; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; +import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.server.mock.KubernetesServer; import org.junit.Rule; import org.junit.jupiter.api.Test; @@ -27,6 +30,8 @@ import java.util.Collections; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -79,6 +84,24 @@ public void testCrud() { secret2 = client.secrets().inNamespace("ns2").withName("secret2").edit(s -> new SecretBuilder(s).removeFromData("one").build()); assertNotNull(secret2); - assertEquals(null, secret2.getData()); + assertNull(secret2.getData()); + } + + @Test + void testSecretWithNoMetadataCreateFails() { + // Given + Secret invalidSecret = new SecretBuilder() + .addToData("key.json", "{\"foo\":\"bar\"}") + .build(); + KubernetesClient client = server.getClient(); + + // When + Then + NonNamespaceOperation> secretOp = client.secrets().inNamespace("foo"); + KubernetesClientException kubernetesClientException = assertThrows(KubernetesClientException.class, () -> secretOp.create(invalidSecret)); + assertEquals(422, kubernetesClientException.getCode()); + assertEquals("Secret is invalid", kubernetesClientException.getStatus().getMessage()); + assertEquals(1, kubernetesClientException.getStatus().getDetails().getCauses().size()); + assertEquals("ValueRequired", kubernetesClientException.getStatus().getDetails().getCauses().get(0).getReason()); + assertEquals("Required value: metadata is required", kubernetesClientException.getStatus().getDetails().getCauses().get(0).getMessage()); } }