GH-324: Rework Kubernetes resource naming & add common unit tests (#326)

Fixes GH-324

Rework the naming of Kubernetes resources for workspaces and sessions
in the common package used by operator and service.

The goal is to make resource names more readable and contain useful information.
Resource names now contain a prefix, optional identifier, user, app definition and
the last segment of the source CRs (session, workspace, app definition) UID.

Also configure unit tests using JUnit 5 for the common maven module
and add some tests for the new name generation.

Misc: Explicitly configure java formatting tab size to 4 spaces:
The setting from the java-formatter.xml does not seem to be respected.
Set the tab width explicitly via `editor.tabSize` to ensure this works independently
of the VSCode settings regarding tab size.

.vscode/settings.json

@@ -16,7 +16,8 @@
   "[java]": {
     "editor.detectIndentation": false,
     "editor.insertSpaces": true,
-    "editor.defaultFormatter": "redhat.java"
+    "editor.defaultFormatter": "redhat.java",
+    "editor.tabSize": 4
   },
   "[dockerfile]": {
     "editor.defaultFormatter": "ms-azuretools.vscode-docker"

java/common/org.eclipse.theia.cloud.common/pom.xml

@@ -31,6 +31,13 @@
             <artifactId>log4j-core</artifactId>
             <version>${log4j.version}</version>
         </dependency>
+
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <version>5.10.3</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>
@@ -43,6 +50,18 @@
                     <target>17</target>
                 </configuration>
             </plugin>
+
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>${surefire-plugin.version}</version>
+                <configuration>
+                    <includes>
+                        <include>**/*Test.java</include>
+                        <include>**/*Tests.java</include>
+                    </includes>
+                </configuration>
+            </plugin>
         </plugins>
     </build>
 

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/util/NamingUtil.java

@@ -1,5 +1,5 @@
 /********************************************************************************
- * Copyright (C) 2022 EclipseSource and others.
+ * Copyright (C) 2022-2024 EclipseSource and others.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v. 2.0 which is available at
@@ -15,7 +15,9 @@
  ********************************************************************************/
 package org.eclipse.theia.cloud.common.util;
 
+import java.util.Arrays;
 import java.util.Locale;
+import java.util.Objects;
 
 import org.eclipse.theia.cloud.common.k8s.resource.appdefinition.AppDefinition;
 import org.eclipse.theia.cloud.common.k8s.resource.session.Session;
@@ -25,8 +27,6 @@ public final class NamingUtil {
 
     public static final int VALID_NAME_LIMIT = 62;
 
-    private static final int MAX_IDENTIFIER_LENGTH = 24;
-
     public static final char VALID_NAME_PREFIX = 'a';
 
     public static final char VALID_NAME_SUFFIX = 'z';
@@ -38,60 +38,105 @@ private NamingUtil() {
     }
 
     /**
-     * Creates a string that can be used as a name for a kubernetes object. When the same arguments are passed to this
+     * @see NamingUtil#createName(AppDefinition, int, String)
+     */
+    public static String createName(AppDefinition appDefinition, int instance) {
+        return createName(appDefinition, instance);
+    }
+
+    /**
+     * Creates a string that can be used as a name for a Kubernetes object. When the same arguments are passed to this
      * method again, the resulting name will be the same. For different arguments the resulting name will be unique in
      * the cluster.
+     * <p>
+     * Typically, giving an <code>identifier</code> is not necessary because the Kubernetes object type (e.g.
+     * Deployment, ConfigMap, ...) already contains this information and names only need to be unique for the same
+     * object type. In turn, it is usually desired to have the same name for objects of different types that belong
+     * together (e.g. deployment and service). An <code>identifier</code> is useful if there are multiple objects of the
+     * same type for an AppDefinition.
+     * </p>
+     * <p>
+     * The created name contains a "session" prefix, the session's user and app definition, the identifier (if given),
+     * and the last segment of the Session's UID. User, app definition and identifier are shortened to keep the name
+     * within Kubernetes' character limit (63) minus 6 characters. The latter allows Kubernetes to add 6 characters at
+     * the end of deployment pod names while the pod names pod names will still contain the whole name of the deployment
+     * </p>
      * 
      * @param appDefinition the {@link AppDefinition}
      * @param instance      instance id
-     * @param identifier    a short description/name of the kubernetes object for which this name will be used. This
-     *                      will be part of the unique sections of the generated name. <b>The combined length of the
-     *                      instance and the identifier must be at most 23 characters long!</b>
+     * @param identifier    an optional short description/name of the Kubernetes object for which this name will be
+     *                      used. <b>This will be shortened to the first 11 characters</b>. May be <code>null</code>.
      * @return the name
      */
     public static String createName(AppDefinition appDefinition, int instance, String identifier) {
-        /*
-         * Kubenertes UIDs are standardized UUIDs/GUIDs. This means the uid string will have a length of 36. Unique part
-         * of the name will consist of the instance followed by the app definition's uuid followed by the identifier.
-         * Parts will be separated with "-". This must be shorter than {@link NamingUtil.VALID_NAME_LIMIT} We fill
-         * remaining space with additional information about the app definition. This may be trimmed away however.
-         */
-        String prefix = instance + "-";
-        return createName(prefix + appDefinition.getMetadata().getUid(), identifier,
-                getAdditionalInformation(appDefinition), MAX_IDENTIFIER_LENGTH - prefix.length());
+        String prefix = "instance-" + instance;
+        return createName(prefix, identifier, null, appDefinition.getSpec().getName(),
+                appDefinition.getMetadata().getUid());
     }
 
     /**
-     * Creates a string that can be used as a name for a kubernetes object. When the same arguments are passed to this
+     * @see NamingUtil#createName(Session, String)
+     */
+    public static String createName(Session session) {
+        return createName("session", null, session.getSpec().getUser(), session.getSpec().getAppDefinition(),
+                session.getMetadata().getUid());
+    }
+
+    /**
+     * Creates a string that can be used as a name for a Kubernetes object. When the same arguments are passed to this
      * method again, the resulting name will be the same. For different arguments the resulting name will be unique in
      * the cluster.
+     * <p>
+     * Typically, giving an <code>identifier</code> is not necessary because the Kubernetes object type (e.g.
+     * Deployment, ConfigMap, ...) already contains this information and names only need to be unique for the same
+     * object type. In turn, it is usually desired to have the same name for objects of different types that belong
+     * together (e.g. deployment and service). An <code>identifier</code> is useful if there are multiple objects of the
+     * same type for a Session.
+     * </p>
+     * <p>
+     * The created name contains a "session" prefix, the session's user and app definition, the identifier (if given),
+     * and the last segment of the Session's UID. User, app definition and identifier are shortened to keep the name
+     * within Kubernetes' character limit (63) minus 6 characters. The latter allows Kubernetes to add 6 characters at
+     * the end of deployment pod names while the pod names pod names will still contain the whole name of the deployment
+     * </p>
      * 
      * @param session    the {@link Session}
-     * @param identifier a short description/name of the kubernetes object for which this name will be used. This will
-     *                   be part of the unique sections of the generated name. <b>Must be at most 24 characters
-     *                   long!</b>
+     * @param identifier an optional short description/name of the Kubernetes object for which this name will be used.
+     *                   <b>This will be shortened to the first 11 characters</b>. May be <code>null</code>.
      * @return the name
      */
     public static String createName(Session session, String identifier) {
-        /*
-         * Kubenertes UIDs are standardized UUIDs/GUIDs. This means the uid string will have a length of 36. Unique part
-         * of the name will consist of the sessions's uuid followed by the identifier. Parts will be separated with "-".
-         * This must be shorter than {@link NamingUtil.VALID_NAME_LIMIT} We fill remaining space with additional
-         * information about the session. This may be trimmed away however.
-         */
-        return createName(session.getMetadata().getUid(), identifier, getAdditionalInformation(session),
-                MAX_IDENTIFIER_LENGTH);
+        return createName("session", identifier, session.getSpec().getUser(), session.getSpec().getAppDefinition(),
+                session.getMetadata().getUid());
+    }
+
+    /**
+     * @see NamingUtil#createName(Workspace, String)
+     */
+    public static String createName(Workspace workspace) {
+        return createName(workspace, null);
     }
 
     /**
-     * Creates a string that can be used as a name for a kubernetes object. When the same arguments are passed to this
+     * Creates a string that can be used as a name for a Kubernetes object. When the same arguments are passed to this
      * method again, the resulting name will be the same. For different arguments the resulting name will be unique in
      * the cluster.
+     * <p>
+     * Typically, giving an <code>identifier</code> is not necessary because the Kubernetes object type (e.g.
+     * Deployment, ConfigMap, ...) already contains this information and names only need to be unique for the same
+     * object type. In turn, it is usually desired to have the same name for objects of different types that belong
+     * together (e.g. deployment and service). An <code>identifier</code> is useful if there are multiple objects of the
+     * same type for a Workspace.
+     * </p>
+     * <p>
+     * The created name contains a "workspace" prefix, the workspace's user and app definition, the identifier (if
+     * given), and the last segment of the Workspace's UID. User, app definition and identifier are shortened to keep
+     * the name within Kubernetes' character limit (63).
+     * </p>
      * 
      * @param workspace  the {@link Workspace}
-     * @param identifier a short description/name of the kubernetes object for which this name will be used. This will
-     *                   be part of the unique sections of the generated name. <b>Must be at most 24 characters
-     *                   long!</b>
+     * @param identifier an optional short description/name of the Kubernetes object for which this name will be used.
+     *                   <b>This will be shortened to the first 11 characters</b>. May be <code>null</code>.
      * @return the name
      */
     public static String createName(Workspace workspace, String identifier) {
@@ -101,43 +146,77 @@ public static String createName(Workspace workspace, String identifier) {
          * This must be shorter than {@link NamingUtil.VALID_NAME_LIMIT} We fill remaining space with additional
          * information about the workspace. This may be trimmed away however.
          */
-        return createName(workspace.getMetadata().getUid(), identifier, getAdditionalInformation(workspace),
-                MAX_IDENTIFIER_LENGTH);
+        return createName("ws", identifier, workspace.getSpec().getUser(), workspace.getSpec().getAppDefinition(),
+                workspace.getMetadata().getUid());
     }
 
     /**
-     * Joins prefix, identifier, and additionalInformation with "-" and enforces conventions to get a valid kubernetes
-     * name.
+     * Builds a valid Kubernetes object names. Except for the prefix, all segments are limited to a fixed number of
+     * characters to ensure that the resulting name includes information from all parameters.
      * 
-     * @param prefix
-     * @param identifier
-     * @param additionalInformation
-     * @param maxIdentifierLength   max length of the identifier
-     * @return the name
-     * @throw {@link IllegalArgumentException} in case the passed identifier is too long
+     * @param prefix        The prefix to start the object name with. Should start with a letter and be at most 13
+     *                      characters long. Longer prefixes are possible but might lead to other info being cut short.
+     * @param identifier    an optional short description/name of the kubernetes object for which this name will be
+     *                      used. <b>This will be shortened to the first 11 characters</b>. May be <code>null</code>.
+     * @param user          The user, may be <code>null</code>
+     * @param appDefinition The app definition name, may be <code>null</code>
+     * @param uid           a unique Kubernetes object id that the name relates to. I.e. the UID of a session when
+     *                      creating the name of a session deployment.
+     * @return the joined and valid Kubernetes name
      */
-    private static String createName(String prefix, String identifier, String additionalInformation,
-            int maxIdentifierLength) {
-        if (identifier.length() > maxIdentifierLength) {
-            throw new IllegalArgumentException(
-                    "Identifier " + identifier + " is too long. Max length is " + maxIdentifierLength);
+    private static String createName(String prefix, String identifier, String user, String appDefinition, String uid) {
+        /*
+         * Kubenertes UIDs are standardized UUIDs/GUIDs. This means the uid string will have a length of 36. We take the
+         * last segment with length 12 to generate unique names for each Session even if user and app definition are the
+         * same.
+         */
+        String shortUid = trimUid(uid);
+
+        // If the user is an email address, only take the part before the @ sign because
+        // this is usually sufficient to identify the user.
+        String userName = user.split("@")[0];
+
+        int infoSegmentLength;
+        String shortenedIdentifier = null;
+        if (identifier == null || identifier.isBlank()) {
+            infoSegmentLength = 17;
+        } else {
+            infoSegmentLength = 11;
+            shortenedIdentifier = trimLength(identifier, infoSegmentLength);
         }
-        return asValidName(String.join("-", prefix, identifier, additionalInformation));
+        String shortUserName = trimLength(userName, infoSegmentLength);
+        String shortAppDef = trimLength(appDefinition, infoSegmentLength);
+
+        return asValidName(prefix, shortenedIdentifier, shortUserName, shortAppDef, shortUid);
     }
 
-    private static String getAdditionalInformation(AppDefinition appDefinition) {
-        return appDefinition.getSpec().getName();
+    /**
+     * Kubenertes UIDs are standardized UUIDs/GUIDs. This means the uid string will have a length of 36. We take the
+     * last segment with length 12 to generate unique names for Kubernetes objects even if other user and app definition
+     * are the same.
+     */
+    private static String trimUid(String uid) {
+        return uid.substring(uid.length() - 12, uid.length());
     }
 
-    private static String getAdditionalInformation(Session session) {
-        String workspace = (session.getSpec().getWorkspace() == null || session.getSpec().getWorkspace().isBlank())
-                ? "none"
-                : session.getSpec().getWorkspace();
-        return session.getSpec().getUser() + "-" + workspace + "-" + session.getSpec().getAppDefinition();
+    private static String trimLength(String text, int maxLength) {
+        if (text == null) {
+            return text;
+        }
+        return text.substring(0, Math.min(text.length(), maxLength));
     }
 
-    private static String getAdditionalInformation(Workspace workspace) {
-        return workspace.getSpec().getName();
+    /**
+     * Joins the given name segments with "-" and enforces conventions to get a valid kubernetes name. Empty or null
+     * segments are removed before joining them.
+     * 
+     * @param segments String segments to join. Segments may be null or empty. These are removed before joining.
+     * @return the name
+     */
+    private static String asValidName(String... segments) {
+        String[] filteredSegments = Arrays.stream(segments).filter(Objects::nonNull).filter(s -> !s.isBlank())
+                .toArray(String[]::new);
+        return asValidName(String.join("-", filteredSegments));
     }
 
     /**

java/common/org.eclipse.theia.cloud.common/src/main/java/org/eclipse/theia/cloud/common/util/WorkspaceUtil.java

@@ -23,7 +23,6 @@
 
 public final class WorkspaceUtil {
     private static final String SESSION_SUFFIX = "-session";
-    private static final String STORAGE_SUFFIX = "pvc";
     private static final String WORKSPACE_PREFIX = "ws-";
     private static final int WORKSPACE_NAME_LIMIT = NamingUtil.VALID_NAME_LIMIT - SESSION_SUFFIX.length();
 
@@ -52,7 +51,7 @@ public static String getSessionName(String user, String appDefinitionName, boole
     }
 
     public static String getStorageName(Workspace workspace) {
-        return NamingUtil.createName(workspace, STORAGE_SUFFIX);
+        return NamingUtil.createName(workspace);
     }
 
     public static String generateWorkspaceLabel(String user, String appDefinitionName) {