Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit a CustomResource should result in a patch #2923

Merged
merged 8 commits into from
Apr 7, 2021
Merged

Edit a CustomResource should result in a patch #2923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
20b8893
CustomResource edit as PATCH
andreaTP Mar 17, 2021
cf76111
cleanup
andreaTP Mar 17, 2021
9de4505
more cleanup
andreaTP Mar 17, 2021
ef9f4d3
more cleanup
andreaTP Mar 17, 2021
1a47309
remove outdated comment
andreaTP Mar 17, 2021
ae80f8c
fix the test
andreaTP Mar 18, 2021
e642d5c
minor cleanups
andreaTP Mar 18, 2021
256511f
patchutils object mapper
andreaTP Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 31 additions & 20 deletions .../main/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -298,6 +300,18 @@ public Map<String, Object> createOrReplace(String namespace, InputStream objectA
return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, null, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).createOrReplace(objectAsStream);
}

private Map<String, Object> replace(String namespace, String name, Map<String, Object> object) throws IOException {
return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).replace(object);
}

private Map<String, Object> replace(String name, Map<String, Object> object) throws IOException {
return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, null, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).replace(object);
}

private Map<String, Object> replace(Map<String, Object> object) throws IOException {
return validateAndSubmitRequest(objectMapper.writeValueAsString(object), HttpCallMethod.PUT);
}

/**
* Edit a custom resource object which is a non-namespaced object.
*
Expand Down Expand Up @@ -332,7 +346,6 @@ public Map<String, Object> edit(String name, String objectAsString) throws IOExc
* @throws IOException in case of network/serialization failures or failures from Kubernetes API
*/
public Map<String, Object> edit(String namespace, String name, Map<String, Object> object) throws IOException {
object = appendResourceVersionInObject(namespace, name, object);
return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).edit(object);
}

Expand All @@ -346,9 +359,6 @@ public Map<String, Object> edit(String namespace, String name, Map<String, Objec
* @throws IOException in case of network/serialization failures or failures from Kubernetes API
*/
public Map<String, Object> 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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already implemented here:

kubernetes-client/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImpl.java

Line 882 in 6e439a8

metadata.put(RESOURCE_VERSION, originalResourceVersion);

objectAsString = appendResourceVersionInObject(namespace, name, objectAsString);
return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).edit(objectAsString);
}

Expand All @@ -360,7 +370,8 @@ public Map<String, Object> edit(String namespace, String name, String objectAsSt
* @throws IOException in case of network/serializatino failures or failures from Kubernetes API
*/
public Map<String, Object> edit(String objectAsString) throws IOException {
return validateAndSubmitRequest(objectAsString, HttpCallMethod.PUT);
Map<String, Object> object = convertJsonOrYamlStringToMap(objectAsString);
return new RawCustomResourceOperationsImpl(client, config, customResourceDefinition, namespace, name, gracePeriodInSeconds, cascading, deletionPropagation, listOptions, dryRun).edit(object);
}

/**
Expand All @@ -371,7 +382,8 @@ public Map<String, Object> edit(String objectAsString) throws IOException {
* @throws IOException in case of network/serializatino failures or failures from Kubernetes API
*/
public Map<String, Object> edit(Map<String, Object> object) throws IOException {
return validateAndSubmitRequest(objectMapper.writeValueAsString(object), HttpCallMethod.PUT);
String objectAsString = getPatchDiff(namespace, name, object);
return validateAndSubmitRequest(objectAsString, HttpCallMethod.PATCH);
}

/**
Expand Down Expand Up @@ -883,7 +895,7 @@ private Map<String, Object> createOrReplaceObject(Map<String, Object> 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."));
}
Expand Down Expand Up @@ -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<String, Object> newObject = convertJsonOrYamlStringToMap(customResourceAsJsonString);

return objectMapper.writeValueAsString(appendResourceVersionInObject(namespace, customResourceName, newObject));
}

private Map<String, Object> appendResourceVersionInObject(String namespace, String customResourceName, Map<String, Object> customResource) throws IOException {
private String getPatchDiff(String namespace, String customResourceName, Map<String, Object> customResource) throws IOException {
Map<String, Object> oldObject = get(namespace, customResourceName);
String resourceVersion = ((Map<String, Object>)oldObject.get(METADATA)).get(RESOURCE_VERSION).toString();

((Map<String, Object>)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) {
Expand Down
20 changes: 16 additions & 4 deletions ...t/java/io/fabric8/kubernetes/client/dsl/internal/RawCustomResourceOperationsImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<String, Object> res = rawCustomResourceOperations.edit(jsonString);

// Then
assertNotEquals(null, res);
}

private void assertRequestCaptured(ArgumentCaptor<Request> captor, String url, String method) {
verify(mockClient).newCall(captor.capture());
assertEquals(url, captor.getValue().url().encodedPath());
Expand Down
5 changes: 4 additions & 1 deletion kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/CustomResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down