From 86aa8c788564ecf4d4c55008070c30cc3d74c20f Mon Sep 17 00:00:00 2001 From: Sri Roopa Ramesh Babu Date: Thu, 5 Feb 2026 12:48:19 -0500 Subject: [PATCH] Generic Provider Config for LCore supported providers. --- api/v1alpha1/olsconfig_types.go | 33 +- api/v1alpha1/zz_generated.deepcopy.go | 7 +- ...tspeed-operator.clusterserviceversion.yaml | 25 +- .../ols.openshift.io_olsconfigs.yaml | 41 ++ bundle/metadata/annotations.yaml | 2 +- .../bases/ols.openshift.io_olsconfigs.yaml | 43 ++ ...tspeed-operator.clusterserviceversion.yaml | 20 + internal/controller/lcore/assets_test.go | 574 +++++++++++++++++- internal/controller/lcore/config.go | 423 +++++-------- internal/controller/lcore/deployment.go | 58 +- internal/controller/lcore/deployment_test.go | 201 ++++++ .../lcore/lcore_generic_provider_test.go | 391 ++++++++++++ internal/controller/utils/constants.go | 4 + internal/controller/utils/test_fixtures.go | 2 +- internal/controller/utils/utils.go | 89 ++- .../utils/utils_generic_provider_test.go | 254 ++++++++ internal/controller/utils/utils_misc_test.go | 187 +++++- 17 files changed, 2056 insertions(+), 298 deletions(-) create mode 100644 internal/controller/lcore/lcore_generic_provider_test.go create mode 100644 internal/controller/utils/utils_generic_provider_test.go diff --git a/api/v1alpha1/olsconfig_types.go b/api/v1alpha1/olsconfig_types.go index e295a2d31..ef14a067f 100644 --- a/api/v1alpha1/olsconfig_types.go +++ b/api/v1alpha1/olsconfig_types.go @@ -21,6 +21,7 @@ import ( corev1 "k8s.io/api/core/v1" resource "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtime "k8s.io/apimachinery/pkg/runtime" ) // OLSConfigSpec defines the desired state of OLSConfig @@ -36,6 +37,7 @@ type OLSConfigSpec struct { // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="OLS Data Collector Settings" OLSDataCollectorConfig OLSDataCollectorSpec `json:"olsDataCollector,omitempty"` // MCP Server settings + // +kubebuilder:validation:MaxItems=20 // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="MCP Server Settings" MCPServers []MCPServerConfig `json:"mcpServers,omitempty"` // Feature Gates holds list of features to be enabled explicitly, otherwise they are disabled by default. @@ -160,6 +162,7 @@ const ( // LLMSpec defines the desired state of the large language model (LLM). type LLMSpec struct { // +kubebuilder:validation:Required + // +kubebuilder:validation:MaxItems=10 // +required // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Providers" Providers []ProviderSpec `json:"providers"` @@ -451,6 +454,11 @@ type ModelSpec struct { // ProviderSpec defines the desired state of LLM provider. // +kubebuilder:validation:XValidation:message="'deploymentName' must be specified for 'azure_openai' provider",rule="self.type != \"azure_openai\" || self.deploymentName != \"\"" // +kubebuilder:validation:XValidation:message="'projectID' must be specified for 'watsonx' provider",rule="self.type != \"watsonx\" || self.projectID != \"\"" +// +kubebuilder:validation:XValidation:message="'providerType' and 'config' must be used together in llamaStackGeneric mode",rule="!has(self.providerType) || has(self.config)" +// +kubebuilder:validation:XValidation:message="'config' requires 'providerType' to be set",rule="!has(self.config) || has(self.providerType)" +// +kubebuilder:validation:XValidation:message="Llama Stack Generic mode (providerType set) requires type='llamaStackGeneric'",rule="!has(self.providerType) || self.type == \"llamaStackGeneric\"" +// +kubebuilder:validation:XValidation:message="Llama Stack Generic mode cannot use legacy provider-specific fields",rule="self.type != \"llamaStackGeneric\" || (!has(self.deploymentName) && !has(self.projectID) && !has(self.url) && !has(self.apiVersion))" +// +kubebuilder:validation:XValidation:message="credentialKey must not be empty or whitespace",rule="!has(self.credentialKey) || !self.credentialKey.matches('^[ \\t\\n\\r\\v\\f]*$')" type ProviderSpec struct { // Provider name // +kubebuilder:validation:Required @@ -468,13 +476,14 @@ type ProviderSpec struct { CredentialsSecretRef corev1.LocalObjectReference `json:"credentialsSecretRef"` // List of models from the provider // +kubebuilder:validation:Required + // +kubebuilder:validation:MaxItems=50 // +required // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Models" Models []ModelSpec `json:"models"` // Provider type // +kubebuilder:validation:Required // +required - // +kubebuilder:validation:Enum=azure_openai;bam;openai;watsonx;rhoai_vllm;rhelai_vllm;fake_provider + // +kubebuilder:validation:Enum=azure_openai;bam;openai;watsonx;rhoai_vllm;rhelai_vllm;fake_provider;llamaStackGeneric // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Provider Type" Type string `json:"type"` // Deployment name for Azure OpenAI provider @@ -493,6 +502,27 @@ type ProviderSpec struct { // +kubebuilder:validation:Optional // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="TLS Security Profile",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} TLSSecurityProfile *configv1.TLSSecurityProfile `json:"tlsSecurityProfile,omitempty"` + // Llama Stack Generic provider type for provider configuration (e.g., "remote::openai", "inline::sentence-transformers") + // When set, this provider uses Llama Stack Generic mode instead of legacy mode. + // Must follow pattern: (inline|remote):: + // +kubebuilder:validation:Pattern=`^(inline|remote)::[a-z0-9][a-z0-9_-]*$` + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Llama Stack Provider Type" + ProviderType string `json:"providerType,omitempty"` + // Arbitrary configuration for the provider (Llama Stack Generic mode only) + // This map is passed directly to Llama Stack provider configuration. + // Credentials are automatically injected as environment variable substitutions. + // Example: {"url": "https://...", "custom_field": "value"} + // +kubebuilder:pruning:PreserveUnknownFields + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Llama Stack Provider Config" + Config *runtime.RawExtension `json:"config,omitempty"` + // Secret key name for provider credentials (defaults to "apitoken" if not set). + // Specifies which key inside credentialsSecretRef to read the credential value from. + // The credential value is always exposed to the container as env var {PROVIDER_NAME}_API_KEY + // (derived from the provider name, not this field), and referenced in the Llama Stack config + // YAML as ${env.PROVIDER_NAME_API_KEY}. This field only controls which secret data key is read. + // +kubebuilder:validation:Optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Credential Key Name" + CredentialKey string `json:"credentialKey,omitempty"` } // UserDataCollectionSpec defines how we collect user data. @@ -687,6 +717,7 @@ type MCPServerConfig struct { // Headers to send to the MCP server // Each header can reference a secret or use a special source (kubernetes token, client token) // +optional + // +kubebuilder:validation:MaxItems=20 // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Headers" Headers []MCPHeader `json:"headers,omitempty"` } diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 6c7473bcc..400f7d9ac 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -24,7 +24,7 @@ import ( configv1 "github.com/openshift/api/config/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" - runtime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime" ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. @@ -517,6 +517,11 @@ func (in *ProviderSpec) DeepCopyInto(out *ProviderSpec) { *out = new(configv1.TLSSecurityProfile) (*in).DeepCopyInto(*out) } + if in.Config != nil { + in, out := &in.Config, &out.Config + *out = new(runtime.RawExtension) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProviderSpec. diff --git a/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml b/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml index e4ad8b023..00135c134 100644 --- a/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml +++ b/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml @@ -81,7 +81,9 @@ spec: - description: The name of the secret object that stores API provider credentials displayName: Credential Secret path: llm.providers[0].credentialsSecretRef - - description: 'Feature Gates holds list of features to be enabled explicitly, otherwise they are disabled by default. possible values: MCPServer, ToolFiltering' + - description: |- + Feature Gates holds list of features to be enabled explicitly, otherwise they are disabled by default. + possible values: MCPServer, ToolFiltering displayName: Feature Gates path: featureGates - displayName: LLM Settings @@ -93,6 +95,21 @@ spec: path: llm.providers[0].apiVersion - description: Deployment name for Azure OpenAI provider displayName: Azure Deployment Name + - description: |- + Arbitrary configuration for the provider (Llama Stack Generic mode only) + This map is passed directly to Llama Stack provider configuration. + Credentials are automatically injected as environment variable substitutions. + Example: {"url": "https://...", "custom_field": "value"} + displayName: Llama Stack Provider Config + path: llm.providers[0].config + - description: |- + Secret key name for provider credentials (defaults to "apitoken" if not set) + Specifies which key in credentialsSecretRef contains the API token. + The operator creates an environment variable named {PROVIDER_NAME}_API_KEY. + displayName: Credential Key Name + path: llm.providers[0].credentialKey + - description: Azure OpenAI deployment name + displayName: Azure OpenAI deployment name path: llm.providers[0].deploymentName - description: Fake Provider MCP Tool Call displayName: Fake Provider MCP Tool Call @@ -118,6 +135,12 @@ spec: - description: Watsonx Project ID displayName: Watsonx Project ID path: llm.providers[0].projectID + - description: |- + Llama Stack Generic provider type for provider configuration (e.g., "remote::openai", "inline::sentence-transformers") + When set, this provider uses Llama Stack Generic mode instead of legacy mode. + Must follow pattern: (inline|remote):: + displayName: Llama Stack Provider Type + path: llm.providers[0].providerType - description: TLS Security Profile used by connection to provider displayName: TLS Security Profile path: llm.providers[0].tlsSecurityProfile diff --git a/bundle/manifests/ols.openshift.io_olsconfigs.yaml b/bundle/manifests/ols.openshift.io_olsconfigs.yaml index b132d5137..c9f0fd679 100644 --- a/bundle/manifests/ols.openshift.io_olsconfigs.yaml +++ b/bundle/manifests/ols.openshift.io_olsconfigs.yaml @@ -61,6 +61,20 @@ spec: apiVersion: description: API Version for Azure OpenAI provider type: string + config: + description: |- + Arbitrary configuration for the provider (Llama Stack Generic mode only) + This map is passed directly to Llama Stack provider configuration. + Credentials are automatically injected as environment variable substitutions. + Example: {"url": "https://...", "custom_field": "value"} + type: object + x-kubernetes-preserve-unknown-fields: true + credentialKey: + description: |- + Secret key name for provider credentials (defaults to "apitoken" if not set) + Specifies which key in credentialsSecretRef contains the API token. + The operator creates an environment variable named {PROVIDER_NAME}_API_KEY. + type: string credentialsSecretRef: description: The name of the secret object that stores API provider credentials @@ -111,6 +125,7 @@ spec: required: - name type: object + maxItems: 50 type: array name: description: Provider name @@ -118,6 +133,13 @@ spec: projectID: description: Watsonx Project ID type: string + providerType: + description: |- + Llama Stack Generic provider type for provider configuration (e.g., "remote::openai", "inline::sentence-transformers") + When set, this provider uses Llama Stack Generic mode instead of legacy mode. + Must follow pattern: (inline|remote):: + pattern: ^(inline|remote)::[a-z0-9][a-z0-9_-]*$ + type: string tlsSecurityProfile: description: TLS Security Profile used by connection to provider @@ -259,6 +281,7 @@ spec: - rhoai_vllm - rhelai_vllm - fake_provider + - llamaStackGeneric type: string url: description: Provider API URL @@ -278,6 +301,22 @@ spec: - message: '''projectID'' must be specified for ''watsonx'' provider' rule: self.type != "watsonx" || self.projectID != "" + - message: '''providerType'' and ''config'' must be used together + in llamaStackGeneric mode' + rule: '!has(self.providerType) || has(self.config)' + - message: '''config'' requires ''providerType'' to be set' + rule: '!has(self.config) || has(self.providerType)' + - message: Llama Stack Generic mode (providerType set) requires + type='llamaStackGeneric' + rule: '!has(self.providerType) || self.type == "llamaStackGeneric"' + - message: Llama Stack Generic mode cannot use legacy provider-specific + fields + rule: self.type != "llamaStackGeneric" || (!has(self.deploymentName) + && !has(self.projectID)) + - message: credentialKey must not be empty or whitespace + rule: '!has(self.credentialKey) || !self.credentialKey.matches(''^[ + \t\n\r]*$'')' + maxItems: 10 type: array required: - providers @@ -347,6 +386,7 @@ spec: - name - valueFrom type: object + maxItems: 20 type: array name: description: Name of the MCP server @@ -364,6 +404,7 @@ spec: - name - url type: object + maxItems: 20 type: array ols: description: OLSSpec defines the desired state of OLS deployment. diff --git a/bundle/metadata/annotations.yaml b/bundle/metadata/annotations.yaml index 6dcedf596..e6c5163cc 100644 --- a/bundle/metadata/annotations.yaml +++ b/bundle/metadata/annotations.yaml @@ -6,7 +6,7 @@ annotations: operators.operatorframework.io.bundle.package.v1: lightspeed-operator operators.operatorframework.io.bundle.channels.v1: alpha operators.operatorframework.io.bundle.channel.default.v1: alpha - operators.operatorframework.io.metrics.builder: operator-sdk-v1.33.0 + operators.operatorframework.io.metrics.builder: operator-sdk-v1.36.1 operators.operatorframework.io.metrics.mediatype.v1: metrics+v1 operators.operatorframework.io.metrics.project_layout: go.kubebuilder.io/v4 # Annotations for testing. diff --git a/config/crd/bases/ols.openshift.io_olsconfigs.yaml b/config/crd/bases/ols.openshift.io_olsconfigs.yaml index 229452e42..353ba6bed 100644 --- a/config/crd/bases/ols.openshift.io_olsconfigs.yaml +++ b/config/crd/bases/ols.openshift.io_olsconfigs.yaml @@ -61,6 +61,22 @@ spec: apiVersion: description: API Version for Azure OpenAI provider type: string + config: + description: |- + Arbitrary configuration for the provider (Llama Stack Generic mode only) + This map is passed directly to Llama Stack provider configuration. + Credentials are automatically injected as environment variable substitutions. + Example: {"url": "https://...", "custom_field": "value"} + type: object + x-kubernetes-preserve-unknown-fields: true + credentialKey: + description: |- + Secret key name for provider credentials (defaults to "apitoken" if not set). + Specifies which key inside credentialsSecretRef to read the credential value from. + The credential value is always exposed to the container as env var {PROVIDER_NAME}_API_KEY + (derived from the provider name, not this field), and referenced in the Llama Stack config + YAML as ${env.PROVIDER_NAME_API_KEY}. This field only controls which secret data key is read. + type: string credentialsSecretRef: description: The name of the secret object that stores API provider credentials @@ -111,6 +127,7 @@ spec: required: - name type: object + maxItems: 50 type: array name: description: Provider name @@ -118,6 +135,13 @@ spec: projectID: description: Watsonx Project ID type: string + providerType: + description: |- + Llama Stack Generic provider type for provider configuration (e.g., "remote::openai", "inline::sentence-transformers") + When set, this provider uses Llama Stack Generic mode instead of legacy mode. + Must follow pattern: (inline|remote):: + pattern: ^(inline|remote)::[a-z0-9][a-z0-9_-]*$ + type: string tlsSecurityProfile: description: TLS Security Profile used by connection to provider @@ -259,6 +283,7 @@ spec: - rhoai_vllm - rhelai_vllm - fake_provider + - llamaStackGeneric type: string url: description: Provider API URL @@ -278,6 +303,22 @@ spec: - message: '''projectID'' must be specified for ''watsonx'' provider' rule: self.type != "watsonx" || self.projectID != "" + - message: '''providerType'' and ''config'' must be used together + in llamaStackGeneric mode' + rule: '!has(self.providerType) || has(self.config)' + - message: '''config'' requires ''providerType'' to be set' + rule: '!has(self.config) || has(self.providerType)' + - message: Llama Stack Generic mode (providerType set) requires + type='llamaStackGeneric' + rule: '!has(self.providerType) || self.type == "llamaStackGeneric"' + - message: Llama Stack Generic mode cannot use legacy provider-specific + fields + rule: self.type != "llamaStackGeneric" || (!has(self.deploymentName) + && !has(self.projectID) && !has(self.url) && !has(self.apiVersion)) + - message: credentialKey must not be empty or whitespace + rule: '!has(self.credentialKey) || !self.credentialKey.matches(''^[ + \t\n\r\v\f]*$'')' + maxItems: 10 type: array required: - providers @@ -347,6 +388,7 @@ spec: - name - valueFrom type: object + maxItems: 20 type: array name: description: Name of the MCP server @@ -364,6 +406,7 @@ spec: - name - url type: object + maxItems: 20 type: array ols: description: OLSSpec defines the desired state of OLS deployment. diff --git a/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml b/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml index 46fb2450e..85154dd10 100644 --- a/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml @@ -59,6 +59,19 @@ spec: - description: API Version for Azure OpenAI provider displayName: Azure OpenAI API Version path: llm.providers[0].apiVersion + - description: |- + Arbitrary configuration for the provider (Llama Stack Generic mode only) + This map is passed directly to Llama Stack provider configuration. + Credentials are automatically injected as environment variable substitutions. + Example: {"url": "https://...", "custom_field": "value"} + displayName: Llama Stack Provider Config + path: llm.providers[0].config + - description: |- + Secret key name for provider credentials (defaults to "apitoken" if not set) + Specifies which key in credentialsSecretRef contains the API token. + The operator creates an environment variable named {PROVIDER_NAME}_API_KEY. + displayName: Credential Key Name + path: llm.providers[0].credentialKey - description: Deployment name for Azure OpenAI provider displayName: Azure Deployment Name path: llm.providers[0].deploymentName @@ -87,6 +100,12 @@ spec: - description: Watsonx Project ID displayName: Watsonx Project ID path: llm.providers[0].projectID + - description: |- + Llama Stack Generic provider type for provider configuration (e.g., "remote::openai", "inline::sentence-transformers") + When set, this provider uses Llama Stack Generic mode instead of legacy mode. + Must follow pattern: (inline|remote):: + displayName: Llama Stack Provider Type + path: llm.providers[0].providerType - description: TLS Security Profile used by connection to provider displayName: TLS Security Profile path: llm.providers[0].tlsSecurityProfile @@ -322,6 +341,7 @@ spec: - tls.key: Private key (PEM format) - REQUIRED - ca.crt: CA certificate for console proxy trust (PEM format) - OPTIONAL + If ca.crt is not provided, the OpenShift Console proxy will use the default system trust store. displayName: TLS Certificate Secret Reference path: ols.tlsConfig.keyCertSecretRef diff --git a/internal/controller/lcore/assets_test.go b/internal/controller/lcore/assets_test.go index bb3e0c44f..8a1878b86 100644 --- a/internal/controller/lcore/assets_test.go +++ b/internal/controller/lcore/assets_test.go @@ -2,6 +2,7 @@ package lcore import ( "context" + "fmt" "strings" "testing" @@ -99,17 +100,14 @@ func TestBuildLlamaStackYAML_UnsupportedProvider(t *testing.T) { // Verify error message mentions the provider expectedErrMsg := "not currently supported by Llama Stack" - if err.Error() == "" || len(err.Error()) == 0 { + if err.Error() == "" { t.Errorf("Error message is empty for unsupported provider '%s'", providerType) } - if err.Error() != "" && len(err.Error()) > 0 { - // Check if error message contains expected text - if !contains(err.Error(), expectedErrMsg) { - t.Errorf("Error message '%s' doesn't contain expected text '%s'", err.Error(), expectedErrMsg) - } - if !contains(err.Error(), providerType) { - t.Errorf("Error message '%s' doesn't mention provider type '%s'", err.Error(), providerType) - } + if !strings.Contains(err.Error(), expectedErrMsg) { + t.Errorf("Error message '%s' doesn't contain expected text '%s'", err.Error(), expectedErrMsg) + } + if !strings.Contains(err.Error(), providerType) { + t.Errorf("Error message '%s' doesn't mention provider type '%s'", err.Error(), providerType) } t.Logf("Correctly rejected unsupported provider '%s' with error: %v", providerType, err) @@ -344,18 +342,286 @@ func TestBuildLlamaStackYAML_AzureProvider(t *testing.T) { t.Logf("Successfully validated Llama Stack YAML with Azure provider (%d bytes)", len(yamlOutput)) } -// Helper function to check if a string contains a substring -func contains(s, substr string) bool { - return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && findSubstring(s, substr)) +// TestBuildLlamaStackYAML_GenericProvider tests the generic provider path using +// remote::openai as the providerType. This validates that a known provider can be +// configured through the generic llamaStackGeneric mechanism instead of the legacy path. +func TestBuildLlamaStackYAML_GenericProvider(t *testing.T) { + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "my-openai", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::openai", + CredentialsSecretRef: corev1.LocalObjectReference{ + Name: "my-openai-secret", + }, + Config: &runtime.RawExtension{ + Raw: []byte(`{"url": "https://api.openai.com/v1", "custom_field": "test_value"}`), + }, + Models: []olsv1alpha1.ModelSpec{ + {Name: "gpt-4o"}, + }, + }, + }, + }, + }, + } + + ctx := context.Background() + yamlOutput, err := buildLlamaStackYAML(nil, ctx, cr) + if err != nil { + t.Fatalf("buildLlamaStackYAML failed for generic provider: %v", err) + } + + if len(yamlOutput) == 0 { + t.Fatal("buildLlamaStackYAML returned empty string for generic provider") + } + + var result map[string]interface{} + err = yaml.Unmarshal([]byte(yamlOutput), &result) + if err != nil { + t.Fatalf("buildLlamaStackYAML produced invalid YAML: %v\nOutput:\n%s", err, yamlOutput) + } + + // Verify providers section + providers, ok := result["providers"].(map[string]interface{}) + if !ok { + t.Fatal("providers section missing or invalid type") + } + inference, ok := providers["inference"].([]interface{}) + if !ok || len(inference) == 0 { + t.Fatal("inference providers missing or empty") + } + + // Find the generic openai provider + var genericProvider map[string]interface{} + for _, provider := range inference { + p, ok := provider.(map[string]interface{}) + if !ok { + continue + } + if p["provider_id"] == "my-openai" { + genericProvider = p + break + } + } + if genericProvider == nil { + t.Fatal("Generic openai provider not found in inference providers") + } + + // Verify provider_type comes from the ProviderType field (generic path) + if genericProvider["provider_type"] != "remote::openai" { + t.Errorf("Expected provider_type='remote::openai', got '%v'", genericProvider["provider_type"]) + } + + // Verify config fields are passed through + config, ok := genericProvider["config"].(map[string]interface{}) + if !ok { + t.Fatal("provider config missing or invalid type") + } + if config["url"] != "https://api.openai.com/v1" { + t.Errorf("Config URL not passed through correctly, got: %v", config["url"]) + } + if config["custom_field"] != "test_value" { + t.Errorf("Custom config field not preserved, got: %v", config["custom_field"]) + } + + // Verify credential auto-injection + apiKey, ok := config["api_key"] + if !ok { + t.Error("api_key not auto-injected into config") + } else { + expectedKey := "${env.MY_OPENAI_API_KEY}" + if apiKey != expectedKey { + t.Errorf("Expected api_key='%s', got '%v'", expectedKey, apiKey) + } + } + + // Verify model mapping + models, ok := result["models"].([]interface{}) + if !ok || len(models) == 0 { + t.Fatal("models section missing or empty") + } + foundModel := false + for _, model := range models { + m, ok := model.(map[string]interface{}) + if !ok { + continue + } + if m["model_id"] == "gpt-4o" { + foundModel = true + if m["provider_id"] != "my-openai" { + t.Errorf("Model not mapped to correct provider, got: %v", m["provider_id"]) + } + break + } + } + if !foundModel { + t.Error("Model not found in models section") + } + + t.Logf("Generic provider with remote::openai generated correctly (%d bytes)", len(yamlOutput)) } -func findSubstring(s, substr string) bool { - for i := 0; i <= len(s)-len(substr); i++ { - if s[i:i+len(substr)] == substr { - return true +// TODO: Add tests for additional generic providers as they become supported +// when they are officially supported. + +func TestBuildLlamaStackYAML_GenericProvider_InvalidValues(t *testing.T) { + // Test multiple invalid configurations + testCases := []struct { + name string + provider olsv1alpha1.ProviderSpec + expectedError string + }{ + { + name: "invalid JSON in config", + provider: olsv1alpha1.ProviderSpec{ + Name: "broken-json", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + Config: &runtime.RawExtension{ + Raw: []byte(`{invalid json syntax`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + expectedError: "failed to unmarshal config", + }, + { + name: "type llamaStackGeneric without providerType", + provider: olsv1alpha1.ProviderSpec{ + Name: "missing-provider-type", + Type: utils.LlamaStackGenericType, + // ProviderType not set - this should fail + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + expectedError: "requires providerType and config fields to be set", + }, + { + name: "empty config Raw bytes", + provider: olsv1alpha1.ProviderSpec{ + Name: "empty-config", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::empty", + Config: &runtime.RawExtension{ + Raw: []byte(""), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + expectedError: "failed to unmarshal config", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{tc.provider}, + }, + }, + } + + ctx := context.Background() + yamlOutput, err := buildLlamaStackYAML(nil, ctx, cr) + + // Should return error + if err == nil { + t.Fatalf("Expected error for %s, got none. Output: %s", tc.name, yamlOutput) + } + + // Verify error message contains expected substring + if !strings.Contains(err.Error(), tc.expectedError) { + t.Errorf("Expected error containing '%s', got: %v", tc.expectedError, err) + } + + // Verify error message mentions provider name (for better debugging) + if !strings.Contains(err.Error(), tc.provider.Name) { + t.Errorf("Error message should mention provider name '%s', got: %v", tc.provider.Name, err) + } + + t.Logf("✓ Correctly rejected %s with error: %v", tc.name, err) + }) + } +} + +func TestBuildLlamaStackYAML_GenericProvider_ConfigWithExistingCredential(t *testing.T) { + // Test generic provider where config already contains an explicit api_key field. + // hasAPIKeyField() detects it and skips auto-injection to avoid overriding the + // caller's value (e.g. a custom env var substitution pattern). + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "custom", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + Config: &runtime.RawExtension{ + // Config explicitly sets api_key to a custom env var substitution. + // The operator must not overwrite it with the default injection. + Raw: []byte(`{ + "url": "https://api.custom.com/v1", + "api_key": "${env.CUSTOM_SECRET_TOKEN}" + }`), + }, + Models: []olsv1alpha1.ModelSpec{ + {Name: "custom-model"}, + }, + }, + }, + }, + }, + } + + // Build the YAML + ctx := context.Background() + yamlOutput, err := buildLlamaStackYAML(nil, ctx, cr) + if err != nil { + t.Fatalf("buildLlamaStackYAML failed: %v", err) + } + + // Parse YAML output + var result map[string]interface{} + err = yaml.Unmarshal([]byte(yamlOutput), &result) + if err != nil { + t.Fatalf("Invalid YAML: %v", err) + } + + // Find the custom provider + providers := result["providers"].(map[string]interface{}) + inference := providers["inference"].([]interface{}) + var customProvider map[string]interface{} + for _, provider := range inference { + p := provider.(map[string]interface{}) + if p["provider_id"] == "custom" { + customProvider = p + break } } - return false + + if customProvider == nil { + t.Fatal("Custom provider not found") + } + + // Get config + config := customProvider["config"].(map[string]interface{}) + + // CRITICAL: Verify the caller's api_key value is preserved unchanged. + apiKey, ok := config["api_key"] + if !ok { + t.Error("api_key field was lost from config") + } else if apiKey != "${env.CUSTOM_SECRET_TOKEN}" { + t.Errorf("api_key was overwritten; expected '${env.CUSTOM_SECRET_TOKEN}', got '%v'", apiKey) + } + + // Verify URL is still present + if config["url"] != "https://api.custom.com/v1" { + t.Errorf("URL not preserved, got: %v", config["url"]) + } + + t.Logf("✓ Explicit api_key in config preserved correctly (auto-injection skipped)") } func TestBuildLCoreConfigYAML(t *testing.T) { @@ -529,3 +795,277 @@ func (m *mockReconcilerForAssets) GetNamespace() string { func (m *mockReconcilerForAssets) GetScheme() *runtime.Scheme { return m.scheme } + +// TestBuildLlamaStackYAML_EdgeCases tests edge cases and error conditions +func TestBuildLlamaStackYAML_EdgeCases(t *testing.T) { + tests := []struct { + name string + provider olsv1alpha1.ProviderSpec + expectError bool + errorContains string + }{ + { + name: "MalformedJSON_Config", + provider: olsv1alpha1.ProviderSpec{ + Name: "bad-json", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + Config: &runtime.RawExtension{ + Raw: []byte(`{invalid json}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + expectError: true, + errorContains: "invalid character", + }, + { + name: "EmptyProviderName", + provider: olsv1alpha1.ProviderSpec{ + Name: "", + Type: "openai", + ProviderType: "", + Models: []olsv1alpha1.ModelSpec{{Name: "gpt-4"}}, + }, + expectError: false, // Name can be empty in provider spec + }, + { + name: "LongProviderName", + provider: olsv1alpha1.ProviderSpec{ + Name: "provider-" + strings.Repeat("a", 200), + Type: "openai", + ProviderType: "", + Models: []olsv1alpha1.ModelSpec{{Name: "gpt-4"}}, + }, + expectError: false, // Should handle long names + }, + { + name: "SpecialCharactersInProviderName", + provider: olsv1alpha1.ProviderSpec{ + Name: "my-provider@#$%", + Type: "openai", + ProviderType: "", + Models: []olsv1alpha1.ModelSpec{{Name: "gpt-4"}}, + }, + expectError: false, // Should not fail on special chars in name + }, + { + name: "VeryLargeConfig", + provider: olsv1alpha1.ProviderSpec{ + Name: "large-config", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + Config: &runtime.RawExtension{ + Raw: []byte(`{"data": "` + strings.Repeat("x", 10000) + `"}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test"}}, + }, + expectError: false, + }, + { + name: "NestedConfig", + provider: olsv1alpha1.ProviderSpec{ + Name: "nested", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + Config: &runtime.RawExtension{ + Raw: []byte(`{"outer": {"inner": {"deep": {"value": 123}}}}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test"}}, + }, + expectError: false, + }, + { + name: "ConfigWithArrays", + provider: olsv1alpha1.ProviderSpec{ + Name: "arrays", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + Config: &runtime.RawExtension{ + Raw: []byte(`{"models": ["model1", "model2"], "endpoints": [{"url": "https://test"}]}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test"}}, + }, + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{tt.provider}, + }, + }, + } + + ctx := context.Background() + yamlOutput, err := buildLlamaStackYAML(nil, ctx, cr) + + if tt.expectError { + if err == nil { + t.Errorf("Expected error, but got none. Output: %s", yamlOutput) + } else if tt.errorContains != "" && !strings.Contains(err.Error(), tt.errorContains) { + t.Errorf("Expected error containing '%s', got: %v", tt.errorContains, err) + } + } else { + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + // Verify output is valid YAML + var result map[string]interface{} + if err := yaml.Unmarshal([]byte(yamlOutput), &result); err != nil { + t.Errorf("buildLlamaStackYAML produced invalid YAML: %v", err) + } + } + }) + } +} + +// TestProviderNameToEnvVarConversion tests environment variable name conversion edge cases +func TestProviderNameToEnvVarConversion(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"simple", "SIMPLE"}, + {"with-hyphens", "WITH_HYPHENS"}, + {"multiple-hyphens-here", "MULTIPLE_HYPHENS_HERE"}, + {"MixedCase", "MIXEDCASE"}, + {"trailing-", "TRAILING_"}, + {"-leading", "_LEADING"}, + {"multiple--hyphens", "MULTIPLE__HYPHENS"}, + {"already_underscore", "ALREADY_UNDERSCORE"}, + {"123numeric", "_123NUMERIC"}, // Leading digit prefixed with underscore + {"", "_"}, // Empty string gets underscore prefix (POSIX compliance, prevents collisions) + // Special characters are stripped to produce POSIX-valid env var names + {"my-provider@#$%", "MY_PROVIDER"}, + {"dots.in.name", "DOTSINNAME"}, + {"provider!with*chars", "PROVIDERWITHCHARS"}, + {"!@#$%", "_"}, // All special chars sanitize to empty, then prefix + {" ", "_"}, // Whitespace sanitizes to empty, then prefix + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("convert_%s", tt.input), func(t *testing.T) { + result := utils.ProviderNameToEnvVarName(tt.input) + if result != tt.expected { + t.Errorf("ProviderNameToEnvVarName(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} + +// TestGenericProviderCredentialInjection verifies that the env var substitution +// pattern (${env.PROVIDER_API_KEY}) is emitted in the Llama Stack YAML config +// for a generic provider. This covers the config-generation side only; the +// SecretKeyRef wiring is tested in deployment_test.go. +func TestGenericProviderCredentialInjection(t *testing.T) { + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "test-provider", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::test-backend", + CredentialsSecretRef: corev1.LocalObjectReference{ + Name: "test-provider-secret", + }, + CredentialKey: "secret_key", + Config: &runtime.RawExtension{ + Raw: []byte(`{"api_endpoint": "https://api.example.com"}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + }, + }, + }, + } + + ctx := context.Background() + yamlOutput, err := buildLlamaStackYAML(nil, ctx, cr) + if err != nil { + t.Fatalf("buildLlamaStackYAML failed: %v", err) + } + + // Verify that the custom credential key is referenced in environment variables + var result map[string]interface{} + if err := yaml.Unmarshal([]byte(yamlOutput), &result); err != nil { + t.Fatalf("Invalid YAML produced: %v", err) + } + + // Check that environment variable substitution is present + if !strings.Contains(yamlOutput, "TEST_PROVIDER_API_KEY") { + t.Errorf("Expected environment variable reference 'TEST_PROVIDER_API_KEY' in output") + } +} + +func TestBuildLlamaStackYAML_GenericProvider_NoCredentials(t *testing.T) { + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "public-llm", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::public-provider", + Config: &runtime.RawExtension{ + Raw: []byte(`{"url": "https://public.example.com"}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "public-model"}}, + // NO CredentialsSecretRef + }, + }, + }, + }, + } + + ctx := context.Background() + yamlOutput, err := buildLlamaStackYAML(nil, ctx, cr) + if err != nil { + t.Fatalf("buildLlamaStackYAML returned error for credential-less generic provider: %v", err) + } + if len(yamlOutput) == 0 { + t.Fatal("buildLlamaStackYAML returned empty string") + } + + var result map[string]interface{} + if err := yaml.Unmarshal([]byte(yamlOutput), &result); err != nil { + t.Fatalf("Invalid YAML: %v", err) + } + + // Find the public-llm provider + providers := result["providers"].(map[string]interface{}) + inference := providers["inference"].([]interface{}) + var publicProvider map[string]interface{} + for _, p := range inference { + pm := p.(map[string]interface{}) + if pm["provider_id"] == "public-llm" { + publicProvider = pm + break + } + } + if publicProvider == nil { + t.Fatal("public-llm provider not found in inference providers") + } + if publicProvider["provider_type"] != "remote::public-provider" { + t.Errorf("Expected provider_type 'remote::public-provider', got '%v'", publicProvider["provider_type"]) + } + + config := publicProvider["config"].(map[string]interface{}) + if config["url"] != "https://public.example.com" { + t.Errorf("Expected url 'https://public.example.com', got '%v'", config["url"]) + } + // api_key should NOT be injected when no credentials are configured + if _, exists := config["api_key"]; exists { + t.Errorf("api_key should not be present for credential-less generic provider, but found: %v", config["api_key"]) + } +} + +func TestDeepCopyMap_NilInput(t *testing.T) { + result := deepCopyMap(nil) + if result != nil { + t.Errorf("deepCopyMap(nil) should return nil, got %v", result) + } +} diff --git a/internal/controller/lcore/config.go b/internal/controller/lcore/config.go index 15bfad0ab..2e18a90df 100644 --- a/internal/controller/lcore/config.go +++ b/internal/controller/lcore/config.go @@ -2,6 +2,7 @@ package lcore import ( "context" + "encoding/json" "fmt" "path" "slices" @@ -78,10 +79,8 @@ func buildLlamaStackCoreConfig(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig // image_name is a semantic identifier for the llama-stack configuration // Note: Does NOT affect PostgreSQL database name (llama-stack uses hardcoded "llamastack") "image_name": "openshift-lightspeed-configuration", - // Minimal APIs for RAG + MCP: agents (for MCP), files, inference, safety (required by agents), telemetry, tool_runtime, vector_io - // Commented out: datasetio, eval, post_training, scoring (not needed for basic RAG + MCP) - // Commented out: datasetio, eval, post_training, prompts, scoring, telemetry - "apis": []string{"agents" /* "datasetio", "eval", */, "files", "inference" /* , "post_training", */, "safety" /* , "scoring", "telemetry"*/, "tool_runtime", "vector_io"}, + // Enabled APIs for RAG + MCP: agents (for MCP), files, inference, safety (required by agents), tool_runtime, vector_io + "apis": []string{"agents", "files", "inference", "safety", "tool_runtime", "vector_io"}, "benchmarks": []interface{}{}, "container_image": nil, "datasets": []interface{}{}, @@ -131,7 +130,7 @@ func buildLlamaStackAgentProviders(_ reconciler.Reconciler, _ *olsv1alpha1.OLSCo "responses": map[string]interface{}{ "backend": "sql_default", "table_name": "agent_responses", - "namespace": "agent_responces", + "namespace": "agent_responses", }, }, }, @@ -139,60 +138,9 @@ func buildLlamaStackAgentProviders(_ reconciler.Reconciler, _ *olsv1alpha1.OLSCo } } -// Commented out - datasetio API not needed for basic RAG + MCP -// Uncomment if you need dataset operations -/* -func buildLlamaStackDatasetIOProviders(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { - return []interface{}{ - map[string]interface{}{ - "provider_id": "huggingface", - "provider_type": "remote::huggingface", - "config": map[string]interface{}{ - "kvstore": map[string]interface{}{ - "db_path": "/tmp/llama-stack/huggingface_datasetio.db", - "namespace": nil, - "type": "sqlite", - }, - }, - }, - map[string]interface{}{ - "provider_id": "localfs", - "provider_type": "inline::localfs", - "config": map[string]interface{}{ - "kvstore": map[string]interface{}{ - "db_path": "/tmp/llama-stack/localfs_datasetio.db", - "namespace": nil, - "type": "sqlite", - }, - }, - }, - } -} -*/ - -// Commented out - eval API not needed for basic RAG + MCP -// Uncomment if you need to run evaluations -/* -func buildLlamaStackEvalProviders(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { - return []interface{}{ - map[string]interface{}{ - "provider_id": "meta-reference", - "provider_type": "inline::meta-reference", - "config": map[string]interface{}{ - "kvstore": map[string]interface{}{ - "db_path": "/tmp/llama-stack/meta_reference_eval.db", - "namespace": nil, - "type": "sqlite", - }, - }, - }, - } -} -*/ - func buildLlamaStackInferenceProviders(_ reconciler.Reconciler, _ context.Context, cr *olsv1alpha1.OLSConfig) ([]interface{}, error) { + // Always include sentence-transformers (required for embeddings) providers := []interface{}{ - // Always include sentence-transformers for embeddings map[string]interface{}{ "provider_id": "sentence-transformers", "provider_type": "inline::sentence-transformers", @@ -200,6 +148,11 @@ func buildLlamaStackInferenceProviders(_ reconciler.Reconciler, _ context.Contex }, } + // Guard against nil CR or empty Providers + if cr == nil || cr.Spec.LLMConfig.Providers == nil { + return providers, nil + } + // Add LLM providers from OLSConfig for _, provider := range cr.Spec.LLMConfig.Providers { providerConfig := map[string]interface{}{ @@ -209,64 +162,92 @@ func buildLlamaStackInferenceProviders(_ reconciler.Reconciler, _ context.Contex // Convert provider name to valid environment variable name envVarName := utils.ProviderNameToEnvVarName(provider.Name) - // Map OLSConfig provider types to Llama Stack provider types - switch provider.Type { - case "openai", "rhoai_vllm", "rhelai_vllm": + // Check if this is Llama Stack Generic provider configuration (providerType is set) + if provider.ProviderType != "" { + // Llama Stack Generic provider configuration: use providerType and config directly + providerConfig["provider_type"] = provider.ProviderType + + // Unmarshal the config from RawExtension config := map[string]interface{}{} - // Determine the appropriate Llama Stack provider type - // - OpenAI uses remote::openai (validates against OpenAI model whitelist) - // - vLLM uses remote::vllm (accepts any custom model names) - if provider.Type == "openai" { - providerConfig["provider_type"] = "remote::openai" - // Set API key from environment variable - // Llama Stack will substitute ${env.VAR_NAME} with the actual env var value - config["api_key"] = fmt.Sprintf("${env.%s_API_KEY}", envVarName) - } else { - providerConfig["provider_type"] = "remote::vllm" - // Set API key from environment variable - // Llama Stack will substitute ${env.VAR_NAME} with the actual env var value - config["api_token"] = fmt.Sprintf("${env.%s_API_KEY}", envVarName) + if provider.Config != nil && provider.Config.Raw != nil { + if err := json.Unmarshal(provider.Config.Raw, &config); err != nil { + return nil, fmt.Errorf("failed to unmarshal config for provider '%s': %w", provider.Name, err) + } } - // Add custom URL if specified - if provider.URL != "" { - config["url"] = provider.URL + // Deep copy to prevent mutations + configCopy := deepCopyMap(config) + + // Auto-inject api_key if not already present in config and credentials are configured. + // Skip injection when: + // - the user has explicitly set api_key in config (custom env var name) + // - no CredentialsSecretRef is configured (public/unauthenticated provider) + if !hasAPIKeyField(configCopy) && provider.CredentialsSecretRef.Name != "" { + configCopy["api_key"] = fmt.Sprintf("${env.%s_API_KEY}", envVarName) } - providerConfig["config"] = config - case "azure_openai": - providerConfig["provider_type"] = "remote::azure" - config := map[string]interface{}{} + providerConfig["config"] = configCopy + + } else { + // Predefined provider types: map to Llama Stack provider types using getProviderType helper + llamaType, err := getProviderType(&provider) + if err != nil { + return nil, err + } + providerConfig["provider_type"] = llamaType + + // Build provider-specific configuration + switch provider.Type { + // fake_provider follows the vLLM credential path (api_token); it is included + // in the CRD enum and mapping solely for operator integration testing. + case "openai", "rhoai_vllm", "rhelai_vllm", "fake_provider": + config := map[string]interface{}{} + // Determine the appropriate config field for credentials + // - OpenAI uses remote::openai (validates against OpenAI model whitelist) + // - vLLM/fake uses remote::vllm / remote::fake (accepts any custom model names) + if provider.Type == "openai" { + // Set API key from environment variable + // Llama Stack will substitute ${env.VAR_NAME} with the actual env var value + config["api_key"] = fmt.Sprintf("${env.%s_API_KEY}", envVarName) + } else { + // Set API token from environment variable for vLLM + // Llama Stack will substitute ${env.VAR_NAME} with the actual env var value + config["api_token"] = fmt.Sprintf("${env.%s_API_KEY}", envVarName) + } - // Azure supports both API key and client credentials authentication - // Always include api_key (required by LiteLLM's Pydantic validation) - config["api_key"] = fmt.Sprintf("${env.%s_API_KEY}", envVarName) + // Add custom URL if specified + if provider.URL != "" { + config["url"] = provider.URL + } + providerConfig["config"] = config - // Also include client credentials fields (will be empty if not using client credentials) - config["client_id"] = fmt.Sprintf("${env.%s_CLIENT_ID:=}", envVarName) - config["tenant_id"] = fmt.Sprintf("${env.%s_TENANT_ID:=}", envVarName) - config["client_secret"] = fmt.Sprintf("${env.%s_CLIENT_SECRET:=}", envVarName) + case "azure_openai": + config := map[string]interface{}{} - // Azure-specific fields - if provider.AzureDeploymentName != "" { - config["deployment_name"] = provider.AzureDeploymentName - } - if provider.APIVersion != "" { - config["api_version"] = provider.APIVersion - } - if provider.URL != "" { - config["api_base"] = provider.URL - } - providerConfig["config"] = config + // Azure supports both API key and client credentials authentication + // Always include api_key (required by LiteLLM's Pydantic validation) + config["api_key"] = fmt.Sprintf("${env.%s_API_KEY}", envVarName) - case "watsonx", "bam": - // These providers are not supported by Llama Stack - // They are handled directly by lightspeed-stack (LCS), not Llama Stack - return nil, fmt.Errorf("provider type '%s' (provider '%s') is not currently supported by Llama Stack. Supported types: openai, azure_openai, rhoai_vllm, rhelai_vllm", provider.Type, provider.Name) + // Also include client credentials fields (will be empty if not using client credentials) + config["client_id"] = fmt.Sprintf("${env.%s_CLIENT_ID:=}", envVarName) + config["tenant_id"] = fmt.Sprintf("${env.%s_TENANT_ID:=}", envVarName) + config["client_secret"] = fmt.Sprintf("${env.%s_CLIENT_SECRET:=}", envVarName) - default: - // Unknown provider type - return nil, fmt.Errorf("unknown provider type '%s' (provider '%s'). Supported types: openai, azure_openai, rhoai_vllm, rhelai_vllm", provider.Type, provider.Name) + // Azure-specific fields + if provider.AzureDeploymentName != "" { + config["deployment_name"] = provider.AzureDeploymentName + } + if provider.APIVersion != "" { + config["api_version"] = provider.APIVersion + } + if provider.URL != "" { + config["api_base"] = provider.URL + } + providerConfig["config"] = config + + default: + return nil, fmt.Errorf("internal error: no config builder for legacy provider type '%s' (provider '%s'); update the switch in buildLlamaStackInferenceProviders", provider.Type, provider.Name) + } } providers = append(providers, providerConfig) @@ -275,24 +256,82 @@ func buildLlamaStackInferenceProviders(_ reconciler.Reconciler, _ context.Contex return providers, nil } -// Commented out - post_training API not needed for basic RAG + MCP -// Uncomment if you need fine-tuning capabilities -/* -func buildLlamaStackPostTraining(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { - return []interface{}{ - map[string]interface{}{ - "provider_id": "huggingface", - "provider_type": "inline::huggingface-gpu", - "config": map[string]interface{}{ - "checkpoint_format": "huggingface", - "device": "cpu", - "distributed_backend": nil, - "dpo_output_dir": ".", - }, - }, +// providerTypeMapping defines how legacy OLSConfig provider types map to Llama Stack provider_type strings. +// New providers that are fully supported without operator changes should use the llamaStackGeneric +// type with the providerType field instead of adding entries here. +// +// To add a new legacy provider: +// 1. Add an entry to this map with OLSConfig type as key +// 2. Set the Llama Stack provider_type value (e.g., "remote::new-provider") +// 3. Add credential/config handling in the switch inside buildLlamaStackInferenceProviders +var providerTypeMapping = map[string]string{ + "openai": "remote::openai", + "rhoai_vllm": "remote::vllm", + "rhelai_vllm": "remote::vllm", + "azure_openai": "remote::azure", + // fake_provider is included in the CRD enum for testing purposes + "fake_provider": "remote::fake", +} + +// getProviderType returns the Llama Stack provider_type string for a legacy OLSConfig +// provider type (openai, azure_openai, rhoai_vllm, rhelai_vllm). +// It is only called for providers where ProviderType == "" (the legacy path); +// generic providers (ProviderType != "") set provider_type directly without this function. +// Returns an error for unsupported types (watsonx, bam) or invalid generic usage. +func getProviderType(provider *olsv1alpha1.ProviderSpec) (string, error) { + // Legacy providers use predefined mapping + if llamaType, exists := providerTypeMapping[provider.Type]; exists { + return llamaType, nil + } + + // Unsupported provider type + switch provider.Type { + case "watsonx", "bam": + return "", fmt.Errorf("provider type '%s' (provider '%s') is not currently supported by Llama Stack. Supported types: openai, azure_openai, rhoai_vllm, rhelai_vllm, %s", provider.Type, provider.Name, utils.LlamaStackGenericType) + case utils.LlamaStackGenericType: + return "", fmt.Errorf("provider type '%s' (provider '%s') requires providerType and config fields to be set", utils.LlamaStackGenericType, provider.Name) + default: + return "", fmt.Errorf("unknown provider type '%s' (provider '%s'). Supported types: openai, azure_openai, rhoai_vllm, rhelai_vllm, %s", provider.Type, provider.Name, utils.LlamaStackGenericType) + } +} + +// deepCopyMap creates a deep copy of a map[string]interface{}, including nested maps +// and slices. This prevents mutations of the copy from affecting the original. +func deepCopyMap(src map[string]interface{}) map[string]interface{} { + if src == nil { + return nil + } + dst := make(map[string]interface{}, len(src)) + for k, v := range src { + dst[k] = deepCopyValue(v) + } + return dst +} + +// deepCopyValue recursively deep-copies a value that may be a primitive, map, or slice. +func deepCopyValue(v interface{}) interface{} { + switch val := v.(type) { + case map[string]interface{}: + return deepCopyMap(val) + case []interface{}: + dstSlice := make([]interface{}, len(val)) + for i, elem := range val { + dstSlice[i] = deepCopyValue(elem) + } + return dstSlice + default: + return v } } -*/ + +// hasAPIKeyField reports whether the config already contains an explicit "api_key" field. +// We only check for "api_key" because that is the field we auto-inject; suppressing +// injection only when the caller has already set it avoids silently skipping injection +// for providers that require api_key but happen to have an unrelated field (e.g. api_token). +func hasAPIKeyField(config map[string]interface{}) bool { + _, ok := config["api_key"] + return ok +} // Safety API - Required by agents provider (for MCP) // Note: You can configure excluded_categories if needed @@ -308,59 +347,6 @@ func buildLlamaStackSafety(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) [] } } -// Commented out - scoring API not needed for basic RAG + MCP -// Uncomment if you need response scoring capabilities -/* -func buildLlamaStackScoring(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { - return []interface{}{ - map[string]interface{}{ - "provider_id": "basic", - "provider_type": "inline::basic", - "config": map[string]interface{}{}, - }, - map[string]interface{}{ - "provider_id": "llm-as-judge", - "provider_type": "inline::llm-as-judge", - "config": map[string]interface{}{}, - }, - map[string]interface{}{ - "provider_id": "braintrust", - "provider_type": "inline::braintrust", - "config": map[string]interface{}{ - "openai_api_key": "********", - }, - }, - } -} -*/ - -// Telemetry provider - commented out as the API doesn't exist in this Llama Stack version -// Uncomment and enable in apis list when telemetry API becomes available -/* -func buildLlamaStackTelemetry(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { - // Console telemetry provider - logs to stdout - return []interface{}{ - map[string]interface{}{ - "provider_id": "console", - "provider_type": "inline::console", - "config": map[string]interface{}{ - "service_name": "lightspeed-stack", - "sinks": []string{"console"}, - }, - }, - } - - // Alternative options (change return statement above): - // SQLite telemetry provider - stores traces in SQLite: - // provider_id: "sqlite", provider_type: "inline::meta-reference" - // config: {service_name, sinks: "sqlite", sqlite_db_path: "/tmp/llama-stack/trace_store.db"} - // - // OTLP telemetry provider - sends traces to Jaeger/OTLP endpoint: - // provider_id: "otlp", provider_type: "inline::otlp" - // config: {service_name, sinks: "otlp", otlp_endpoint: "http://jaeger:4317"} -} -*/ - func buildLlamaStackToolRuntime(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { return []interface{}{ map[string]interface{}{ @@ -407,20 +393,6 @@ func buildLlamaStackServerConfig(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConf } } -// Commented out - shields not needed without safety API -// Uncomment if you enable safety API and need shields -/* -func buildLlamaStackShields(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { - return []interface{}{ - map[string]interface{}{ - "shield_id": "llama-guard-shield", - "provider_id": "llama-guard", - "provider_shield_id": "gpt-3.5-turbo", - }, - } -} -*/ - func buildLlamaStackVectorDBs(_ reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) []interface{} { vectorDBs := []interface{}{} @@ -589,22 +561,19 @@ func buildLlamaStackYAML(r reconciler.Reconciler, ctx context.Context, cr *olsv1 } // Build providers map - only include providers for enabled APIs - // Note: telemetry provider commented out as the API doesn't exist in this Llama Stack version config["providers"] = map[string]interface{}{ - "files": buildLlamaStackFileProviders(r, cr), - "agents": buildLlamaStackAgentProviders(r, cr), // Required for MCP - "inference": inferenceProviders, - "safety": buildLlamaStackSafety(r, cr), // Required by agents provider - // "telemetry": buildLlamaStackTelemetry(r, cr), // Telemetry and tracing - "tool_runtime": buildLlamaStackToolRuntime(r, cr), // Required for RAG - "vector_io": buildLlamaStackVectorDB(r, cr), // Required for RAG + "files": buildLlamaStackFileProviders(r, cr), + "agents": buildLlamaStackAgentProviders(r, cr), + "inference": inferenceProviders, + "safety": buildLlamaStackSafety(r, cr), + "tool_runtime": buildLlamaStackToolRuntime(r, cr), + "vector_io": buildLlamaStackVectorDB(r, cr), } // Add top-level fields - config["scoring_fns"] = []interface{}{} // Keep empty for now + config["scoring_fns"] = []interface{}{} config["server"] = buildLlamaStackServerConfig(r, cr) - config["storage"] = buildLlamaStackStorage(r, cr) // Persistent storage configuration - // config["shields"] = buildLlamaStackShields(r, cr) // Commented out - not needed without safety API + config["storage"] = buildLlamaStackStorage(r, cr) config["vector_dbs"] = buildLlamaStackVectorDBs(r, cr) config["models"] = buildLlamaStackModels(r, cr) config["tool_groups"] = buildLlamaStackToolGroups(r, cr) @@ -699,10 +668,6 @@ func buildLCoreInferenceConfig(_ reconciler.Reconciler, cr *olsv1alpha1.OLSConfi } } -// ============================================================================ -// Optional Configuration Builders (commented out - uncomment to implement) -// ============================================================================ - // buildLCoreDatabaseConfig configures persistent database storage // Supports SQLite (file-based) or PostgreSQL (server-based) // Default: PostgreSQL (shared with App Server) @@ -825,28 +790,6 @@ func buildLCoreMCPServersConfig(r reconciler.Reconciler, cr *olsv1alpha1.OLSConf return mcpServers } -// buildLCoreAuthorizationConfig configures role-based access control -// Defines which actions different roles can perform -// Actions: query, list_models, list_providers, get_provider, get_metrics, get_config, info, model_override -// -//func buildLCoreAuthorizationConfig(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) map[string]interface{} { -// return map[string]interface{}{ -// "access_rules": []interface{}{ -// map[string]interface{}{ -// "role": "admin", -// "actions": []string{ -// "query", "list_models", "list_providers", "get_provider", -// "get_metrics", "get_config", "info", "model_override", -// }, -// }, -// map[string]interface{}{ -// "role": "user", -// "actions": []string{"query", "info"}, -// }, -// }, -// } -//} - // buildLCoreCustomizationConfig configures system prompt customization // Uses CR field if set, otherwise falls back to default (same as lightspeed-service) func buildLCoreCustomizationConfig(_ reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) map[string]interface{} { @@ -880,34 +823,8 @@ func buildLCoreConversationCacheConfig(r reconciler.Reconciler, _ *olsv1alpha1.O "namespace": "conversation_cache", // Separate schema for conversation cache }, } - - // Example: SQLite cache (requires persistent volume) - // return map[string]interface{}{ - // "type": "sqlite", - // "sqlite": map[string]interface{}{ - // "db_path": "/app-root/data/conversation_cache.db", - // }, - // } - } -// buildLCoreByokRagConfig configures Bring Your Own Knowledge RAG sources -// Allows adding custom document collections beyond default RAG databases -// Requires vector database setup and embedding model configuration -// -//func buildLCoreByokRagConfig(_ reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) []interface{} { -// return []interface{}{ -// map[string]interface{}{ -// "rag_id": "custom-docs", -// "rag_type": "chromadb", // or "pgvector" -// "embedding_model": "sentence-transformers/all-mpnet-base-v2", -// "embedding_dimension": 768, -// "vector_db_id": "custom-vectordb", -// "db_path": "/app-root/data/custom_rag.db", -// }, -// } -//} - // buildLCoreQuotaHandlersConfig configures token usage rate limiting // Controls how many tokens users or clusters can consume // Useful for cost management and preventing abuse @@ -1009,10 +926,6 @@ func buildLCoreConfigYAML(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) (s config["mcp_servers"] = mcpServers // Model Context Protocol servers } - // Optional features (uncomment to enable): - // "authorization": buildLCoreAuthorizationConfig(r, cr), // Role-based access control - // "byok_rag": buildLCoreByokRagConfig(r, cr), // Custom RAG sources - // Convert to YAML yamlBytes, err := yaml.Marshal(config) if err != nil { diff --git a/internal/controller/lcore/deployment.go b/internal/controller/lcore/deployment.go index ae212aae2..bab9f2189 100644 --- a/internal/controller/lcore/deployment.go +++ b/internal/controller/lcore/deployment.go @@ -192,8 +192,46 @@ func buildLlamaStackEnvVars(r reconciler.Reconciler, ctx context.Context, cr *ol } } - // For Azure providers, read the secret to support both authentication methods - if provider != nil && provider.Type == "azure_openai" { + // Handle credential environment variables based on provider configuration + if provider == nil { + // This should never happen in normal operation because ForEachExternalSecret + // uses "llm-provider-" sourced directly from cr.Spec.LLMConfig.Providers. + // Guard against any future refactoring that could break the invariant. + return fmt.Errorf("internal: provider '%s' not found in CR but has a registered secret '%s'", providerName, name) + } else if provider.ProviderType != "" { + // Generic provider configuration: use credentialKey field. + // Re-fetch the secret here as a fail-safe: the secret or its keys could have been + // modified after ValidateLLMCredentials ran earlier in the reconcile loop. + credentialKey := provider.CredentialKey + if credentialKey == "" { + credentialKey = utils.DefaultCredentialKey + } + + secret := &corev1.Secret{} + err := r.Get(ctx, client.ObjectKey{ + Name: name, + Namespace: r.GetNamespace(), + }, secret) + if err != nil { + return fmt.Errorf("failed to get secret %s: %w", name, err) + } + + // Create env var for the primary credential + if _, ok := secret.Data[credentialKey]; !ok { + return fmt.Errorf("secret %s missing credential key '%s' for provider '%s'", name, credentialKey, providerName) + } + envVars = append(envVars, corev1.EnvVar{ + Name: envVarBase + "_API_KEY", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: name}, + Key: credentialKey, + }, + }, + }) + + } else if provider.Type == "azure_openai" { + // Azure OpenAI provider: read secret to support both authentication methods secret := &corev1.Secret{} err := r.Get(ctx, client.ObjectKey{ Name: name, @@ -206,10 +244,10 @@ func buildLlamaStackEnvVars(r reconciler.Reconciler, ctx context.Context, cr *ol // Create environment variables for each key in the secret // Azure supports both API key (apitoken) and client credentials (client_id, tenant_id, client_secret) keyToEnvSuffix := map[string]string{ - "apitoken": "_API_KEY", - "client_id": "_CLIENT_ID", - "tenant_id": "_TENANT_ID", - "client_secret": "_CLIENT_SECRET", + utils.DefaultCredentialKey: "_API_KEY", + "client_id": "_CLIENT_ID", + "tenant_id": "_TENANT_ID", + "client_secret": "_CLIENT_SECRET", } for key := range secret.Data { @@ -244,13 +282,13 @@ func buildLlamaStackEnvVars(r reconciler.Reconciler, ctx context.Context, cr *ol }) } } else { - // For non-Azure providers, always use API key + // Standard providers: use API key from default credential key envVars = append(envVars, corev1.EnvVar{ Name: envVarBase + "_API_KEY", ValueFrom: &corev1.EnvVarSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: "apitoken", + Key: utils.DefaultCredentialKey, }, }, }) @@ -553,7 +591,7 @@ func addMCPHeaderSecretVolumesAndMounts(r reconciler.Reconciler, ctx context.Con } // Mount MCP header secrets using the same pattern as appserver - _ = utils.ForEachExternalSecret(cr, func(name, source string) error { + err := utils.ForEachExternalSecret(cr, func(name, source string) error { if strings.HasPrefix(source, "mcp-") { // Validate secret exists and has correct structure serverName := strings.TrimPrefix(source, "mcp-") @@ -580,7 +618,7 @@ func addMCPHeaderSecretVolumesAndMounts(r reconciler.Reconciler, ctx context.Con return nil }) - return nil + return err } // addDataCollectorVolumesAndMounts adds volumes and mounts needed for data collection (feedback/transcripts) diff --git a/internal/controller/lcore/deployment_test.go b/internal/controller/lcore/deployment_test.go index ac17120ae..651362442 100644 --- a/internal/controller/lcore/deployment_test.go +++ b/internal/controller/lcore/deployment_test.go @@ -1243,3 +1243,204 @@ func (m *mockReconcilerWithTelemetry) Get(ctx context.Context, key client.Object func (m *mockReconcilerWithTelemetry) GetDataverseExporterImage() string { return utils.DataverseExporterImageDefault } + +// mockReconcilerWithSecrets extends mockReconciler to return pre-configured fake +// secrets keyed by name. Use this when the code under test calls r.Get for a Secret. +type mockReconcilerWithSecrets struct { + mockReconciler + secrets map[string]*corev1.Secret // name -> secret +} + +func (m *mockReconcilerWithSecrets) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if secret, ok := obj.(*corev1.Secret); ok { + if fakeSecret, found := m.secrets[key.Name]; found { + *secret = *fakeSecret + return nil + } + } + return m.mockReconciler.Get(ctx, key, obj, opts...) +} + +// TestBuildLlamaStackEnvVars_GenericProvider_CustomCredentialKey verifies that when +// a generic provider sets CredentialKey to a non-default value, buildLlamaStackEnvVars +// wires the env var to SecretKeyRef.Key equal to that custom key name. +// This is the authoritative test for the CredentialKey field's runtime behaviour. +func TestBuildLlamaStackEnvVars_GenericProvider_CustomCredentialKey(t *testing.T) { + const secretName = "custom-provider-creds" + const customKey = "bearer_token" // non-default; default is "apitoken" + + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "custom-provider", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + CredentialKey: customKey, + CredentialsSecretRef: corev1.LocalObjectReference{Name: secretName}, + Config: &runtime.RawExtension{ + Raw: []byte(`{"url": "https://api.example.com/v1"}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + }, + }, + }, + } + + fakeSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName}, + Data: map[string][]byte{ + customKey: []byte("my-custom-token"), + }, + } + + r := &mockReconcilerWithSecrets{ + secrets: map[string]*corev1.Secret{secretName: fakeSecret}, + } + + ctx := context.Background() + envVars, err := buildLlamaStackEnvVars(r, ctx, cr) + if err != nil { + t.Fatalf("buildLlamaStackEnvVars returned error: %v", err) + } + + // Find the CUSTOM_PROVIDER_API_KEY env var + var apiKeyEnv *corev1.EnvVar + for i := range envVars { + if envVars[i].Name == "CUSTOM_PROVIDER_API_KEY" { + apiKeyEnv = &envVars[i] + break + } + } + + if apiKeyEnv == nil { + t.Fatal("CUSTOM_PROVIDER_API_KEY env var not found in generated env vars") + } + + // CRITICAL: The SecretKeyRef.Key must be the custom key, not the default "apitoken" + if apiKeyEnv.ValueFrom == nil || apiKeyEnv.ValueFrom.SecretKeyRef == nil { + t.Fatal("CUSTOM_PROVIDER_API_KEY env var does not reference a secret") + } + if apiKeyEnv.ValueFrom.SecretKeyRef.Name != secretName { + t.Errorf("Expected SecretKeyRef.Name=%q, got %q", secretName, apiKeyEnv.ValueFrom.SecretKeyRef.Name) + } + if apiKeyEnv.ValueFrom.SecretKeyRef.Key != customKey { + t.Errorf("Expected SecretKeyRef.Key=%q (custom credentialKey), got %q", customKey, apiKeyEnv.ValueFrom.SecretKeyRef.Key) + } + + t.Logf("✓ CredentialKey=%q correctly wired to SecretKeyRef.Key", customKey) +} + +// TestBuildLlamaStackEnvVars_GenericProvider_DefaultCredentialKey verifies that when +// CredentialKey is not set, buildLlamaStackEnvVars falls back to the default "apitoken" key. +func TestBuildLlamaStackEnvVars_GenericProvider_DefaultCredentialKey(t *testing.T) { + const secretName = "generic-provider-creds" + + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "generic-provider", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::custom", + // CredentialKey intentionally not set — should default to "apitoken" + CredentialsSecretRef: corev1.LocalObjectReference{Name: secretName}, + Config: &runtime.RawExtension{ + Raw: []byte(`{"url": "https://api.example.com/v1"}`), + }, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + }, + }, + }, + } + + fakeSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName}, + Data: map[string][]byte{ + "apitoken": []byte("my-generic-token"), + }, + } + + r := &mockReconcilerWithSecrets{ + secrets: map[string]*corev1.Secret{secretName: fakeSecret}, + } + + ctx := context.Background() + envVars, err := buildLlamaStackEnvVars(r, ctx, cr) + if err != nil { + t.Fatalf("buildLlamaStackEnvVars returned error: %v", err) + } + + var apiKeyEnv *corev1.EnvVar + for i := range envVars { + if envVars[i].Name == "GENERIC_PROVIDER_API_KEY" { + apiKeyEnv = &envVars[i] + break + } + } + + if apiKeyEnv == nil { + t.Fatal("GENERIC_PROVIDER_API_KEY env var not found") + } + + if apiKeyEnv.ValueFrom == nil || apiKeyEnv.ValueFrom.SecretKeyRef == nil { + t.Fatal("GENERIC_PROVIDER_API_KEY does not reference a secret") + } + if apiKeyEnv.ValueFrom.SecretKeyRef.Key != "apitoken" { + t.Errorf("Expected default SecretKeyRef.Key=%q, got %q", "apitoken", apiKeyEnv.ValueFrom.SecretKeyRef.Key) + } + + t.Logf("✓ Default credentialKey 'apitoken' correctly wired to SecretKeyRef.Key") +} + +// TestBuildLlamaStackEnvVars_GenericProvider_MissingCredentialKey verifies that when +// the secret does not contain the expected credentialKey, an error is returned rather +// than silently omitting the env var. +func TestBuildLlamaStackEnvVars_GenericProvider_MissingCredentialKey(t *testing.T) { + const secretName = "broken-creds" + + cr := &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{ + Providers: []olsv1alpha1.ProviderSpec{ + { + Name: "broken", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::broken", + CredentialKey: "my_api_key", + CredentialsSecretRef: corev1.LocalObjectReference{Name: secretName}, + Config: &runtime.RawExtension{Raw: []byte(`{}`)}, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }, + }, + }, + }, + } + + // Secret exists but does NOT contain the expected key + fakeSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: secretName}, + Data: map[string][]byte{ + "wrong_key": []byte("some-token"), + }, + } + + r := &mockReconcilerWithSecrets{ + secrets: map[string]*corev1.Secret{secretName: fakeSecret}, + } + + ctx := context.Background() + _, err := buildLlamaStackEnvVars(r, ctx, cr) + if err == nil { + t.Fatal("Expected error when credentialKey is absent from secret, got nil") + } + if !strings.Contains(err.Error(), "my_api_key") { + t.Errorf("Error should mention the missing key 'my_api_key', got: %v", err) + } + + t.Logf("✓ Missing credentialKey correctly returns error: %v", err) +} diff --git a/internal/controller/lcore/lcore_generic_provider_test.go b/internal/controller/lcore/lcore_generic_provider_test.go new file mode 100644 index 000000000..5218c275d --- /dev/null +++ b/internal/controller/lcore/lcore_generic_provider_test.go @@ -0,0 +1,391 @@ +package lcore + +import ( + "context" + + . "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" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +// ─── helpers ──────────────────────────────────────────────────────────────── + +// genericProvider builds a llamaStackGeneric ProviderSpec. +// Pass secretName="" to simulate a public/unauthenticated endpoint. +// Pass configRaw=nil to omit the Config field entirely. +func genericProvider(name, providerType string, configRaw []byte, secretName string) olsv1alpha1.ProviderSpec { + p := olsv1alpha1.ProviderSpec{ + Name: name, + Type: utils.LlamaStackGenericType, + ProviderType: providerType, + CredentialsSecretRef: corev1.LocalObjectReference{Name: secretName}, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + } + if configRaw != nil { + p.Config = &runtime.RawExtension{Raw: configRaw} + } + return p +} + +// crWith wraps providers into a minimal OLSConfig. +func crWith(providers ...olsv1alpha1.ProviderSpec) *olsv1alpha1.OLSConfig { + return &olsv1alpha1.OLSConfig{ + Spec: olsv1alpha1.OLSConfigSpec{ + LLMConfig: olsv1alpha1.LLMSpec{Providers: providers}, + }, + } +} + +// providerConfig extracts the "config" map from providers[idx]. +// [0] is always sentence-transformers; user providers start at [1]. +func providerConfig(providers []interface{}, idx int) map[string]interface{} { + return providers[idx].(map[string]interface{})["config"].(map[string]interface{}) +} + +// ─── tests ────────────────────────────────────────────────────────────────── + +var _ = Describe("Generic provider", func() { + + // ── buildLlamaStackInferenceProviders ──────────────────────────────────── + Describe("buildLlamaStackInferenceProviders", func() { + + It("returns only sentence-transformers for a nil CR", func() { + providers, err := buildLlamaStackInferenceProviders(nil, context.Background(), nil) + Expect(err).NotTo(HaveOccurred()) + Expect(providers).To(HaveLen(1)) + Expect(providers[0].(map[string]interface{})["provider_id"]).To(Equal("sentence-transformers")) + }) + + It("returns a clear error for invalid JSON in config", func() { + cr := crWith(genericProvider("p", "remote::openai", []byte(`{invalid`), "s")) + _, err := buildLlamaStackInferenceProviders(nil, context.Background(), cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to unmarshal config")) + }) + + It("auto-injects api_key when credentials are configured", func() { + cr := crWith(genericProvider("my-provider", "remote::openai", + []byte(`{"url":"https://example.com"}`), "my-secret")) + + providers, err := buildLlamaStackInferenceProviders(nil, context.Background(), cr) + Expect(err).NotTo(HaveOccurred()) + // [0] sentence-transformers, [1] my-provider + Expect(providers).To(HaveLen(2)) + + p := providers[1].(map[string]interface{}) + Expect(p["provider_id"]).To(Equal("my-provider")) + Expect(p["provider_type"]).To(Equal("remote::openai")) + Expect(providerConfig(providers, 1)["api_key"]).To(Equal("${env.MY_PROVIDER_API_KEY}")) + }) + + It("does not overwrite an api_key the user already supplied", func() { + cr := crWith(genericProvider("my-provider", "remote::openai", + []byte(`{"api_key":"${env.CUSTOM_KEY}"}`), "my-secret")) + + providers, err := buildLlamaStackInferenceProviders(nil, context.Background(), cr) + Expect(err).NotTo(HaveOccurred()) + Expect(providerConfig(providers, 1)["api_key"]).To(Equal("${env.CUSTOM_KEY}")) + }) + + It("does not inject api_key for a public/unauthenticated provider", func() { + cr := crWith(genericProvider("pub", "remote::ollama", + []byte(`{"url":"http://localhost:11434"}`), "" /* no secret */)) + + providers, err := buildLlamaStackInferenceProviders(nil, context.Background(), cr) + Expect(err).NotTo(HaveOccurred()) + Expect(providerConfig(providers, 1)).NotTo(HaveKey("api_key")) + }) + + It("injects api_key even for an empty config object {}", func() { + cr := crWith(genericProvider("p", "remote::openai", []byte(`{}`), "my-secret")) + + providers, err := buildLlamaStackInferenceProviders(nil, context.Background(), cr) + Expect(err).NotTo(HaveOccurred()) + Expect(providerConfig(providers, 1)).To(HaveKey("api_key")) + }) + + It("configures multiple generic providers independently", func() { + cr := crWith( + genericProvider("alpha", "remote::openai", + []byte(`{"url":"https://alpha.example.com"}`), "s-alpha"), + genericProvider("beta", "remote::vllm", + []byte(`{"url":"https://beta.example.com"}`), "s-beta"), + ) + + providers, err := buildLlamaStackInferenceProviders(nil, context.Background(), cr) + Expect(err).NotTo(HaveOccurred()) + // [0] sentence-transformers, [1] alpha, [2] beta + Expect(providers).To(HaveLen(3)) + + alpha := providers[1].(map[string]interface{}) + Expect(alpha["provider_id"]).To(Equal("alpha")) + Expect(alpha["provider_type"]).To(Equal("remote::openai")) + + beta := providers[2].(map[string]interface{}) + Expect(beta["provider_id"]).To(Equal("beta")) + Expect(beta["provider_type"]).To(Equal("remote::vllm")) + + Expect(providerConfig(providers, 1)["api_key"]).To(Equal("${env.ALPHA_API_KEY}")) + Expect(providerConfig(providers, 2)["api_key"]).To(Equal("${env.BETA_API_KEY}")) + }) + + DescribeTable("produces the correct env-var substitution for various provider names", + func(providerName, wantAPIKey string) { + cr := crWith(genericProvider(providerName, "remote::openai", []byte(`{}`), "s")) + providers, err := buildLlamaStackInferenceProviders(nil, context.Background(), cr) + Expect(err).NotTo(HaveOccurred()) + Expect(providerConfig(providers, 1)["api_key"]).To(Equal(wantAPIKey)) + }, + Entry("hyphenated name", "my-openai", "${env.MY_OPENAI_API_KEY}"), + Entry("multiple hyphens", "provider-with-hyphens", "${env.PROVIDER_WITH_HYPHENS_API_KEY}"), + Entry("plain lowercase", "simpleprovider", "${env.SIMPLEPROVIDER_API_KEY}"), + Entry("leading digits are prefixed with underscore", "123provider", "${env._123PROVIDER_API_KEY}"), + ) + }) + + // ── buildLlamaStackYAML ────────────────────────────────────────────────── + Describe("buildLlamaStackYAML", func() { + + It("propagates a generic-provider invalid-JSON error to the caller", func() { + cr := crWith(genericProvider("p", "remote::openai", []byte(`{invalid`), "s")) + _, err := buildLlamaStackYAML(nil, context.Background(), cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to build inference providers")) + }) + + It("generates YAML containing the credential injection placeholder for a generic provider", func() { + cr := crWith(genericProvider("my-provider", "remote::openai", + []byte(`{"url":"https://example.com"}`), "my-secret")) + + yamlStr, err := buildLlamaStackYAML(nil, context.Background(), cr) + Expect(err).NotTo(HaveOccurred()) + // The auto-injected api_key must use the env-var substitution pattern. + Expect(yamlStr).To(ContainSubstring("${env.MY_PROVIDER_API_KEY}")) + // Provider metadata must appear in the YAML. + Expect(yamlStr).To(ContainSubstring("remote::openai")) + Expect(yamlStr).To(ContainSubstring("my-provider")) + }) + }) + + // ── buildLCoreConfigYAML ───────────────────────────────────────────────── + Describe("buildLCoreConfigYAML", func() { + + It("uses the generic provider name as inference default_provider", func() { + cr := crWith(genericProvider("my-generic-provider", "remote::openai", []byte(`{}`), "")) + cr.Spec.OLSConfig.DefaultProvider = "my-generic-provider" + cr.Spec.OLSConfig.DefaultModel = "test-model" + + yamlStr, err := buildLCoreConfigYAML(testReconcilerInstance, cr) + Expect(err).NotTo(HaveOccurred()) + Expect(yamlStr).To(ContainSubstring("my-generic-provider")) + Expect(yamlStr).To(ContainSubstring("test-model")) + }) + }) + + // ── getProviderType ────────────────────────────────────────────────────── + Describe("getProviderType", func() { + + DescribeTable("returns an error for unsupported or misused provider types", + func(providerType, expectedMsg string) { + p := &olsv1alpha1.ProviderSpec{Name: "test", Type: providerType} + _, err := getProviderType(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring(expectedMsg)) + }, + Entry("llamaStackGeneric without providerType field", + utils.LlamaStackGenericType, "requires providerType and config fields"), + Entry("watsonx is unsupported", "watsonx", "not currently supported by Llama Stack"), + Entry("bam is unsupported", "bam", "not currently supported by Llama Stack"), + Entry("completely unknown type", "totally-unknown", "unknown provider type"), + ) + }) + + // ── deepCopyMap ────────────────────────────────────────────────────────── + Describe("deepCopyMap", func() { + + It("returns nil for a nil input", func() { + Expect(deepCopyMap(nil)).To(BeNil()) + }) + + It("returns an empty (non-nil) map for an empty input", func() { + result := deepCopyMap(map[string]interface{}{}) + Expect(result).NotTo(BeNil()) + Expect(result).To(BeEmpty()) + }) + + It("copies primitive values correctly", func() { + src := map[string]interface{}{"s": "hello", "n": 42, "b": true} + Expect(deepCopyMap(src)).To(Equal(src)) + }) + + It("prevents mutation of nested maps", func() { + src := map[string]interface{}{ + "nested": map[string]interface{}{"key": "original"}, + } + result := deepCopyMap(src) + result["nested"].(map[string]interface{})["key"] = "modified" + + Expect(src["nested"].(map[string]interface{})["key"]).To(Equal("original"), + "modifying the copy must not affect the original") + }) + + It("prevents mutation of slice values", func() { + src := map[string]interface{}{"items": []interface{}{"a", "b", "c"}} + result := deepCopyMap(src) + result["items"].([]interface{})[0] = "modified" + + Expect(src["items"].([]interface{})[0]).To(Equal("a"), + "modifying the copy's slice must not affect the original") + }) + + It("deep-copies three levels of nesting", func() { + src := map[string]interface{}{ + "l1": map[string]interface{}{ + "l2": map[string]interface{}{"value": "deep"}, + }, + } + result := deepCopyMap(src) + result["l1"].(map[string]interface{})["l2"].(map[string]interface{})["value"] = "changed" + + got := src["l1"].(map[string]interface{})["l2"].(map[string]interface{})["value"] + Expect(got).To(Equal("deep"), "3-level deep value must not be mutated") + }) + + It("deep-copies mixed nested slices and maps", func() { + src := map[string]interface{}{ + "config": map[string]interface{}{ + "tags": []interface{}{"tag1", "tag2"}, + "meta": map[string]interface{}{"version": "1.0"}, + }, + } + result := deepCopyMap(src) + result["config"].(map[string]interface{})["tags"].([]interface{})[0] = "mutated" + result["config"].(map[string]interface{})["meta"].(map[string]interface{})["version"] = "2.0" + + Expect(src["config"].(map[string]interface{})["tags"].([]interface{})[0]).To(Equal("tag1")) + Expect(src["config"].(map[string]interface{})["meta"].(map[string]interface{})["version"]).To(Equal("1.0")) + }) + }) + + // ── CRD validation – ProviderSpec CEL rules ────────────────────────────── + Describe("CRD validation – ProviderSpec CEL rules", Ordered, func() { + + // validGenericProvider returns a fully valid llamaStackGeneric ProviderSpec + // that satisfies all five CRD CEL rules. + validGenericProvider := func() olsv1alpha1.ProviderSpec { + return olsv1alpha1.ProviderSpec{ + Name: "test-provider", + Type: utils.LlamaStackGenericType, + ProviderType: "remote::openai", + Config: &runtime.RawExtension{Raw: []byte(`{}`)}, + CredentialsSecretRef: corev1.LocalObjectReference{Name: "test-secret"}, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + } + } + + // withInvalidProvider attempts to update the singleton "cluster" CR with the + // given provider spec. Because the CRD enforces name == "cluster", all + // tests use Update so the name constraint never interferes with the CEL + // assertion under test. + withInvalidProvider := func(provider olsv1alpha1.ProviderSpec) error { + existing := &olsv1alpha1.OLSConfig{} + Expect(k8sClient.Get(ctx, crNamespacedName, existing)).To(Succeed()) + updated := existing.DeepCopy() + updated.Spec.LLMConfig.Providers = []olsv1alpha1.ProviderSpec{provider} + return k8sClient.Update(ctx, updated) + } + + // Capture the original spec once before any test in this block runs and + // restore it afterwards so the reconciler tests that follow are unaffected. + var savedSpec olsv1alpha1.OLSConfigSpec + + BeforeAll(func() { + existing := &olsv1alpha1.OLSConfig{} + Expect(k8sClient.Get(ctx, crNamespacedName, existing)).To(Succeed()) + savedSpec = *existing.Spec.DeepCopy() + }) + + AfterAll(func() { + existing := &olsv1alpha1.OLSConfig{} + Expect(k8sClient.Get(ctx, crNamespacedName, existing)).To(Succeed()) + restored := existing.DeepCopy() + restored.Spec = savedSpec + Expect(k8sClient.Update(ctx, restored)).To(Succeed()) + }) + + // Rule 1: !has(self.providerType) || has(self.config) + It("rejects a provider that sets providerType without config (Rule 1)", func() { + p := validGenericProvider() + p.Config = nil // violates Rule 1 + Expect(withInvalidProvider(p)).To(HaveOccurred()) + }) + + // Rule 2: !has(self.config) || has(self.providerType) + It("rejects a provider that sets config without providerType (Rule 2)", func() { + p := validGenericProvider() + p.ProviderType = "" // violates Rule 2 + err := withInvalidProvider(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("config")) + }) + + // Rule 3: !has(self.providerType) || self.type == "llamaStackGeneric" + It("rejects a provider that sets providerType with type != llamaStackGeneric (Rule 3)", func() { + p := validGenericProvider() + p.Type = "openai" // violates Rule 3 + err := withInvalidProvider(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("llamaStackGeneric")) + }) + + // Rule 4: self.type != "llamaStackGeneric" || (!has(self.deploymentName) && !has(self.projectID) && !has(self.url) && !has(self.apiVersion)) + It("rejects a llamaStackGeneric provider that also sets deploymentName (Rule 4)", func() { + p := validGenericProvider() + p.AzureDeploymentName = "my-deployment" // violates Rule 4 + err := withInvalidProvider(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("legacy")) + }) + + It("rejects a llamaStackGeneric provider that also sets url (Rule 4)", func() { + p := validGenericProvider() + p.URL = "https://my-endpoint.example.com" // violates Rule 4 + err := withInvalidProvider(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("legacy")) + }) + + It("rejects a llamaStackGeneric provider that also sets apiVersion (Rule 4)", func() { + p := validGenericProvider() + p.APIVersion = "2024-01" // violates Rule 4 + err := withInvalidProvider(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("legacy")) + }) + + // Rule 5: !has(self.credentialKey) || !self.credentialKey.matches('^[ \t\n\r\v\f]*$') + It("rejects a provider with a whitespace-only credentialKey (Rule 5)", func() { + p := validGenericProvider() + p.CredentialKey = " " // violates Rule 5 + err := withInvalidProvider(p) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("credentialKey")) + }) + + // Positive: a fully valid generic provider must be accepted. + // This is the only test that successfully mutates the CR; AfterAll restores it. + It("accepts a fully valid llamaStackGeneric provider", func() { + existing := &olsv1alpha1.OLSConfig{} + Expect(k8sClient.Get(ctx, crNamespacedName, existing)).To(Succeed()) + good := existing.DeepCopy() + good.Spec.LLMConfig.Providers = []olsv1alpha1.ProviderSpec{validGenericProvider()} + good.Spec.OLSConfig.DefaultProvider = "test-provider" + good.Spec.OLSConfig.DefaultModel = "test-model" + Expect(k8sClient.Update(ctx, good)).To(Succeed()) + }) + }) +}) diff --git a/internal/controller/utils/constants.go b/internal/controller/utils/constants.go index 8835eb09a..a36a9b6ed 100644 --- a/internal/controller/utils/constants.go +++ b/internal/controller/utils/constants.go @@ -118,6 +118,10 @@ const ( // Annotation key for serving certificate secret name // #nosec G101 ServingCertSecretAnnotationKey = "service.beta.openshift.io/serving-cert-secret-name" + // LlamaStackGenericType is the provider type for Llama Stack Generic mode + LlamaStackGenericType = "llamaStackGeneric" + // DefaultCredentialKey is the default secret key name for provider credentials + DefaultCredentialKey = "apitoken" // AzureOpenAIType is the name of the Azure OpenAI provider type AzureOpenAIType = "azure_openai" // FakeProviderType is the name of the fake provider type used for testing diff --git a/internal/controller/utils/test_fixtures.go b/internal/controller/utils/test_fixtures.go index fdadf579f..0384d3914 100644 --- a/internal/controller/utils/test_fixtures.go +++ b/internal/controller/utils/test_fixtures.go @@ -256,7 +256,7 @@ func GenerateRandomSecret() (*corev1.Secret, error) { Namespace: OLSNamespaceDefault, }, Data: map[string][]byte{ - "apitoken": []byte(token), + DefaultCredentialKey: []byte(token), }, } diff --git a/internal/controller/utils/utils.go b/internal/controller/utils/utils.go index 44c47984a..64d34e95b 100644 --- a/internal/controller/utils/utils.go +++ b/internal/controller/utils/utils.go @@ -20,6 +20,7 @@ import ( "crypto/sha1" //nolint:gosec "crypto/x509" "encoding/hex" + "encoding/json" "encoding/pem" "fmt" "os" @@ -87,15 +88,38 @@ func GetContainerIndex(deployment *appsv1.Deployment, containerName string) (int // Kubernetes resource names typically use hyphens (DNS-1123), but environment variable // names cannot contain hyphens. This function replaces hyphens with underscores and // converts to uppercase for consistency with environment variable naming conventions. +// Characters that are not valid in POSIX environment variable names ([A-Za-z0-9_]) +// are stripped before conversion. Hyphens are replaced with underscores. +// IMPORTANT: Names that sanitize to empty or start with digits are prefixed with an underscore +// to ensure compliance with POSIX environment variable naming rules (must not be empty, +// must not start with a digit). // -// Example: "my-provider" -> "MY_PROVIDER" +// Example: "my-provider" -> "MY_PROVIDER" +// Example: "provider@test" -> "PROVIDERTEST" +// Example: "123provider" -> "_123PROVIDER" +// Example: "!@#$%" -> "_" func ProviderNameToEnvVarName(providerName string) string { + // Strip characters that are invalid in POSIX environment variable names. + // Only alphanumeric, hyphens (converted below), and underscores are kept. + sanitized := envVarSanitizeRegex.ReplaceAllString(providerName, "") // Replace hyphens with underscores for valid environment variable names - envVarName := strings.ReplaceAll(providerName, "-", "_") + envVarName := strings.ReplaceAll(sanitized, "-", "_") // Convert to uppercase for standard environment variable convention - return strings.ToUpper(envVarName) + result := strings.ToUpper(envVarName) + + // POSIX env var names must not be empty and must not start with a digit. + // Prefix with underscore to satisfy both rules. + if len(result) == 0 || (result[0] >= '0' && result[0] <= '9') { + result = "_" + result + } + + return result } +// envVarSanitizeRegex strips characters that are not valid in environment variable names. +// Keeps alphanumeric, hyphens (converted to underscores later), and underscores. +var envVarSanitizeRegex = regexp.MustCompile(`[^a-zA-Z0-9_-]`) + // GetResourcesOrDefault returns custom resources from CR if specified, otherwise returns defaults. // This is a common pattern used across all component resource getters to avoid repetitive // null-checking logic. It provides a consistent way to handle user-configurable container resources @@ -609,11 +633,30 @@ func GetCAFromSecret(rclient client.Client, ctx context.Context, namespace, secr // ValidateLLMCredentials validates that all LLM provider credentials are present and valid. // It checks that each provider's credential secret exists and contains the required keys. +// For generic providers with custom config, it validates the config JSON is well-formed. func ValidateLLMCredentials(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) error { for _, provider := range cr.Spec.LLMConfig.Providers { + // Llama Stack Generic providers require LCore backend (AppServer does not support llamaStackGeneric providers) + // LCore is the future direction; AppServer is being deprecated + if provider.Type == LlamaStackGenericType && !r.UseLCore() { + return fmt.Errorf("LLM provider '%s' uses type '%s' which requires LCore backend. Enable LCore with --enable-lcore operator flag", provider.Name, LlamaStackGenericType) + } + + // Generic providers may operate without credentials (public/unauthenticated endpoints) if provider.CredentialsSecretRef.Name == "" { + if provider.Type == LlamaStackGenericType { + // Still validate Config JSON for public endpoints, even without credentials + if provider.Config != nil && provider.Config.Raw != nil { + var config map[string]interface{} + if err := json.Unmarshal(provider.Config.Raw, &config); err != nil { + return fmt.Errorf("LLM provider %s config is not valid JSON: %w", provider.Name, err) + } + } + continue + } return fmt.Errorf("provider %s missing credentials secret", provider.Name) } + secret := &corev1.Secret{} err := r.Get(ctx, client.ObjectKey{Name: provider.CredentialsSecretRef.Name, Namespace: r.GetNamespace()}, secret) if err != nil { @@ -622,9 +665,37 @@ func ValidateLLMCredentials(r reconciler.Reconciler, ctx context.Context, cr *ol } return fmt.Errorf("failed to get LLM provider %s credential secret %s: %w", provider.Name, provider.CredentialsSecretRef.Name, err) } - if provider.Type == AzureOpenAIType { - // Azure OpenAI secret must contain "apitoken" or 3 keys named "client_id", "tenant_id", "client_secret" - if _, ok := secret.Data["apitoken"]; ok { + + // Validate credential keys based on provider configuration + if provider.ProviderType != "" { + // Generic provider configuration: validate credentialKey exists + credentialKey := provider.CredentialKey + if credentialKey == "" { + credentialKey = DefaultCredentialKey + } + + // Validate credentialKey is not empty (should be caught by CRD validation but double-check) + if strings.TrimSpace(credentialKey) == "" { + return fmt.Errorf("LLM provider %s: credentialKey must not be empty or whitespace", provider.Name) + } + + // Check if the specified credential key exists in secret + if _, ok := secret.Data[credentialKey]; !ok { + return fmt.Errorf("LLM provider %s credential secret %s missing key '%s'", provider.Name, provider.CredentialsSecretRef.Name, credentialKey) + } + + // Validate provider config JSON is well-formed + if provider.Config != nil && provider.Config.Raw != nil { + var config map[string]interface{} + if err := json.Unmarshal(provider.Config.Raw, &config); err != nil { + // Strict validation: reject malformed JSON in generic provider config + return fmt.Errorf("LLM provider %s config is not valid JSON: %w", provider.Name, err) + } + } + + } else if provider.Type == AzureOpenAIType { + // Azure OpenAI provider: secret must contain default credential key or 3 keys named "client_id", "tenant_id", "client_secret" + if _, ok := secret.Data[DefaultCredentialKey]; ok { continue } for _, key := range []string{"client_id", "tenant_id", "client_secret"} { @@ -633,9 +704,9 @@ func ValidateLLMCredentials(r reconciler.Reconciler, ctx context.Context, cr *ol } } } else { - // Other providers (e.g. WatsonX, OpenAI) must contain a key named "apitoken" - if _, ok := secret.Data["apitoken"]; !ok { - return fmt.Errorf("LLM provider %s credential secret %s missing key 'apitoken'", provider.Name, provider.CredentialsSecretRef.Name) + // Standard providers: must contain the default credential key + if _, ok := secret.Data[DefaultCredentialKey]; !ok { + return fmt.Errorf("LLM provider %s credential secret %s missing key '%s'", provider.Name, provider.CredentialsSecretRef.Name, DefaultCredentialKey) } } } diff --git a/internal/controller/utils/utils_generic_provider_test.go b/internal/controller/utils/utils_generic_provider_test.go new file mode 100644 index 000000000..25fed77cf --- /dev/null +++ b/internal/controller/utils/utils_generic_provider_test.go @@ -0,0 +1,254 @@ +package utils + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + olsv1alpha1 "github.com/openshift/lightspeed-operator/api/v1alpha1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" +) + +// createTestSecret creates a Secret in the test namespace and registers a +// DeferCleanup to delete it at the end of the enclosing It block. +// This eliminates the need for a shared AfterEach + nil-check pattern. +func createTestSecret(name string, data map[string][]byte) { + s := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: OLSNamespaceDefault}, + Data: data, + } + Expect(k8sClient.Create(testCtx, s)).To(Succeed()) + DeferCleanup(k8sClient.Delete, testCtx, s) +} + +// newGenericReconciler returns a fresh TestReconciler with LCore enabled. +// Calling this at the start of each It block prevents state leaking between tests. +func newGenericReconciler() *TestReconciler { + r := NewTestReconciler(k8sClient, logf.Log.WithName("test"), k8sClient.Scheme(), OLSNamespaceDefault) + r.SetUseLCore(true) + return r +} + +// ─── ProviderNameToEnvVarName – edge cases ─────────────────────────────────── + +var _ = Describe("ProviderNameToEnvVarName", func() { + + DescribeTable("converts provider names to valid env-var identifiers", + func(input, expected string) { + Expect(ProviderNameToEnvVarName(input)).To(Equal(expected)) + }, + // Existing cases (canonical examples) + Entry("hyphen becomes underscore", "my-provider", "MY_PROVIDER"), + Entry("all uppercase passthrough", "PROVIDER", "PROVIDER"), + Entry("lowercase to uppercase", "provider", "PROVIDER"), + Entry("empty string gets underscore prefix (POSIX compliance)", "", "_"), + Entry("mixed case with hyphen", "OpenAI-Provider", "OPENAI_PROVIDER"), + // Edge cases with leading digits + Entry("leading digit gets underscore prefix (POSIX compliance)", "123provider", "_123PROVIDER"), + Entry("digit-only name gets underscore prefix (POSIX compliance)", "12345", "_12345"), + // Special character stripping + Entry("dot is stripped (not alphanumeric/hyphen/underscore)", "my.provider", "MYPROVIDER"), + Entry("at-sign is stripped", "provider@test", "PROVIDERTEST"), + // All special characters result in underscore (collision prevention) + Entry("all special characters sanitize to empty, then prefix", "!@#$%", "_"), + Entry("whitespace-only sanitizes to empty, then prefix", " ", "_"), + ) +}) + +// ─── ValidateLLMCredentials – generic provider scenarios ───────────────────── + +var _ = Describe("ValidateLLMCredentials – generic provider", func() { + + It("fails when the credential secret does not exist", func() { + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + cr.Spec.LLMConfig.Providers[0].CredentialKey = DefaultCredentialKey + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "does-not-exist" + + err := ValidateLLMCredentials(r, testCtx, cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("does-not-exist not found")) + }) + + It("succeeds when no credentials are configured (public/unauthenticated endpoint)", func() { + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::ollama" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "" + + Expect(ValidateLLMCredentials(r, testCtx, cr)).To(Succeed()) + }) + + It("fails when public generic provider has malformed JSON config", func() { + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::ollama" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "" + cr.Spec.LLMConfig.Providers[0].Config = &runtime.RawExtension{Raw: []byte(`{invalid json`)} + + err := ValidateLLMCredentials(r, testCtx, cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not valid JSON")) + }) + + It("succeeds when public generic provider has valid JSON config", func() { + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::ollama" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "" + cr.Spec.LLMConfig.Providers[0].Config = &runtime.RawExtension{Raw: []byte(`{"url":"http://localhost:11434"}`)} + + Expect(ValidateLLMCredentials(r, testCtx, cr)).To(Succeed()) + }) + + It("succeeds when public generic provider has no config at all", func() { + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::ollama" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "" + cr.Spec.LLMConfig.Providers[0].Config = nil + + Expect(ValidateLLMCredentials(r, testCtx, cr)).To(Succeed()) + }) + + It("succeeds with a custom credentialKey that exists in the secret", func() { + createTestSecret("gen-custom-key-secret", map[string][]byte{ + "bearer_token": []byte("tok"), + }) + + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + cr.Spec.LLMConfig.Providers[0].CredentialKey = "bearer_token" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "gen-custom-key-secret" + + Expect(ValidateLLMCredentials(r, testCtx, cr)).To(Succeed()) + }) + + It("succeeds with the default credentialKey (apitoken) when credentialKey is omitted", func() { + createTestSecret("gen-default-key-secret", map[string][]byte{ + DefaultCredentialKey: []byte("tok"), + }) + + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + cr.Spec.LLMConfig.Providers[0].CredentialKey = "" // omitted → defaults to "apitoken" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "gen-default-key-secret" + + Expect(ValidateLLMCredentials(r, testCtx, cr)).To(Succeed()) + }) + + It("fails when the secret exists but lacks the specified credentialKey", func() { + createTestSecret("gen-missing-key-secret", map[string][]byte{ + "wrong_key": []byte("tok"), + }) + + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + cr.Spec.LLMConfig.Providers[0].CredentialKey = "expected_key" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "gen-missing-key-secret" + + err := ValidateLLMCredentials(r, testCtx, cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing key 'expected_key'")) + }) + + It("fails at runtime when credentialKey is whitespace-only", func() { + createTestSecret("gen-whitespace-key-secret", map[string][]byte{ + DefaultCredentialKey: []byte("tok"), + }) + + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + cr.Spec.LLMConfig.Providers[0].CredentialKey = " " + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "gen-whitespace-key-secret" + + err := ValidateLLMCredentials(r, testCtx, cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("credentialKey must not be empty or whitespace")) + }) + + It("fails when config contains malformed JSON", func() { + createTestSecret("gen-invalid-json-secret", map[string][]byte{ + DefaultCredentialKey: []byte("tok"), + }) + + r := newGenericReconciler() + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + cr.Spec.LLMConfig.Providers[0].CredentialKey = DefaultCredentialKey + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "gen-invalid-json-secret" + cr.Spec.LLMConfig.Providers[0].Config = &runtime.RawExtension{Raw: []byte(`{invalid`)} + + err := ValidateLLMCredentials(r, testCtx, cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not valid JSON")) + }) + + It("fails when generic provider requires LCore backend but LCore is disabled", func() { + createTestSecret("gen-lcore-required-secret", map[string][]byte{ + DefaultCredentialKey: []byte("tok"), + }) + + r := newGenericReconciler() + r.SetUseLCore(false) // override to disable LCore + + cr := GetDefaultOLSConfigCR() + cr.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + cr.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "gen-lcore-required-secret" + + err := ValidateLLMCredentials(r, testCtx, cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("requires LCore backend")) + Expect(err.Error()).To(ContainSubstring("--enable-lcore")) + }) + + It("fails on the generic provider even when a valid legacy provider precedes it", func() { + // Scenario: [legacy openai, generic llamaStackGeneric] with LCore disabled. + // The legacy provider passes secret validation; the generic provider must + // still be rejected because LCore is disabled. + createTestSecret("mix-legacy-secret", map[string][]byte{ + DefaultCredentialKey: []byte("tok"), + }) + + r := newGenericReconciler() + r.SetUseLCore(false) + + cr := GetDefaultOLSConfigCR() + // Make Provider[0] a valid legacy openai provider so it passes validation. + cr.Spec.LLMConfig.Providers[0].Type = "openai" + cr.Spec.LLMConfig.Providers[0].ProviderType = "" + cr.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "mix-legacy-secret" + + // Append Provider[1] as a llamaStackGeneric provider. + cr.Spec.LLMConfig.Providers = append(cr.Spec.LLMConfig.Providers, olsv1alpha1.ProviderSpec{ + Name: "generic-mixed", + Type: LlamaStackGenericType, + ProviderType: "remote::openai", + CredentialsSecretRef: corev1.LocalObjectReference{Name: "mix-legacy-secret"}, + Models: []olsv1alpha1.ModelSpec{{Name: "test-model"}}, + }) + + err := ValidateLLMCredentials(r, testCtx, cr) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("requires LCore backend")) + Expect(err.Error()).To(ContainSubstring("generic-mixed")) + }) +}) diff --git a/internal/controller/utils/utils_misc_test.go b/internal/controller/utils/utils_misc_test.go index 740625d75..71a3e82e1 100644 --- a/internal/controller/utils/utils_misc_test.go +++ b/internal/controller/utils/utils_misc_test.go @@ -10,6 +10,7 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/intstr" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -183,7 +184,7 @@ var _ = Describe("Utility Functions", func() { It("should handle empty string", func() { result := ProviderNameToEnvVarName("") - Expect(result).To(Equal("")) + Expect(result).To(Equal("_")) }) It("should handle mixed case with hyphens", func() { @@ -891,7 +892,7 @@ cNHlzbRSivTDuHmXJdCYIdd8cnH6EbPm3zNg0jU5Au6OrvDZYifP+DtuiLmJct4= Namespace: OLSNamespaceDefault, }, Data: map[string][]byte{ - "apitoken": []byte("test-token"), + DefaultCredentialKey: []byte("test-token"), }, } err := k8sClient.Create(testCtx, testSecret) @@ -977,6 +978,188 @@ cNHlzbRSivTDuHmXJdCYIdd8cnH6EbPm3zNg0jU5Au6OrvDZYifP+DtuiLmJct4= err = ValidateLLMCredentials(testReconciler, testCtx, testCR) Expect(err).NotTo(HaveOccurred()) }) + + It("should validate generic provider with custom credentialKey", func() { + By("Create a test secret with custom key") + testSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-generic-custom-key-secret", + Namespace: OLSNamespaceDefault, + }, + Data: map[string][]byte{ + "bearer_token": []byte("test-token"), + }, + } + err := k8sClient.Create(testCtx, testSecret) + Expect(err).NotTo(HaveOccurred()) + + By("Create a test CR with custom credentialKey") + testCR := GetDefaultOLSConfigCR() + testCR.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + testCR.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + testCR.Spec.LLMConfig.Providers[0].CredentialKey = "bearer_token" + testCR.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "test-generic-custom-key-secret" + testReconciler.SetUseLCore(true) + + By("Check LLM credentials - should succeed") + err = ValidateLLMCredentials(testReconciler, testCtx, testCR) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should validate generic provider with default credentialKey", func() { + By("Create a test secret with apitoken key") + testSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-generic-default-key-secret", + Namespace: OLSNamespaceDefault, + }, + Data: map[string][]byte{ + DefaultCredentialKey: []byte("test-token"), + }, + } + err := k8sClient.Create(testCtx, testSecret) + Expect(err).NotTo(HaveOccurred()) + + By("Create a test CR with default credentialKey (apitoken)") + testCR := GetDefaultOLSConfigCR() + testCR.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + testCR.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + testCR.Spec.LLMConfig.Providers[0].CredentialKey = DefaultCredentialKey + testCR.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "test-generic-default-key-secret" + testReconciler.SetUseLCore(true) + + By("Check LLM credentials - should succeed") + err = ValidateLLMCredentials(testReconciler, testCtx, testCR) + Expect(err).NotTo(HaveOccurred()) + }) + + It("should fail when generic provider has whitespace-only credentialKey", func() { + By("Create a test secret") + testSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-generic-whitespace-secret", + Namespace: OLSNamespaceDefault, + }, + Data: map[string][]byte{ + DefaultCredentialKey: []byte("test-token"), + }, + } + err := k8sClient.Create(testCtx, testSecret) + Expect(err).NotTo(HaveOccurred()) + + By("Create a test CR with whitespace-only credentialKey") + testCR := GetDefaultOLSConfigCR() + testCR.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + testCR.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + testCR.Spec.LLMConfig.Providers[0].CredentialKey = " " + testCR.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "test-generic-whitespace-secret" + testReconciler.SetUseLCore(true) + + By("Check LLM credentials - should fail") + err = ValidateLLMCredentials(testReconciler, testCtx, testCR) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("credentialKey must not be empty or whitespace")) + }) + + It("should fail when generic provider has invalid JSON in Config", func() { + By("Create a test secret") + testSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-generic-invalid-json-secret", + Namespace: OLSNamespaceDefault, + }, + Data: map[string][]byte{ + DefaultCredentialKey: []byte("test-token"), + }, + } + err := k8sClient.Create(testCtx, testSecret) + Expect(err).NotTo(HaveOccurred()) + + By("Create a test CR with invalid JSON config") + testCR := GetDefaultOLSConfigCR() + testCR.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + testCR.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + testCR.Spec.LLMConfig.Providers[0].CredentialKey = DefaultCredentialKey + testCR.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "test-generic-invalid-json-secret" + testCR.Spec.LLMConfig.Providers[0].Config = &runtime.RawExtension{ + Raw: []byte(`{invalid json`), + } + testReconciler.SetUseLCore(true) + + By("Check LLM credentials - should fail due to invalid JSON") + err = ValidateLLMCredentials(testReconciler, testCtx, testCR) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not valid JSON")) + }) + + It("should fail when generic provider secret is missing the specified credential key", func() { + By("Create a test secret without the expected key") + testSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-generic-missing-key-secret", + Namespace: OLSNamespaceDefault, + }, + Data: map[string][]byte{ + "wrong_key": []byte("test-token"), + }, + } + err := k8sClient.Create(testCtx, testSecret) + Expect(err).NotTo(HaveOccurred()) + + By("Create a test CR with credentialKey that doesn't exist in secret") + testCR := GetDefaultOLSConfigCR() + testCR.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + testCR.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + testCR.Spec.LLMConfig.Providers[0].CredentialKey = "my_api_key" + testCR.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "test-generic-missing-key-secret" + testReconciler.SetUseLCore(true) + + By("Check LLM credentials - should fail due to missing key") + err = ValidateLLMCredentials(testReconciler, testCtx, testCR) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing key 'my_api_key'")) + }) + + It("should fail when llamaStackGeneric provider is used with LCore disabled", func() { + By("Create a test CR with llamaStackGeneric provider") + testCR := GetDefaultOLSConfigCR() + testCR.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + testCR.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + testReconciler.SetUseLCore(false) + + By("Check LLM credentials - should fail because LCore is disabled") + err := ValidateLLMCredentials(testReconciler, testCtx, testCR) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("requires LCore backend")) + Expect(err.Error()).To(ContainSubstring("--enable-lcore")) + }) + + It("should succeed when llamaStackGeneric provider is used with LCore enabled", func() { + By("Create a test secret with custom credentialKey") + testSecret = &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-generic-lcore-secret", + Namespace: OLSNamespaceDefault, + }, + Data: map[string][]byte{ + DefaultCredentialKey: []byte("test-token"), + }, + } + err := k8sClient.Create(testCtx, testSecret) + Expect(err).NotTo(HaveOccurred()) + + By("Create a test CR with llamaStackGeneric provider and LCore enabled") + testCR := GetDefaultOLSConfigCR() + testCR.Spec.LLMConfig.Providers[0].Type = LlamaStackGenericType + testCR.Spec.LLMConfig.Providers[0].ProviderType = "remote::openai" + testCR.Spec.LLMConfig.Providers[0].CredentialKey = DefaultCredentialKey + testCR.Spec.LLMConfig.Providers[0].CredentialsSecretRef.Name = "test-generic-lcore-secret" + testReconciler.SetUseLCore(true) + + By("Check LLM credentials - should succeed") + err = ValidateLLMCredentials(testReconciler, testCtx, testCR) + Expect(err).NotTo(HaveOccurred()) + }) }) })