Merge pull request jenkinsci#1690 from cronik/feature/ephemeral-containers-pod-utils

Extract pod name generation functions

Author: amuniz

src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesSlave.java

@@ -34,7 +34,6 @@
 import java.io.IOException;
 import java.time.Instant;
 import java.util.HashSet;
-import java.util.Locale;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -47,7 +46,6 @@
 import jenkins.metrics.api.Metrics;
 import jenkins.model.Jenkins;
 import jenkins.security.MasterToSlaveCallable;
-import org.apache.commons.lang.RandomStringUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.Validate;
 import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention;
@@ -302,19 +300,16 @@ private static KubernetesCloud getKubernetesCloud(String cloudName) {
     }
 
     static String getSlaveName(PodTemplate template) {
-        String randString = RandomStringUtils.random(5, "bcdfghjklmnpqrstvwxz0123456789");
         String name = template.getName();
         if (StringUtils.isEmpty(name)) {
-            return String.format("%s-%s", DEFAULT_AGENT_PREFIX, randString);
+            name = DEFAULT_AGENT_PREFIX;
         }
-        // no spaces
-        name = name.replaceAll("[ _]", "-").toLowerCase(Locale.getDefault());
-        // keep it under 63 chars (62 is used to account for the '-')
-        name = name.substring(0, Math.min(name.length(), 62 - randString.length()));
-        String slaveName = String.format("%s-%s", name, randString);
-        if (!slaveName.matches("[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*")) {
-            return String.format("%s-%s", DEFAULT_AGENT_PREFIX, randString);
+
+        String slaveName = PodUtils.createNameWithRandomSuffix(name);
+        if (!PodUtils.isValidName(slaveName)) {
+            slaveName = PodUtils.createNameWithRandomSuffix(DEFAULT_AGENT_PREFIX);
         }
+
         return slaveName;
     }
 

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodUtils.java

@@ -29,12 +29,15 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 import java.util.Optional;
 import java.util.function.Predicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import jenkins.model.Jenkins;
+import org.apache.commons.lang.RandomStringUtils;
 import org.apache.commons.lang.StringUtils;
 import org.csanchez.jenkins.plugins.kubernetes.pipeline.PodTemplateStepExecution;
 
@@ -43,6 +46,9 @@ private PodUtils() {}
 
     private static final Logger LOGGER = Logger.getLogger(PodUtils.class.getName());
 
+    private static final Pattern NAME_PATTERN =
+            Pattern.compile("[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*");
+
     public static final Predicate<ContainerStatus> CONTAINER_IS_TERMINATED =
             cs -> cs.getState().getTerminated() != null;
     public static final Predicate<ContainerStatus> CONTAINER_IS_WAITING =
@@ -57,15 +63,27 @@ public static List<ContainerStatus> getWaitingContainers(Pod pod) {
         return getContainers(pod, CONTAINER_IS_WAITING);
     }
 
-    public static List<ContainerStatus> getContainerStatus(Pod pod) {
+    /**
+     * Get all container statuses (does not include ephemeral or init containers).
+     * @param pod pod to get container statuses for
+     * @return list of statuses, possibly empty, never null
+     */
+    @NonNull
+    public static List<ContainerStatus> getContainerStatus(@NonNull Pod pod) {
         PodStatus podStatus = pod.getStatus();
         if (podStatus == null) {
             return Collections.emptyList();
         }
         return podStatus.getContainerStatuses();
     }
 
-    public static List<ContainerStatus> getContainers(Pod pod, Predicate<ContainerStatus> predicate) {
+    /**
+     * Get pod container statuses (does not include ephemeral or init containers) that match the given filter.
+     * @param pod pod to get container statuses for
+     * @param predicate container status predicate
+     * @return list of statuses, possibly empty, never null
+     */
+    public static List<ContainerStatus> getContainers(@NonNull Pod pod, @NonNull Predicate<ContainerStatus> predicate) {
         return getContainerStatus(pod).stream().filter(predicate).collect(Collectors.toList());
     }
 
@@ -182,4 +200,39 @@ public static String logLastLines(@NonNull Pod pod, @NonNull KubernetesClient cl
         }
         return Util.fixEmpty(sb.toString());
     }
+
+    /**
+     * Generate a random string to be used as the suffix for dynamic resource names.
+     * @return random string suitable for kubernetes resources
+     */
+    @NonNull
+    public static String generateRandomSuffix() {
+        return RandomStringUtils.random(5, "bcdfghjklmnpqrstvwxz0123456789");
+    }
+
+    /**
+     * Create kubernetes resource name with a random suffix appended to the given base name. This method
+     * performs some basic transforms to make the base name compliant (i.e. spaces and underscores). The
+     * returned string will also be truncated to a max of 63 characters.
+     * @param name base name to append to
+     * @return resource name with random suffix and maximum length of 63 characters
+     */
+    @NonNull
+    public static String createNameWithRandomSuffix(@NonNull String name) {
+        String suffix = generateRandomSuffix();
+        // no spaces
+        name = name.replaceAll("[ _]", "-").toLowerCase(Locale.getDefault());
+        // keep it under 63 chars (62 is used to account for the '-')
+        name = name.substring(0, Math.min(name.length(), 62 - suffix.length()));
+        return String.join("-", name, suffix);
+    }
+
+    /**
+     * Check if the given name is a valid pod resource name. Does not validate string length.
+     * @param name name to check
+     * @return true if the given string contains valid pod resource name characters
+     */
+    public static boolean isValidName(@NonNull String name) {
+        return PodUtils.NAME_PATTERN.matcher(name).matches();
+    }
 }

src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodUtilsTest.java

@@ -0,0 +1,59 @@
+package org.csanchez.jenkins.plugins.kubernetes;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.IntStream;
+import org.apache.commons.lang.StringUtils;
+import org.junit.Test;
+
+public class PodUtilsTest {
+
+    @Test
+    public void generateRandomSuffix() {
+        List<String> generated = IntStream.range(0, 100)
+                .mapToObj(i -> PodUtils.generateRandomSuffix())
+                .toList();
+        Set<String> unique = new HashSet<>(generated);
+        assertEquals(generated.size(), unique.size());
+
+        for (String suffix : generated) {
+            assertEquals(5, suffix.length());
+            assertTrue(PodUtils.isValidName(suffix));
+        }
+    }
+
+    @Test
+    public void createNameWithRandomSuffix() {
+        String name = PodUtils.createNameWithRandomSuffix("foo");
+        assertEquals(9, name.length());
+        assertTrue(name.startsWith("foo-"));
+        assertTrue(PodUtils.isValidName(name));
+
+        // names with invalid characters
+        name = PodUtils.createNameWithRandomSuffix("foo bar_cat");
+        assertEquals(17, name.length());
+        assertTrue(name.startsWith("foo-bar-cat-"));
+
+        // very long names
+        name = PodUtils.createNameWithRandomSuffix(StringUtils.repeat("a", 70));
+        assertEquals(63, name.length());
+        assertTrue(name.startsWith(StringUtils.repeat("a", 57) + "-"));
+    }
+
+    @Test
+    public void isValidName() {
+        assertTrue(PodUtils.isValidName("foo"));
+        assertTrue(PodUtils.isValidName("foo-bar"));
+        assertTrue(PodUtils.isValidName("foo.bar"));
+        assertTrue(PodUtils.isValidName("foo.123"));
+        assertTrue(PodUtils.isValidName("123-foo"));
+        assertFalse(PodUtils.isValidName("foo bar"));
+        assertFalse(PodUtils.isValidName("-foo"));
+        assertFalse(PodUtils.isValidName(".foo"));
+    }
+}