Skip to content

Commit d8aafce

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1518 from JoaoFula/reenable-proxy-test
OLS-2893 - reenabling proxy test and fixing configmap issue
2 parents 61089fd + f7956e3 commit d8aafce

7 files changed

Lines changed: 273 additions & 29 deletions

File tree

‎internal/controller/appserver/deployment.go‎

Lines changed: 10 additions & 9 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{
@@ -389,7 +389,7 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) (
389389
Annotations: map[string]string{
390390
utils.OLSConfigMapResourceVersionAnnotation: configMapResourceVersion,
391391
utils.OpenShiftMCPServerConfigMapResourceVersionAnnotation: mcpConfigMapResourceVersion,
392-
utils.ProxyCACertResourceVersionAnnotation: proxyCACMResourceVersion,
392+
utils.ProxyCACertHashAnnotation: proxyCACMResourceVersion,
393393
},
394394
},
395395
Spec: appsv1.DeploymentSpec{
@@ -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.ProxyCACertHashAnnotation]
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.ProxyCACertHashAnnotation] = currentProxyCACMHash
590591

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

‎internal/controller/lcore/deployment.go‎

Lines changed: 13 additions & 12 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
@@ -997,7 +997,7 @@ func generateLCoreServerDeployment(r reconciler.Reconciler, ctx context.Context,
997997
utils.LCoreConfigMapResourceVersionAnnotation: lcoreConfigMapResourceVersion,
998998
utils.LlamaStackConfigMapResourceVersionAnnotation: llamaStackConfigMapResourceVersion,
999999
utils.OpenShiftMCPServerConfigMapResourceVersionAnnotation: mcpConfigMapResourceVersion,
1000-
utils.ProxyCACertResourceVersionAnnotation: proxyCACMResourceVersion,
1000+
utils.ProxyCACertHashAnnotation: proxyCACMResourceVersion,
10011001
},
10021002
},
10031003
Spec: appsv1.DeploymentSpec{
@@ -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.ProxyCACertHashAnnotation]
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.ProxyCACertHashAnnotation] = 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
@@ -1275,7 +1276,7 @@ func generateLCoreLibraryDeployment(r reconciler.Reconciler, ctx context.Context
12751276
utils.LCoreConfigMapResourceVersionAnnotation: lcoreConfigMapResourceVersion,
12761277
utils.LlamaStackConfigMapResourceVersionAnnotation: llamaStackConfigMapResourceVersion,
12771278
utils.OpenShiftMCPServerConfigMapResourceVersionAnnotation: mcpConfigMapResourceVersion,
1278-
utils.ProxyCACertResourceVersionAnnotation: proxyCACMResourceVersion,
1279+
utils.ProxyCACertHashAnnotation: proxyCACMResourceVersion,
12791280
},
12801281
},
12811282
Spec: appsv1.DeploymentSpec{

‎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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,8 @@ 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
111-
ProxyCACertResourceVersionAnnotation = "ols.openshift.io/proxy-ca-configmap-version"
110+
// ProxyCACertHashAnnotation is the annotation key for tracking Proxy CA certificate content hash.
111+
ProxyCACertHashAnnotation = "ols.openshift.io/proxy-ca-configmap-hash"
112112
// OLSAppServerContainerPort is the port number of the lightspeed-service-api container exposes
113113
OLSAppServerContainerPort = 8443
114114
// OLSAppServerServicePort is the port number for OLS application server service.

‎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.

0 commit comments

Comments
 (0)