From 07f77b1d3a5f5ee54cb71faa412e95d503999c89 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Wed, 7 Apr 2021 10:17:49 +0100 Subject: [PATCH] Edit a CustomResource should result in a patch (#2923) * CustomResource edit as PATCH * fix the test * patchutils object mapper --- .../RawCustomResourceOperationsImpl.java | 51 +++++++++++-------- .../RawCustomResourceOperationsImplTest.java | 20 ++++++-- .../client/mock/CustomResourceTest.java | 5 +- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImpl.java index 4f0b9a75d9e..c640389727a 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImpl.java @@ -26,6 +26,7 @@ import java.util.concurrent.TimeUnit; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import io.fabric8.kubernetes.api.model.DeleteOptions; @@ -48,11 +49,12 @@ import io.fabric8.kubernetes.client.dsl.Namespaceable; import io.fabric8.kubernetes.client.dsl.base.CustomResourceDefinitionContext; import io.fabric8.kubernetes.client.dsl.base.OperationSupport; +import io.fabric8.kubernetes.client.internal.PatchUtils; import io.fabric8.kubernetes.client.utils.HttpClientUtils; import io.fabric8.kubernetes.client.utils.IOHelpers; -import io.fabric8.kubernetes.client.utils.Serialization; import io.fabric8.kubernetes.client.utils.Utils; import io.fabric8.kubernetes.client.utils.WatcherToggle; +import io.fabric8.zjsonpatch.JsonDiff; import okhttp3.HttpUrl; import okhttp3.MediaType; import okhttp3.OkHttpClient; @@ -88,12 +90,12 @@ public class RawCustomResourceOperationsImpl extends OperationSupport implements private final String deletionPropagation; private final boolean cascading; - private enum HttpCallMethod { GET, POST, PUT, DELETE } + private enum HttpCallMethod { GET, POST, PUT, PATCH, DELETE } private RawCustomResourceOperationsImpl(OkHttpClient client, Config config, CustomResourceDefinitionContext crdContext, String namespace, String name, long gracePeriodInSeconds, boolean cascading, String deletionPropagation, ListOptions listOptions, boolean dryRun) { super(client, config); this.customResourceDefinition = crdContext; - this.objectMapper = Serialization.jsonMapper(); + this.objectMapper = PatchUtils.patchMapper(); this.namespace = namespace; this.name = name; this.gracePeriodInSeconds = gracePeriodInSeconds; @@ -298,6 +300,18 @@ public Map createOrReplace(String namespace, InputStream objectA return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, null, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).createOrReplace(objectAsStream); } + private Map replace(String namespace, String name, Map object) throws IOException { + return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).replace(object); + } + + private Map replace(String name, Map object) throws IOException { + return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, null, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).replace(object); + } + + private Map replace(Map object) throws IOException { + return validateAndSubmitRequest(objectMapper.writeValueAsString(object), HttpCallMethod.PUT); + } + /** * Edit a custom resource object which is a non-namespaced object. * @@ -332,7 +346,6 @@ public Map edit(String name, String objectAsString) throws IOExc * @throws IOException in case of network/serialization failures or failures from Kubernetes API */ public Map edit(String namespace, String name, Map object) throws IOException { - object = appendResourceVersionInObject(namespace, name, object); return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).edit(object); } @@ -346,9 +359,6 @@ public Map edit(String namespace, String name, Map edit(String namespace, String name, String objectAsString) throws IOException { - // Append resourceVersion in object metadata in order to - // avoid : https://github.com/fabric8io/kubernetes-client/issues/1724 - objectAsString = appendResourceVersionInObject(namespace, name, objectAsString); return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).edit(objectAsString); } @@ -360,7 +370,8 @@ public Map edit(String namespace, String name, String objectAsSt * @throws IOException in case of network/serializatino failures or failures from Kubernetes API */ public Map edit(String objectAsString) throws IOException { - return validateAndSubmitRequest(objectAsString, HttpCallMethod.PUT); + Map object = convertJsonOrYamlStringToMap(objectAsString); + return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).edit(object); } /** @@ -371,7 +382,8 @@ public Map edit(String objectAsString) throws IOException { * @throws IOException in case of network/serializatino failures or failures from Kubernetes API */ public Map edit(Map object) throws IOException { - return validateAndSubmitRequest(objectMapper.writeValueAsString(object), HttpCallMethod.PUT); + String objectAsString = getPatchDiff(namespace, name, object); + return validateAndSubmitRequest(objectAsString, HttpCallMethod.PATCH); } /** @@ -883,7 +895,7 @@ private Map createOrReplaceObject(Map objectAsMa } String nameFromObject = (String) metadata.get("name"); ret = this.namespace != null ? - edit(this.namespace, nameFromObject, objectAsMap) : edit(nameFromObject, objectAsMap); + replace(this.namespace, nameFromObject, objectAsMap) : replace(nameFromObject, objectAsMap); } catch (NullPointerException nullPointerException) { throw KubernetesClientException.launderThrowable(new IllegalStateException("Invalid object provided -- metadata.name is required.")); } @@ -1057,23 +1069,22 @@ private Request getRequest(String url, String body, HttpCallMethod httpCallMetho return requestBuilder.post(requestBody).url(url).build(); case PUT: return requestBuilder.put(requestBody).url(url).build(); + case PATCH: + return requestBuilder.patch(RequestBody.create(JSON_PATCH, body)).url(url).build(); } return requestBuilder.build(); } - private String appendResourceVersionInObject(String namespace, String customResourceName, String customResourceAsJsonString) throws IOException { - Map newObject = convertJsonOrYamlStringToMap(customResourceAsJsonString); - - return objectMapper.writeValueAsString(appendResourceVersionInObject(namespace, customResourceName, newObject)); - } - - private Map appendResourceVersionInObject(String namespace, String customResourceName, Map customResource) throws IOException { + private String getPatchDiff(String namespace, String customResourceName, Map customResource) throws IOException { Map oldObject = get(namespace, customResourceName); - String resourceVersion = ((Map)oldObject.get(METADATA)).get(RESOURCE_VERSION).toString(); - ((Map)customResource.get(METADATA)).put(RESOURCE_VERSION, resourceVersion); + // Exclude changes to the status + oldObject.put("status", null); + customResource.put("status", null); + + JsonNode newone = JsonDiff.asJson(PatchUtils.patchMapper().valueToTree(oldObject), PatchUtils.patchMapper().valueToTree(customResource)); - return customResource; + return objectMapper.writeValueAsString(newone); } private DeleteOptions fetchDeleteOptions(boolean cascading, String propagationPolicy) { diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImplTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImplTest.java index c03ccbcd98d..4249ac22f69 100644 --- a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImplTest.java +++ b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImplTest.java @@ -15,10 +15,7 @@ */ package io.fabric8.kubernetes.client.dsl.internal; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.*; import java.io.IOException; @@ -30,6 +27,8 @@ import io.fabric8.kubernetes.api.model.DeleteOptionsBuilder; import io.fabric8.kubernetes.api.model.DeletionPropagation; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.utils.Serialization; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -657,6 +656,19 @@ void testGetConfigmap() { assertRequestCaptured(captor, "/api/v1/namespaces/default/configmaps/game-config", "GET"); } + @Test + void testEditCR() throws IOException { + // Given + RawCustomResourceOperationsImpl rawCustomResourceOperations = new RawCustomResourceOperationsImpl(mockClient, config, clusterCustomResourceDefinitionContext); + String jsonString = "{ \"metadata\": " + Serialization.jsonMapper().writeValueAsString(new ObjectMetaBuilder().withName("myresource").withNamespace("mynamespace").build()) + "}"; + + // When + Map res = rawCustomResourceOperations.edit(jsonString); + + // Then + assertNotEquals(null, res); + } + private void assertRequestCaptured(ArgumentCaptor captor, String url, String method) { verify(mockClient).newCall(captor.capture()); assertEquals(url, captor.getValue().url().encodedPath()); diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CustomResourceTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CustomResourceTest.java index 78ef651cf6c..e39870a2058 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CustomResourceTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CustomResourceTest.java @@ -188,9 +188,12 @@ void testGet() { @Test void testEdit() throws IOException { + String jsonObject = "{\"apiVersion\": \"test.fabric8.io/v1alpha1\",\"kind\": \"Hello\"," + + "\"metadata\": {\"resourceVersion\": \"1\", \"name\": \"example-hello\"},\"spec\": {\"size\": 3}}"; String jsonObjectNew = "{\"apiVersion\": \"test.fabric8.io/v1alpha1\",\"kind\": \"Hello\"," + "\"metadata\": {\"resourceVersion\": \"1\", \"name\": \"example-hello\"},\"spec\": {\"size\": 4}}"; - server.expect().put().withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos/example-hello").andReturn(HttpURLConnection.HTTP_OK, jsonObjectNew).once(); + server.expect().patch().withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos/example-hello").andReturn(HttpURLConnection.HTTP_OK, jsonObjectNew).once(); + server.expect().get().withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos/example-hello").andReturn(HttpURLConnection.HTTP_OK, jsonObject).once(); server.expect().get().withPath("/apis/test.fabric8.io/v1alpha1/namespaces/ns1/hellos/example-hello").andReturn(HttpURLConnection.HTTP_OK, jsonObjectNew).once(); KubernetesClient client = server.getClient();