chore: use dynamic delimiter for advanced authorization values

pkg/common/utils/utils.go

@@ -29,6 +29,8 @@ import (
 	"sigs.k8s.io/yaml"
 )
 
+var delimiters = []string{",", "|", ";", ":", "#", "\t"}
+
 func Contains(list []string, s string) bool {
 	for _, v := range list {
 		if v == s {
@@ -242,6 +244,20 @@ func FormatLabels(m map[string]string) string {
 	return labels.FormatLabels(m)
 }
 
+// FindAvailableDelimiter returns the first delimiter from a predefined list (comma, pipe, semicolon,
+// colon, hash, tab) that is not present in the given string. This is useful for selecting a safe
+// separator character when joining values that may themselves contain common delimiters.
+// Returns an empty string if all candidate delimiters are already present in the input.
+func FindAvailableDelimiter(s string) string {
+	for _, delimiter := range delimiters {
+		if !strings.Contains(s, delimiter) {
+			return delimiter
+		}
+	}
+
+	return ""
+}
+
 // Whitelists the host.
 // Sample: Whitelist("che.yourcompany.com") -> ".yourcompany.com"
 func Whitelist(hostname string) (value string) {

pkg/deploy/server/server_configmap.go

@@ -170,7 +170,9 @@ func (s *CheServerReconciler) getConfigMapData(ctx *chetypes.DeployContext) (che
 	maps.Copy(cheEnv, ctx.CheCluster.Spec.Components.CheServer.ExtraProperties)
 
 	// Updates `CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION__<...>`
-	s.updateAdvancedAuthorizationEnv(ctx, cheEnv)
+	if err := s.updateAdvancedAuthorizationEnv(ctx, cheEnv); err != nil {
+		return nil, err
+	}
 
 	// Updates `CHE_INFRA_KUBERNETES_USER__CLUSTER__ROLES`
 	s.updateUserClusterRoleEnv(ctx, cheEnv)
@@ -183,25 +185,43 @@ func (s *CheServerReconciler) getConfigMapData(ctx *chetypes.DeployContext) (che
 	return cheEnv, nil
 }
 
-func (s *CheServerReconciler) updateAdvancedAuthorizationEnv(ctx *chetypes.DeployContext, cheEnv map[string]string) {
+func (s *CheServerReconciler) updateAdvancedAuthorizationEnv(ctx *chetypes.DeployContext, cheEnv map[string]string) error {
 	if ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization != nil {
+		delimiter := cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER"]
+		if delimiter == "" {
+			allFields := strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowUsers, "") +
+				strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyUsers, "") +
+				strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowGroups, "") +
+				strings.Join(ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyGroups, "")
+
+			delimiter = utils.FindAvailableDelimiter(allFields)
+		}
+
+		if delimiter == "" {
+			return fmt.Errorf("failed to find an available delimiter for advanced authorization values")
+		}
+
 		cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS"] = strings.Join(
 			ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowUsers,
-			",",
+			delimiter,
 		)
 		cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__USERS"] = strings.Join(
 			ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyUsers,
-			",",
+			delimiter,
 		)
 		cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__GROUPS"] = strings.Join(
 			ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.AllowGroups,
-			",",
+			delimiter,
 		)
 		cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__GROUPS"] = strings.Join(
 			ctx.CheCluster.Spec.Networking.Auth.AdvancedAuthorization.DenyGroups,
-			",",
+			delimiter,
 		)
+
+		cheEnv["CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER"] = delimiter
 	}
