fix #3745: throw better exceptions when a namespace is missing

fabric8io · Mar 3, 2022 · 9e5d116 · 9e5d116
1 parent 3c8a074
commit 9e5d116
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 * Fix #3582: SSL truststore can be loaded in FIPS enabled environments
 * Fix #3818: adding missing throws to launderThrowable
 * Fix #3859: refined how a deserialization class is chosen to not confuse types with the same kind
+* Fix #3745: the client will throw better exceptions when a namespace is not discernible for an operation
 
 #### Improvements
 * Fix #3811: Reintroduce `Replaceable` interface in `NonNamespaceOperation`

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/BaseOperation.java
@@ -291,31 +291,28 @@ public ExtensibleResource<T> fromServer() {
   @Override
   public final T createOrReplace(T... items) {
     T itemToCreateOrReplace = getItem();
+    Resource<T> resource;
     if (items.length > 1) {
       throw new IllegalArgumentException("Too many items to create.");
     } else if (items.length == 1) {
       itemToCreateOrReplace = items[0];
+      resource = withItem(itemToCreateOrReplace);
+    } else {
+      resource = this;
     }
 
     if (itemToCreateOrReplace == null) {
       throw new IllegalArgumentException("Nothing to create.");
     }
-
-    if (Utils.isNullOrEmpty(name)) {
-
-      return withName(itemToCreateOrReplace.getMetadata().getName()).createOrReplace(itemToCreateOrReplace);
-    }
-    T finalItemToCreateOrReplace = itemToCreateOrReplace;
-    // use the Resource in case create or replace is overriden
-    Resource<T> resource = newResource(context);
+
     CreateOrReplaceHelper<T> createOrReplaceHelper = new CreateOrReplaceHelper<>(
       resource::create,
       resource::replace,
-      m -> waitUntilCondition(Objects::nonNull, 1, TimeUnit.SECONDS),
-      m -> fromServer().get()
+      m -> resource.waitUntilCondition(Objects::nonNull, 1, TimeUnit.SECONDS),
+      m -> resource.fromServer().get()
     );
 
-    return createOrReplaceHelper.createOrReplace(finalItemToCreateOrReplace);
+    return createOrReplaceHelper.createOrReplace(itemToCreateOrReplace);
   }
 
   @Override
@@ -704,12 +701,7 @@ protected T handleGet(URL resourceUrl) throws InterruptedException, IOException
   }
 
   private URL getCompleteResourceUrl() throws MalformedURLException {
-    URL requestUrl = null;
-    if (item != null) {
-      requestUrl = getNamespacedUrl(checkNamespace(item));
-    } else {
-      requestUrl = getNamespacedUrl();
-    }
+    URL requestUrl = getNamespacedUrl(checkNamespace(item));
     if (name != null) {
       requestUrl = new URL(URLUtils.join(requestUrl.toString(), name));
     } else if (item != null && reloadingFromServer) {

diff --git a/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationContext.java b/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationContext.java
@@ -129,9 +129,9 @@ private void setNamespace(String namespace, boolean defaultNamespace) {
     if (!defaultNamespace || Utils.isNotNullOrEmpty(namespace)) {
       this.namespace = namespace;
       this.defaultNamespace = defaultNamespace;
-    } else {
-      this.namespace = config != null && Utils.isNotNullOrEmpty(config.getNamespace()) ? config.getNamespace() : null;
-      this.defaultNamespace = config != null ? config.isDefaultNamespace() : true;
+    } else if (config != null) {
+      this.namespace = config.getNamespace();
+      this.defaultNamespace = config.isDefaultNamespace();
     }
   }
 

diff --git a/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java b/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/OperationSupport.java
@@ -233,6 +233,9 @@ protected <T> String checkNamespace(T item) {
     String operationNs = getNamespace();
     String itemNs = (item instanceof HasMetadata) ? KubernetesResourceUtil.getNamespace((HasMetadata)item) : null;
     if (Utils.isNullOrEmpty(operationNs) && Utils.isNullOrEmpty(itemNs)) {
+      if (context.isDefaultNamespace()) {
+        throw new KubernetesClientException("namespace not specified for an operation requiring one and no default was found in the Config.");  
+      }
       throw new KubernetesClientException("namespace not specified for an operation requiring one.");
     } else if (!Utils.isNullOrEmpty(itemNs) && (Utils.isNullOrEmpty(operationNs)
         || this.context.isDefaultNamespace())) {

diff --git a/...netes-tests/src/test/java/io/fabric8/openshift/client/server/mock/NamespacedItemTest.java b/...netes-tests/src/test/java/io/fabric8/openshift/client/server/mock/NamespacedItemTest.java
@@ -28,6 +28,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @EnableOpenShiftMockClient(crud = true)
 class NamespacedItemTest {
@@ -116,4 +117,11 @@ void testOperationNullNamespace() {
     assertThrows(KubernetesClientException.class, () -> configMaps.inNamespace(null));
   }
 
+  @Test
+  void testInAnyNamespaceWithNamespacedOperation() {
+    Resource<ConfigMap> op = client.adapt(NamespacedKubernetesClient.class).inAnyNamespace().configMaps().withName("x");
+    KubernetesClientException e = assertThrows(KubernetesClientException.class, () -> op.get());
+    assertTrue(e.getMessage().contains("namespace not"));
+  }
+
 }