diff --git a/infra/feast-operator/internal/controller/featurestore_controller_oidc_auth_test.go b/infra/feast-operator/internal/controller/featurestore_controller_oidc_auth_test.go index 82a7ebeb4ad..bb5cc4fb4b2 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_oidc_auth_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_oidc_auth_test.go @@ -487,6 +487,9 @@ func expectedServerOidcAuthorizConfig() services.AuthzConfig { OidcParameters: map[string]interface{}{ string(services.OidcAuthDiscoveryUrl): "auth-discovery-url", string(services.OidcClientId): "client-id", + string(services.OidcClientSecret): "client-secret", + string(services.OidcUsername): "username", + string(services.OidcPassword): "password", }, } } @@ -494,9 +497,11 @@ func expectedClientOidcAuthorizConfig() services.AuthzConfig { return services.AuthzConfig{ Type: services.OidcAuthType, OidcParameters: map[string]interface{}{ - string(services.OidcClientSecret): "client-secret", - string(services.OidcUsername): "username", - string(services.OidcPassword): "password"}, + string(services.OidcClientId): "client-id", + string(services.OidcAuthDiscoveryUrl): "auth-discovery-url", + string(services.OidcClientSecret): "client-secret", + string(services.OidcUsername): "username", + string(services.OidcPassword): "password"}, } } diff --git a/infra/feast-operator/internal/controller/services/repo_config.go b/infra/feast-operator/internal/controller/services/repo_config.go index b893f031d41..00c09563fd9 100644 --- a/infra/feast-operator/internal/controller/services/repo_config.go +++ b/infra/feast-operator/internal/controller/services/repo_config.go @@ -102,15 +102,15 @@ func getBaseServiceRepoConfig( return repoConfig, authSecretErr } - oidcServerProperties := map[string]interface{}{} - for _, oidcServerProperty := range OidcServerProperties { - if val, exists := propertiesMap[string(oidcServerProperty)]; exists { - oidcServerProperties[string(oidcServerProperty)] = val + oidcParameters := map[string]interface{}{} + for _, oidcProperty := range OidcProperties { + if val, exists := propertiesMap[string(oidcProperty)]; exists { + oidcParameters[string(oidcProperty)] = val } else { - return repoConfig, missingOidcSecretProperty(oidcServerProperty) + return repoConfig, missingOidcSecretProperty(oidcProperty) } } - repoConfig.AuthzConfig.OidcParameters = oidcServerProperties + repoConfig.AuthzConfig.OidcParameters = oidcParameters } return repoConfig, nil @@ -327,11 +327,11 @@ func getRepoConfig( } oidcClientProperties := map[string]interface{}{} - for _, oidcClientProperty := range OidcClientProperties { - if val, exists := propertiesMap[string(oidcClientProperty)]; exists { - oidcClientProperties[string(oidcClientProperty)] = val + for _, oidcProperty := range OidcProperties { + if val, exists := propertiesMap[string(oidcProperty)]; exists { + oidcClientProperties[string(oidcProperty)] = val } else { - return repoConfig, missingOidcSecretProperty(oidcClientProperty) + return repoConfig, missingOidcSecretProperty(oidcProperty) } } repoConfig.AuthzConfig.OidcParameters = oidcClientProperties diff --git a/infra/feast-operator/internal/controller/services/repo_config_test.go b/infra/feast-operator/internal/controller/services/repo_config_test.go index b31e40d7382..efde05df335 100644 --- a/infra/feast-operator/internal/controller/services/repo_config_test.go +++ b/infra/feast-operator/internal/controller/services/repo_config_test.go @@ -214,9 +214,12 @@ var _ = Describe("Repo Config", func() { repoConfig, err = getServiceRepoConfig(featureStore, secretExtractionFunc) Expect(err).NotTo(HaveOccurred()) Expect(repoConfig.AuthzConfig.Type).To(Equal(OidcAuthType)) - Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveLen(2)) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveLen(5)) Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcClientId))) Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcAuthDiscoveryUrl))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcClientSecret))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcUsername))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcPassword))) Expect(repoConfig.OfflineStore).To(Equal(expectedOfflineConfig)) Expect(repoConfig.OnlineStore).To(Equal(defaultOnlineStoreConfig(featureStore))) Expect(repoConfig.Registry).To(Equal(defaultRegistryConfig(featureStore))) @@ -224,7 +227,9 @@ var _ = Describe("Repo Config", func() { repoConfig, err = getClientRepoConfig(featureStore, secretExtractionFunc, nil) Expect(err).NotTo(HaveOccurred()) Expect(repoConfig.AuthzConfig.Type).To(Equal(OidcAuthType)) - Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveLen(3)) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveLen(5)) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcClientId))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcAuthDiscoveryUrl))) Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcClientSecret))) Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcUsername))) Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcPassword))) @@ -314,14 +319,9 @@ var _ = Describe("Repo Config", func() { _, err := getServiceRepoConfig(featureStore, secretExtractionFunc) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) - _, err = getServiceRepoConfig(featureStore, secretExtractionFunc) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) - _, err = getServiceRepoConfig(featureStore, secretExtractionFunc) + _, err = getClientRepoConfig(featureStore, secretExtractionFunc, nil) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) - _, err = getClientRepoConfig(featureStore, secretExtractionFunc, nil) - Expect(err).ToNot(HaveOccurred()) By("Having invalid client oidc authorization") featureStore.Spec.AuthzConfig = &feastdevv1.AuthzConfig{ @@ -341,12 +341,6 @@ var _ = Describe("Repo Config", func() { _, err = getServiceRepoConfig(featureStore, secretExtractionFunc) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) - _, err = getServiceRepoConfig(featureStore, secretExtractionFunc) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) - _, err = getServiceRepoConfig(featureStore, secretExtractionFunc) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) _, err = getClientRepoConfig(featureStore, secretExtractionFunc, nil) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) diff --git a/infra/feast-operator/internal/controller/services/services_types.go b/infra/feast-operator/internal/controller/services/services_types.go index 5997307ab1c..d437c703a6a 100644 --- a/infra/feast-operator/internal/controller/services/services_types.go +++ b/infra/feast-operator/internal/controller/services/services_types.go @@ -210,6 +210,7 @@ var ( OidcServerProperties = []OidcPropertyType{OidcClientId, OidcAuthDiscoveryUrl} OidcClientProperties = []OidcPropertyType{OidcClientSecret, OidcUsername, OidcPassword} + OidcProperties = []OidcPropertyType{OidcClientId, OidcAuthDiscoveryUrl, OidcClientSecret, OidcUsername, OidcPassword} ) // Feast server types: Reserved only for server types like Online, Offline, and Registry servers. Should not be used for client types like the UI, etc.