Skip to content

Commit

Permalink
fix (kubernetes-client) : edit(), patch() and other mutating oper…
Browse files Browse the repository at this point in the history
…ations shouldn't be allowed in NonNamespaceOperation (#3772)

Remove reference to `WritableOperation<T>` interface from
NonNamespaceOperation as it exposes `edit()`, `patch()` and other
mutating operations.

Signed-off-by: Rohan Kumar <rohaan@redhat.com>
  • Loading branch information
rohanKanojia authored and manusa committed Feb 4, 2022
1 parent 5d5ff8e commit 89ec394
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -4,6 +4,7 @@

#### Bugs
* Fix #3776: VerticalPodAutoscaler cannot load yaml with "controlledResources"
* Fix #3772: `edit()` should not be allowed as a NonNamespaceOperation

#### Improvements

Expand Down
Expand Up @@ -25,7 +25,8 @@
public interface NonNamespaceOperation<T, L, R> extends
Nameable<R>,
FilterWatchListMultiDeletable<T, L>,
WritableOperation<T>,
Createable<T>,
CreateOrReplaceable<T>,
DryRunable<WritableOperation<T>>,
Loadable<R> {
}
Expand Down
Expand Up @@ -24,6 +24,7 @@ public interface Resource<T> extends CreateOrReplaceable<T>,
CreateFromServerGettable<T>,
CascadingEditReplacePatchDeletable<T>,
VersionWatchAndWaitable<T>,
WritableOperation<T>,
DryRunable<WritableOperation<T>>,
Requirable<T>, Readiable, Informable<T>,
CreateOrDeleteable<T> {
Expand Down
Expand Up @@ -128,7 +128,7 @@ public void testFullObjectPatch() {
// When
ConfigMap configMapFromServer = client.configMaps().inNamespace(currentNamespace).withName(name).get();
configMapFromServer.setData(Collections.singletonMap("foo", "bar"));
ConfigMap patchedConfigMap = client.configMaps().patch(configMapFromServer);
ConfigMap patchedConfigMap = client.configMaps().inNamespace(currentNamespace).withName(name).patch(configMapFromServer);

// Then
assertThat(patchedConfigMap).isNotNull();
Expand All @@ -148,7 +148,7 @@ public void testFullObjectPatchWithConcurrentChange() {
// concurrent change to empty
ConfigMap baseCopy = new ConfigMapBuilder(base).build();
baseCopy.setData(Collections.emptyMap());
client.configMaps().patch(baseCopy);
client.configMaps().inNamespace(currentNamespace).withName(name).patch(baseCopy);

// concurrent change to empty
ConfigMap baseCopy2 = new ConfigMapBuilder(base).build();
Expand Down
Expand Up @@ -214,7 +214,7 @@ public void replaceStatusSubresource() {
// use the original pet, no need to pick up the resourceVersion
pet.getSpec().setType("shouldn't change");
pet.setStatus(petStatusToUpdate);
Pet updatedPet = petClient.inNamespace(currentNamespace).replaceStatus(pet);
Pet updatedPet = petClient.inNamespace(currentNamespace).withName(pet.getMetadata().getName()).replaceStatus(pet);

// Then
assertPet(updatedPet, "pet-replacestatus", "Pigeon", "Sleeping");
Expand All @@ -234,7 +234,7 @@ public void patchStatusSubresource() {
// use the original pet, no need to pick up the resourceVersion
pet.getSpec().setType("shouldn't change");
pet.setStatus(petStatusToUpdate);
Pet updatedPet = petClient.inNamespace(currentNamespace).patchStatus(pet);
Pet updatedPet = petClient.inNamespace(currentNamespace).withName(pet.getMetadata().getName()).patchStatus(pet);

// Then
assertPet(updatedPet, "pet-applystatus", "Pigeon", "Sleeping");
Expand Down
Expand Up @@ -175,7 +175,7 @@ void testStatusSubresourceHandling() {

cronTab.getMetadata().setLabels(labels);

result = cronTabClient.replace(cronTab);
result = cronTabClient.withName(cronTab.getMetadata().getName()).replace(cronTab);

String originalUid = result.getMetadata().getUid();

Expand All @@ -187,13 +187,13 @@ void testStatusSubresourceHandling() {
labels.put("other", "label");
cronTab.setStatus(null);

result = cronTabClient.replace(cronTab);
result = cronTabClient.withName(cronTab.getMetadata().getName()).replace(cronTab);

// should retain the existing
assertNotNull(result.getStatus());

labels.put("another", "label");
result = cronTabClient.patch(cronTab);
result = cronTabClient.withName(cronTab.getMetadata().getName()).patch(cronTab);

// should retain the existing
assertNotNull(result.getStatus());
Expand All @@ -220,7 +220,7 @@ void testStatusPatch() {

result.setStatus(status);

result = cronTabClient.patchStatus(result);
result = cronTabClient.withName(cronTab.getMetadata().getName()).patchStatus(result);
assertNotNull(result.getStatus());
}

Expand Down
Expand Up @@ -157,7 +157,7 @@ void testStatusReplace() throws InterruptedException {
server.expect().put().withPath("/apis/example.crd.com/v1alpha1/stars/sun/status").andReturn(200, "{\"apiVersion\":\"example.crd.com/v1alpha1\",\"kind\":\"Star\",\"metadata\":{\"name\":\"sun\",\"resourceVersion\":\"2\"},\"spec\":{\"type\":\"G\",\"location\":\"Galaxy\"},\"status\":{\"location\":\"M\"}}").once();
starClient = client.customResources(Star.class);

Star replaced = starClient.inNamespace("test").replaceStatus(updatedStar);
Star replaced = starClient.inNamespace("test").withName(updatedStar.getMetadata().getName()).replaceStatus(updatedStar);
assertEquals("2", replaced.getMetadata().getResourceVersion());
RecordedRequest recordedRequest = server.getLastRequest();
// get of the latest version, put of status
Expand Down

0 comments on commit 89ec394

Please sign in to comment.