From df25706615740c538d448d0c06a5749148fdc5bd Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 18 Feb 2026 10:12:05 +0100 Subject: [PATCH 1/3] fix: Ensure certificates order Signed-off-by: Anatolii Bazko --- .../usernamespace/usernamespace_controller.go | 8 +- pkg/common/constants/constants.go | 37 +++++--- pkg/deploy/tls/certificates.go | 40 +++++--- pkg/deploy/tls/certificates_test.go | 92 +++++++++++++++++++ 4 files changed, 148 insertions(+), 29 deletions(-) diff --git a/controllers/usernamespace/usernamespace_controller.go b/controllers/usernamespace/usernamespace_controller.go index 36dec2920..4e08ef1d9 100644 --- a/controllers/usernamespace/usernamespace_controller.go +++ b/controllers/usernamespace/usernamespace_controller.go @@ -460,7 +460,7 @@ func (r *CheUserNamespaceReconciler) reconcileGitTlsCertificate(ctx context.Cont return delConfigMap() } - if gitCert.Data["ca.crt"] == "" { + if gitCert.Data[constants.GitSelfSignedCertsConfigMapCertKey] == "" { return delConfigMap() } @@ -479,12 +479,12 @@ func (r *CheUserNamespaceReconciler) reconcileGitTlsCertificate(ctx context.Cont }), }, Data: map[string]string{ - "certificate": gitCert.Data["ca.crt"], + "certificate": gitCert.Data[constants.GitSelfSignedCertsConfigMapCertKey], }, } - if gitCert.Data["githost"] != "" { - target.Data["host"] = gitCert.Data["githost"] + if gitCert.Data[constants.GitSelfSignedCertsConfigMapGitHostKey] != "" { + target.Data["host"] = gitCert.Data[constants.GitSelfSignedCertsConfigMapGitHostKey] } _, err := deploy.Sync(deployContext, &target, diffs.ConfigMapAllLabels) diff --git a/pkg/common/constants/constants.go b/pkg/common/constants/constants.go index 2572fc859..50d2b703b 100644 --- a/pkg/common/constants/constants.go +++ b/pkg/common/constants/constants.go @@ -37,22 +37,31 @@ const ( DefaultPluginRegistryCpuRequest = "100m" // Server - DefaultServerMemoryLimit = "1024Mi" - DefaultServerMemoryRequest = "512Mi" - DefaultServerCpuLimit = "1" - DefaultServerCpuRequest = "100m" - DefaultServerLogLevel = "INFO" - DefaultServerDebug = false - DefaultServerMetricsPort = int32(8087) - DefaultServerDebugPort = int32(8000) - DefaultServerPort = int32(8080) + DefaultServerMemoryLimit = "1024Mi" + DefaultServerMemoryRequest = "512Mi" + DefaultServerCpuLimit = "1" + DefaultServerCpuRequest = "100m" + DefaultServerLogLevel = "INFO" + DefaultServerDebug = false + DefaultServerMetricsPort = int32(8087) + DefaultServerDebugPort = int32(8000) + DefaultServerPort = int32(8080) + DefaultProxyCredentialsSecret = "proxy-credentials" + DefaultJavaOpts = "-XX:MaxRAMPercentage=85.0" + DefaultSecurityContextFsGroup = 1724 + DefaultSecurityContextRunAsUser = 1724 + DefaultCheServiceAccountName = "che" + + // Certificates DefaultCaBundleCertsCMName = "ca-certs" - DefaultProxyCredentialsSecret = "proxy-credentials" DefaultGitSelfSignedCertsConfigMapName = "che-git-self-signed-cert" - DefaultJavaOpts = "-XX:MaxRAMPercentage=85.0" - DefaultSecurityContextFsGroup = 1724 - DefaultSecurityContextRunAsUser = 1724 - DefaultCheServiceAccountName = "che" + // GitSelfSignedCertsConfigMapCertKey is the ConfigMap data key that holds the CA certificate + // in the git self-signed certificates ConfigMap. + GitSelfSignedCertsConfigMapCertKey = "ca.crt" + // GitSelfSignedCertsConfigMapGitHostKey is the ConfigMap data key that holds the git server hostname + // in the git self-signed certificates ConfigMap. This value is not a certificate and must be + // excluded when merging CA bundles. + GitSelfSignedCertsConfigMapGitHostKey = "githost" // OAuth BitBucketOAuthConfigClientIdFileName = "id" diff --git a/pkg/deploy/tls/certificates.go b/pkg/deploy/tls/certificates.go index 44d884dda..905fb449a 100644 --- a/pkg/deploy/tls/certificates.go +++ b/pkg/deploy/tls/certificates.go @@ -15,7 +15,9 @@ package tls import ( "errors" "fmt" + "maps" "os" + "slices" "sort" "strings" @@ -213,7 +215,7 @@ func (c *CertificatesReconciler) syncGitTrustedCertificates(ctx *chetypes.Deploy return err == nil, err } - if gitTrustedCertsCM.Data["ca.crt"] != "" { + if gitTrustedCertsCM.Data[constants.GitSelfSignedCertsConfigMapCertKey] != "" { gitTrustedCertsCM.TypeMeta = metav1.TypeMeta{ Kind: "ConfigMap", APIVersion: "v1", @@ -318,22 +320,28 @@ func (c *CertificatesReconciler) syncCheCABundleCerts(ctx *chetypes.DeployContex return false, err } - // Sort configmaps by name, always have the same order and content - // to avoid endless reconcile loop + // Sort ConfigMaps to avoid endless reconcile loop sort.Slice(cheCABundlesCMs, func(i, j int) bool { return strings.Compare(cheCABundlesCMs[i].Name, cheCABundlesCMs[j].Name) < 0 }) - // Calculated revisions and content cheCABundlesContent := "" for _, cm := range cheCABundlesCMs { - for dataKey, dataValue := range cm.Data { - cheCABundlesContent += fmt.Sprintf( - "# ConfigMap: %s, Key: %s\n%s\n\n", - cm.Name, - dataKey, - dataValue, - ) + // Sort keys to produce deterministic output and avoid endless reconcile loop + dataKeys := slices.Collect(maps.Keys(cm.Data)) + sort.Strings(dataKeys) + + for _, dataKey := range dataKeys { + // Skip the "githost" key from the git trusted certs ConfigMap: + // it contains a hostname, not a certificate, and should not be included in the CA bundle. + if ctx.CheCluster.Spec.DevEnvironments.TrustedCerts != nil && + cm.Name == ctx.CheCluster.Spec.DevEnvironments.TrustedCerts.GitTrustedCertsConfigMapName && + dataKey == constants.GitSelfSignedCertsConfigMapGitHostKey { + + continue + } + + cheCABundlesContent += printCert(&cm, dataKey) } } @@ -394,3 +402,13 @@ func readKubernetesCaBundle() ([]byte, error) { return data, nil } + +// printCert formats a single certificate entry with its ConfigMap name and key as a header comment. +func printCert(cm *corev1.ConfigMap, key string) string { + return fmt.Sprintf( + "# ConfigMap: %s, Key: %s\n%s\n\n", + cm.Name, + key, + cm.Data[key], + ) +} diff --git a/pkg/deploy/tls/certificates_test.go b/pkg/deploy/tls/certificates_test.go index 6b438f4cb..6f7f3ccdf 100644 --- a/pkg/deploy/tls/certificates_test.go +++ b/pkg/deploy/tls/certificates_test.go @@ -263,6 +263,98 @@ func TestSyncCheCABundleCerts(t *testing.T) { assert.Equal(t, cm.Data[kubernetesCABundleCertsFile], "# ConfigMap: cert1, Key: a1\nb1\n\n# ConfigMap: cert2, Key: a2\nb2\n\n") } +func TestSyncCheCABundleCertsDeterministicKeyOrder(t *testing.T) { + ctx := test.NewCtxBuilder().WithObjects( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cert1", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": "ca-bundle", + "app.kubernetes.io/part-of": "che.eclipse.org", + }, + }, + Data: map[string]string{ + "z-key": "z-value", + "a-key": "a-value", + "m-key": "m-value", + }, + }).Build() + + certificatesReconciler := NewCertificatesReconciler() + + _, err := certificatesReconciler.syncCheCABundleCerts(ctx) + assert.NoError(t, err) + + cm := &corev1.ConfigMap{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: CheMergedCABundleCertsCMName, Namespace: "eclipse-che"}, cm) + assert.Nil(t, err) + + expected := "# ConfigMap: cert1, Key: a-key\na-value\n\n" + + "# ConfigMap: cert1, Key: m-key\nm-value\n\n" + + "# ConfigMap: cert1, Key: z-key\nz-value\n\n" + assert.Equal(t, expected, cm.Data[kubernetesCABundleCertsFile]) +} + +func TestSyncCheCABundleCertsGitTrustedCertsExcludesGitHostKey(t *testing.T) { + ctx := test.NewCtxBuilder().WithCheCluster( + &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eclipse-che", + Namespace: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + DevEnvironments: chev2.CheClusterDevEnvironments{ + TrustedCerts: &chev2.TrustedCerts{ + GitTrustedCertsConfigMapName: "git-trusted-certs", + }, + }, + }, + }).WithObjects( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "git-trusted-certs", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + Data: map[string]string{ + constants.GitSelfSignedCertsConfigMapCertKey: "git-cert-value", + constants.GitSelfSignedCertsConfigMapGitHostKey: "https://git.example.com", + }, + }, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-cert", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + Data: map[string]string{ + "cert-key": "other-cert-value", + }, + }).Build() + + certificates := NewCertificatesReconciler() + + _, err := certificates.syncCheCABundleCerts(ctx) + assert.NoError(t, err) + + cm := &corev1.ConfigMap{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: CheMergedCABundleCertsCMName, Namespace: "eclipse-che"}, cm) + assert.NoError(t, err) + + // Verify that the githost key is excluded from the merged bundle + expected := "# ConfigMap: git-trusted-certs, Key: ca.crt\ngit-cert-value\n\n" + + "# ConfigMap: other-cert, Key: cert-key\nother-cert-value\n\n" + + assert.Equal(t, expected, cm.Data[kubernetesCABundleCertsFile]) +} + func TestToggleDisableWorkspaceCaBundleMount(t *testing.T) { // Enable workspace CA bundle mount ctx := test.NewCtxBuilder().WithObjects(&corev1.ConfigMap{ From 30d860e29b9cc2bb2ec41a43f573660dc7590988 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 18 Feb 2026 13:30:48 +0100 Subject: [PATCH 2/3] fix: Ensure certificates order Signed-off-by: Anatolii Bazko --- pkg/deploy/tls/certificates.go | 12 +++++++----- pkg/deploy/tls/certificates_test.go | 30 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/pkg/deploy/tls/certificates.go b/pkg/deploy/tls/certificates.go index 905fb449a..8329ca350 100644 --- a/pkg/deploy/tls/certificates.go +++ b/pkg/deploy/tls/certificates.go @@ -320,7 +320,9 @@ func (c *CertificatesReconciler) syncCheCABundleCerts(ctx *chetypes.DeployContex return false, err } - // Sort ConfigMaps to avoid endless reconcile loop + // Sort ConfigMaps by name and their data keys alphabetically to ensure + // deterministic ordering. This prevents spurious reconcile loops that occur + // when Go's random map iteration produces different output each time. sort.Slice(cheCABundlesCMs, func(i, j int) bool { return strings.Compare(cheCABundlesCMs[i].Name, cheCABundlesCMs[j].Name) < 0 }) @@ -334,10 +336,10 @@ func (c *CertificatesReconciler) syncCheCABundleCerts(ctx *chetypes.DeployContex for _, dataKey := range dataKeys { // Skip the "githost" key from the git trusted certs ConfigMap: // it contains a hostname, not a certificate, and should not be included in the CA bundle. - if ctx.CheCluster.Spec.DevEnvironments.TrustedCerts != nil && - cm.Name == ctx.CheCluster.Spec.DevEnvironments.TrustedCerts.GitTrustedCertsConfigMapName && - dataKey == constants.GitSelfSignedCertsConfigMapGitHostKey { - + if dataKey == constants.GitSelfSignedCertsConfigMapGitHostKey && + (cm.Name == constants.DefaultGitSelfSignedCertsConfigMapName || + (ctx.CheCluster.Spec.DevEnvironments.TrustedCerts != nil && + cm.Name == ctx.CheCluster.Spec.DevEnvironments.TrustedCerts.GitTrustedCertsConfigMapName)) { continue } diff --git a/pkg/deploy/tls/certificates_test.go b/pkg/deploy/tls/certificates_test.go index 6f7f3ccdf..4e12a4076 100644 --- a/pkg/deploy/tls/certificates_test.go +++ b/pkg/deploy/tls/certificates_test.go @@ -355,6 +355,36 @@ func TestSyncCheCABundleCertsGitTrustedCertsExcludesGitHostKey(t *testing.T) { assert.Equal(t, expected, cm.Data[kubernetesCABundleCertsFile]) } +func TestSyncCheCABundleCertsExcludesGitHostKeyWithDefaultConfigMap(t *testing.T) { + ctx := test.NewCtxBuilder().WithObjects( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DefaultGitSelfSignedCertsConfigMapName, // "che-git-self-signed-cert" + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + Data: map[string]string{ + constants.GitSelfSignedCertsConfigMapCertKey: "cert-data", + constants.GitSelfSignedCertsConfigMapGitHostKey: "https://git.example.com", + }, + }).Build() + + certificates := NewCertificatesReconciler() + _, err := certificates.syncCheCABundleCerts(ctx) + assert.NoError(t, err) + + cm := &corev1.ConfigMap{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: CheMergedCABundleCertsCMName, Namespace: "eclipse-che"}, cm) + assert.NoError(t, err) + + // Verify githost is excluded even when using default ConfigMap name + expected := "# ConfigMap: che-git-self-signed-cert, Key: ca.crt\ncert-data\n\n" + assert.Equal(t, expected, cm.Data[kubernetesCABundleCertsFile]) +} + func TestToggleDisableWorkspaceCaBundleMount(t *testing.T) { // Enable workspace CA bundle mount ctx := test.NewCtxBuilder().WithObjects(&corev1.ConfigMap{ From d977e3ab9b107f31f91d06df806ad327dca5091c Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Wed, 18 Feb 2026 16:49:18 +0100 Subject: [PATCH 3/3] fix: Ensure certificates order Signed-off-by: Anatolii Bazko --- pkg/deploy/tls/certificates.go | 18 +++- pkg/deploy/tls/certificates_test.go | 137 ++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 4 deletions(-) diff --git a/pkg/deploy/tls/certificates.go b/pkg/deploy/tls/certificates.go index 8329ca350..d386bb8ab 100644 --- a/pkg/deploy/tls/certificates.go +++ b/pkg/deploy/tls/certificates.go @@ -336,10 +336,7 @@ func (c *CertificatesReconciler) syncCheCABundleCerts(ctx *chetypes.DeployContex for _, dataKey := range dataKeys { // Skip the "githost" key from the git trusted certs ConfigMap: // it contains a hostname, not a certificate, and should not be included in the CA bundle. - if dataKey == constants.GitSelfSignedCertsConfigMapGitHostKey && - (cm.Name == constants.DefaultGitSelfSignedCertsConfigMapName || - (ctx.CheCluster.Spec.DevEnvironments.TrustedCerts != nil && - cm.Name == ctx.CheCluster.Spec.DevEnvironments.TrustedCerts.GitTrustedCertsConfigMapName)) { + if dataKey == constants.GitSelfSignedCertsConfigMapGitHostKey && isGitTrustedCertsConfigMap(ctx, &cm) { continue } @@ -414,3 +411,16 @@ func printCert(cm *corev1.ConfigMap, key string) string { cm.Data[key], ) } + +func isGitTrustedCertsConfigMap(ctx *chetypes.DeployContext, cm *corev1.ConfigMap) bool { + if cm.Name == constants.DefaultGitSelfSignedCertsConfigMapName { + return true + } + + if ctx.CheCluster.Spec.DevEnvironments.TrustedCerts != nil && + cm.Name == ctx.CheCluster.Spec.DevEnvironments.TrustedCerts.GitTrustedCertsConfigMapName { + return true + } + + return false +} diff --git a/pkg/deploy/tls/certificates_test.go b/pkg/deploy/tls/certificates_test.go index 4e12a4076..4f3e93a75 100644 --- a/pkg/deploy/tls/certificates_test.go +++ b/pkg/deploy/tls/certificates_test.go @@ -494,3 +494,140 @@ func TestToggleDisableWorkspaceCaBundleMount(t *testing.T) { assert.Equal(t, caCertsMergedCM.Data["tls-ca-bundle.pem"], "# ConfigMap: ca-certs, Key: ca-bundle.crt\nopenshift-ca-bundle-new\n\n") assert.Equal(t, 1, len(caCertsMergedCM.Data)) } + +func TestSyncCheCABundleCertsWithEmptyConfigMap(t *testing.T) { + // A CA bundle ConfigMap exists but has no data entries + emptyCert := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-cert", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + Data: map[string]string{}, + } + ctx := test.NewCtxBuilder().WithObjects(emptyCert).Build() + + certificates := NewCertificatesReconciler() + + _, err := certificates.syncCheCABundleCerts(ctx) + assert.NoError(t, err) + + cm := &corev1.ConfigMap{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: CheMergedCABundleCertsCMName, Namespace: "eclipse-che"}, cm) + assert.NoError(t, err) + + // Merged CM should have no tls-ca-bundle.pem key when source ConfigMap is empty + assert.Empty(t, cm.Data) +} + +func TestSyncCheCABundleCertsWithEmptyAndNonEmptyConfigMaps(t *testing.T) { + emptyCert := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "empty-cert", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + Data: map[string]string{}, + } + nonEmptyCert := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "non-empty-cert", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + Data: map[string]string{"ca.crt": "some-cert"}, + } + ctx := test.NewCtxBuilder().WithObjects(emptyCert, nonEmptyCert).Build() + + certificates := NewCertificatesReconciler() + + _, err := certificates.syncCheCABundleCerts(ctx) + assert.NoError(t, err) + + cm := &corev1.ConfigMap{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: CheMergedCABundleCertsCMName, Namespace: "eclipse-che"}, cm) + assert.NoError(t, err) + + // Only the non-empty ConfigMap's cert should be in the merged bundle + expected := "# ConfigMap: non-empty-cert, Key: ca.crt\nsome-cert\n\n" + assert.Equal(t, expected, cm.Data[kubernetesCABundleCertsFile]) +} + +func TestSyncCheCABundleCertsWithNilDataConfigMap(t *testing.T) { + // A CA bundle ConfigMap exists with nil Data (not initialized) + nilDataCert := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nil-data-cert", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + } + ctx := test.NewCtxBuilder().WithObjects(nilDataCert).Build() + + certificates := NewCertificatesReconciler() + + _, err := certificates.syncCheCABundleCerts(ctx) + assert.NoError(t, err) + + cm := &corev1.ConfigMap{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: CheMergedCABundleCertsCMName, Namespace: "eclipse-che"}, cm) + assert.NoError(t, err) + + // Merged CM should have no tls-ca-bundle.pem key when source ConfigMap has nil Data + assert.Empty(t, cm.Data) +} + +func TestSyncCheCABundleCertsGitTrustedCertsOnlyGitHostKey(t *testing.T) { + // Git trusted certs ConfigMap has only the githost key (no actual cert) + ctx := test.NewCtxBuilder().WithCheCluster( + &chev2.CheCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "eclipse-che", + Namespace: "eclipse-che", + }, + Spec: chev2.CheClusterSpec{ + DevEnvironments: chev2.CheClusterDevEnvironments{ + TrustedCerts: &chev2.TrustedCerts{ + GitTrustedCertsConfigMapName: "git-trusted-certs", + }, + }, + }, + }).WithObjects( + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "git-trusted-certs", + Namespace: "eclipse-che", + Labels: map[string]string{ + "app.kubernetes.io/component": constants.CheCABundle, + "app.kubernetes.io/part-of": constants.CheEclipseOrg, + }, + }, + Data: map[string]string{ + constants.GitSelfSignedCertsConfigMapGitHostKey: "https://git.example.com", + }, + }).Build() + + certificates := NewCertificatesReconciler() + + _, err := certificates.syncCheCABundleCerts(ctx) + assert.NoError(t, err) + + cm := &corev1.ConfigMap{} + err = ctx.ClusterAPI.Client.Get(context.TODO(), types.NamespacedName{Name: CheMergedCABundleCertsCMName, Namespace: "eclipse-che"}, cm) + assert.NoError(t, err) + + // All keys were skipped (githost is excluded), so merged CM should be empty + assert.Empty(t, cm.Data) +}