improve: better namespace naming for junit extension and custom patte…

…rns (#2171) Signed-off-by: Attila Mészáros <csviri@gmail.com>
operator-framework · Jan 8, 2024 · af84f6a · af84f6a
1 parent 6f2be6a
commit af84f6a
Show file tree

Hide file tree

Showing 9 changed files with 282 additions and 24 deletions.
diff --git a/operator-framework-junit5/pom.xml b/operator-framework-junit5/pom.xml
@@ -39,6 +39,11 @@
       <groupId>org.awaitility</groupId>
       <artifactId>awaitility</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-core</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
 </project>
diff --git a/...ork-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java b/...ork-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java
@@ -4,10 +4,9 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Locale;
-import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
+import java.util.function.Function;
 
 import org.awaitility.Awaitility;
 import org.junit.jupiter.api.extension.AfterAllCallback;
@@ -23,7 +22,6 @@
 import io.fabric8.kubernetes.client.KubernetesClientBuilder;
 import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
-import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
 import io.fabric8.kubernetes.client.utils.Utils;
 import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider;
 
@@ -34,6 +32,7 @@ public abstract class AbstractOperatorExtension implements HasKubernetesClient,
     AfterEachCallback {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(AbstractOperatorExtension.class);
+  public static final int MAX_NAMESPACE_NAME_LENGTH = 63;
   public static final int CRD_READY_WAIT = 2000;
   public static final int DEFAULT_NAMESPACE_DELETE_TIMEOUT = 90;
 
@@ -44,6 +43,8 @@ public abstract class AbstractOperatorExtension implements HasKubernetesClient,
   protected final boolean preserveNamespaceOnError;
   protected final boolean waitForNamespaceDeletion;
   protected final int namespaceDeleteTimeout = DEFAULT_NAMESPACE_DELETE_TIMEOUT;
+  protected final Function<ExtensionContext, String> namespaceNameSupplier;
+  protected final Function<ExtensionContext, String> perClassNamespaceNameSupplier;
 
   protected String namespace;
 
@@ -53,14 +54,18 @@ protected AbstractOperatorExtension(
       boolean oneNamespacePerClass,
       boolean preserveNamespaceOnError,
       boolean waitForNamespaceDeletion,
-      KubernetesClient kubernetesClient) {
+      KubernetesClient kubernetesClient,
+      Function<ExtensionContext, String> namespaceNameSupplier,
+      Function<ExtensionContext, String> perClassNamespaceNameSupplier) {
     this.kubernetesClient = kubernetesClient != null ? kubernetesClient
         : new KubernetesClientBuilder().build();
     this.infrastructure = infrastructure;
     this.infrastructureTimeout = infrastructureTimeout;
     this.oneNamespacePerClass = oneNamespacePerClass;
     this.preserveNamespaceOnError = preserveNamespaceOnError;
     this.waitForNamespaceDeletion = waitForNamespaceDeletion;
+    this.namespaceNameSupplier = namespaceNameSupplier;
+    this.perClassNamespaceNameSupplier = perClassNamespaceNameSupplier;
   }
 
 
@@ -132,26 +137,14 @@ public <T extends HasMetadata> boolean delete(Class<T> type, T resource) {
 
   protected void beforeAllImpl(ExtensionContext context) {
     if (oneNamespacePerClass) {
-      namespace = context.getRequiredTestClass().getSimpleName();
-      namespace += "-";
-      namespace += UUID.randomUUID();
-      namespace = KubernetesResourceUtil.sanitizeName(namespace).toLowerCase(Locale.US);
-      namespace = namespace.substring(0, Math.min(namespace.length(), 63));
-
+      namespace = perClassNamespaceNameSupplier.apply(context);
       before(context);
     }
   }
 
   protected void beforeEachImpl(ExtensionContext context) {
     if (!oneNamespacePerClass) {
-      namespace = context.getRequiredTestClass().getSimpleName();
-      namespace += "-";
-      namespace += context.getRequiredTestMethod().getName();
-      namespace += "-";
-      namespace += UUID.randomUUID();
-      namespace = KubernetesResourceUtil.sanitizeName(namespace).toLowerCase(Locale.US);
-      namespace = namespace.substring(0, Math.min(namespace.length(), 63));
-
+      namespace = namespaceNameSupplier.apply(context);
       before(context);
     }
   }
@@ -219,6 +212,10 @@ public static abstract class AbstractBuilder<T extends AbstractBuilder<T>> {
     protected boolean oneNamespacePerClass;
     protected int namespaceDeleteTimeout;
     protected Consumer<ConfigurationServiceOverrider> configurationServiceOverrider;
+    protected Function<ExtensionContext, String> namespaceNameSupplier =
+        new DefaultNamespaceNameSupplier();
+    protected Function<ExtensionContext, String> perClassNamespaceNameSupplier =
+        new DefaultPerClassNamespaceNameSupplier();
 
     protected AbstractBuilder() {
       this.infrastructure = new ArrayList<>();
@@ -280,5 +277,17 @@ public T withNamespaceDeleteTimeout(int timeout) {
       this.namespaceDeleteTimeout = timeout;
       return (T) this;
     }
+
+    public AbstractBuilder<T> withNamespaceNameSupplier(
+        Function<ExtensionContext, String> namespaceNameSupplier) {
+      this.namespaceNameSupplier = namespaceNameSupplier;
+      return this;
+    }
+
+    public AbstractBuilder<T> withPerClassNamespaceNameSupplier(
+        Function<ExtensionContext, String> perClassNamespaceNameSupplier) {
+      this.perClassNamespaceNameSupplier = perClassNamespaceNameSupplier;
+      return this;
+    }
   }
 }
diff --git a/...it5/src/main/java/io/javaoperatorsdk/operator/junit/ClusterDeployedOperatorExtension.java b/...it5/src/main/java/io/javaoperatorsdk/operator/junit/ClusterDeployedOperatorExtension.java
@@ -11,6 +11,7 @@
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
+import java.util.function.Function;
 
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.slf4j.Logger;
@@ -37,11 +38,13 @@ private ClusterDeployedOperatorExtension(
       boolean preserveNamespaceOnError,
       boolean waitForNamespaceDeletion,
       boolean oneNamespacePerClass,
-      KubernetesClient kubernetesClient) {
+      KubernetesClient kubernetesClient,
+      Function<ExtensionContext, String> namespaceNameSupplier,
+      Function<ExtensionContext, String> perClassNamespaceNameSupplier) {
     super(infrastructure, infrastructureTimeout, oneNamespacePerClass,
         preserveNamespaceOnError,
         waitForNamespaceDeletion,
-        kubernetesClient);
+        kubernetesClient, namespaceNameSupplier, perClassNamespaceNameSupplier);
     this.operatorDeployment = operatorDeployment;
     this.operatorDeploymentTimeout = operatorDeploymentTimeout;
   }
@@ -152,7 +155,9 @@ public ClusterDeployedOperatorExtension build() {
           preserveNamespaceOnError,
           waitForNamespaceDeletion,
           oneNamespacePerClass,
-          kubernetesClient != null ? kubernetesClient : new KubernetesClientBuilder().build());
+          kubernetesClient != null ? kubernetesClient : new KubernetesClientBuilder().build(),
+          namespaceNameSupplier,
+          perClassNamespaceNameSupplier);
     }
   }
 }
