From b403fd6febc160fdec033a4cb6fe16fdfd223507 Mon Sep 17 00:00:00 2001 From: Sri Roopa Ramesh Babu Date: Tue, 17 Feb 2026 14:45:08 -0500 Subject: [PATCH] Adding key field to proxyCA config. --- api/v1alpha1/olsconfig_types.go | 19 +++- api/v1alpha1/zz_generated.deepcopy.go | 18 +++- .../bases/ols.openshift.io_olsconfigs.yaml | 11 ++- internal/controller/appserver/assets.go | 11 ++- internal/controller/appserver/assets_test.go | 45 +++++++-- internal/controller/appserver/deployment.go | 23 +++++ internal/controller/appserver/reconciler.go | 22 +---- .../controller/appserver/reconciler_test.go | 9 +- internal/controller/lcore/config.go | 24 ++++- internal/controller/lcore/deployment.go | 58 +++++++++++- internal/controller/lcore/reconciler.go | 4 +- internal/controller/lcore/reconciler_test.go | 7 +- internal/controller/olsconfig_helpers_test.go | 6 +- internal/controller/utils/constants.go | 2 + internal/controller/utils/utils.go | 93 +++++++++++++++++-- test/e2e/proxy_test.go | 48 ++-------- 16 files changed, 307 insertions(+), 93 deletions(-) diff --git a/api/v1alpha1/olsconfig_types.go b/api/v1alpha1/olsconfig_types.go index e295a2d31..d69e28538 100644 --- a/api/v1alpha1/olsconfig_types.go +++ b/api/v1alpha1/olsconfig_types.go @@ -532,9 +532,24 @@ type ProxyConfig struct { // +kubebuilder:validation:Pattern=`^https?://.*$` // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Proxy URL" ProxyURL string `json:"proxyURL,omitempty"` - // The configmap holding proxy CA certificate + // The configmap and key holding proxy CA certificate. + // The key is optional and defaults to "proxy-ca.crt" for backward compatibility. + // If you use a different key name in your ConfigMap, specify it in the Key field of ProxyCACertConfigMapRef. // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Proxy CA Certificate" - ProxyCACertificateRef *corev1.LocalObjectReference `json:"proxyCACertificate,omitempty"` + ProxyCACertificateRef *ProxyCACertConfigMapRef `json:"proxyCACertificate,omitempty"` +} + +// ProxyCACertConfigMapRef references a ConfigMap containing the proxy CA certificate. +// Provides backward compatibility by making the key field optional with a default value. +// +structType=atomic +type ProxyCACertConfigMapRef struct { + // The ConfigMap to select from + corev1.LocalObjectReference `json:",inline"` + // Key in the ConfigMap that contains the proxy CA certificate. + // Defaults to "proxy-ca.crt" if not specified. + // +kubebuilder:default="proxy-ca.crt" + // +optional + Key string `json:"key,omitempty"` } // ToolFilteringConfig defines configuration for tool filtering using hybrid RAG retrieval. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 6c7473bcc..ca9b24d2f 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -529,12 +529,28 @@ func (in *ProviderSpec) DeepCopy() *ProviderSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ProxyCACertConfigMapRef) DeepCopyInto(out *ProxyCACertConfigMapRef) { + *out = *in + out.LocalObjectReference = in.LocalObjectReference +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxyCACertConfigMapRef. +func (in *ProxyCACertConfigMapRef) DeepCopy() *ProxyCACertConfigMapRef { + if in == nil { + return nil + } + out := new(ProxyCACertConfigMapRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ProxyConfig) DeepCopyInto(out *ProxyConfig) { *out = *in if in.ProxyCACertificateRef != nil { in, out := &in.ProxyCACertificateRef, &out.ProxyCACertificateRef - *out = new(corev1.LocalObjectReference) + *out = new(ProxyCACertConfigMapRef) **out = **in } } diff --git a/config/crd/bases/ols.openshift.io_olsconfigs.yaml b/config/crd/bases/ols.openshift.io_olsconfigs.yaml index 229452e42..2350f11eb 100644 --- a/config/crd/bases/ols.openshift.io_olsconfigs.yaml +++ b/config/crd/bases/ols.openshift.io_olsconfigs.yaml @@ -4352,8 +4352,17 @@ spec: such as LLM providers. properties: proxyCACertificate: - description: The configmap holding proxy CA certificate + description: |- + The configmap and key holding proxy CA certificate. + The key is optional and defaults to "proxy-ca.crt" for backward compatibility. + If you use a different key name in your ConfigMap, specify it in the Key field of ProxyCACertConfigMapRef. properties: + key: + default: proxy-ca.crt + description: |- + Key in the ConfigMap that contains the proxy CA certificate. + Defaults to "proxy-ca.crt" if not specified. + type: string name: default: "" description: |- diff --git a/internal/controller/appserver/assets.go b/internal/controller/appserver/assets.go index 573b668e2..026719d8b 100644 --- a/internal/controller/appserver/assets.go +++ b/internal/controller/appserver/assets.go @@ -183,12 +183,15 @@ func GenerateOLSConfigMap(r reconciler.Reconciler, ctx context.Context, cr *olsv ProxyURL: cr.Spec.OLSConfig.ProxyConfig.ProxyURL, ProxyCACertPath: "", } - if cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef != nil && cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef.Name != "" { - err := validateCertificateInConfigMap(r, ctx, cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef.Name, utils.ProxyCACertFileName) + proxyCACertRef := cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef + cmName := utils.GetProxyCACertConfigMapName(proxyCACertRef) + if cmName != "" { + certKey := utils.GetProxyCACertKey(proxyCACertRef) + err := validateCertificateInConfigMap(r, ctx, cmName, certKey) if err != nil { - return nil, fmt.Errorf("failed to validate proxy CA certificate %s: %w", cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef.Name, err) + return nil, fmt.Errorf("failed to validate proxy CA certificate %s: %w", cmName, err) } - proxyConfig.ProxyCACertPath = path.Join(utils.OLSAppCertsMountRoot, utils.ProxyCACertVolumeName, utils.ProxyCACertFileName) + proxyConfig.ProxyCACertPath = path.Join(utils.OLSAppCertsMountRoot, utils.ProxyCACertVolumeName, certKey) } } diff --git a/internal/controller/appserver/assets_test.go b/internal/controller/appserver/assets_test.go index 3008df50b..f4ff97f96 100644 --- a/internal/controller/appserver/assets_test.go +++ b/internal/controller/appserver/assets_test.go @@ -1654,6 +1654,7 @@ user_data_collector_config: {} Context("Proxy settings", func() { const caConfigMapName = "test-ca-configmap" + const proxyURL = "https://proxy.example.com:8080" var proxyCACm *corev1.ConfigMap BeforeEach(func() { @@ -1727,15 +1728,19 @@ user_data_collector_config: {} )) cr.Spec.OLSConfig.ProxyConfig = &olsv1alpha1.ProxyConfig{ - ProxyURL: "https://proxy.example.com:8080", - ProxyCACertificateRef: &corev1.LocalObjectReference{ - Name: caConfigMapName, + ProxyURL: proxyURL, + ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: caConfigMapName, + }, + // No Key specified - tests backward compatibility }, } olsCm, err = GenerateOLSConfigMap(testReconcilerInstance, ctx, cr) Expect(err).NotTo(HaveOccurred()) - Expect(olsCm.Data[utils.OLSConfigFilename]).To(ContainSubstring("proxy_config:\n proxy_ca_cert_path: /etc/certs/proxy-ca/proxy-ca.crt\n proxy_url: https://proxy.example.com:8080\n")) + Expect(olsCm.Data[utils.OLSConfigFilename]).To(ContainSubstring("proxy_ca_cert_path: /etc/certs/proxy-ca/" + utils.ProxyCACertFileName)) + Expect(olsCm.Data[utils.OLSConfigFilename]).To(ContainSubstring("proxy_url: " + proxyURL)) dep, err = GenerateOLSDeployment(testReconcilerInstance, cr) Expect(err).NotTo(HaveOccurred()) @@ -1748,6 +1753,9 @@ user_data_collector_config: {} Name: caConfigMapName, }, DefaultMode: &defaultVolumeMode, + Items: []corev1.KeyToPath{ + {Key: utils.ProxyCACertFileName, Path: utils.ProxyCACertFileName}, + }, }, }, })) @@ -1766,15 +1774,38 @@ user_data_collector_config: {} Expect(err).NotTo(HaveOccurred()) cr.Spec.OLSConfig.ProxyConfig = &olsv1alpha1.ProxyConfig{ - ProxyURL: "https://proxy.example.com:8080", - ProxyCACertificateRef: &corev1.LocalObjectReference{ - Name: caConfigMapName, + ProxyURL: proxyURL, + ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: caConfigMapName, + }, + // No Key specified - tests backward compatibility }, } _, err = GenerateOLSConfigMap(testReconcilerInstance, ctx, cr) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to validate proxy CA certificate")) }) + + It("should use custom Key when ProxyCACertificateRef.Key is set", func() { + const proxyCertKey = "my-proxy-ca.crt" + proxyCACm.Data[proxyCertKey] = utils.TestCACert + err := testReconcilerInstance.Update(ctx, proxyCACm) + Expect(err).NotTo(HaveOccurred()) + + cr.Spec.OLSConfig.ProxyConfig = &olsv1alpha1.ProxyConfig{ + ProxyURL: proxyURL, + ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{ + LocalObjectReference: corev1.LocalObjectReference{Name: caConfigMapName}, + Key: proxyCertKey, + }, + } + + olsCm, err := GenerateOLSConfigMap(testReconcilerInstance, ctx, cr) + Expect(err).NotTo(HaveOccurred()) + Expect(olsCm.Data[utils.OLSConfigFilename]).To(ContainSubstring("proxy_ca_cert_path: /etc/certs/proxy-ca/" + proxyCertKey)) + Expect(olsCm.Data[utils.OLSConfigFilename]).To(ContainSubstring("proxy_url: " + proxyURL)) + }) }) }) diff --git a/internal/controller/appserver/deployment.go b/internal/controller/appserver/deployment.go index 5c10e5328..ffdf5e660 100644 --- a/internal/controller/appserver/deployment.go +++ b/internal/controller/appserver/deployment.go @@ -266,6 +266,7 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) ( // Note: Callback never returns an error, using ForEach for convenient iteration _ = utils.ForEachExternalConfigMap(cr, func(name, source string) error { var volumeName, mountPath string + var items []corev1.KeyToPath switch source { case "additional-ca": volumeName = utils.AdditionalCAVolumeName @@ -273,6 +274,10 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) ( case "proxy-ca": volumeName = utils.ProxyCACertVolumeName mountPath = path.Join(utils.OLSAppCertsMountRoot, utils.ProxyCACertVolumeName) + certKey := utils.GetProxyCACertKey(cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef) + items = []corev1.KeyToPath{ + {Key: certKey, Path: certKey}, + } default: return nil } @@ -283,6 +288,7 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) ( ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{Name: name}, DefaultMode: &volumeDefaultMode, + Items: items, }, }, }) @@ -367,6 +373,8 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) ( return nil, fmt.Errorf("failed to get ConfigMap resource version: %w", err) } + proxyCACMResourceVersion := utils.GetProxyCACertResourceVersion(r, ctx, cr) + deployment := appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: utils.OLSAppServerDeploymentName, @@ -374,6 +382,7 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) ( Labels: utils.GenerateAppServerSelectorLabels(), Annotations: map[string]string{ utils.OLSConfigMapResourceVersionAnnotation: configMapResourceVersion, + utils.ProxyCACertResourceVersionAnnotation: proxyCACMResourceVersion, }, }, Spec: appsv1.DeploymentSpec{ @@ -530,6 +539,13 @@ func updateOLSDeployment(r reconciler.Reconciler, ctx context.Context, existingD } } + // Step 3: Check if Proxy CA ConfigMap ResourceVersion has changed + storedProxyCACMVersion := existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] + currentProxyCACMVersion := desiredDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] + if storedProxyCACMVersion != currentProxyCACMVersion { + changed = true + } + // If nothing changed, skip update if !changed { return nil @@ -537,7 +553,14 @@ func updateOLSDeployment(r reconciler.Reconciler, ctx context.Context, existingD // Apply changes - always update spec and annotations since something changed existingDeployment.Spec = desiredDeployment.Spec + + // Initialize annotations if nil + if existingDeployment.Annotations == nil { + existingDeployment.Annotations = make(map[string]string) + } + existingDeployment.Annotations[utils.OLSConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.OLSConfigMapResourceVersionAnnotation] + existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] = desiredDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] r.GetLogger().Info("updating OLS deployment", "name", existingDeployment.Name) diff --git a/internal/controller/appserver/reconciler.go b/internal/controller/appserver/reconciler.go index 3ba18c22f..be60375fe 100644 --- a/internal/controller/appserver/reconciler.go +++ b/internal/controller/appserver/reconciler.go @@ -64,7 +64,7 @@ func ReconcileAppServerResources(r reconciler.Reconciler, ctx context.Context, o }, { Name: "reconcile Additional CA ConfigMap", - Task: reconcileOLSAdditionalCAConfigMap, + Task: utils.ReconcileOLSAdditionalCAConfigMap, }, { Name: "reconcile Metrics Reader Secret", @@ -76,7 +76,7 @@ func ReconcileAppServerResources(r reconciler.Reconciler, ctx context.Context, o }, { Name: "reconcile Proxy CA ConfigMap", - Task: reconcileProxyCAConfigMap, + Task: utils.ReconcileProxyCAConfigMap, }, { Name: "reconcile ImageStreams", @@ -189,24 +189,6 @@ func reconcileOLSConfigMap(r reconciler.Reconciler, ctx context.Context, cr *ols return nil } -func reconcileOLSAdditionalCAConfigMap(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) error { - if cr.Spec.OLSConfig.AdditionalCAConfigMapRef == nil { - // no additional CA certs, skip - r.GetLogger().Info("Additional CA not configured, reconciliation skipped") - return nil - } - - // Verify the configmap exists (annotation is handled by main controller) - cm := &corev1.ConfigMap{} - err := r.Get(ctx, client.ObjectKey{Name: cr.Spec.OLSConfig.AdditionalCAConfigMapRef.Name, Namespace: r.GetNamespace()}, cm) - if err != nil { - return fmt.Errorf("%s: %w", utils.ErrGetAdditionalCACM, err) - } - - r.GetLogger().Info("additional CA configmap reconciled", "configmap", cm.Name) - return nil -} - func reconcileExporterConfigMap(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) error { // Check if data collector is enabled enabled, err := dataCollectorEnabled(r, cr) diff --git a/internal/controller/appserver/reconciler_test.go b/internal/controller/appserver/reconciler_test.go index d76f67638..0334ed1cc 100644 --- a/internal/controller/appserver/reconciler_test.go +++ b/internal/controller/appserver/reconciler_test.go @@ -981,8 +981,10 @@ var _ = Describe("App server reconciliator", Ordered, func() { By("Set up a proxy CA cert") cr.Spec.OLSConfig.ProxyConfig = &olsv1alpha1.ProxyConfig{ ProxyURL: "https://proxy.example.com:8080", - ProxyCACertificateRef: &corev1.LocalObjectReference{ - Name: cmCACertName, + ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: cmCACertName, + }, }, } err := ReconcileAppServer(testReconcilerInstance, ctx, cr) @@ -1008,6 +1010,9 @@ var _ = Describe("App server reconciliator", Ordered, func() { Name: cmCACertName, }, DefaultMode: &volumeDefaultMode, + Items: []corev1.KeyToPath{ + {Key: utils.ProxyCACertFileName, Path: utils.ProxyCACertFileName}, + }, }, }, }, diff --git a/internal/controller/lcore/config.go b/internal/controller/lcore/config.go index 15bfad0ab..09381b1a3 100644 --- a/internal/controller/lcore/config.go +++ b/internal/controller/lcore/config.go @@ -637,7 +637,7 @@ func buildLCoreServiceConfig(_ reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) // color_log: enable colored logs for DEBUG, disable for production (INFO+) colorLog := logLevel == olsv1alpha1.LogLevelDebug - return map[string]interface{}{ + serviceConfig := map[string]interface{}{ "host": "0.0.0.0", "port": utils.OLSAppServerContainerPort, "auth_enabled": false, @@ -651,6 +651,28 @@ func buildLCoreServiceConfig(_ reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) "tls_key_path": "/etc/certs/lightspeed-tls/tls.key", }, } + + // Add proxy configuration if specified + if cr.Spec.OLSConfig.ProxyConfig != nil { + proxyConfigMap := map[string]interface{}{} + + if cr.Spec.OLSConfig.ProxyConfig.ProxyURL != "" { + proxyConfigMap["proxy_url"] = cr.Spec.OLSConfig.ProxyConfig.ProxyURL + } + + proxyCACertRef := cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef + cmName := utils.GetProxyCACertConfigMapName(proxyCACertRef) + if cmName != "" { + certKey := utils.GetProxyCACertKey(proxyCACertRef) + proxyConfigMap["proxy_ca_cert_path"] = path.Join(utils.OLSAppCertsMountRoot, utils.ProxyCACertVolumeName, certKey) + } + + if len(proxyConfigMap) > 0 { + serviceConfig["proxy_config"] = proxyConfigMap + } + } + + return serviceConfig } func buildLCoreLlamaStackConfig(r reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) map[string]interface{} { diff --git a/internal/controller/lcore/deployment.go b/internal/controller/lcore/deployment.go index ae212aae2..02d30d353 100644 --- a/internal/controller/lcore/deployment.go +++ b/internal/controller/lcore/deployment.go @@ -524,6 +524,36 @@ func addUserCAVolumesAndMounts(volumes *[]corev1.Volume, volumeMounts *[]corev1. }) } +// addProxyCACertVolumeAndMount adds the proxy CA ConfigMap volume and mount if a proxy CA is configured. +func addProxyCACertVolumeAndMount(volumes *[]corev1.Volume, volumeMounts *[]corev1.VolumeMount, cr *olsv1alpha1.OLSConfig, volumeDefaultMode *int32) { + if cr.Spec.OLSConfig.ProxyConfig == nil { + return + } + proxyCACertRef := cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef + cmName := utils.GetProxyCACertConfigMapName(proxyCACertRef) + if cmName == "" { + return + } + certKey := utils.GetProxyCACertKey(proxyCACertRef) + *volumes = append(*volumes, corev1.Volume{ + Name: utils.ProxyCACertVolumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: cmName}, + DefaultMode: volumeDefaultMode, + Items: []corev1.KeyToPath{ + {Key: certKey, Path: certKey}, + }, + }, + }, + }) + *volumeMounts = append(*volumeMounts, corev1.VolumeMount{ + Name: utils.ProxyCACertVolumeName, + MountPath: path.Join(utils.OLSAppCertsMountRoot, utils.ProxyCACertVolumeName), + ReadOnly: true, + }) +} + // addCustomTLSVolumesAndMounts adds user-provided custom TLS certificate volumes and mounts if specified func addCustomTLSVolumesAndMounts(volumes *[]corev1.Volume, volumeMounts *[]corev1.VolumeMount, cr *olsv1alpha1.OLSConfig, volumeDefaultMode *int32) { if cr.Spec.OLSConfig.TLSConfig != nil && cr.Spec.OLSConfig.TLSConfig.KeyCertSecretRef.Name != "" { @@ -714,6 +744,7 @@ func generateLCoreServerDeployment(r reconciler.Reconciler, ctx context.Context, // If they don't exist, we'll get empty strings which is fine for initial creation lcoreConfigMapResourceVersion, _ := utils.GetConfigMapResourceVersion(r, ctx, utils.LCoreConfigCmName) llamaStackConfigMapResourceVersion, _ := utils.GetConfigMapResourceVersion(r, ctx, utils.LlamaStackConfigCmName) + proxyCACMResourceVersion := utils.GetProxyCACertResourceVersion(r, ctx, cr) // Use helper functions to build common components labels := buildCommonLabels() @@ -766,6 +797,9 @@ func generateLCoreServerDeployment(r reconciler.Reconciler, ctx context.Context, // Add user-provided CA certificates to llama-stack container addUserCAVolumesAndMounts(&volumes, &llamaStackVolumeMounts, cr, &volumeDefaultMode) + // Proxy CA ConfigMap volume and mount (for proxy certificate verification) + addProxyCACertVolumeAndMount(&volumes, &llamaStackVolumeMounts, cr, &volumeDefaultMode) + // Build environment variables for LLM providers llamaStackEnvVars, err := buildLlamaStackEnvVars(r, ctx, cr) if err != nil { @@ -908,8 +942,9 @@ func generateLCoreServerDeployment(r reconciler.Reconciler, ctx context.Context, Namespace: r.GetNamespace(), Labels: labels, Annotations: map[string]string{ - utils.LCoreConfigMapResourceVersionAnnotation: lcoreConfigMapResourceVersion, + utils.LCoreConfigMapResourceVersionAnnotation: lcoreConfigMapResourceVersion, utils.LlamaStackConfigMapResourceVersionAnnotation: llamaStackConfigMapResourceVersion, + utils.ProxyCACertResourceVersionAnnotation: proxyCACMResourceVersion, }, }, Spec: appsv1.DeploymentSpec{ @@ -1029,6 +1064,13 @@ func updateLCoreDeployment(r reconciler.Reconciler, ctx context.Context, existin } } + // Check if Proxy CA ConfigMap ResourceVersion has changed + storedProxyCACMVersion := existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] + currentProxyCACMVersion := desiredDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] + if storedProxyCACMVersion != currentProxyCACMVersion { + changed = true + } + // If nothing changed, skip update if !changed { return nil @@ -1036,8 +1078,15 @@ func updateLCoreDeployment(r reconciler.Reconciler, ctx context.Context, existin // Apply changes - always update spec and annotations since something changed existingDeployment.Spec = desiredDeployment.Spec + + // Initialize annotations if nil + if existingDeployment.Annotations == nil { + existingDeployment.Annotations = make(map[string]string) + } + existingDeployment.Annotations[utils.LCoreConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.LCoreConfigMapResourceVersionAnnotation] existingDeployment.Annotations[utils.LlamaStackConfigMapResourceVersionAnnotation] = desiredDeployment.Annotations[utils.LlamaStackConfigMapResourceVersionAnnotation] + existingDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] = desiredDeployment.Annotations[utils.ProxyCACertResourceVersionAnnotation] r.GetLogger().Info("updating LCore deployment", "name", existingDeployment.Name) @@ -1062,6 +1111,7 @@ func generateLCoreLibraryDeployment(r reconciler.Reconciler, ctx context.Context // Get ResourceVersions for tracking lcoreConfigMapResourceVersion, _ := utils.GetConfigMapResourceVersion(r, ctx, utils.LCoreConfigCmName) llamaStackConfigMapResourceVersion, _ := utils.GetConfigMapResourceVersion(r, ctx, utils.LlamaStackConfigCmName) + proxyCACMResourceVersion := utils.GetProxyCACertResourceVersion(r, ctx, cr) // Use helper functions to build common components labels := buildCommonLabels() @@ -1104,6 +1154,9 @@ func generateLCoreLibraryDeployment(r reconciler.Reconciler, ctx context.Context // Add user CA certificates addUserCAVolumesAndMounts(&volumes, &volumeMounts, cr, &volumeDefaultMode) + // Proxy CA ConfigMap volume and mount (for proxy certificate verification) + addProxyCACertVolumeAndMount(&volumes, &volumeMounts, cr, &volumeDefaultMode) + // Add MCP header secrets for HTTP MCP servers if err := addMCPHeaderSecretVolumesAndMounts(r, ctx, &volumes, &volumeMounts, cr, &volumeDefaultMode); err != nil { return nil, err @@ -1132,8 +1185,9 @@ func generateLCoreLibraryDeployment(r reconciler.Reconciler, ctx context.Context Namespace: r.GetNamespace(), Labels: labels, Annotations: map[string]string{ - utils.LCoreConfigMapResourceVersionAnnotation: lcoreConfigMapResourceVersion, + utils.LCoreConfigMapResourceVersionAnnotation: lcoreConfigMapResourceVersion, utils.LlamaStackConfigMapResourceVersionAnnotation: llamaStackConfigMapResourceVersion, + utils.ProxyCACertResourceVersionAnnotation: proxyCACMResourceVersion, }, }, Spec: appsv1.DeploymentSpec{ diff --git a/internal/controller/lcore/reconciler.go b/internal/controller/lcore/reconciler.go index 98b367829..d0274c2b0 100644 --- a/internal/controller/lcore/reconciler.go +++ b/internal/controller/lcore/reconciler.go @@ -65,11 +65,11 @@ func ReconcileLCoreResources(r reconciler.Reconciler, ctx context.Context, olsco }, { Name: "reconcile OLS Additional CA ConfigMap", - Task: reconcileOLSAdditionalCAConfigMap, + Task: utils.ReconcileOLSAdditionalCAConfigMap, }, { Name: "reconcile Proxy CA ConfigMap", - Task: reconcileProxyCAConfigMap, + Task: utils.ReconcileProxyCAConfigMap, }, { Name: "reconcile Metrics Reader Secret", diff --git a/internal/controller/lcore/reconciler_test.go b/internal/controller/lcore/reconciler_test.go index 00d480794..ef4a3372d 100644 --- a/internal/controller/lcore/reconciler_test.go +++ b/internal/controller/lcore/reconciler_test.go @@ -248,8 +248,11 @@ var _ = Describe("LCore reconciliator", Ordered, func() { Expect(err).NotTo(HaveOccurred()) cr.Spec.OLSConfig.ProxyConfig = &olsv1alpha1.ProxyConfig{ ProxyURL: "https://proxy.example.com:8443", - ProxyCACertificateRef: &corev1.LocalObjectReference{ - Name: proxyCACMName, + ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: proxyCACMName, + }, + // Key is omitted - will default to "proxy-ca.crt" for backward compatibility }, } // LCore requires supported Llama Stack provider types diff --git a/internal/controller/olsconfig_helpers_test.go b/internal/controller/olsconfig_helpers_test.go index f991bdc8a..5137ab649 100644 --- a/internal/controller/olsconfig_helpers_test.go +++ b/internal/controller/olsconfig_helpers_test.go @@ -647,8 +647,10 @@ var _ = Describe("Helper Functions", func() { Name: "test-additional-ca", }, ProxyConfig: &olsv1alpha1.ProxyConfig{ - ProxyCACertificateRef: &corev1.LocalObjectReference{ - Name: "test-proxy-ca", + ProxyCACertificateRef: &olsv1alpha1.ProxyCACertConfigMapRef{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "test-proxy-ca", + }, }, }, }, diff --git a/internal/controller/utils/constants.go b/internal/controller/utils/constants.go index 8835eb09a..bd3f1eea1 100644 --- a/internal/controller/utils/constants.go +++ b/internal/controller/utils/constants.go @@ -107,6 +107,8 @@ const ( OLSConsoleTLSHashKey = "hash/olsconsoletls" // AdditionalCAHashKey is the key of the hash value of the additional CA certificates in the deployment annotations AdditionalCAHashKey = "hash/additionalca" + // ProxyCACertResourceVersionAnnotation is the annotation key for tracking Proxy CA ConfigMap ResourceVersion + ProxyCACertResourceVersionAnnotation = "ols.openshift.io/proxy-ca-configmap-version" // OLSAppServerContainerPort is the port number of the lightspeed-service-api container exposes OLSAppServerContainerPort = 8443 // OLSAppServerServicePort is the port number for OLS application server service. diff --git a/internal/controller/utils/utils.go b/internal/controller/utils/utils.go index 44c47984a..28d17533e 100644 --- a/internal/controller/utils/utils.go +++ b/internal/controller/utils/utils.go @@ -727,6 +727,21 @@ func GetConfigMapResourceVersion(r reconciler.Reconciler, ctx context.Context, c return configMap.ResourceVersion, nil } +// GetProxyCACertResourceVersion returns the ResourceVersion of the proxy CA ConfigMap +// if proxy CA is configured. Returns empty string if proxy is not configured or the +// ConfigMap is not found. +func GetProxyCACertResourceVersion(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) string { + if cr.Spec.OLSConfig.ProxyConfig == nil { + return "" + } + cmName := GetProxyCACertConfigMapName(cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef) + if cmName == "" { + return "" + } + version, _ := GetConfigMapResourceVersion(r, ctx, cmName) + return version +} + // GetSecretResourceVersion returns the ResourceVersion of a Secret. func GetSecretResourceVersion(r reconciler.Reconciler, ctx context.Context, secretName string) (string, error) { secret := &corev1.Secret{} @@ -813,12 +828,12 @@ func ForEachExternalConfigMap(cr *olsv1alpha1.OLSConfig, fn func(name string, so } // 2. Proxy CA certificate - if cr.Spec.OLSConfig.ProxyConfig != nil && - cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef != nil && - cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef.Name != "" { - cmName := cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef.Name - if err := fn(cmName, "proxy-ca"); err != nil { - return err + if cr.Spec.OLSConfig.ProxyConfig != nil { + cmName := GetProxyCACertConfigMapName(cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef) + if cmName != "" { + if err := fn(cmName, "proxy-ca"); err != nil { + return err + } } } @@ -881,3 +896,69 @@ func GenerateServiceAccount(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig, } return &sa, nil } + +// GetProxyCACertKey returns the ConfigMap key for the proxy CA certificate. +// If not specified, defaults to ProxyCACertFileName for backward compatibility. +func GetProxyCACertKey(proxyCACertRef *olsv1alpha1.ProxyCACertConfigMapRef) string { + if proxyCACertRef == nil { + return ProxyCACertFileName + } + if proxyCACertRef.Key != "" { + return proxyCACertRef.Key + } + return ProxyCACertFileName // Default for backward compatibility +} + +// GetProxyCACertConfigMapName returns the ConfigMap name for the proxy CA certificate. +// Returns empty string if the reference is nil. +func GetProxyCACertConfigMapName(proxyCACertRef *olsv1alpha1.ProxyCACertConfigMapRef) string { + if proxyCACertRef == nil || proxyCACertRef.Name == "" { + return "" + } + return proxyCACertRef.Name +} + +// ReconcileOLSAdditionalCAConfigMap validates that the externally referenced Additional CA ConfigMap exists. +// Annotation handling is managed by the main controller. +func ReconcileOLSAdditionalCAConfigMap(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) error { + if cr.Spec.OLSConfig.AdditionalCAConfigMapRef == nil { + // no additional CA certs, skip + r.GetLogger().Info("Additional CA not configured, reconciliation skipped") + return nil + } + + cm := &corev1.ConfigMap{} + err := r.Get(ctx, client.ObjectKey{Name: cr.Spec.OLSConfig.AdditionalCAConfigMapRef.Name, Namespace: r.GetNamespace()}, cm) + if err != nil { + return fmt.Errorf("%s: %w", ErrGetAdditionalCACM, err) + } + + r.GetLogger().Info("additional CA configmap reconciled", "configmap", cm.Name) + return nil +} + +// ReconcileProxyCAConfigMap validates that the externally referenced Proxy CA ConfigMap exists. +// Annotation handling is managed by the main controller. +func ReconcileProxyCAConfigMap(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) error { + if cr.Spec.OLSConfig.ProxyConfig == nil { + // no proxy CA certs, skip + r.GetLogger().Info("Proxy CA not configured, reconciliation skipped") + return nil + } + + cmName := GetProxyCACertConfigMapName(cr.Spec.OLSConfig.ProxyConfig.ProxyCACertificateRef) + if cmName == "" { + // no proxy CA certs, skip + r.GetLogger().Info("Proxy CA not configured, reconciliation skipped") + return nil + } + + cm := &corev1.ConfigMap{} + err := r.Get(ctx, client.ObjectKey{Name: cmName, Namespace: r.GetNamespace()}, cm) + if err != nil { + return fmt.Errorf("%s: %w", ErrGetProxyCACM, err) + } + + r.GetLogger().Info("proxy CA configmap reconciled", "configmap", cm.Name) + return nil +} diff --git a/test/e2e/proxy_test.go b/test/e2e/proxy_test.go index 662be3357..b69cfc5a5 100644 --- a/test/e2e/proxy_test.go +++ b/test/e2e/proxy_test.go @@ -26,7 +26,6 @@ const ( httpsPort = 3349 httpPort = 3128 squidConfigName = "squid-config" - proxyConfigmapName = "proxy-ca" ) // Test Design Notes: @@ -52,7 +51,7 @@ var _ = Describe("Proxy test", Ordered, Label("Proxy"), FlakeAttempts(5), func() var secret *corev1.Secret // Helper function to setup proxy configuration - setupProxyConfig := func(proxyURL string, proxyCACertRef *corev1.LocalObjectReference) { + setupProxyConfig := func(proxyURL string, proxyCACertRef *olsv1alpha1.ProxyCACertConfigMapRef) { By("modifying the olsconfig to use proxy") err = client.Update(cr, func(obj ctrlclient.Object) error { cr := obj.(*olsv1alpha1.OLSConfig) @@ -306,32 +305,6 @@ var _ = Describe("Proxy test", Ordered, Label("Proxy"), FlakeAttempts(5), func() err = client.WaitForDeploymentRollout(deployment) Expect(err).NotTo(HaveOccurred()) - By("copy contents of openshift-service-ca.crt to proxy-ca") - serviceCAConfigMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "openshift-service-ca.crt", - Namespace: OLSNameSpace, - }, - } - err = client.Get(serviceCAConfigMap) - Expect(err).NotTo(HaveOccurred()) - serviceCACrt, ok := serviceCAConfigMap.Data["service-ca.crt"] - Expect(ok).To(BeTrue()) - - configmap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: proxyConfigmapName, - Namespace: OLSNameSpace, - }, - Data: map[string]string{ - "proxy-ca.crt": serviceCACrt, - }, - } - err = client.Create(configmap) - Expect(err).NotTo(HaveOccurred()) - err = client.WaitForObjectCreated(configmap) - Expect(err).NotTo(HaveOccurred()) - By("get the squid service hostname") squidService = &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -423,18 +396,8 @@ var _ = Describe("Proxy test", Ordered, Label("Proxy"), FlakeAttempts(5), func() Expect(err).NotTo(HaveOccurred()) } - By("Deleting the proxy configmap") - configmap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: proxyConfigmapName, - Namespace: OLSNameSpace, - }, - } - err = client.DeleteAndWait(configmap, 30*time.Second) - Expect(err).NotTo(HaveOccurred()) - By("Deleting the squid-config configmap") - configmap = &corev1.ConfigMap{ + configmap := &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: squidConfigName, Namespace: OLSNameSpace, @@ -487,8 +450,11 @@ var _ = Describe("Proxy test", Ordered, Label("Proxy"), FlakeAttempts(5), func() }) It("should be able to query the application server with https proxy", func() { - setupProxyConfig("https://"+squidHostname+":"+strconv.Itoa(httpsPort), &corev1.LocalObjectReference{ - Name: "proxy-ca", + setupProxyConfig("https://"+squidHostname+":"+strconv.Itoa(httpsPort), &olsv1alpha1.ProxyCACertConfigMapRef{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "openshift-service-ca.crt", + }, + Key: "service-ca.crt", }) waitForAppServerRollout() setupHTTPSClient()