Fixup: Extract name generation to method and add tests

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
eclipse-che · Oct 24, 2019 · fd98a46 · fd98a46
1 parent 8101022
commit fd98a46
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 12 deletions.
diff --git a/.../workspace/infrastructure/kubernetes/wsplugins/brokerphases/BrokerEnvironmentFactory.java b/.../workspace/infrastructure/kubernetes/wsplugins/brokerphases/BrokerEnvironmentFactory.java
@@ -34,8 +34,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity;
@@ -73,8 +71,6 @@ public abstract class BrokerEnvironmentFactory<E extends KubernetesEnvironment>
   private static final String CONF_FOLDER = "/broker-config";
   private static final String PLUGINS_VOLUME_NAME = "plugins";
   private static final String BROKERS_POD_NAME = "che-plugin-broker";
-  private static final Pattern IMAGE_PATTERN =
-      Pattern.compile("(?<registry>[^/]+/)?(?<org>[^/]+)/(?<image>[^/]+)");
 
   private final ObjectMapper objectMapper = new ObjectMapper();
   private final String cheWebsocketEndpoint;
@@ -157,21 +153,19 @@ private String generateUniqueName(String suffix) {
     return NameGenerator.generate(suffix, 6);
   }
 
+  @VisibleForTesting
+  protected String generateContainerNameFromImageRef(String image) {
+    return image.toLowerCase().replaceAll("[^/]*/", "").replaceAll("[^\\d\\w-]", "-");
+  }
+
   private Container newContainer(
       RuntimeIdentity runtimeId,
       List<EnvVar> envVars,
       String image,
       @Nullable String brokerVolumeName) {
     // There's a chance the full image reference may be over the name limit of 63 chars
     // so we need to remove registry hostname
-    Matcher matcher = IMAGE_PATTERN.matcher(image);
-    String containerName;
-    if (matcher.matches()) {
-      containerName = String.format("%s/%s", matcher.group("org"), matcher.group("image"));
-    } else {
-      containerName = image;
-    }
-    containerName = containerName.toLowerCase().replaceAll("[^\\d\\w-]", "-");
+    String containerName = generateContainerNameFromImageRef(image);
     final ContainerBuilder cb =
         new ContainerBuilder()
             .withName(containerName)

diff --git a/...kspace/infrastructure/kubernetes/wsplugins/brokerphases/BrokerEnvironmentFactoryTest.java b/...kspace/infrastructure/kubernetes/wsplugins/brokerphases/BrokerEnvironmentFactoryTest.java
@@ -42,6 +42,7 @@
 import org.mockito.Mock;
 import org.mockito.testng.MockitoTestNGListener;
 import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
 
@@ -238,4 +239,24 @@ public void shouldCreateConfigMapWithPluginFQNs() throws Exception {
               "Missing field from serialized config: expected '%s' in '%s'", expect, config));
     }
   }
+
+  @Test(dataProvider = "imageRefs")
+  public void testImageToContainerNameConversion(Object image, Object expected) {
+    String actual = factory.generateContainerNameFromImageRef((String) image);
+    assertEquals(
+        actual,
+        expected,
+        String.format("Should remove registry and organization from image '%s'.", image));
+  }
+
+  @DataProvider(name = "imageRefs")
+  public Object[][] imageRefs() {
+    return new Object[][] {
+      {"quay.io/eclipse/che-unified-plugin-broker:v0.20", "che-unified-plugin-broker-v0-20"},
+      {"very-long-registry-hostname-url.service/eclipse/image:tag", "image-tag"},
+      {"eclipse/che-unified-plugin-broker:v0.20", "che-unified-plugin-broker-v0-20"},
+      {"very-long-organization.name-eclipse-che/image:tag", "image-tag"},
+      {"very-long-registry-hostname-url.service/very-long-organization/image:tag", "image-tag"}
+    };
+  }
 }