diff --git a/...-junit5/src/main/java/io/javaoperatorsdk/operator/junit/DefaultNamespaceNameSupplier.java b/...-junit5/src/main/java/io/javaoperatorsdk/operator/junit/DefaultNamespaceNameSupplier.java
@@ -0,0 +1,49 @@
+package io.javaoperatorsdk.operator.junit;
+
+import java.util.Locale;
+import java.util.UUID;
+import java.util.function.Function;
+
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
+
+import static io.javaoperatorsdk.operator.junit.AbstractOperatorExtension.MAX_NAMESPACE_NAME_LENGTH;
+
+public class DefaultNamespaceNameSupplier implements Function<ExtensionContext, String> {
+
+  public static final int RANDOM_SUFFIX_LENGTH = 5;
+  public static final int DELIMITERS_LENGTH = 2;
+
+  public static final int MAX_NAME_LENGTH_TOGETHER =
+      MAX_NAMESPACE_NAME_LENGTH - DELIMITERS_LENGTH - RANDOM_SUFFIX_LENGTH;
+  public static final int PART_RESERVED_NAME_LENGTH = MAX_NAME_LENGTH_TOGETHER / 2;
+
+  public static final String DELIMITER = "-";
+
+  @Override
+  public String apply(ExtensionContext context) {
+    String classPart = context.getRequiredTestClass().getSimpleName();
+    String methodPart = context.getRequiredTestMethod().getName();
+    if (classPart.length() + methodPart.length() + DELIMITERS_LENGTH
+        + RANDOM_SUFFIX_LENGTH > MAX_NAMESPACE_NAME_LENGTH) {
+      if (classPart.length() > PART_RESERVED_NAME_LENGTH) {
+        int classPartMaxLength =
+            methodPart.length() > PART_RESERVED_NAME_LENGTH ? PART_RESERVED_NAME_LENGTH
+                : MAX_NAME_LENGTH_TOGETHER - methodPart.length();
+        classPart = classPart.substring(0, Math.min(classPartMaxLength, classPart.length()));
+      }
+      if (methodPart.length() > PART_RESERVED_NAME_LENGTH) {
+        int methodPartMaxLength =
+            classPart.length() > PART_RESERVED_NAME_LENGTH ? PART_RESERVED_NAME_LENGTH
+                : MAX_NAME_LENGTH_TOGETHER - classPart.length();
+        methodPart = methodPart.substring(0, Math.min(methodPartMaxLength, methodPart.length()));
+      }
+    }
+
+    String namespace = classPart + DELIMITER + methodPart + DELIMITER + UUID.randomUUID().toString()
+        .substring(0, RANDOM_SUFFIX_LENGTH);
+    namespace = KubernetesResourceUtil.sanitizeName(namespace).toLowerCase(Locale.US);
+    return namespace;
+  }
+}
diff --git a/...src/main/java/io/javaoperatorsdk/operator/junit/DefaultPerClassNamespaceNameSupplier.java b/...src/main/java/io/javaoperatorsdk/operator/junit/DefaultPerClassNamespaceNameSupplier.java
@@ -0,0 +1,32 @@
+package io.javaoperatorsdk.operator.junit;
+
+import java.util.Locale;
+import java.util.UUID;
+import java.util.function.Function;
+
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
+
+import static io.javaoperatorsdk.operator.junit.AbstractOperatorExtension.MAX_NAMESPACE_NAME_LENGTH;
+import static io.javaoperatorsdk.operator.junit.DefaultNamespaceNameSupplier.DELIMITER;
+import static io.javaoperatorsdk.operator.junit.DefaultNamespaceNameSupplier.RANDOM_SUFFIX_LENGTH;
+
+public class DefaultPerClassNamespaceNameSupplier implements Function<ExtensionContext, String> {
+
+  public static final int MAX_CLASS_NAME_LENGTH =
+      MAX_NAMESPACE_NAME_LENGTH - RANDOM_SUFFIX_LENGTH - 1;
+
+  @Override
+  public String apply(ExtensionContext context) {
+    String className = context.getRequiredTestClass().getSimpleName();
+    String namespace =
+        className.length() > MAX_CLASS_NAME_LENGTH ? className.substring(0, MAX_CLASS_NAME_LENGTH)
+            : className;
+    namespace += DELIMITER;
+    namespace += UUID.randomUUID().toString().substring(0, RANDOM_SUFFIX_LENGTH);
+    namespace = KubernetesResourceUtil.sanitizeName(namespace).toLowerCase(Locale.US);
+    namespace = namespace.substring(0, Math.min(namespace.length(), MAX_NAMESPACE_NAME_LENGTH));
+    return namespace;
+  }
+}
diff --git a/...k-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java b/...k-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java
@@ -9,6 +9,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -53,14 +54,18 @@ private LocallyRunOperatorExtension(
       boolean waitForNamespaceDeletion,
       boolean oneNamespacePerClass,
       KubernetesClient kubernetesClient,
-      Consumer<ConfigurationServiceOverrider> configurationServiceOverrider) {
+      Consumer<ConfigurationServiceOverrider> configurationServiceOverrider,
+      Function<ExtensionContext, String> namespaceNameSupplier,
+      Function<ExtensionContext, String> perClassNamespaceNameSupplier) {
     super(
         infrastructure,
         infrastructureTimeout,
         oneNamespacePerClass,
         preserveNamespaceOnError,
         waitForNamespaceDeletion,
-        kubernetesClient);
+        kubernetesClient,
+        namespaceNameSupplier,
+        perClassNamespaceNameSupplier);
     this.reconcilers = reconcilers;
     this.portForwards = portForwards;
     this.localPortForwards = new ArrayList<>(portForwards.size());
