Skip to content

Commit c18b9b6

Browse files
JoaoFulaJoaoFula
committed
checking hash instead of resourceVersion for configmap
checking hash instead of resourceVersion for configmap checking hash instead of resourceVersion for configmap
1 parent 6fca12a commit c18b9b6

7 files changed

Lines changed: 51 additions & 29 deletions

File tree

‎internal/controller/appserver/assets_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2045,11 +2045,11 @@ var _ = Describe("Helper function unit tests", func() {
20452045
It("should return error when proxy CA certificate ConfigMap does not exist", func() {
20462046
cr.Spec.OLSConfig.ProxyConfig = &olsv1alpha1.ProxyConfig{
20472047
ProxyURL: "http://proxy.example.com:8080",
2048-
ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{
2049-
LocalObjectReference: corev1.LocalObjectReference{
2050-
Name: "nonexistent-proxy-ca",
2048+
ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{
2049+
LocalObjectReference: corev1.LocalObjectReference{
2050+
Name: "nonexistent-proxy-ca",
2051+
},
20512052
},
2052-
},
20532053
}
20542054
// Don't create the ConfigMap - validation should fail
20552055
_, err := buildOLSConfig(testReconcilerInstance, ctx, cr, false)

‎internal/controller/appserver/deployment.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,9 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) (
376376
if err != nil && !apierrors.IsNotFound(err) {
377377
return nil, fmt.Errorf("failed to get MCP Server ConfigMap resource version: %w", err)
378378
}
379-
proxyCACMResourceVersion, err := utils.GetProxyCACertResourceVersion(r, ctx, cr)
379+
proxyCACMResourceVersion, err := utils.GetProxyCACertHash(r, ctx, cr)
380380
if err != nil && !apierrors.IsNotFound(err) {
381-
return nil, fmt.Errorf("failed to get Proxy CA ConfigMap resource version: %w", err)
381+
return nil, fmt.Errorf("failed to get Proxy CA certificate hash: %w", err)
382382
}
383383

384384
deployment := appsv1.Deployment{
@@ -559,14 +559,15 @@ func updateOLSDeployment(r reconciler.Reconciler, ctx context.Context, cr *olsv1
559559
}
560560
}
561561

562-
// Step 4: Check if Proxy CA ConfigMap ResourceVersion has changed
563-
currentProxyCACMVersion, err := utils.GetProxyCACertResourceVersion(r, ctx, cr)
562+
// Step 4: Check if Proxy CA certificate content has changed
563+
currentProxyCACMHash, err := utils.GetProxyCACertHash(r, ctx, cr)
564564
if err != nil && !apierrors.IsNotFound(err) {
565-
r.GetLogger().Info("failed to get Proxy CA ConfigMap ResourceVersion", "error", err)
565+
r.GetLogger().Info("failed to get Proxy CA certificate hash", "error", err)
566566
changed = true
567567
} else {
568-
storedProxyCACMVersion := existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation]
569-
if storedProxyCACMVersion != currentProxyCACMVersion {
568+
storedProxyCACMHash := existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation]
569+
if storedProxyCACMHash != currentProxyCACMHash {
570+
r.GetLogger().Info("Proxy CA certificate content changed, updating deployment")
570571
changed = true
571572
}
572573
}
@@ -586,7 +587,7 @@ func updateOLSDeployment(r reconciler.Reconciler, ctx context.Context, cr *olsv1
586587

587588
existingDeployment.Annotations[utils.OLSConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.OLSConfigMapResourceVersionAnnotation]
588589
existingDeployment.Annotations[utils.OpenShiftMCPServerConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.OpenShiftMCPServerConfigMapResourceVersionAnnotation]
589-
existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] = currentProxyCACMVersion
590+
existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] = currentProxyCACMHash
590591

591592
r.GetLogger().Info("updating OLS deployment", "name", existingDeployment.Name)
592593

‎internal/controller/lcore/deployment.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -793,9 +793,9 @@ func generateLCoreServerDeployment(r reconciler.Reconciler, ctx context.Context,
793793
if err != nil && !apierrors.IsNotFound(err) {
794794
return nil, fmt.Errorf("failed to get MCP Server ConfigMap resource version: %w", err)
795795
}
796-
proxyCACMResourceVersion, err := utils.GetProxyCACertResourceVersion(r, ctx, cr)
796+
proxyCACMResourceVersion, err := utils.GetProxyCACertHash(r, ctx, cr)
797797
if err != nil && !apierrors.IsNotFound(err) {
798-
return nil, fmt.Errorf("failed to get Proxy CA ConfigMap resource version: %w", err)
798+
return nil, fmt.Errorf("failed to get Proxy CA certificate hash: %w", err)
799799
}
800800

801801
// Use helper functions to build common components
@@ -1130,14 +1130,15 @@ func updateLCoreDeployment(r reconciler.Reconciler, ctx context.Context, cr *ols
11301130
}
11311131
}
11321132

1133-
// Check if Proxy CA ConfigMap ResourceVersion has changed
1134-
currentProxyCACMVersion, err := utils.GetProxyCACertResourceVersion(r, ctx, cr)
1133+
// Check if Proxy CA certificate content has changed
1134+
currentProxyCACMHash, err := utils.GetProxyCACertHash(r, ctx, cr)
11351135
if err != nil && !apierrors.IsNotFound(err) {
1136-
r.GetLogger().Info("failed to get Proxy CA ConfigMap ResourceVersion", "error", err)
1136+
r.GetLogger().Info("failed to get Proxy CA certificate hash", "error", err)
11371137
changed = true
11381138
} else {
1139-
storedProxyCACMVersion := existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation]
1140-
if storedProxyCACMVersion != currentProxyCACMVersion {
1139+
storedProxyCACMHash := existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation]
1140+
if storedProxyCACMHash != currentProxyCACMHash {
1141+
r.GetLogger().Info("Proxy CA certificate content changed, updating deployment")
11411142
changed = true
11421143
}
11431144
}
@@ -1158,7 +1159,7 @@ func updateLCoreDeployment(r reconciler.Reconciler, ctx context.Context, cr *ols
11581159
existingDeployment.Annotations[utils.LCoreConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.LCoreConfigMapResourceVersionAnnotation]
11591160
existingDeployment.Annotations[utils.LlamaStackConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.LlamaStackConfigMapResourceVersionAnnotation]
11601161
existingDeployment.Annotations[utils.OpenShiftMCPServerConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.OpenShiftMCPServerConfigMapResourceVersionAnnotation]
1161-
existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] = currentProxyCACMVersion
1162+
existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] = currentProxyCACMHash
11621163

11631164
r.GetLogger().Info("updating LCore deployment", "name", existingDeployment.Name)
11641165

@@ -1195,9 +1196,9 @@ func generateLCoreLibraryDeployment(r reconciler.Reconciler, ctx context.Context
11951196
if err != nil && !apierrors.IsNotFound(err) {
11961197
return nil, fmt.Errorf("failed to get MCP Server ConfigMap resource version: %w", err)
11971198
}
1198-
proxyCACMResourceVersion, err := utils.GetProxyCACertResourceVersion(r, ctx, cr)
1199+
proxyCACMResourceVersion, err := utils.GetProxyCACertHash(r, ctx, cr)
11991200
if err != nil && !apierrors.IsNotFound(err) {
1200-
return nil, fmt.Errorf("failed to get Proxy CA ConfigMap resource version: %w", err)
1201+
return nil, fmt.Errorf("failed to get Proxy CA certificate hash: %w", err)
12011202
}
12021203

12031204
// Use helper functions to build common components

‎internal/controller/lcore/reconciler_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ var _ = Describe("LCore reconciliator", Ordered, func() {
229229
cm.Name = proxyCACMName
230230
cm.Namespace = utils.OLSNamespaceDefault
231231
cm.Data = map[string]string{
232-
"ca-bundle.crt": "test-proxy-ca-cert-content",
232+
utils.ProxyCACertFileName: "test-proxy-ca-cert-content",
233233
}
234234
err = k8sClient.Create(ctx, cm)
235235
Expect(err).NotTo(HaveOccurred())

‎internal/controller/utils/constants.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,9 @@ const (
107107
OLSConsoleTLSHashKey = "hash/olsconsoletls"
108108
// AdditionalCAHashKey is the key of the hash value of the additional CA certificates in the deployment annotations
109109
AdditionalCAHashKey = "hash/additionalca"
110-
// ProxyCACertResourceVersionAnnotation is the annotation key for tracking Proxy CA ConfigMap ResourceVersion
110+
// ProxyCACertResourceVersionAnnotation is the annotation key for tracking Proxy CA certificate content hash.
111+
// Despite the name referencing "ResourceVersion" for backward compatibility, this now stores a SHA256 hash
112+
// of the certificate content to ensure deployments only restart when the cert actually changes.
111113
ProxyCACertResourceVersionAnnotation = "ols.openshift.io/proxy-ca-configmap-version"
112114
// OLSAppServerContainerPort is the port number of the lightspeed-service-api container exposes
113115
OLSAppServerContainerPort = 8443

‎internal/controller/utils/utils.go‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package utils
1818
import (
1919
"context"
2020
"crypto/sha1" //nolint:gosec
21+
"crypto/sha256"
2122
"crypto/x509"
2223
"encoding/hex"
2324
"encoding/json"
@@ -798,17 +799,34 @@ func GetConfigMapResourceVersion(r reconciler.Reconciler, ctx context.Context, c
798799
return configMap.ResourceVersion, nil
799800
}
800801

801-
// GetProxyCACertResourceVersion returns the ResourceVersion of the proxy CA ConfigMap
802-
// if proxy CA is configured. Returns empty string and nil error if proxy is not configured.
803-
func GetProxyCACertResourceVersion(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) (string, error) {
802+
// GetProxyCACertHash returns a SHA256 hash of the proxy CA certificate content
803+
// if proxy CA is configured. This ensures deployments only restart when the certificate
804+
// content actually changes, not just when the ConfigMap ResourceVersion changes
805+
// (which can happen frequently for service-ca managed ConfigMaps).
806+
// Returns empty string and nil error if proxy is not configured.
807+
func GetProxyCACertHash(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) (string, error) {
804808
if cr.Spec.OLSConfig.ProxyConfig == nil {
805809
return "", nil
806810
}
807811
cmName := GetProxyCACertConfigMapName(cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef)
808812
if cmName == "" {
809813
return "", nil
810814
}
811-
return GetConfigMapResourceVersion(r, ctx, cmName)
815+
816+
cm := &corev1.ConfigMap{}
817+
err := r.Get(ctx, client.ObjectKey{Name: cmName, Namespace: r.GetNamespace()}, cm)
818+
if err != nil {
819+
return "", err
820+
}
821+
822+
certKey := GetProxyCACertKey(cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef)
823+
certData, ok := cm.Data[certKey]
824+
if !ok {
825+
return "", fmt.Errorf("proxy CA certificate key %s not found in ConfigMap %s", certKey, cmName)
826+
}
827+
828+
hash := sha256.Sum256([]byte(certData))
829+
return hex.EncodeToString(hash[:]), nil
812830
}
813831

814832
// GetSecretResourceVersion returns the ResourceVersion of a Secret.

‎test/e2e/proxy_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const (
3434
// - Creates test infrastructure (squid proxy) and cleans up with DeleteAndWait
3535
// - All tests share a single cluster-scoped OLSConfig CR
3636
// - FlakeAttempts(5) handles transient network and port-forwarding issues
37-
var _ = XDescribe("Proxy test", Ordered, Label("Proxy"), FlakeAttempts(5), func() {
37+
var _ = Describe("Proxy test", Ordered, Label("Proxy"), FlakeAttempts(5), func() {
3838

3939
var cr *olsv1alpha1.OLSConfig
4040
var err error

0 commit comments

Comments
 (0)