Skip to content

Commit

Permalink
fix #4142: additional patch methods
Browse files Browse the repository at this point in the history
also correcting the javadocs wrt the patch base item.

removing logic that infers from the passed in item as the context will
already have the necessary information
  • Loading branch information
shawkins authored and manusa committed May 17, 2022
1 parent 78e74e2 commit 5133ad8
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
SharedIndexInformer allows for the addition and removal of indexes even after starting, and you can remove the default namespace index if you wish.
And Store.getKey can be used rather than directly referencing static Cache functions.
* Fix #4065: Client.getAPIResources("v1") can be used to obtain the core/legacy resources
* Fix #4142: Added patch() and patch(PatchContext) methods for use with resource and load
* Fix #4093: adding a possibility to get a log as an `InputStream` from the `Loggable` resources

#### Dependency Upgrade
Expand Down
2 changes: 2 additions & 0 deletions doc/MIGRATION-v6.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ Client.adapt will no longer perform the isAdaptable check - that is you may free

- DSL methods available off of a collection context involving a resource - client.configMaps().create(configMap) - should instead use a no-argument method - client.configMaps().resource(configMap).create() or client.resource(configMap).create().

The only exception to the above is patch(PatchContext, item) - it is valid to be called after withName.

## Object Sorting

KubernetesList and Template will no longer automatically sort their objects by default. You may use the HasMetadataComparator to sort the items as needed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,19 @@ public interface EditReplacePatchable<T>

