From 20b88939f652fb8320e68f47aa99f0aa35be668f Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Wed, 17 Mar 2021 19:00:22 +0000 Subject: [PATCH 1/8] CustomResource edit as PATCH --- .../RawCustomResourceOperationsImpl.java | 50 ++++++++++++++----- .../RawCustomResourceOperationsImplTest.java | 31 ++++++++++-- 2 files changed, 64 insertions(+), 17 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..1ad2efb9b8c 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,13 @@ 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 +91,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 = new ObjectMapper(); this.namespace = namespace; this.name = name; this.gracePeriodInSeconds = gracePeriodInSeconds; @@ -298,6 +301,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 +347,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 +360,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 +371,10 @@ 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); + // Append resourceVersion in object metadata in order to + // avoid : https://github.com/fabric8io/kubernetes-client/issues/1724 + objectAsString = getPatchDiff(namespace, name, objectAsString); + return validateAndSubmitRequest(objectAsString, HttpCallMethod.PATCH); } /** @@ -371,7 +385,7 @@ 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); + return validateAndSubmitRequest(objectMapper.writeValueAsString(object), HttpCallMethod.PATCH); } /** @@ -883,7 +897,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 +1071,33 @@ 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 { + private String getPatchDiff(String namespace, String customResourceName, String customResourceAsJsonString) throws IOException { Map newObject = convertJsonOrYamlStringToMap(customResourceAsJsonString); - return objectMapper.writeValueAsString(appendResourceVersionInObject(namespace, customResourceName, newObject)); + return getPatchDiff(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); - return customResource; + // 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)); + + String patch = objectMapper.writeValueAsString(newone); + + return patch; } 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..1d66ba62059 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,30 @@ 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) { + public Map get(String namespace, String name) { + + Map metadata = new HashMap(); + metadata.put("resourceVersion", "123"); + + Map res = new HashMap(); + res.put("metadata", metadata); + + return res; + } + }; + String jsonString = "{ \"metadata\": " + Serialization.jsonMapper().writeValueAsString(new ObjectMetaBuilder().withName("myresource").withNamespace("mynamespace").build()) + "}"; + + // When + Map res = rawCustomResourceOperations.edit(jsonString); + + // Then + assertNotEquals(res, null); + } + private void assertRequestCaptured(ArgumentCaptor captor, String url, String method) { verify(mockClient).newCall(captor.capture()); assertEquals(url, captor.getValue().url().encodedPath()); From cf7611109f26e4660eb47db593491f7acc9af4df Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Wed, 17 Mar 2021 19:00:51 +0000 Subject: [PATCH 2/8] cleanup --- .../RawCustomResourceOperationsImpl.java | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 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 1ad2efb9b8c..805da098c70 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 @@ -371,10 +371,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 { - // Append resourceVersion in object metadata in order to - // avoid : https://github.com/fabric8io/kubernetes-client/issues/1724 - objectAsString = getPatchDiff(namespace, name, objectAsString); - return validateAndSubmitRequest(objectAsString, HttpCallMethod.PATCH); + Map object = convertJsonOrYamlStringToMap(objectAsString); + return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).edit(object); } /** @@ -385,7 +383,10 @@ 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.PATCH); + // Append resourceVersion in object metadata in order to + // avoid : https://github.com/fabric8io/kubernetes-client/issues/1724 + String objectAsString = getPatchDiff(namespace, name, object); + return validateAndSubmitRequest(objectAsString, HttpCallMethod.PATCH); } /** @@ -1077,12 +1078,6 @@ private Request getRequest(String url, String body, HttpCallMethod httpCallMetho return requestBuilder.build(); } - private String getPatchDiff(String namespace, String customResourceName, String customResourceAsJsonString) throws IOException { - Map newObject = convertJsonOrYamlStringToMap(customResourceAsJsonString); - - return getPatchDiff(namespace, customResourceName, newObject); - } - 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(); From 9de450536df4f1916321ee254602dedfc46e5188 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Wed, 17 Mar 2021 19:04:49 +0000 Subject: [PATCH 3/8] more cleanup --- .../client/dsl/internal/RawCustomResourceOperationsImpl.java | 3 --- 1 file changed, 3 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 805da098c70..05c21edcecd 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 @@ -1080,9 +1080,6 @@ private Request getRequest(String url, String body, HttpCallMethod httpCallMetho 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); From ef9f4d3e0ea1ee3b00ab1d5bb4ac4ab29494d876 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Wed, 17 Mar 2021 19:18:07 +0000 Subject: [PATCH 4/8] more cleanup --- .../RawCustomResourceOperationsImplTest.java | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) 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 1d66ba62059..4ed9c7e3fb5 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 @@ -659,18 +659,7 @@ void testGetConfigmap() { @Test void testEditCR() throws IOException { // Given - RawCustomResourceOperationsImpl rawCustomResourceOperations = new RawCustomResourceOperationsImpl(mockClient, config, clusterCustomResourceDefinitionContext) { - public Map get(String namespace, String name) { - - Map metadata = new HashMap(); - metadata.put("resourceVersion", "123"); - - Map res = new HashMap(); - res.put("metadata", metadata); - - return res; - } - }; + RawCustomResourceOperationsImpl rawCustomResourceOperations = new RawCustomResourceOperationsImpl(mockClient, config, clusterCustomResourceDefinitionContext); String jsonString = "{ \"metadata\": " + Serialization.jsonMapper().writeValueAsString(new ObjectMetaBuilder().withName("myresource").withNamespace("mynamespace").build()) + "}"; // When From 1a473099411253fb1b9a5e967d257b9c53f35e6b Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Wed, 17 Mar 2021 19:35:53 +0000 Subject: [PATCH 5/8] remove outdated comment --- .../client/dsl/internal/RawCustomResourceOperationsImpl.java | 2 -- 1 file changed, 2 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 05c21edcecd..b638a312a6a 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 @@ -383,8 +383,6 @@ 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 { - // Append resourceVersion in object metadata in order to - // avoid : https://github.com/fabric8io/kubernetes-client/issues/1724 String objectAsString = getPatchDiff(namespace, name, object); return validateAndSubmitRequest(objectAsString, HttpCallMethod.PATCH); } From ae80f8ca813e688bc4652f15991166350a5c4663 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Thu, 18 Mar 2021 11:20:23 +0000 Subject: [PATCH 6/8] fix the test --- .../fabric8/kubernetes/client/mock/CustomResourceTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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(); From e642d5ce9a682d9aafcc1e4dcdb52fe20dc3b1f0 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Thu, 18 Mar 2021 11:59:33 +0000 Subject: [PATCH 7/8] minor cleanups --- .../client/dsl/internal/RawCustomResourceOperationsImpl.java | 4 +--- .../dsl/internal/RawCustomResourceOperationsImplTest.java | 2 +- 2 files changed, 2 insertions(+), 4 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 b638a312a6a..86dc521008f 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 @@ -1085,9 +1085,7 @@ private String getPatchDiff(String namespace, String customResourceName, Map res = rawCustomResourceOperations.edit(jsonString); // Then - assertNotEquals(res, null); + assertNotEquals(null, res); } private void assertRequestCaptured(ArgumentCaptor captor, String url, String method) { From 256511f302dd318abbe7e1f35e5f42a7f847f377 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Wed, 7 Apr 2021 09:42:25 +0100 Subject: [PATCH 8/8] patchutils object mapper --- .../client/dsl/internal/RawCustomResourceOperationsImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 86dc521008f..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 @@ -52,7 +52,6 @@ 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; @@ -96,7 +95,7 @@ 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 = new ObjectMapper(); + this.objectMapper = PatchUtils.patchMapper(); this.namespace = namespace; this.name = name; this.gracePeriodInSeconds = gracePeriodInSeconds;