Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions api/v1alpha1/olsconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 17 additions & 1 deletion api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 10 additions & 1 deletion config/crd/bases/ols.openshift.io_olsconfigs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There definitely should be a default value for the key matching the value we currently expect. We should not require clients who have gotten this to work to now have to specify the 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: |-
Expand Down
11 changes: 7 additions & 4 deletions internal/controller/appserver/assets.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
45 changes: 38 additions & 7 deletions internal/controller/appserver/assets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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())
Expand All @@ -1748,6 +1753,9 @@ user_data_collector_config: {}
Name: caConfigMapName,
},
DefaultMode: &defaultVolumeMode,
Items: []corev1.KeyToPath{
{Key: utils.ProxyCACertFileName, Path: utils.ProxyCACertFileName},
},
},
},
}))
Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new Key field is not exercised in tests, can look like this:

		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: "https://proxy.example.com:8080",
				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: https://proxy.example.com:8080"))
		})

Can make proxyURL a const while we're at it.

},
}
_, 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))
})
})
})

Expand Down
23 changes: 23 additions & 0 deletions internal/controller/appserver/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,18 @@ 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
mountPath = UserCAMountPath
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
}
Expand All @@ -283,6 +288,7 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) (
ConfigMap: &corev1.ConfigMapVolumeSource{
LocalObjectReference: corev1.LocalObjectReference{Name: name},
DefaultMode: &volumeDefaultMode,
Items: items,
},
},
})
Expand Down Expand Up @@ -367,13 +373,16 @@ 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,
Namespace: r.GetNamespace(),
Labels: utils.GenerateAppServerSelectorLabels(),
Annotations: map[string]string{
utils.OLSConfigMapResourceVersionAnnotation: configMapResourceVersion,
utils.ProxyCACertResourceVersionAnnotation: proxyCACMResourceVersion,
},
},
Spec: appsv1.DeploymentSpec{
Expand Down Expand Up @@ -530,14 +539,28 @@ 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
}

// 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)

Expand Down
22 changes: 2 additions & 20 deletions internal/controller/appserver/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 7 additions & 2 deletions internal/controller/appserver/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -1008,6 +1010,9 @@ var _ = Describe("App server reconciliator", Ordered, func() {
Name: cmCACertName,
},
DefaultMode: &volumeDefaultMode,
Items: []corev1.KeyToPath{
{Key: utils.ProxyCACertFileName, Path: utils.ProxyCACertFileName},
},
},
},
},
Expand Down
24 changes: 23 additions & 1 deletion internal/controller/lcore/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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{} {
Expand Down
Loading