/**
* Update field(s) of a resource using a JSON patch.
*
* <br>
* <p>
* It is the same as calling {@link #patch(PatchContext, Object)} with {@link PatchType#JSON} specified.
*
* <p>
* WARNING: This may overwrite concurrent changes (between when you obtained your item and the current state) in an unexpected
* way.
* Consider using edit instead or ensure you have called load or resource to define the base of your patch
* <p>
* Consider using edit, which allows for a known base, and a builder instead.
*
* @param item to be patched with patched values
* @return returns deserialized version of api server response
* @deprecated use resource(item).patch() or edit instead
*/
@Deprecated
default T patch(T item) {
return patch(PatchContext.of(PatchType.JSON), item);
}
Expand All @@ -83,8 +85,7 @@ default T patch(T item) {
* Update field(s) of a resource using type specified in {@link PatchContext}(defaults to strategic merge if not specified).
*
* <ul>
* <li>{@link PatchType#JSON} - will create a JSON patch against the current item. See the note in {@link #patch(Object)}
* about what is used for the base object.
* <li>{@link PatchType#JSON} - will create a JSON patch against the latest server state
* <li>{@link PatchType#JSON_MERGE} - will send the serialization of the item as a JSON MERGE patch.
* Set the resourceVersion to null to prevent optimistic locking.
* <li>{@link PatchType#STRATEGIC_MERGE} - will send the serialization of the item as a STRATEGIC MERGE patch.
Expand Down Expand Up @@ -137,4 +138,40 @@ default T patch(String patch) {
* @return updated object
*/
T patchStatus();

/**
* Update field(s) of a resource using a JSON patch.
* <p>
* It is the same as calling {@link #patch(PatchContext, Object)} with {@link PatchType#JSON} specified.
* <p>
* WARNING: This may overwrite concurrent changes (between when you obtained your item and the current state) in an unexpected
* way.
* <p>
* Consider using edit instead.
*
* @return returns deserialized version of api server response
*/
T patch();

/**
* Update field(s) of a resource using type specified in {@link PatchContext}(defaults to strategic merge if not specified).
* <p>
* For use when you are providing a complete object to be patched:
* resource(item).patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY))
*
* <ul>
* <li>{@link PatchType#JSON} - will create a JSON patch against the latest server state
* <li>{@link PatchType#JSON_MERGE} - will send the serialization of the item as a JSON MERGE patch.
* Set the resourceVersion to null to prevent optimistic locking.
* <li>{@link PatchType#STRATEGIC_MERGE} - will send the serialization of the item as a STRATEGIC MERGE patch.
* Set the resourceVersion to null to prevent optimistic locking.
* <li>{@link PatchType#SERVER_SIDE_APPLY} - will send the serialization of the item as a SERVER SIDE APPLY patch.
* You may explicitly set the {@link PatchContext#getFieldManager()} as well to override the default.
* </ul>
*
* @param patchContext {@link PatchContext} for patch request
* @return returns deserialized version of api server response
*/
T patch(PatchContext patchContext);

}
Original file line number Diff line number Diff line change
Expand Up @@ -291,4 +291,14 @@ public T patchStatus() {
return resource.patchStatus();
}

@Override
public T patch() {
return resource.patch();
}

@Override
public T patch(PatchContext patchContext) {
return resource.patch(patchContext);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,25 @@ public T patchStatus(T item) {

@Override
public T patchStatus() {
throw new KubernetesClientException(READ_ONLY_UPDATE_EXCEPTION_MESSAGE);
return patchStatus(getNonNullItem());
}

@Override
public T patch() {
return patch(getNonNullItem());
}

@Override
public T patch(PatchContext patchContext) {
return patch(patchContext, getNonNullItem());
}

protected T getNonNullItem() {
T result = getItem();
if (result == null) {
throw new KubernetesClientException("item required");
}
return result;
}

@Override
Expand Down Expand Up @@ -1045,7 +1063,7 @@ public ExtensibleResource<T> dryRun() {

@Override
public ExtensibleResource<T> lockResourceVersion() {
return lockResourceVersion(KubernetesResourceUtil.getResourceVersion(getItem()));
return lockResourceVersion(KubernetesResourceUtil.getResourceVersion(getNonNullItem()));
}

@Override
Expand All @@ -1055,7 +1073,7 @@ public T updateStatus(T item) {

@Override
public T create() {
return create(getItem());
return create(getNonNullItem());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,6 @@ public T edit(Visitor... visitors) {
* Will always return non-null or throw an exception.
*/
protected T requireFromServer() {
return requireFromServer(null);
}

/**
* Get the current item from the server, consulting the metadata for the name if needed
* <br>
* Will always return non-null or throw an exception.
*/
protected T requireFromServer(ObjectMeta metadata) {
try {
if (Utils.isNotNullOrEmpty(getName())) {
return newInstance(context.withItem(null)).require();
Expand All @@ -109,9 +100,6 @@ protected T requireFromServer(ObjectMeta metadata) {
return newInstance(context.withItem(null)).withName(name).require();
}
}
if (metadata != null && Utils.isNotNullOrEmpty(metadata.getName())) {
return newInstance(context.withItem(null)).withName(metadata.getName()).require();
}
} catch (ResourceNotFoundException e) {
if (e.getCause() instanceof KubernetesClientException) {
throw (KubernetesClientException) e.getCause();
Expand Down Expand Up @@ -155,7 +143,7 @@ protected T replace(T item, boolean status) {
if (!status) {
try {
ObjectMeta metadata = item.getMetadata();
item = modifyItemForReplaceOrPatch(() -> requireFromServer(metadata), item);
item = modifyItemForReplaceOrPatch(() -> requireFromServer(), item);
} catch (Exception e) {
throw KubernetesClientException.launderThrowable(forOperationType(REPLACE_OPERATION), e);
}
Expand All @@ -170,7 +158,7 @@ protected T replace(T item, boolean status) {
// if a resourceVersion is already there, try it first
resourceVersion = existingResourceVersion;
} else {
T got = requireFromServer(item.getMetadata());
T got = requireFromServer();
resourceVersion = KubernetesResourceUtil.getResourceVersion(got);
}

Expand Down Expand Up @@ -207,7 +195,7 @@ protected T replace(T item, boolean status) {

protected T patch(PatchContext context, T base, T item, boolean status) {
if (base == null && context != null && context.getPatchType() == PatchType.JSON) {
base = requireFromServer(item.getMetadata());
base = getMandatory();
if (base.getMetadata() != null) {
// prevent the resourceVersion from being modified in the patch
if (item.getMetadata() == null) {
Expand All @@ -233,11 +221,6 @@ protected T patch(PatchContext context, T base, T item, boolean status) {
return visitor.apply(item);
}

@Override
public T patchStatus() {
return patchStatus(getItem());
}

@Override
public T patchStatus(T item) {
return patch(PatchContext.of(PatchType.JSON_MERGE), null, clone(item), true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,31 @@ void testCrud() {
assertEquals("THREE", configmap2.getData().get("III"));
}

@Test
void testPatchBase() {
ConfigMap configmap2 = new ConfigMapBuilder()
.withNewMetadata()
.addToLabels("foo", "bar")
.withName("configmap2")
.endMetadata()
.addToData("two", "2")
.build();
ConfigMap configmap2a = new ConfigMapBuilder()
.withNewMetadata()
.addToLabels("foo", "bar")
.withName("configmap2")
.endMetadata()
.addToData("three", "3")
.build();

client.configMaps().inNamespace("ns1").resource(configmap2).create();
client.configMaps().inNamespace("ns1").resource(configmap2a).replace();

// should still diff to latest
ConfigMap result = client.configMaps().inNamespace("ns1").resource(configmap2).patch(configmap2);
assertEquals(Collections.singletonMap("three", "3"), result.getData());
}

@Test
@DisplayName("edit, should add and remove data entries")
void edit() {
Expand Down

0 comments on commit 5133ad8

Please sign in to comment.