fail if the provided user name is not k8s compliant (#94)

pkg/cmd/generate/cluster.go

@@ -1,7 +1,11 @@
 package generate
 
 import (
+	"errors"
+	"strings"
+
 	"github.com/kubesaw/ksctl/pkg/configuration"
+	"k8s.io/apimachinery/pkg/util/validation"
 )
 
 type clusterContext struct {
@@ -49,6 +53,9 @@ func ensureUsers(ctx *clusterContext, objsCache objectsCache) error {
 		if user.Selector.ShouldBeSkippedForMember(ctx.specificKMemberName) {
 			continue
 		}
+		if err := validateUserName(user.Name); err != nil {
+			return err
+		}
 		m := &permissionsManager{
 			objectsCache:    objsCache,
 			createSubject:   ensureUserIdentityAndGroups(user.ID, user.Groups),
@@ -67,3 +74,27 @@ func ensureUsers(ctx *clusterContext, objsCache objectsCache) error {
 
 	return nil
 }
+
+func validateUserName(userName string) error {
+	// check if the userName matches the format for k8s resource names
+	validationErrors := validation.IsDNS1123Subdomain(userName)
+	if len(validationErrors) == 0 {
+		// check if the userName matches the format for k8s label values
+		validationErrors = validation.IsValidLabelValue(userName)
+		if len(validationErrors) == 0 {
+			return nil
+		}
+	}
+	errs := make([]error, len(validationErrors))
+	for i := 0; i < len(validationErrors); i++ {
+		// the returned messages from the validators contain strings mentioning
+		// labels (in the case of label value validators), and subdomains (in the
+		// case of resource name validators). This can be confusing so let's replace
+		// these two strings with "User name" so it refers to the name defined in
+		// kubesaw-admins.yaml file.
+		message := strings.ReplaceAll(validationErrors[i], "label", "User name")
+		message = strings.ReplaceAll(message, "subdomain", "User name")
+		errs[i] = errors.New(message)
+	}
+	return errors.Join(errs...)
+}

pkg/cmd/generate/cluster_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/kubesaw/ksctl/pkg/assets"
 	"github.com/kubesaw/ksctl/pkg/configuration"
 	. "github.com/kubesaw/ksctl/pkg/test"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -230,6 +231,26 @@ func TestUsers(t *testing.T) {
 			hasNsClusterRole("toolchain-member-operator", "view", configuration.Member.AsSuffix("clusterrole-view-bob-crtadmin")).
 			hasClusterRoleBinding("cluster-monitoring-view", configuration.Member.AsSuffix("clusterrole-cluster-monitoring-view-bob-crtadmin"))
 	})
+
+	t.Run("returns error for invalid username", func(t *testing.T) {
+		for _, username := range invalidUserNames {
+			t.Run("username: "+username, func(t *testing.T) {
+				// given
+				kubeSawAdmins := newKubeSawAdminsWithDefaultClusters(
+					ServiceAccounts(),
+					Users(User("john+user", []string{"12345"}, true, "", permissionsForAllNamespaces...)),
+				)
+				ctx := newAdminManifestsContextWithDefaultFiles(t, kubeSawAdmins)
+				clusterCtx := newFakeClusterContext(ctx, configuration.Host)
+
+				// when
+				err := ensureUsers(clusterCtx, objectsCache{})
+
+				// then
+				require.Error(t, err)
+			})
+		}
+	})
 }
 
 func newKubeSawAdminsWithDefaultClusters(serviceAccounts []assets.ServiceAccount, users []assets.User) *assets.KubeSawAdmins {
@@ -238,3 +259,36 @@ func newKubeSawAdminsWithDefaultClusters(serviceAccounts []assets.ServiceAccount
 		serviceAccounts,
 		users)
 }
+
+var invalidUserNames = []string{
+	"special#-name", "special:-name", "-name", "name-", "special_name", "specialName",
+	"some-very-very-very-very-very-very-very-very-very-very-very-long-name",
+}
+
+func TestSanitizeUserName(t *testing.T) {
+	t.Run("valid usernames", func(t *testing.T) {
+		require.NoError(t, validateUserName("special-name"))
+		require.NoError(t, validateUserName("special.name"))
+	})
+	t.Run("invalid usernames", func(t *testing.T) {
+		for _, username := range invalidUserNames {
+			t.Run("username: "+username, func(t *testing.T) {
+				// when
+				err := validateUserName(username)
+
+				// then
+				// Let's not hard-code the actual expected errors here. It's something that we don't own,
+				// and it would be just matter of copy-pasting what is in the apimachinery library. Apart
+				// from that, the error are not short which would make the code really messy.
+				require.Error(t, err)
+				// the returned messages from the k8s validators contain strings mentioning
+				// labels (in the case of label value validators), and subdomains (in the
+				// case of resource name validators). The validateUserName function replaced
+				// these words to not make the final error message confusing.
+				// Let's verify that the final error doesn't contain these words.
+				assert.NotContains(t, err.Error(), "label")
+				assert.NotContains(t, err.Error(), "subdomain")
+			})
+		}
+	})
+}