From 45763146a953e3e30608274fa77b247b2c124287 Mon Sep 17 00:00:00 2001
From: Prashant Jaikumar <pjaikumar@google.com>
Date: Mon, 8 Jan 2024 19:44:21 +0000
Subject: [PATCH] [CDAP-20933] Fix k8s resource name cleaning logic

---
 .../io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java | 13 ++++++++++---
 .../cdap/k8s/runtime/KubeTwillPreparerTest.java     | 13 +++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java b/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java
index 46797f0c77b4..e00b3364073f 100644
--- a/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java
+++ b/cdap-kubernetes/src/main/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparer.java
@@ -104,6 +104,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.twill.api.ClassAcceptor;
 import org.apache.twill.api.Configs;
 import org.apache.twill.api.LocalFile;
@@ -780,11 +781,17 @@ private String asLabel(String val) {
 
   /**
    * Kubernetes names must be lowercase alphanumeric, or '-'. Some are less restrictive, but those
-   * characters should always be ok.
+   * characters should always be ok. They should also start and end with an alphanumeric.
    */
-  private String cleanse(String val, int maxLength) {
+  @VisibleForTesting
+  static String cleanse(String val, int maxLength) {
+    // Replace characters that are not alphanumeric or '=' with '-'
     String cleansed = val.replaceAll("[^A-Za-z0-9\\-]", "-").toLowerCase();
-    return cleansed.length() > maxLength ? cleansed.substring(0, maxLength) : cleansed;
+    // Truncate to maxLength if needed
+    cleansed = cleansed.length() > maxLength ? cleansed.substring(0, maxLength) : cleansed;
+    // Remove leading and trailing '-'
+    cleansed = StringUtils.stripEnd(cleansed, "-");
+    return StringUtils.stripStart(cleansed, "-");
   }
 
   /**
diff --git a/cdap-kubernetes/src/test/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparerTest.java b/cdap-kubernetes/src/test/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparerTest.java
index 5200987f1f12..7014c4ce8ea0 100644
--- a/cdap-kubernetes/src/test/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparerTest.java
+++ b/cdap-kubernetes/src/test/java/io/cdap/cdap/k8s/runtime/KubeTwillPreparerTest.java
@@ -382,6 +382,19 @@ public void testCreatePodSpecSystemNamespaceWithConfigmapOption()
         .anyMatch(v -> v.getName().equals("cdap-config-abc-123") && v.getMountPath().equals("/config")));
   }
 
+  @Test
+  public void testResourceNameCleanse() {
+    // Trailing '-' is stripped
+    Assert.assertEquals("name", KubeTwillPreparer.cleanse("name-1",
+        5));
+    // Leading '-' removed
+    Assert.assertEquals("name", KubeTwillPreparer.cleanse("--name",
+        6));
+    // '_' is replaced with '-'
+    Assert.assertEquals("name-1", KubeTwillPreparer.cleanse("name_1",
+        6));
+  }
+
   private static Map<String, String> getTwillConfigs() {
     HashMap<String, String> cConf = new HashMap<>();
     cConf.put(Configs.Keys.JAVA_RESERVED_MEMORY_MB, "1024");