fix: Ensure certificates order (#2087)

* fix: Ensure certificates order

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Author: tolusha

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)

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"

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,27 @@ 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 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
 	})
 
-	// 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 dataKey == constants.GitSelfSignedCertsConfigMapGitHostKey && isGitTrustedCertsConfigMap(ctx, &cm) {
+				continue
+			}
+
+			cheCABundlesContent += printCert(&cm, dataKey)
 		}
 	}
 
@@ -394,3 +401,26 @@ 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],
+	)
+}
+
+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
+}