diff --git a/api/v1alpha1/olsconfig_types.go b/api/v1alpha1/olsconfig_types.go index 7abcd1cbc..8a6fc97ba 100644 --- a/api/v1alpha1/olsconfig_types.go +++ b/api/v1alpha1/olsconfig_types.go @@ -319,6 +319,16 @@ type OLSSpec struct { // +kubebuilder:validation:Optional // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Tools Approval Configuration",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} ToolsApprovalConfig *ToolsApprovalConfig `json:"toolsApprovalConfig,omitempty"` + // Enable in-process credential hot-reload for LLM provider secrets. + // When true, the operator will not restart the app-server when LLM credential + // secret data is rotated — the service re-reads credentials from disk on each request. + // IMPORTANT: Requires lightspeed-service with get_credentials() hot-reload support + // (service PR #2955 / RFE-9380). If enabled with an older service image, rotated + // credentials (including revoked keys) will remain stale until the pod is manually restarted. + // +kubebuilder:default=false + // +kubebuilder:validation:Optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Credential Hot Reload",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:booleanSwitch"} + CredentialHotReload *bool `json:"credentialHotReload,omitempty"` } // Persistent Storage Configuration diff --git a/bundle/manifests/ols.openshift.io_olsconfigs.yaml b/bundle/manifests/ols.openshift.io_olsconfigs.yaml index a785c5980..5b088403a 100644 --- a/bundle/manifests/ols.openshift.io_olsconfigs.yaml +++ b/bundle/manifests/ols.openshift.io_olsconfigs.yaml @@ -1,9 +1,9 @@ +--- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition metadata: annotations: controller-gen.kubebuilder.io/version: v0.19.0 - creationTimestamp: null name: olsconfigs.ols.openshift.io spec: group: ols.openshift.io @@ -642,6 +642,16 @@ spec: - postgres type: string type: object + credentialHotReload: + default: false + description: |- + Enable in-process credential hot-reload for LLM provider secrets. + When true, the operator will not restart the app-server when LLM credential + secret data is rotated — the service re-reads credentials from disk on each request. + IMPORTANT: Requires lightspeed-service with get_credentials() hot-reload support + (service PR #2955 / RFE-9380). If enabled with an older service image, rotated + credentials (including revoked keys) will remain stale until the pod is manually restarted. + type: boolean defaultModel: description: Default model for usage type: string @@ -2302,9 +2312,3 @@ spec: storage: true subresources: status: {} -status: - acceptedNames: - kind: "" - plural: "" - conditions: null - storedVersions: null diff --git a/config/crd/bases/ols.openshift.io_olsconfigs.yaml b/config/crd/bases/ols.openshift.io_olsconfigs.yaml index 27bbe04a2..5b088403a 100644 --- a/config/crd/bases/ols.openshift.io_olsconfigs.yaml +++ b/config/crd/bases/ols.openshift.io_olsconfigs.yaml @@ -642,6 +642,16 @@ spec: - postgres type: string type: object + credentialHotReload: + default: false + description: |- + Enable in-process credential hot-reload for LLM provider secrets. + When true, the operator will not restart the app-server when LLM credential + secret data is rotated — the service re-reads credentials from disk on each request. + IMPORTANT: Requires lightspeed-service with get_credentials() hot-reload support + (service PR #2955 / RFE-9380). If enabled with an older service image, rotated + credentials (including revoked keys) will remain stale until the pod is manually restarted. + type: boolean defaultModel: description: Default model for usage type: string diff --git a/docs/credential-hot-reload.md b/docs/credential-hot-reload.md new file mode 100644 index 000000000..c914403a2 --- /dev/null +++ b/docs/credential-hot-reload.md @@ -0,0 +1,52 @@ +# Credential Hot-Reload (RFE-9380) + +## Overview + +When `credentialHotReload` is enabled on the OLSConfig CR, the operator skips +rolling restarts of the app-server pod when LLM credential secret **data** +changes. The service re-reads credential files from disk on every LLM request, +so rotated tokens take effect without downtime. + +This feature is a companion to +[lightspeed-service PR #2955](https://github.com/openshift/lightspeed-service/pull/2955), +which adds the in-process credential reload on the service side. + +## Configuration + +```yaml +apiVersion: ols.openshift.io/v1alpha1 +kind: OLSConfig +metadata: + name: cluster +spec: + ols: + credentialHotReload: true # default: false +``` + +## Behavior + +| Event | `credentialHotReload: false` (default) | `credentialHotReload: true` | +|---|---|---| +| LLM secret **data** rotated (same secret name) | Rolling restart | **No restart** — service picks up new credentials on next request | +| LLM secret **ref** changed in CR (different secret name) | Rolling restart | Rolling restart (deployment volume spec changes) | +| TLS / MCP / Postgres secret changed | Rolling restart | Rolling restart (unchanged) | + +## Prerequisites + +- The lightspeed-service image must include the `get_credentials()` hot-reload + support (lightspeed-service >= the version containing PR #2955). If an older + service image is used with this flag enabled, rotated credentials will not be + picked up until the pod is manually restarted. + +## How It Works + +1. During reconciliation, the operator reads `spec.ols.credentialHotReload` from + the OLSConfig CR and stores it in the internal `WatcherConfig`, along with the + set of LLM provider secret names. + +2. When the secret watcher detects a `.data` change on an annotated secret, it + checks whether the secret is an LLM credential and the hot-reload flag is + enabled. If both conditions are true, the restart is skipped. + +3. Non-LLM secrets (TLS, MCP headers, Postgres) always trigger restarts + regardless of the flag. diff --git a/internal/controller/appserver/assets.go b/internal/controller/appserver/assets.go index c3480a3d1..6f53df3de 100644 --- a/internal/controller/appserver/assets.go +++ b/internal/controller/appserver/assets.go @@ -321,6 +321,10 @@ func buildOLSConfig(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha } } + if cr.Spec.OLSConfig.CredentialHotReload != nil && *cr.Spec.OLSConfig.CredentialHotReload { + olsConfig.CredentialHotReload = true + } + tlsProfile := cr.Spec.OLSConfig.TLSSecurityProfile if tlsProfile == nil { apiServerProfile, err := utiltls.FetchAPIServerTlsProfile(r) diff --git a/internal/controller/olsconfig_controller.go b/internal/controller/olsconfig_controller.go index 066077356..a983c67ac 100644 --- a/internal/controller/olsconfig_controller.go +++ b/internal/controller/olsconfig_controller.go @@ -746,6 +746,12 @@ func (r *OLSConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, fmt.Errorf("failed to annotate external resources: %w", err) } + if olsconfig.Spec.OLSConfig.CredentialHotReload != nil && *olsconfig.Spec.OLSConfig.CredentialHotReload { + r.Logger.Info("credentialHotReload is enabled — LLM credential secret rotations will not "+ + "trigger app-server restarts. Requires lightspeed-service with get_credentials() "+ + "hot-reload support (service PR #2955 / RFE-9380)") + } + // 5. Phase 1: Reconcile independent resources if err := r.reconcileIndependentResources(ctx, olsconfig); err != nil { if isRESTMappingError(err) { diff --git a/internal/controller/olsconfig_helpers.go b/internal/controller/olsconfig_helpers.go index 2a3359be6..6407b84d8 100644 --- a/internal/controller/olsconfig_helpers.go +++ b/internal/controller/olsconfig_helpers.go @@ -406,6 +406,9 @@ func (r *OLSConfigReconciler) annotateExternalResources(ctx context.Context, r.WatcherConfig.AnnotatedSecretMapping = make(map[string][]string) } + credentialHotReload := cr.Spec.OLSConfig.CredentialHotReload != nil && + *cr.Spec.OLSConfig.CredentialHotReload + var errs []error // Annotate all external secrets @@ -418,11 +421,21 @@ func (r *OLSConfigReconciler) annotateExternalResources(ctx context.Context, } } + // When credentialHotReload is enabled, LLM secrets are not watched — + // the service re-reads credentials from disk (RFE-9380). + if credentialHotReload && strings.HasPrefix(source, "llm-provider-") { + if err := r.removeSecretAnnotationIfNeeded(ctx, name, r.Options.Namespace); err != nil { + r.Logger.Error(err, "Failed to remove annotation from secret", "secret", name) + errs = append(errs, err) + } + return nil + } + if err := r.annotateSecretIfNeeded(ctx, name, r.Options.Namespace); err != nil { r.Logger.Error(err, "Failed to annotate secret", "source", source, "secret", name) errs = append(errs, err) } - return nil // Continue iteration even on error + return nil }) if err != nil { errs = append(errs, err) @@ -502,6 +515,28 @@ func (r *OLSConfigReconciler) annotateSecretIfNeeded(ctx context.Context, name, return r.Update(ctx, secret) } +// removeSecretAnnotationIfNeeded removes the watcher annotation from a secret if present. +func (r *OLSConfigReconciler) removeSecretAnnotationIfNeeded(ctx context.Context, name, namespace string) error { + secret := &corev1.Secret{} + err := r.Get(ctx, client.ObjectKey{Name: name, Namespace: namespace}, secret) + if err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return err + } + + if secret.Annotations == nil { + return nil + } + if _, exists := secret.Annotations[utils.WatcherAnnotationKey]; !exists { + return nil + } + + delete(secret.Annotations, utils.WatcherAnnotationKey) + return r.Update(ctx, secret) +} + // annotateConfigMapIfNeeded annotates a configmap with the watcher annotation if it doesn't already have it. // Returns nil if the configmap doesn't exist (will be picked up on next reconciliation). func (r *OLSConfigReconciler) annotateConfigMapIfNeeded(ctx context.Context, name, namespace string) error { diff --git a/internal/controller/olsconfig_helpers_test.go b/internal/controller/olsconfig_helpers_test.go index 6dc793562..b8b24ab71 100644 --- a/internal/controller/olsconfig_helpers_test.go +++ b/internal/controller/olsconfig_helpers_test.go @@ -854,4 +854,48 @@ var _ = Describe("Helper Functions", func() { Expect(found).To(BeTrue()) }) }) + + Context("removeSecretAnnotationIfNeeded", func() { + It("should remove annotation when present", func() { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "remove-test-secret", + Namespace: testNamespace, + Annotations: map[string]string{ + utils.WatcherAnnotationKey: utils.OLSConfigName, + }, + }, + Data: map[string][]byte{"key": []byte("val")}, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, secret) }() + + err := reconciler.removeSecretAnnotationIfNeeded(ctx, "remove-test-secret", testNamespace) + Expect(err).NotTo(HaveOccurred()) + + fetched := &corev1.Secret{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: "remove-test-secret", Namespace: testNamespace}, fetched)).To(Succeed()) + Expect(fetched.Annotations).NotTo(HaveKey(utils.WatcherAnnotationKey)) + }) + + It("should do nothing when annotation is absent", func() { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-annot-secret", + Namespace: testNamespace, + }, + Data: map[string][]byte{"key": []byte("val")}, + } + Expect(k8sClient.Create(ctx, secret)).To(Succeed()) + defer func() { _ = k8sClient.Delete(ctx, secret) }() + + err := reconciler.removeSecretAnnotationIfNeeded(ctx, "no-annot-secret", testNamespace) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should do nothing when secret does not exist", func() { + err := reconciler.removeSecretAnnotationIfNeeded(ctx, "nonexistent-secret", testNamespace) + Expect(err).NotTo(HaveOccurred()) + }) + }) }) diff --git a/internal/controller/utils/types.go b/internal/controller/utils/types.go index 52ed1a62f..43e3a8db0 100644 --- a/internal/controller/utils/types.go +++ b/internal/controller/utils/types.go @@ -69,7 +69,12 @@ type ConfigMapWatcherConfig struct { SystemResources []SystemConfigMap } -// WatcherConfig contains all watcher configuration +// WatcherConfig contains all watcher configuration. +// NOTE: This struct is written by the reconciler and read by watcher event handlers +// (including predicate filters such as SecretWatcherFilter). This is safe because +// controller-runtime serializes reconcile calls and predicate evaluations on the same +// controller work queue. If MaxConcurrentReconciles is ever increased above 1, +// a sync.RWMutex must be added here. type WatcherConfig struct { Secrets SecretWatcherConfig ConfigMaps ConfigMapWatcherConfig @@ -239,6 +244,8 @@ type OLSConfig struct { Audit *AuditYAMLConfig `json:"audit,omitempty"` // Solr hybrid RAG (portal-rag /hybrid-search); mirrors lightspeed-service solr_hybrid SolrHybrid *SolrHybridSettings `json:"solr_hybrid,omitempty"` + // Enable in-process credential hot-reload for LLM provider secrets + CredentialHotReload bool `json:"credential_hot_reload,omitempty"` } type AuditYAMLConfig struct { diff --git a/internal/controller/watchers/watchers.go b/internal/controller/watchers/watchers.go index 9881ed265..375acc26e 100644 --- a/internal/controller/watchers/watchers.go +++ b/internal/controller/watchers/watchers.go @@ -274,8 +274,9 @@ func SecretWatcherFilter(r reconciler.Reconciler, ctx context.Context, obj clien // Check 2: Look for watcher annotation (user-provided secrets) if _, exist := annotations[utils.WatcherAnnotationKey]; exist { - // For annotated secrets, determine affected deployments from mapping secretName := obj.GetName() + + // For annotated secrets, determine affected deployments from mapping var affectedDeployments []string var found bool if watcherConfig != nil { diff --git a/internal/controller/watchers/watchers_test.go b/internal/controller/watchers/watchers_test.go index 6e291f833..909a024dc 100644 --- a/internal/controller/watchers/watchers_test.go +++ b/internal/controller/watchers/watchers_test.go @@ -444,4 +444,5 @@ var _ = Describe("Watchers", func() { Expect(updated.Spec.Template.Annotations).To(HaveKey(utils.ForceReloadAnnotationKey)) }) }) + }) diff --git a/test/e2e/reconciliation_test.go b/test/e2e/reconciliation_test.go index 7c91d3da1..952687a2e 100644 --- a/test/e2e/reconciliation_test.go +++ b/test/e2e/reconciliation_test.go @@ -9,6 +9,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" olsv1alpha1 "github.com/openshift/lightspeed-operator/api/v1alpha1" + "github.com/openshift/lightspeed-operator/internal/controller/utils" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" @@ -389,4 +390,71 @@ var _ = Describe("Reconciliation From OLSConfig CR", Ordered, func() { }) + It("should remove LLM secret annotation and propagate config when credentialHotReload is enabled", func() { + llmSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: LLMTokenSecondSecretName, + Namespace: OLSNameSpace, + }, + } + + By("verify the LLM secret is annotated before enabling hot-reload") + Eventually(func() bool { + if err := client.Get(llmSecret); err != nil { + return false + } + _, exists := llmSecret.Annotations[utils.WatcherAnnotationKey] + return exists + }, 30*time.Second, 2*time.Second).Should(BeTrue()) + + By("enable credentialHotReload on the CR") + err = client.Update(cr, func(obj ctrlclient.Object) error { + config := obj.(*olsv1alpha1.OLSConfig) + hotReload := true + config.Spec.OLSConfig.CredentialHotReload = &hotReload + // The previous CA-cert test deletes its ConfigMap via defer but + // leaves the dangling ref on the CR. Clear it so + // GenerateOLSConfigMap won't fail with a NotFound error. + config.Spec.OLSConfig.AdditionalCAConfigMapRef = nil + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + By("verify the LLM secret annotation is removed") + Eventually(func() bool { + if err := client.Get(llmSecret); err != nil { + return false + } + _, exists := llmSecret.Annotations[utils.WatcherAnnotationKey] + return !exists + }, 30*time.Second, 2*time.Second).Should(BeTrue()) + + By("verify the olsconfig ConfigMap contains credential_hot_reload: true") + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: AppServerConfigMapName, + Namespace: OLSNameSpace, + }, + } + err = client.WaitForConfigMapContainString(configMap, AppServerConfigMapKey, "credential_hot_reload: true") + Expect(err).NotTo(HaveOccurred()) + + By("disable credentialHotReload to restore default behavior") + err = client.Update(cr, func(obj ctrlclient.Object) error { + config := obj.(*olsv1alpha1.OLSConfig) + config.Spec.OLSConfig.CredentialHotReload = nil + return nil + }) + Expect(err).NotTo(HaveOccurred()) + + By("verify the LLM secret annotation is restored") + Eventually(func() bool { + if err := client.Get(llmSecret); err != nil { + return false + } + _, exists := llmSecret.Annotations[utils.WatcherAnnotationKey] + return exists + }, 30*time.Second, 2*time.Second).Should(BeTrue()) + }) + })