Merge pull request #678 from vdice/fix/trim-k8s-name-to-last-alphanumeric-char

fix(shim_controller): ensure trimmed K8s resource name ends in an alphanumeric char

Author: vdice

internal/controller/shim_controller.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"math"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -481,24 +482,23 @@ func (sr *ShimReconciler) createJobManifest(shim *rcmv1.Shim, node *corev1.Node,
 	}
 	sr.setOperationConfiguration(shim, &opConfig, artifact)
 
-	name := node.Name + "-" + shim.Name + "-" + operation
-	nameMax := int(math.Min(float64(len(name)), K8sNameMaxLength))
+	name := trimK8sName(node.Name + "-" + shim.Name + "-" + operation)
 
 	job := &batchv1.Job{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "batch/v1",
 			Kind:       "Job",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      name[:nameMax],
+			Name:      name,
 			Namespace: os.Getenv("CONTROLLER_NAMESPACE"),
 			Annotations: map[string]string{
 				"spinkube.dev/nodeName":  node.Name,
 				"spinkube.dev/shimName":  shim.Name,
 				"spinkube.dev/operation": operation,
 			},
 			Labels: map[string]string{
-				name[:nameMax]:           "true",
+				name:                     "true",
 				"spinkube.dev/shimName":  shim.Name,
 				"spinkube.dev/operation": operation,
 				"spinkube.dev/job":       "true",
@@ -612,8 +612,7 @@ func (sr *ShimReconciler) handleDeployRuntimeClass(ctx context.Context, shim *rc
 
 // createRuntimeClassManifest creates a RuntimeClass manifest for a Shim.
 func (sr *ShimReconciler) createRuntimeClassManifest(shim *rcmv1.Shim) (*nodev1.RuntimeClass, error) {
-	name := shim.Spec.RuntimeClass.Name
-	nameMax := int(math.Min(float64(len(name)), K8sNameMaxLength))
+	name := trimK8sName(shim.Spec.RuntimeClass.Name)
 
 	nodeSelector := shim.Spec.NodeSelector
 	if nodeSelector == nil {
@@ -626,8 +625,8 @@ func (sr *ShimReconciler) createRuntimeClassManifest(shim *rcmv1.Shim) (*nodev1.
 			Kind:       "RuntimeClass",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:   name[:nameMax],
-			Labels: map[string]string{name[:nameMax]: "true"},
+			Name:   name,
+			Labels: map[string]string{name: "true"},
 		},
 		Handler: shim.Spec.RuntimeClass.Handler,
 		Scheduling: &nodev1.Scheduling{
@@ -730,3 +729,12 @@ func (sr *ShimReconciler) ensureFinalizerForShim(ctx context.Context, shim *rcmv
 func ptr[T any](v T) *T {
 	return &v
 }
+
+// trims the K8s name per the K8sNameMaxLength default and ensures it ends with an alphanumeric character
+func trimK8sName(name string) string {
+	var trailingNonAlphaNumRegex = regexp.MustCompile(`[^a-zA-Z0-9]+$`)
+
+	nameMax := int(math.Min(float64(len(name)), K8sNameMaxLength))
+
+	return trailingNonAlphaNumRegex.ReplaceAllString(name[:nameMax], "")
+}

internal/controller/shim_controller_test.go

@@ -261,3 +261,36 @@ func TestNormalizeArch(t *testing.T) {
 		})
 	}
 }
+
+func TestTrimK8sName(t *testing.T) {
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{
+			"k8s-name",
+			"k8s-name",
+		},
+		{
+			"k8s-name!",
+			"k8s-name",
+		},
+		{
+			"verrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrylong-k8s-nameeeeeeeeeeeeeeeee",
+			"verrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrylong-k8s-nameeeeeeeeeeee",
+		},
+		{
+			"verrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrylong-k8s-name----------------",
+			"verrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrylong-k8s-name",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := trimK8sName(tt.input)
+			if got != tt.want {
+				t.Errorf("trimK8sName(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}