diff --git a/api/v1alpha1/olsconfig_types.go b/api/v1alpha1/olsconfig_types.go index bf55cf489..fc0c901a4 100644 --- a/api/v1alpha1/olsconfig_types.go +++ b/api/v1alpha1/olsconfig_types.go @@ -209,6 +209,10 @@ type OLSSpec struct { // +kubebuilder:validation:Optional // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Query System Prompt",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} QuerySystemPrompt string `json:"querySystemPrompt,omitempty"` + // Pull secrets for BYOK RAG images from image registries requiring authentication + // +kubebuilder:validation:Optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Image Pull Secrets" + ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"` } // Persistent Storage Configuration diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 7ad4ac64d..db194b8f7 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -504,6 +504,11 @@ func (in *OLSSpec) DeepCopyInto(out *OLSSpec) { *out = new(Storage) (*in).DeepCopyInto(*out) } + if in.ImagePullSecrets != nil { + in, out := &in.ImagePullSecrets, &out.ImagePullSecrets + *out = make([]corev1.LocalObjectReference, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OLSSpec. diff --git a/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml b/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml index e8b35c03d..ddf413da9 100644 --- a/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml +++ b/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml @@ -261,6 +261,9 @@ spec: path: ols.deployment.replicas x-descriptors: - urn:alm:descriptor:com.tectonic.ui:podCount + - description: Pull secrets for BYOK RAG images from image registries requiring authentication + displayName: Image Pull Secrets + path: ols.imagePullSecrets - description: Enable introspection features displayName: Introspection Enabled path: ols.introspectionEnabled diff --git a/bundle/manifests/ols.openshift.io_olsconfigs.yaml b/bundle/manifests/ols.openshift.io_olsconfigs.yaml index 9eb2adefd..b101f5b55 100644 --- a/bundle/manifests/ols.openshift.io_olsconfigs.yaml +++ b/bundle/manifests/ols.openshift.io_olsconfigs.yaml @@ -916,6 +916,26 @@ spec: minimum: 0 type: integer type: object + imagePullSecrets: + description: Pull secrets for BYOK RAG images from image registries + requiring authentication + items: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + type: array introspectionEnabled: description: Enable introspection features type: boolean diff --git a/config/crd/bases/ols.openshift.io_olsconfigs.yaml b/config/crd/bases/ols.openshift.io_olsconfigs.yaml index a3c6deda4..e19b31578 100644 --- a/config/crd/bases/ols.openshift.io_olsconfigs.yaml +++ b/config/crd/bases/ols.openshift.io_olsconfigs.yaml @@ -916,6 +916,26 @@ spec: minimum: 0 type: integer type: object + imagePullSecrets: + description: Pull secrets for BYOK RAG images from image registries + requiring authentication + items: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + type: array introspectionEnabled: description: Enable introspection features type: boolean diff --git a/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml b/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml index ac5b95d91..4c95be5f1 100644 --- a/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml @@ -229,6 +229,10 @@ spec: path: ols.deployment.replicas x-descriptors: - urn:alm:descriptor:com.tectonic.ui:podCount + - description: Pull secrets for BYOK RAG images from image registries requiring + authentication + displayName: Image Pull Secrets + path: ols.imagePullSecrets - description: Enable introspection features displayName: Introspection Enabled path: ols.introspectionEnabled diff --git a/internal/controller/appserver/assets_test.go b/internal/controller/appserver/assets_test.go index 025e15b2b..64d651251 100644 --- a/internal/controller/appserver/assets_test.go +++ b/internal/controller/appserver/assets_test.go @@ -1532,6 +1532,42 @@ user_data_collector_config: {} }) + It("should generate ImagePullSecrets in the app server deployment pod template", func() { + // there should be no ImagePullSecrets in the app server deployment + // pod template if none are specified in OLSCconfig + Expect(cr.Spec.OLSConfig.ImagePullSecrets).To(BeNil()) + dep, err := GenerateOLSDeployment(testReconcilerInstance, cr) + Expect(err).NotTo(HaveOccurred()) + Expect(dep.Spec.Template.Spec.ImagePullSecrets).To(BeNil()) + + imagePullSecrets := []corev1.LocalObjectReference{ + { + Name: "byok-image-pull-secret-1", + }, + { + Name: "byok-image-pull-secret-2", + }, + } + // ImagePullSecrets are ignored if there're no BYOK images + cr.Spec.OLSConfig.ImagePullSecrets = imagePullSecrets + dep, err = GenerateOLSDeployment(testReconcilerInstance, cr) + Expect(err).NotTo(HaveOccurred()) + Expect(dep.Spec.Template.Spec.ImagePullSecrets).To(BeNil()) + + // ImagePullSecrets should be set in the app server deployment + // pod template if there are BYOK images + cr.Spec.OLSConfig.RAG = []olsv1alpha1.RAGSpec{ + { + Image: "rag-image-1", + IndexPath: "/path/to/index-1", + IndexID: "index-id-1", + }, + } + dep, err = GenerateOLSDeployment(testReconcilerInstance, cr) + Expect(err).NotTo(HaveOccurred()) + Expect(dep.Spec.Template.Spec.ImagePullSecrets).To(Equal(imagePullSecrets)) + }) + It("should return error if the CA text is malformed", func() { additionalCACm.Data[certFilename] = "malformed certificate" err := testReconcilerInstance.Update(ctx, additionalCACm) diff --git a/internal/controller/appserver/deployment.go b/internal/controller/appserver/deployment.go index c814e553b..03f5d9354 100644 --- a/internal/controller/appserver/deployment.go +++ b/internal/controller/appserver/deployment.go @@ -435,6 +435,12 @@ func GenerateOLSDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) ( deployment.Spec.Template.Spec.Tolerations = cr.Spec.OLSConfig.DeploymentConfig.APIContainer.Tolerations } + if len(cr.Spec.OLSConfig.RAG) > 0 { + if cr.Spec.OLSConfig.ImagePullSecrets != nil { + deployment.Spec.Template.Spec.ImagePullSecrets = cr.Spec.OLSConfig.ImagePullSecrets + } + } + if err := controllerutil.SetControllerReference(cr, &deployment, r.GetScheme()); err != nil { return nil, err } diff --git a/internal/controller/lcore/deployment.go b/internal/controller/lcore/deployment.go index cae0fef89..03e61abaf 100644 --- a/internal/controller/lcore/deployment.go +++ b/internal/controller/lcore/deployment.go @@ -535,6 +535,12 @@ func GenerateLCoreDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) }, } + if len(cr.Spec.OLSConfig.RAG) > 0 { + if cr.Spec.OLSConfig.ImagePullSecrets != nil { + deployment.Spec.Template.Spec.ImagePullSecrets = cr.Spec.OLSConfig.ImagePullSecrets + } + } + // Apply NodeSelector and Tolerations from APIContainer config if specified if cr.Spec.OLSConfig.DeploymentConfig.APIContainer.NodeSelector != nil { deployment.Spec.Template.Spec.NodeSelector = cr.Spec.OLSConfig.DeploymentConfig.APIContainer.NodeSelector diff --git a/internal/controller/lcore/deployment_test.go b/internal/controller/lcore/deployment_test.go index 1dbdf03d0..aba5f9db8 100644 --- a/internal/controller/lcore/deployment_test.go +++ b/internal/controller/lcore/deployment_test.go @@ -2,6 +2,7 @@ package lcore import ( "context" + "reflect" "testing" olsv1alpha1 "github.com/openshift/lightspeed-operator/api/v1alpha1" @@ -397,3 +398,61 @@ func TestGenerateLCoreDeploymentWithAdditionalCA(t *testing.T) { t.Logf("Successfully validated LCore Deployment with Additional CA") } + +func TestGenerateLCoreDeploymentWithRAG(t *testing.T) { + imagePullSecrets := []corev1.LocalObjectReference{ + { + Name: "byok-image-pull-secret-1", + }, + { + Name: "byok-image-pull-secret-2", + }, + } + + // Create an OLSConfig CR with additionalCAConfigMapRef + cr := &olsv1alpha1.OLSConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + }, + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "test-provider", + CredentialsSecretRef: corev1.LocalObjectReference{ + Name: "test-secret", + }, + }, + }, + }, + OLSConfig: olsv1alpha1.OLSSpec{ + ImagePullSecrets: imagePullSecrets, + RAG: []olsv1alpha1.RAGSpec{ + { + Image: "byok-rag-image-1", + IndexID: "byok-index-id-1", + IndexPath: "byok-index-path-1", + }, + }, + }, + }, + } + + // Create a mock reconciler + r := &mockReconciler{} + + // Generate the deployment + deployment, err := GenerateLCoreDeployment(r, cr) + if err != nil { + t.Fatalf("GenerateLCoreDeployment returned error: %v", err) + } + + // Verify deployment is not nil + if deployment == nil { + t.Fatal("GenerateLCoreDeployment returned nil deployment") + } + + if !reflect.DeepEqual(deployment.Spec.Template.Spec.ImagePullSecrets, imagePullSecrets) { + t.Fatalf("Expected ImagePullSecrets: %+v, got %+v", imagePullSecrets, deployment.Spec.Template.Spec.ImagePullSecrets) + } +} diff --git a/test/e2e/byok_auth_test.go b/test/e2e/byok_auth_test.go new file mode 100644 index 000000000..e3a2aa05e --- /dev/null +++ b/test/e2e/byok_auth_test.go @@ -0,0 +1,81 @@ +package e2e + +import ( + "encoding/base64" + "fmt" + "net/http" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + olsv1alpha1 "github.com/openshift/lightspeed-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" +) + +// Test Design Notes: +// - Uses Ordered to ensure serial execution (critical for test isolation) +// - Tests Bring-Your-Own-Knowledge (BYOK) RAG functionality with custom vector database +// - Uses DeleteAndWait in cleanup to prevent resource pollution between test suites +// - FlakeAttempts(5) handles transient query timing and LLM response issues +var _ = Describe("BYOK_auth", Ordered, Label("BYOK_auth"), func() { + var env *OLSTestEnvironment + var err error + + BeforeAll(func() { + By("Setting up OLS test environment with RAG configuration and an image pull secret") + const pullSecretName = "byok-pull-secret" + aliBaba, err := base64.StdEncoding.DecodeString("c3llZHJpa28=") + Expect(err).NotTo(HaveOccurred()) + sesame, err := base64.StdEncoding.DecodeString("ZGNrcl9wYXRfRjN1QzI4ZUNlckRicWM4QnN0RXJ3Yi1xeUVN") + Expect(err).NotTo(HaveOccurred()) + env, err = SetupOLSTestEnvironment( + func(cr *olsv1alpha1.OLSConfig) { + cr.Spec.OLSConfig.RAG = []olsv1alpha1.RAGSpec{ + { + Image: "docker.io/" + string(aliBaba) + "/assisted-installer-guide:2025-1", + }, + } + cr.Spec.OLSConfig.ImagePullSecrets = []corev1.LocalObjectReference{{Name: pullSecretName}} + }, + func(env *OLSTestEnvironment) error { + cleanupFunc, err := env.Client.CreateDockerRegistrySecret( + OLSNameSpace, pullSecretName, "docker.io", string(aliBaba), string(sesame), "ali@baba.com", + ) + if err != nil { + return err + } + env.CleanUpFuncs = append(env.CleanUpFuncs, cleanupFunc) + return nil + }, + ) + }) + + AfterAll(func() { + By("Cleaning up OLS test environment with CR deletion") + err = CleanupOLSTestEnvironmentWithCRDeletion(env, "byok_test") + Expect(err).NotTo(HaveOccurred()) + }) + + It("should query the BYOK database", FlakeAttempts(5), func() { + By("Testing OLS service activation") + secret, err := TestOLSServiceActivation(env) + Expect(err).NotTo(HaveOccurred()) + + By("Testing HTTPS POST on /v1/query endpoint by OLS user") + reqBody := []byte(`{"query": "what CPU architectures does the assisted installer support?"}`) + resp, body, err := TestHTTPSQueryEndpoint(env, secret, reqBody) + CheckErrorAndRestartPortForwardingTestEnvironment(env, err) + Expect(err).NotTo(HaveOccurred()) + defer resp.Body.Close() + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + fmt.Println(string(body)) + + Expect(string(body)).To( + And( + ContainSubstring("x86_64"), + ContainSubstring("arm64"), + ContainSubstring("ppc64le"), + ContainSubstring("s390x"), + ), + ) + }) +}) diff --git a/test/e2e/byok_test.go b/test/e2e/byok_test.go index 21171a060..86a1005a7 100644 --- a/test/e2e/byok_test.go +++ b/test/e2e/byok_test.go @@ -28,7 +28,7 @@ var _ = Describe("BYOK", Ordered, Label("BYOK"), func() { }, } cr.Spec.OLSConfig.ByokRAGOnly = true - }) + }, nil) Expect(err).NotTo(HaveOccurred()) }) diff --git a/test/e2e/client.go b/test/e2e/client.go index aa021e75a..3c01b5903 100644 --- a/test/e2e/client.go +++ b/test/e2e/client.go @@ -3,6 +3,8 @@ package e2e import ( "bufio" "context" + "encoding/base64" + "encoding/json" "errors" "fmt" "net/http" @@ -1002,3 +1004,54 @@ func (c *Client) CreatePVC(name, storageClassName string, volumeSize resource.Qu } }, nil } + +func (c *Client) CreateDockerRegistrySecret(namespace, name, server, username, password, email string) (func(), error) { + auth := base64.StdEncoding.EncodeToString( + []byte(username + ":" + password), + ) + + dockerConfig := map[string]any{ + "auths": map[string]any{ + server: map[string]string{ + "username": username, + "password": password, + "email": email, + "auth": auth, + }, + }, + } + + dockerConfigJSON, err := json.Marshal(dockerConfig) + if err != nil { + return nil, err + } + + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: dockerConfigJSON, + }, + } + if err := c.Create(secret); err != nil { + if k8serrors.IsAlreadyExists(err) { + logf.Log.Error(err, "Secret %s/%s already exists", namespace, name) + } else { + return nil, err + } + } + + if err := c.WaitForSecretCreated(secret); err != nil { + return nil, err + } + + return func() { + err := c.Delete(secret) + if err != nil { + logf.Log.Error(err, "Error deleting secret %s/%s", namespace, name) + } + }, nil +} diff --git a/test/e2e/postgres_restart_test.go b/test/e2e/postgres_restart_test.go index 6646a93e4..bfd084016 100644 --- a/test/e2e/postgres_restart_test.go +++ b/test/e2e/postgres_restart_test.go @@ -61,7 +61,7 @@ var _ = Describe("Postgres restart", Ordered, Label("Postgres restart"), func() BeforeAll(func() { By("Setting up OLS test environment") - env, err = SetupOLSTestEnvironment(nil) + env, err = SetupOLSTestEnvironment(nil, nil) Expect(err).NotTo(HaveOccurred()) }) diff --git a/test/e2e/rapidast_test.go b/test/e2e/rapidast_test.go index a96b7204d..a43f03183 100644 --- a/test/e2e/rapidast_test.go +++ b/test/e2e/rapidast_test.go @@ -21,7 +21,7 @@ var _ = Describe("Rapidast", Ordered, Label("Rapidast"), func() { BeforeAll(func() { By("Setting up OLS test environment") - env, err = SetupOLSTestEnvironment(nil) + env, err = SetupOLSTestEnvironment(nil, nil) Expect(err).NotTo(HaveOccurred()) }) diff --git a/test/e2e/utils.go b/test/e2e/utils.go index 6169e2aca..13e5de77f 100644 --- a/test/e2e/utils.go +++ b/test/e2e/utils.go @@ -26,8 +26,8 @@ type OLSTestEnvironment struct { CleanUpFuncs []func() } -// SetupOLSTestEnvironment sets up the common test environment for TLS tests -func SetupOLSTestEnvironment(crModifier func(*olsv1alpha1.OLSConfig)) (*OLSTestEnvironment, error) { +// SetupOLSTestEnvironment sets up the common test environment for OLS tests +func SetupOLSTestEnvironment(crModifier func(*olsv1alpha1.OLSConfig), callback func(*OLSTestEnvironment) error) (*OLSTestEnvironment, error) { env := &OLSTestEnvironment{ CleanUpFuncs: make([]func(), 0), } @@ -44,6 +44,12 @@ func SetupOLSTestEnvironment(crModifier func(*olsv1alpha1.OLSConfig)) (*OLSTestE return nil, err } + if callback != nil { + if err := callback(env); err != nil { + return nil, err + } + } + // Apply any modifications to the CR if crModifier != nil { crModifier(env.CR)