From 10f80abc343142131c28916535f66e7124ce2fc9 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 23 Mar 2026 10:51:38 +0100 Subject: [PATCH 1/3] chore: use dynamic delimiter for advanced authorization values Signed-off-by: Anatolii Bazko --- pkg/common/utils/utils.go | 11 ++ pkg/deploy/server/server_configmap.go | 32 ++++- pkg/deploy/server/server_configmap_test.go | 141 +++++++++++++++++++++ 3 files changed, 178 insertions(+), 6 deletions(-) diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index f4264e53ce..3896a04668 100644 --- a/pkg/common/utils/utils.go +++ b/pkg/common/utils/utils.go @@ -242,6 +242,17 @@ func FormatLabels(m map[string]string) string { return labels.FormatLabels(m) } +func FindAvailableDelimiter(s string) string { + delimiters := []string{",", "|", ";", ":", "#", "\t"} + 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) { diff --git a/pkg/deploy/server/server_configmap.go b/pkg/deploy/server/server_configmap.go index 4ad150e124..17124564aa 100644 --- a/pkg/deploy/server/server_configmap.go +++ b/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) { diff --git a/pkg/deploy/server/server_configmap_test.go b/pkg/deploy/server/server_configmap_test.go index 54b2ebf0a5..7d0050d0e8 100644 --- a/pkg/deploy/server/server_configmap_test.go +++ b/pkg/deploy/server/server_configmap_test.go @@ -403,3 +403,144 @@ 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"}, + }, + } + + 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) + } + } + }) + } +} From d94e5444b74b92806bc0003e0a0f79fda15a8b38 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Tue, 24 Mar 2026 11:43:22 +0100 Subject: [PATCH 2/3] fixup Signed-off-by: Anatolii Bazko --- pkg/common/utils/utils.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/common/utils/utils.go b/pkg/common/utils/utils.go index 3896a04668..bebc2c5658 100644 --- a/pkg/common/utils/utils.go +++ b/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,8 +244,11 @@ 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 { - delimiters := []string{",", "|", ";", ":", "#", "\t"} for _, delimiter := range delimiters { if !strings.Contains(s, delimiter) { return delimiter From a86bdc0d50466deffd5b9cb3b3cfa5bf711004e6 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 25 Mar 2026 12:29:49 +0100 Subject: [PATCH 3/3] fixup Signed-off-by: Anatolii Bazko --- pkg/deploy/server/server_configmap_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/deploy/server/server_configmap_test.go b/pkg/deploy/server/server_configmap_test.go index 7d0050d0e8..a64772f45c 100644 --- a/pkg/deploy/server/server_configmap_test.go +++ b/pkg/deploy/server/server_configmap_test.go @@ -522,7 +522,10 @@ func TestUpdateAdvancedAuthorizationEnv(t *testing.T) { }, }, }, - expectedData: map[string]string{"CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS": ",|;:#\t@,|;:#\t"}, + expectedData: map[string]string{ + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_ALLOW__USERS": ",|;:#\t@,|;:#\t", + "CHE_INFRA_KUBERNETES_ADVANCED__AUTHORIZATION_DELIMITER": "@", + }, }, }