+
+	return nil
 }
 
 func (s *CheServerReconciler) updateUserClusterRoleEnv(ctx *chetypes.DeployContext, cheEnv map[string]string) {

pkg/deploy/server/server_configmap_test.go

@@ -403,3 +403,147 @@ func TestGetConfigMapDataWithUserClusterRoles(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateAdvancedAuthorizationEnv(t *testing.T) {
+	type testCase struct {
+		name          string
+		cheCluster    *chev2.CheCluster
+		expectedData  map[string]string
+		errorExpected bool
+	}
+
+	testCases := []testCase{
+		{
+			name: "Test advanced authorization with comma (default) delimiter",
+			cheCluster: &chev2.CheCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "eclipse-che",
+					Name:      "eclipse-che",
+				},
+				Spec: chev2.CheClusterSpec{
+					Networking: chev2.CheClusterSpecNetworking{
+						Auth: chev2.Auth{
+							AdvancedAuthorization: &chev2.AdvancedAuthorization{
+								AllowUsers: []string{"user1", "user2"},
+							},
+						},
+					},
+				},
+			},
+			expectedData: map[string]string{
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS": "user1,user2",
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER":    ",",
+			},
+		},
+		{
+			name: "Test advanced authorization picks alternative delimiter",
+			cheCluster: &chev2.CheCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "eclipse-che",
+					Name:      "eclipse-che",
+				},
+				Spec: chev2.CheClusterSpec{
+					Networking: chev2.CheClusterSpecNetworking{
+						Auth: chev2.Auth{
+							AdvancedAuthorization: &chev2.AdvancedAuthorization{
+								AllowUsers:  []string{"user1,a", "user2,b"},
+								DenyUsers:   []string{},
+								AllowGroups: []string{},
+								DenyGroups:  []string{},
+							},
+						},
+					},
+				},
+			},
+			expectedData: map[string]string{
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS":  "user1,a|user2,b",
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__USERS":   "",
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__GROUPS": "",
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DENY__GROUPS":  "",
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER":     "|",
+			},
+		},
+		{
+			name: "Test advanced authorization nil does nothing",
+			cheCluster: &chev2.CheCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "eclipse-che",
+					Name:      "eclipse-che",
+				},
+			},
+			expectedData: map[string]string{},
+		},
+		{
+			name: "Test advanced authorization fails when no delimiter available",
+			cheCluster: &chev2.CheCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "eclipse-che",
+					Name:      "eclipse-che",
+				},
+				Spec: chev2.CheClusterSpec{
+					Networking: chev2.CheClusterSpecNetworking{
+						Auth: chev2.Auth{
+							AdvancedAuthorization: &chev2.AdvancedAuthorization{
+								AllowUsers:  []string{",|;:#\t"},
+								DenyUsers:   []string{},
+								AllowGroups: []string{},
+								DenyGroups:  []string{},
+							},
+						},
+					},
+				},
+			},
+			errorExpected: true,
+		},
+		{
+			name: "Test advanced authorization picks up configured delimiter",
+			cheCluster: &chev2.CheCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "eclipse-che",
+					Name:      "eclipse-che",
+				},
+				Spec: chev2.CheClusterSpec{
+					Components: chev2.CheClusterComponents{
+						CheServer: chev2.CheServer{
+							ExtraProperties: map[string]string{
+								"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER": "@",
+							},
+						},
+					},
+					Networking: chev2.CheClusterSpecNetworking{
+						Auth: chev2.Auth{
+							AdvancedAuthorization: &chev2.AdvancedAuthorization{
+								AllowUsers:  []string{",|;:#\t", ",|;:#\t"},
+								DenyUsers:   []string{},
+								AllowGroups: []string{},
+								DenyGroups:  []string{},
+							},
+						},
+					},
+				},
+			},
+			expectedData: map[string]string{
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS": ",|;:#\t@,|;:#\t",
+				"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER":    "@",
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			ctx := test.NewCtxBuilder().WithCheCluster(testCase.cheCluster).Build()
+
+			serverReconciler := NewCheServerReconciler()
+			cheEnv, err := serverReconciler.getConfigMapData(ctx)
+
+			if testCase.errorExpected {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+				for k, v := range testCase.expectedData {
+					assert.Equal(t, v, cheEnv[k], "key: %s", k)
+				}
+			}
+		})
+	}
+}