improve: mapping from annotation depends on type (#2606)

* improve: mapping from annotation depends on type Signed-off-by: Attila Mészáros <[email protected]> compiles Signed-off-by: Attila Mészáros <[email protected]> * refactor: avoid creating useless strings Signed-off-by: Chris Laprun <[email protected]> * refactor: ensure roundtrip works Signed-off-by: Chris Laprun <[email protected]> * refactor: rename toSimpleString to toGVKString for greater clarity Signed-off-by: Chris Laprun <[email protected]> * refactor: minor improvements Signed-off-by: Chris Laprun <[email protected]> --------- Signed-off-by: Attila Mészáros <[email protected]> Signed-off-by: Chris Laprun <[email protected]> Co-authored-by: Chris Laprun <[email protected]>
operator-framework · Nov 27, 2024 · 84debd8 · 84debd8
1 parent b81012b
commit 84debd8
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 33 deletions.
diff --git a/...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java b/...framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java
@@ -7,6 +7,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 
 public class GroupVersionKind {
+  private static final String SEPARATOR = "/";
   private final String group;
   private final String version;
   private final String kind;
@@ -16,7 +17,7 @@ public class GroupVersionKind {
 
   public GroupVersionKind(String apiVersion, String kind) {
     this.kind = kind;
-    String[] groupAndVersion = apiVersion.split("/");
+    String[] groupAndVersion = apiVersion.split(SEPARATOR);
     if (groupAndVersion.length == 1) {
       this.group = null;
       this.version = groupAndVersion[0];
@@ -40,24 +41,24 @@ public GroupVersionKind(String group, String version, String kind) {
     this.group = group;
     this.version = version;
     this.kind = kind;
-    this.apiVersion = (group == null || group.isBlank()) ? version : group + "/" + version;
+    this.apiVersion = (group == null || group.isBlank()) ? version : group + SEPARATOR + version;
   }
 
   /**
    * Parse GVK from a String representation. Expected format is: [group]/[version]/[kind]
-   * 
+   *
    * <pre>
    *   Sample: "apps/v1/Deployment"
    * </pre>
-   * 
+   *
    * or: [version]/[kind]
-   * 
+   *
    * <pre>
    *     Sample: v1/ConfigMap
    * </pre>
    **/
   public static GroupVersionKind fromString(String gvk) {
-    String[] parts = gvk.split("/");
+    String[] parts = gvk.split(SEPARATOR);
     if (parts.length == 3) {
       return new GroupVersionKind(parts[0], parts[1], parts[2]);
     } else if (parts.length == 2) {
@@ -68,6 +69,19 @@ public static GroupVersionKind fromString(String gvk) {
     }
   }
 
+  /**
+   * Reverse to {@link #fromString(String)}.
+   *
+   * @return gvk encoded in simple string.
+   */
+  public String toGVKString() {
+    if (group != null) {
+      return group + SEPARATOR + version + SEPARATOR + kind;
+    } else {
+      return version + SEPARATOR + kind;
+    }
+  }
+
   public String getGroup() {
     return group;
   }

diff --git a/...javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/...javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
@@ -19,6 +19,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.Ignore;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ConfiguredDependentResource;
+import io.javaoperatorsdk.operator.processing.GroupVersionKind;
 import io.javaoperatorsdk.operator.processing.dependent.AbstractEventSourceHolderDependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -208,17 +209,18 @@ private boolean useNonOwnerRefBasedSecondaryToPrimaryMapping() {
 
   protected void addSecondaryToPrimaryMapperAnnotations(R desired, P primary) {
     addSecondaryToPrimaryMapperAnnotations(desired, primary, Mappers.DEFAULT_ANNOTATION_FOR_NAME,
-        Mappers.DEFAULT_ANNOTATION_FOR_NAMESPACE);
+        Mappers.DEFAULT_ANNOTATION_FOR_NAMESPACE, Mappers.DEFAULT_ANNOTATION_FOR_PRIMARY_TYPE);
   }
 
   protected void addSecondaryToPrimaryMapperAnnotations(R desired, P primary, String nameKey,
-      String namespaceKey) {
+      String namespaceKey, String typeKey) {
     var annotations = desired.getMetadata().getAnnotations();
     annotations.put(nameKey, primary.getMetadata().getName());
     var primaryNamespaces = primary.getMetadata().getNamespace();
     if (primaryNamespaces != null) {
       annotations.put(namespaceKey, primary.getMetadata().getNamespace());
     }
+    annotations.put(typeKey, GroupVersionKind.gvkFor(primary.getClass()).toGVKString());
   }
 
   @Override
@@ -274,7 +276,7 @@ protected Optional<SecondaryToPrimaryMapper<R>> getSecondaryToPrimaryMapper(
         return Optional
             .of(Mappers.fromOwnerReferences(context.getPrimaryResourceClass(), clustered));
       } else if (isCreatable()) {
-        return Optional.of(Mappers.fromDefaultAnnotations());
+        return Optional.of(Mappers.fromDefaultAnnotations(context.getPrimaryResourceClass()));
       }
     }
     return Optional.empty();

diff --git a/...e/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java b/...e/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java
@@ -5,6 +5,7 @@
 import java.util.stream.Collectors;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.processing.GroupVersionKind;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper;
 
@@ -13,33 +14,41 @@ public class Mappers {
   public static final String DEFAULT_ANNOTATION_FOR_NAME = "io.javaoperatorsdk/primary-name";
   public static final String DEFAULT_ANNOTATION_FOR_NAMESPACE =
       "io.javaoperatorsdk/primary-namespace";
+  public static final String DEFAULT_ANNOTATION_FOR_PRIMARY_TYPE =
+      "io.javaoperatorsdk/primary-type";
 
   private Mappers() {}
 
   public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromAnnotation(
-      String nameKey) {
-    return fromMetadata(nameKey, null, false);
+      String nameKey, String typeKey, Class<? extends HasMetadata> primaryResourceType) {
+    return fromAnnotation(nameKey, null, typeKey, primaryResourceType);
   }
 
   @SuppressWarnings("unused")
   public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromAnnotation(
-      String nameKey, String namespaceKey) {
-    return fromMetadata(nameKey, namespaceKey, false);
+      String nameKey, String namespaceKey, String typeKey,
+      Class<? extends HasMetadata> primaryResourceType) {
+    return fromMetadata(nameKey, namespaceKey, typeKey, primaryResourceType, false);
   }
 
   @SuppressWarnings("unused")
-  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(String nameKey) {
-    return fromMetadata(nameKey, null, true);
+  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(String nameKey,
+      String typeKey,
+      Class<? extends HasMetadata> primaryResourceType) {
+    return fromLabel(nameKey, null, typeKey, primaryResourceType);
   }
 
-  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromDefaultAnnotations() {
-    return fromMetadata(DEFAULT_ANNOTATION_FOR_NAME, DEFAULT_ANNOTATION_FOR_NAMESPACE, false);
+  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromDefaultAnnotations(
+      Class<? extends HasMetadata> primaryResourceType) {
+    return fromAnnotation(DEFAULT_ANNOTATION_FOR_NAME, DEFAULT_ANNOTATION_FOR_NAMESPACE,
+        DEFAULT_ANNOTATION_FOR_PRIMARY_TYPE, primaryResourceType);
   }
 
   @SuppressWarnings("unused")
   public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(
-      String nameKey, String namespaceKey) {
-    return fromMetadata(nameKey, namespaceKey, true);
+      String nameKey, String namespaceKey, String typeKey,
+      Class<? extends HasMetadata> primaryResourceType) {
+    return fromMetadata(nameKey, namespaceKey, typeKey, primaryResourceType, true);
   }
 
   public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
@@ -78,7 +87,8 @@ public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerRefer
   }
 
   private static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromMetadata(
-      String nameKey, String namespaceKey, boolean isLabel) {
+      String nameKey, String namespaceKey, String typeKey,
+      Class<? extends HasMetadata> primaryResourceType, boolean isLabel) {
     return resource -> {
       final var metadata = resource.getMetadata();
       if (metadata == null) {
@@ -94,6 +104,15 @@ private static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromMetadata(
         }
         var namespace =
             namespaceKey == null ? resource.getMetadata().getNamespace() : map.get(namespaceKey);
+
+        String gvkSimple = map.get(typeKey);
+
+        if (gvkSimple != null &&
+            !GroupVersionKind.fromString(gvkSimple)
+                .equals(GroupVersionKind.gvkFor(primaryResourceType))) {
+          return Set.of();
+        }
+
         return Set.of(new ResourceID(name, namespace));
       }
     };
@@ -105,14 +124,11 @@ public static ResourceID fromString(String cacheKey) {
     }
 
     final String[] split = cacheKey.split("/");
-    switch (split.length) {
-      case 1:
-        return new ResourceID(split[0]);
-      case 2:
-        return new ResourceID(split[1], split[0]);
-      default:
-        throw new IllegalArgumentException("Cannot extract a ResourceID from " + cacheKey);
-    }
+    return switch (split.length) {
+      case 1 -> new ResourceID(split[0]);
+      case 2 -> new ResourceID(split[1], split[0]);
+      default -> throw new IllegalArgumentException("Cannot extract a ResourceID from " + cacheKey);
+    };
   }
 
   /**
@@ -139,9 +155,17 @@ public static <OWNER extends HasMetadata, T extends HasMetadata> SecondaryToPrim
 
   public static class SecondaryToPrimaryFromDefaultAnnotation
       implements SecondaryToPrimaryMapper<HasMetadata> {
+
+    private final Class<? extends HasMetadata> primaryResourceType;
+
+    public SecondaryToPrimaryFromDefaultAnnotation(
+        Class<? extends HasMetadata> primaryResourceType) {
+      this.primaryResourceType = primaryResourceType;
+    }
+
     @Override
     public Set<ResourceID> toPrimaryResourceIDs(HasMetadata resource) {
-      return Mappers.fromDefaultAnnotations().toPrimaryResourceIDs(resource);
+      return Mappers.fromDefaultAnnotations(primaryResourceType).toPrimaryResourceIDs(resource);
     }
   }
 

diff --git a/...ework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java b/...ework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java
@@ -75,4 +75,16 @@ void pluralShouldOverrideDefaultComputedVersionIfProvided() {
         "MyPlural");
     assertThat(gvk.getPlural()).hasValue("MyPlural");
   }
+
+  @Test
+  void encodesToGVKString() {
+    final var deploymentGVK = "apps/v1/Deployment";
+    var gvk = GroupVersionKind.fromString(deploymentGVK);
+    assertThat(gvk.toGVKString()).isEqualTo(deploymentGVK);
+    assertThat(gvk).isEqualTo(GroupVersionKind.fromString(gvk.toGVKString()));
+
+    gvk = GroupVersionKind.fromString("v1/ConfigMap");
+    assertThat(gvk.toGVKString()).isEqualTo("v1/ConfigMap");
+    assertThat(gvk).isEqualTo(GroupVersionKind.fromString(gvk.toGVKString()));
+  }
 }
diff --git a/...atorsdk/operator/baseapi/informereventsource/InformerEventSourceTestCustomReconciler.java b/...atorsdk/operator/baseapi/informereventsource/InformerEventSourceTestCustomReconciler.java
@@ -26,6 +26,7 @@ public class InformerEventSourceTestCustomReconciler
       LoggerFactory.getLogger(InformerEventSourceTestCustomReconciler.class);
 
   public static final String RELATED_RESOURCE_NAME = "relatedResourceName";
+  public static final String RELATED_RESOURCE_TYPE = "relatedResourceType";
   public static final String TARGET_CONFIG_MAP_KEY = "targetStatus";
   public static final String MISSING_CONFIG_MAP = "Missing Config Map";
 
@@ -38,7 +39,9 @@ public List<EventSource<?, InformerEventSourceTestCustomResource>> prepareEventS
     InformerEventSourceConfiguration<ConfigMap> config =
         InformerEventSourceConfiguration
             .from(ConfigMap.class, InformerEventSourceTestCustomResource.class)
-            .withSecondaryToPrimaryMapper(Mappers.fromAnnotation(RELATED_RESOURCE_NAME))
+            .withSecondaryToPrimaryMapper(
+                Mappers.fromAnnotation(RELATED_RESOURCE_NAME, RELATED_RESOURCE_TYPE,
+                    InformerEventSourceTestCustomResource.class))
             .build();
 
     return List.of(new InformerEventSource<>(config, context));

diff --git a/...vaoperatorsdk/operator/baseapi/informerremotecluster/InformerRemoteClusterReconciler.java b/...vaoperatorsdk/operator/baseapi/informerremotecluster/InformerRemoteClusterReconciler.java
@@ -53,7 +53,8 @@ public List<EventSource<?, InformerRemoteClusterCustomResource>> prepareEventSou
         .from(ConfigMap.class, InformerRemoteClusterCustomResource.class)
         // owner references do not work cross cluster, using
         // annotations here to reference primary resource
-        .withSecondaryToPrimaryMapper(Mappers.fromDefaultAnnotations())
+        .withSecondaryToPrimaryMapper(
+            Mappers.fromDefaultAnnotations(InformerRemoteClusterCustomResource.class))
         // setting remote client for informer
         .withKubernetesClient(remoteClient)
         .withInformerConfiguration(

diff --git a/...r/dependent/dependentcustommappingannotation/CustomMappingConfigMapDependentResource.java b/...r/dependent/dependentcustommappingannotation/CustomMappingConfigMapDependentResource.java
@@ -20,10 +20,12 @@ public class CustomMappingConfigMapDependentResource
 
   public static final String CUSTOM_NAME_KEY = "customNameKey";
   public static final String CUSTOM_NAMESPACE_KEY = "customNamespaceKey";
+  public static final String CUSTOM_TYPE_KEY = "customTypeKey";
   public static final String KEY = "key";
 
   private static final SecondaryToPrimaryMapper<ConfigMap> mapper =
-      Mappers.fromAnnotation(CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY);
+      Mappers.fromAnnotation(CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY, CUSTOM_TYPE_KEY,
+          DependentCustomMappingCustomResource.class);
 
   public CustomMappingConfigMapDependentResource() {
     super(ConfigMap.class);
@@ -44,7 +46,8 @@ protected ConfigMap desired(DependentCustomMappingCustomResource primary,
   @Override
   protected void addSecondaryToPrimaryMapperAnnotations(ConfigMap desired,
       DependentCustomMappingCustomResource primary) {
-    addSecondaryToPrimaryMapperAnnotations(desired, primary, CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY);
+    addSecondaryToPrimaryMapperAnnotations(desired, primary, CUSTOM_NAME_KEY, CUSTOM_NAMESPACE_KEY,
+        CUSTOM_TYPE_KEY);
   }