@@ -285,7 +290,7 @@ public LocallyRunOperatorExtension build() {
           waitForNamespaceDeletion,
           oneNamespacePerClass,
           kubernetesClient,
-          configurationServiceOverrider);
+          configurationServiceOverrider, namespaceNameSupplier, perClassNamespaceNameSupplier);
     }
   }
 

diff --git a/...it5/src/test/java/io/javaoperatorsdk/operator/junit/DefaultNamespaceNameSupplierTest.java b/...it5/src/test/java/io/javaoperatorsdk/operator/junit/DefaultNamespaceNameSupplierTest.java
@@ -0,0 +1,56 @@
+package io.javaoperatorsdk.operator.junit;
+
+import org.junit.jupiter.api.Test;
+
+import static io.javaoperatorsdk.operator.junit.AbstractOperatorExtension.MAX_NAMESPACE_NAME_LENGTH;
+import static io.javaoperatorsdk.operator.junit.DefaultNamespaceNameSupplier.*;
+import static io.javaoperatorsdk.operator.junit.NamespaceNamingTestUtils.*;
+import static org.assertj.core.api.Assertions.assertThat;
+
+class DefaultNamespaceNameSupplierTest {
+
+
+  DefaultNamespaceNameSupplier supplier = new DefaultNamespaceNameSupplier();
+
+  @Test
+  void trivialCase() {
+    String ns = supplier.apply(mockExtensionContext(SHORT_CLASS_NAME, SHORT_METHOD_NAME));
+
+    assertThat(ns).startsWith(SHORT_CLASS_NAME + DELIMITER + SHORT_METHOD_NAME + DELIMITER);
+    shortEnoughAndEndsWithRandomString(ns);
+  }
+
+  @Test
+  void classPartLongerCase() {
+    String ns = supplier.apply(mockExtensionContext(LONG_CLASS_NAME, SHORT_METHOD_NAME));
+
+    assertThat(ns).startsWith(LONG_CLASS_NAME + DELIMITER + SHORT_METHOD_NAME + DELIMITER);
+    shortEnoughAndEndsWithRandomString(ns);
+  }
+
+  @Test
+  void methodPartLonger() {
+    String ns = supplier.apply(mockExtensionContext(SHORT_CLASS_NAME, LONG_METHOD_NAME));
+
+    assertThat(ns).startsWith(SHORT_CLASS_NAME + DELIMITER + LONG_METHOD_NAME + DELIMITER);
+    shortEnoughAndEndsWithRandomString(ns);
+  }
+
+  @Test
+  void methodPartAndClassPartLonger() {
+    String ns = supplier.apply(mockExtensionContext(LONG_CLASS_NAME, LONG_METHOD_NAME));
+
+    assertThat(ns).startsWith(LONG_CLASS_NAME.substring(0, PART_RESERVED_NAME_LENGTH) + DELIMITER
+        + LONG_METHOD_NAME.substring(0, PART_RESERVED_NAME_LENGTH)
+        + DELIMITER);
+    shortEnoughAndEndsWithRandomString(ns);
+  }
+
+
+  private static void shortEnoughAndEndsWithRandomString(String ns) {
+    assertThat(ns.length()).isLessThanOrEqualTo(MAX_NAMESPACE_NAME_LENGTH);
+    assertThat(ns.split("-")[2]).hasSize(RANDOM_SUFFIX_LENGTH);
+  }
+
+
+}
diff --git a/...test/java/io/javaoperatorsdk/operator/junit/DefaultPerClassNamespaceNameSupplierTest.java b/...test/java/io/javaoperatorsdk/operator/junit/DefaultPerClassNamespaceNameSupplierTest.java
@@ -0,0 +1,44 @@
+package io.javaoperatorsdk.operator.junit;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+import static io.javaoperatorsdk.operator.junit.AbstractOperatorExtension.MAX_NAMESPACE_NAME_LENGTH;
+import static io.javaoperatorsdk.operator.junit.DefaultNamespaceNameSupplier.DELIMITER;
+import static io.javaoperatorsdk.operator.junit.DefaultNamespaceNameSupplier.RANDOM_SUFFIX_LENGTH;
+import static io.javaoperatorsdk.operator.junit.DefaultPerClassNamespaceNameSupplier.MAX_CLASS_NAME_LENGTH;
+import static io.javaoperatorsdk.operator.junit.NamespaceNamingTestUtils.SHORT_CLASS_NAME;
+import static io.javaoperatorsdk.operator.junit.NamespaceNamingTestUtils.VERY_LONG_CLASS_NAME;
+import static org.assertj.core.api.Assertions.assertThat;
+
+class DefaultPerClassNamespaceNameSupplierTest {
+
+  DefaultPerClassNamespaceNameSupplier supplier = new DefaultPerClassNamespaceNameSupplier();
+
+  @Test
+  void shortClassCase() {
+    var ns = supplier.apply(mockExtensionContext(SHORT_CLASS_NAME));
+
+    assertThat(ns).startsWith(SHORT_CLASS_NAME + DELIMITER);
+    shortEnoughAndEndsWithRandomString(ns);
+  }
+
+  @Test
+  void longClassCase() {
+    var ns = supplier.apply(mockExtensionContext(VERY_LONG_CLASS_NAME));
+
+    assertThat(ns).startsWith(VERY_LONG_CLASS_NAME.substring(0, MAX_CLASS_NAME_LENGTH) + DELIMITER);
+    shortEnoughAndEndsWithRandomString(ns);
+    assertThat(ns).hasSize(MAX_NAMESPACE_NAME_LENGTH);
+  }
+
+  public static ExtensionContext mockExtensionContext(String className) {
+    return NamespaceNamingTestUtils.mockExtensionContext(className, null);
+  }
+
+  private static void shortEnoughAndEndsWithRandomString(String ns) {
+    assertThat(ns.length()).isLessThanOrEqualTo(MAX_NAMESPACE_NAME_LENGTH);
+    assertThat(ns.split("-")[1]).hasSize(RANDOM_SUFFIX_LENGTH);
+  }
+
+}