feat: throw exception on problematic owner ference adding in dependen…

…t resources (#2190) Signed-off-by: Attila Mészáros <csviri@gmail.com>
operator-framework · Jan 10, 2024 · cd0fe64 · cd0fe64
1 parent 39818f8
commit cd0fe64
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 10 deletions.
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java
@@ -6,12 +6,15 @@
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Locale;
+import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import io.fabric8.kubernetes.api.builder.Builder;
+import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.Namespaced;
 import io.fabric8.kubernetes.client.CustomResource;
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.utils.Serialization;
@@ -71,6 +74,30 @@ public static String getNameFor(Class<? extends Reconciler> reconcilerClass) {
     return getDefaultNameFor(reconcilerClass);
   }
 
+  public static void checkIfCanAddOwnerReference(HasMetadata owner, HasMetadata resource) {
+    if (owner instanceof GenericKubernetesResource
+        || resource instanceof GenericKubernetesResource) {
+      return;
+    }
+    if (owner instanceof Namespaced) {
+      if (!(resource instanceof Namespaced)) {
+        throw new OperatorException(
+            "Cannot add owner reference from a cluster scoped to a namespace scoped resource." +
+                resourcesIdentifierDescription(owner, resource));
+      } else if (!Objects.equals(owner.getMetadata().getNamespace(),
+          resource.getMetadata().getNamespace())) {
+        throw new OperatorException(
+            "Cannot add owner reference between two resource in different namespaces." +
+                resourcesIdentifierDescription(owner, resource));
+      }
+    }
+  }
+
+  private static String resourcesIdentifierDescription(HasMetadata owner, HasMetadata resource) {
+    return " Owner name: " + owner.getMetadata().getName() + " Kind: " + owner.getKind()
+        + ", Resource name: " + resource.getMetadata().getName() + " Kind: " + resource.getKind();
+  }
+
   public static String getNameFor(Reconciler reconciler) {
     return getNameFor(reconciler.getClass());
   }

diff --git a/...javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/...javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
@@ -10,6 +10,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.javaoperatorsdk.operator.OperatorException;
+import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.dependent.Configured;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Constants;
@@ -236,6 +237,7 @@ protected Resource<R> prepare(Context<P> context, R desired, P primary, String a
 
   protected void addReferenceHandlingMetadata(R desired, P primary) {
     if (addOwnerReference()) {
+      ReconcilerUtils.checkIfCanAddOwnerReference(primary, desired);
       desired.addOwnerReference(primary);
     } else if (useNonOwnerRefBasedSecondaryToPrimaryMapping()) {
       addSecondaryToPrimaryMapperAnnotations(desired, primary);

diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java
@@ -4,16 +4,12 @@
 
 import org.junit.jupiter.api.Test;
 
-import io.fabric8.kubernetes.api.model.ContainerBuilder;
-import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.fabric8.kubernetes.api.model.Namespace;
-import io.fabric8.kubernetes.api.model.Namespaced;
-import io.fabric8.kubernetes.api.model.Pod;
-import io.fabric8.kubernetes.api.model.PodSpec;
-import io.fabric8.kubernetes.api.model.PodTemplateSpec;
+import io.fabric8.kubernetes.api.model.*;
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
 import io.fabric8.kubernetes.api.model.apps.DeploymentSpec;
+import io.fabric8.kubernetes.api.model.rbac.ClusterRole;
+import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBuilder;
 import io.fabric8.kubernetes.client.CustomResource;
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.http.HttpRequest;
@@ -29,9 +25,7 @@
 import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException;
 import static io.javaoperatorsdk.operator.ReconcilerUtils.isFinalizerValid;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -155,6 +149,46 @@ void handleKubernetesExceptionShouldThrowMissingCRDExceptionWhenAppropriate() {
         HasMetadata.getFullResourceName(Tomcat.class)));
   }
 
+
+  @Test
+  void checksIfOwnerReferenceCanBeAdded() {
+    assertThrows(OperatorException.class,
+        () -> ReconcilerUtils.checkIfCanAddOwnerReference(namespacedResource(),
+            namespacedResourceFromOtherNamespace()));
+
+    assertThrows(OperatorException.class,
+        () -> ReconcilerUtils.checkIfCanAddOwnerReference(namespacedResource(),
+            clusterScopedResource()));
+
+    assertDoesNotThrow(() -> {
+      ReconcilerUtils.checkIfCanAddOwnerReference(clusterScopedResource(), clusterScopedResource());
+      ReconcilerUtils.checkIfCanAddOwnerReference(namespacedResource(), namespacedResource());
+    });
+  }
+
+  private ClusterRole clusterScopedResource() {
+    return new ClusterRoleBuilder()
+        .withMetadata(new ObjectMetaBuilder()
+            .build())
+        .build();
+  }
+
+  private ConfigMap namespacedResource() {
+    return new ConfigMapBuilder()
+        .withMetadata(new ObjectMetaBuilder()
+            .withNamespace("testns1")
+            .build())
+        .build();
+  }
+
+  private ConfigMap namespacedResourceFromOtherNamespace() {
+    return new ConfigMapBuilder()
+        .withMetadata(new ObjectMetaBuilder()
+            .withNamespace("testns2")
+            .build())
+        .build();
+  }
+
   @Group("tomcatoperator.io")
   @Version("v1")
   @ShortNames("tc")