diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 4bc0aa7c5e..bdeecd570d 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -280,8 +280,10 @@ type OptionalConfigs struct { } // AuthzConfig defines the authorization settings for the deployed Feast services. +// +kubebuilder:validation:XValidation:rule="[has(self.kubernetes), has(self.oidc)].exists_one(c, c)",message="One selection required between kubernetes or oidc." type AuthzConfig struct { KubernetesAuthz *KubernetesAuthz `json:"kubernetes,omitempty"` + OidcAuthz *OidcAuthz `json:"oidc,omitempty"` } // KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. @@ -296,6 +298,12 @@ type KubernetesAuthz struct { Roles []string `json:"roles,omitempty"` } +// OidcAuthz defines the authorization settings for deployments using an Open ID Connect identity provider. +// https://auth0.com/docs/authenticate/protocols/openid-connect-protocol +type OidcAuthz struct { + SecretRef corev1.LocalObjectReference `json:"secretRef"` +} + // TlsConfigs configures server TLS for a feast service. in an openshift cluster, this is configured by default using service serving certificates. // +kubebuilder:validation:XValidation:rule="(!has(self.disable) || !self.disable) ? has(self.secretRef) : true",message="`secretRef` required if `disable` is false." type TlsConfigs struct { diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index 8675397fee..3f317c650e 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -34,6 +34,11 @@ func (in *AuthzConfig) DeepCopyInto(out *AuthzConfig) { *out = new(KubernetesAuthz) (*in).DeepCopyInto(*out) } + if in.OidcAuthz != nil { + in, out := &in.OidcAuthz, &out.OidcAuthz + *out = new(OidcAuthz) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthzConfig. @@ -373,6 +378,22 @@ func (in *OfflineTlsConfigs) DeepCopy() *OfflineTlsConfigs { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OidcAuthz) DeepCopyInto(out *OidcAuthz) { + *out = *in + out.SecretRef = in.SecretRef +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OidcAuthz. +func (in *OidcAuthz) DeepCopy() *OidcAuthz { + if in == nil { + return nil + } + out := new(OidcAuthz) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OnlineStore) DeepCopyInto(out *OnlineStore) { *out = *in diff --git a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml index 958c7cdddb..fddac5458f 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -69,7 +69,31 @@ spec: type: string type: array type: object + oidc: + description: |- + OidcAuthz defines the authorization settings for deployments using an Open ID Connect identity provider. + https://auth0.com/docs/authenticate/protocols/openid-connect-protocol + properties: + secretRef: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + required: + - secretRef + type: object type: object + x-kubernetes-validations: + - message: One selection required between kubernetes or oidc. + rule: '[has(self.kubernetes), has(self.oidc)].exists_one(c, c)' feastProject: description: FeastProject is the Feast project id. This can be any alphanumeric string with underscores, but it cannot start with an @@ -1238,7 +1262,32 @@ spec: type: string type: array type: object + oidc: + description: |- + OidcAuthz defines the authorization settings for deployments using an Open ID Connect identity provider. + https://auth0.com/docs/authenticate/protocols/openid-connect-protocol + properties: + secretRef: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + required: + - secretRef + type: object type: object + x-kubernetes-validations: + - message: One selection required between kubernetes or oidc. + rule: '[has(self.kubernetes), has(self.oidc)].exists_one(c, + c)' feastProject: description: FeastProject is the Feast project id. This can be any alphanumeric string with underscores, but it cannot start diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml new file mode 100644 index 0000000000..c70f172ded --- /dev/null +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml @@ -0,0 +1,35 @@ +apiVersion: feast.dev/v1alpha1 +kind: FeatureStore +metadata: + name: sample-oidc-auth +spec: + feastProject: my_project + services: + onlineStore: + persistence: + file: + path: /data/online_store.db + offlineStore: + persistence: + file: + type: dask + registry: + local: + persistence: + file: + path: /data/registry.db + authz: + oidc: + secretRef: + name: oidc-secret +--- +kind: Secret +apiVersion: v1 +metadata: + name: oidc-secret +stringData: + client_id: client_id + auth_discovery_url: auth_discovery_url + client_secret: client_secret + username: username + password: password diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index b5e103f969..59ca5505b9 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -77,7 +77,31 @@ spec: type: string type: array type: object + oidc: + description: |- + OidcAuthz defines the authorization settings for deployments using an Open ID Connect identity provider. + https://auth0.com/docs/authenticate/protocols/openid-connect-protocol + properties: + secretRef: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + required: + - secretRef + type: object type: object + x-kubernetes-validations: + - message: One selection required between kubernetes or oidc. + rule: '[has(self.kubernetes), has(self.oidc)].exists_one(c, c)' feastProject: description: FeastProject is the Feast project id. This can be any alphanumeric string with underscores, but it cannot start with an @@ -1246,7 +1270,32 @@ spec: type: string type: array type: object + oidc: + description: |- + OidcAuthz defines the authorization settings for deployments using an Open ID Connect identity provider. + https://auth0.com/docs/authenticate/protocols/openid-connect-protocol + properties: + secretRef: + description: |- + LocalObjectReference contains enough information to let you locate the + referenced object inside the same namespace. + properties: + name: + description: |- + Name of the referent. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + TODO: Add other useful fields. apiVersion, kind, uid? + type: string + type: object + x-kubernetes-map-type: atomic + required: + - secretRef + type: object type: object + x-kubernetes-validations: + - message: One selection required between kubernetes or oidc. + rule: '[has(self.kubernetes), has(self.oidc)].exists_one(c, + c)' feastProject: description: FeastProject is the Feast project id. This can be any alphanumeric string with underscores, but it cannot start diff --git a/infra/feast-operator/internal/controller/authz/authz.go b/infra/feast-operator/internal/controller/authz/authz.go index c7d4b89606..efcae23a4b 100644 --- a/infra/feast-operator/internal/controller/authz/authz.go +++ b/infra/feast-operator/internal/controller/authz/authz.go @@ -18,15 +18,13 @@ import ( // Deploy the feast authorization func (authz *FeastAuthorization) Deploy() error { if authz.isKubernetesAuth() { - if err := authz.deployKubernetesAuth(); err != nil { - return err - } - } else { - authz.removeOrphanedRoles() - _ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRole()) - _ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRoleBinding()) - apimeta.RemoveStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue].Type) + return authz.deployKubernetesAuth() } + + authz.removeOrphanedRoles() + _ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRole()) + _ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRoleBinding()) + apimeta.RemoveStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue].Type) return nil } diff --git a/infra/feast-operator/internal/controller/featurestore_controller_kubernetes_auth_test.go b/infra/feast-operator/internal/controller/featurestore_controller_kubernetes_auth_test.go index 6589e181af..4930f3fc59 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_kubernetes_auth_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_kubernetes_auth_test.go @@ -333,9 +333,9 @@ var _ = Describe("FeatureStore Controller-Kubernetes authorization", func() { Expect(err).To(HaveOccurred()) Expect(errors.IsNotFound(err)).To(BeTrue()) - By("Clearing the kubernetes authorizatino and reconciling") + By("Clearing the kubernetes authorization and reconciling") resourceNew = resource.DeepCopy() - resourceNew.Spec.AuthzConfig = &feastdevv1alpha1.AuthzConfig{} + resourceNew.Spec.AuthzConfig = nil err = k8sClient.Update(ctx, resourceNew) Expect(err).NotTo(HaveOccurred()) _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ 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 new file mode 100644 index 0000000000..c062a573df --- /dev/null +++ b/infra/feast-operator/internal/controller/featurestore_controller_oidc_auth_test.go @@ -0,0 +1,589 @@ +/* +Copyright 2024 Feast Community. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/base64" + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "gopkg.in/yaml.v3" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/feast-dev/feast/infra/feast-operator/api/feastversion" + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/authz" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/handler" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" +) + +var _ = Describe("FeatureStore Controller-OIDC authorization", func() { + Context("When deploying a resource with all ephemeral services and OIDC authorization", func() { + const resourceName = "oidc-authorization" + const oidcSecretName = "oidc-secret" + var pullPolicy = corev1.PullAlways + + ctx := context.Background() + + typeNamespacedSecretName := types.NamespacedName{ + Name: oidcSecretName, + Namespace: "default", + } + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", + } + featurestore := &feastdevv1alpha1.FeatureStore{} + + BeforeEach(func() { + By("creating the OIDC secret") + oidcSecret := createValidOidcSecret(oidcSecretName) + err := k8sClient.Get(ctx, typeNamespacedSecretName, oidcSecret) + if err != nil && errors.IsNotFound(err) { + Expect(k8sClient.Create(ctx, oidcSecret)).To(Succeed()) + } + + By("creating the custom resource for the Kind FeatureStore") + err = k8sClient.Get(ctx, typeNamespacedName, featurestore) + if err != nil && errors.IsNotFound(err) { + resource := createFeatureStoreResource(resourceName, image, pullPolicy, &[]corev1.EnvVar{}) + resource.Spec.AuthzConfig = &feastdevv1alpha1.AuthzConfig{OidcAuthz: &feastdevv1alpha1.OidcAuthz{ + SecretRef: corev1.LocalObjectReference{ + Name: oidcSecretName, + }, + }} + + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + } + }) + AfterEach(func() { + resource := &feastdevv1alpha1.FeatureStore{} + err := k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + oidcSecret := createValidOidcSecret(oidcSecretName) + err = k8sClient.Get(ctx, typeNamespacedSecretName, oidcSecret) + if err != nil && errors.IsNotFound(err) { + By("Cleanup the OIDC secret") + Expect(k8sClient.Delete(ctx, oidcSecret)).To(Succeed()) + } + + By("Cleanup the specific resource instance FeatureStore") + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + }) + + It("should successfully reconcile the resource", func() { + By("Reconciling the created resource") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + feast := services.FeastServices{ + Handler: handler.FeastHandler{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + }, + } + Expect(resource.Status).NotTo(BeNil()) + Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion)) + Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType))) + Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject)) + expectedAuthzConfig := &feastdevv1alpha1.AuthzConfig{ + OidcAuthz: &feastdevv1alpha1.OidcAuthz{ + SecretRef: corev1.LocalObjectReference{ + Name: oidcSecretName, + }, + }, + } + Expect(resource.Status.Applied.AuthzConfig).To(Equal(expectedAuthzConfig)) + Expect(resource.Status.Applied.Services).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Persistence.FilePersistence.Type).To(Equal(string(services.OfflineFilePersistenceDaskConfigType))) + Expect(resource.Status.Applied.Services.OfflineStore.ImagePullPolicy).To(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Resources).To(BeNil()) + Expect(resource.Status.Applied.Services.OfflineStore.Image).To(Equal(&services.DefaultImage)) + Expect(resource.Status.Applied.Services.OnlineStore).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Persistence.FilePersistence.Path).To(Equal(services.DefaultOnlineStoreEphemeralPath)) + Expect(resource.Status.Applied.Services.OnlineStore.Env).To(Equal(&[]corev1.EnvVar{})) + Expect(resource.Status.Applied.Services.OnlineStore.ImagePullPolicy).To(Equal(&pullPolicy)) + Expect(resource.Status.Applied.Services.OnlineStore.Resources).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.OnlineStore.Image).To(Equal(&image)) + Expect(resource.Status.Applied.Services.Registry).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence).NotTo(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.Path).To(Equal(services.DefaultRegistryEphemeralPath)) + Expect(resource.Status.Applied.Services.Registry.Local.ImagePullPolicy).To(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Resources).To(BeNil()) + Expect(resource.Status.Applied.Services.Registry.Local.Image).To(Equal(&services.DefaultImage)) + + Expect(resource.Status.ServiceHostnames.OfflineStore).To(Equal(feast.GetFeastServiceName(services.OfflineFeastType) + "." + resource.Namespace + domain)) + Expect(resource.Status.ServiceHostnames.OnlineStore).To(Equal(feast.GetFeastServiceName(services.OnlineFeastType) + "." + resource.Namespace + domain)) + Expect(resource.Status.ServiceHostnames.Registry).To(Equal(feast.GetFeastServiceName(services.RegistryFeastType) + "." + resource.Namespace + domain)) + + Expect(resource.Status.Conditions).NotTo(BeEmpty()) + cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.AuthorizationReadyType) + Expect(cond).To(BeNil()) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.RegistryReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.RegistryReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ClientReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ClientReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.ClientReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OfflineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OfflineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OfflineStoreReadyMessage)) + + cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.OnlineStoreReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.OnlineStoreReadyType)) + Expect(cond.Message).To(Equal(feastdevv1alpha1.OnlineStoreReadyMessage)) + + Expect(resource.Status.Phase).To(Equal(feastdevv1alpha1.ReadyPhase)) + + // check offline deployment + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes).To(HaveLen(0)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(0)) + + // check online deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes).To(HaveLen(0)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(0)) + + // check registry deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.Spec.Replicas).To(Equal(&services.DefaultReplicas)) + Expect(controllerutil.HasControllerReference(deploy)).To(BeTrue()) + Expect(deploy.Spec.Template.Spec.Containers).To(HaveLen(1)) + Expect(deploy.Spec.Template.Spec.Volumes).To(HaveLen(0)) + Expect(deploy.Spec.Template.Spec.Containers[0].VolumeMounts).To(HaveLen(0)) + + // check Feast Role + feastRole := &rbacv1.Role{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: authz.GetFeastRoleName(resource), + Namespace: resource.Namespace, + }, + feastRole) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + + // check RoleBinding + roleBinding := &rbacv1.RoleBinding{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: authz.GetFeastRoleName(resource), + Namespace: resource.Namespace, + }, + roleBinding) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + + // check ServiceAccounts + for _, serviceType := range []services.FeastServiceType{services.RegistryFeastType, services.OnlineFeastType, services.OfflineFeastType} { + sa := &corev1.ServiceAccount{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(serviceType), + Namespace: resource.Namespace, + }, + sa) + Expect(err).NotTo(HaveOccurred()) + } + + By("Clearing the OIDC authorization and reconciling") + resourceNew := resource.DeepCopy() + resourceNew.Spec.AuthzConfig = nil + err = k8sClient.Update(ctx, resourceNew) + Expect(err).NotTo(HaveOccurred()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource = &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + feast.Handler.FeatureStore = resource + + // check no RoleBinding + roleBinding = &rbacv1.RoleBinding{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: authz.GetFeastRoleName(resource), + Namespace: resource.Namespace, + }, + roleBinding) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + }) + + It("should properly encode a feature_store.yaml config", func() { + By("Reconciling the created resource") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + _, err := controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + req, err := labels.NewRequirement(services.NameLabelKey, selection.Equals, []string{resource.Name}) + Expect(err).NotTo(HaveOccurred()) + labelSelector := labels.NewSelector().Add(*req) + listOpts := &client.ListOptions{Namespace: resource.Namespace, LabelSelector: labelSelector} + deployList := appsv1.DeploymentList{} + err = k8sClient.List(ctx, &deployList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(deployList.Items).To(HaveLen(3)) + + svcList := corev1.ServiceList{} + err = k8sClient.List(ctx, &svcList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(svcList.Items).To(HaveLen(3)) + + cmList := corev1.ConfigMapList{} + err = k8sClient.List(ctx, &cmList, listOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(cmList.Items).To(HaveLen(1)) + + feast := services.FeastServices{ + Handler: handler.FeastHandler{ + Client: controllerReconciler.Client, + Context: ctx, + Scheme: controllerReconciler.Scheme, + FeatureStore: resource, + }, + } + + // check registry deployment + deploy := &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.RegistryFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env := getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + // check registry config + fsYamlStr, err := feast.GetServiceFeatureStoreYamlBase64(services.RegistryFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err := base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfig := &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfig) + Expect(err).NotTo(HaveOccurred()) + testConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + Registry: services.RegistryConfig{ + RegistryType: services.RegistryFileConfigType, + Path: services.DefaultRegistryEphemeralPath, + S3AdditionalKwargs: nil, + }, + AuthzConfig: expectedServerOidcAuthorizConfig(), + } + Expect(repoConfig).To(Equal(testConfig)) + + // check offline deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OfflineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + // check offline config + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OfflineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfig = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfig) + Expect(err).NotTo(HaveOccurred()) + regRemote := services.RegistryConfig{ + RegistryType: services.RegistryRemoteConfigType, + Path: fmt.Sprintf("feast-%s-registry.default.svc.cluster.local:80", resourceName), + } + testConfig = &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: services.OfflineStoreConfig{ + Type: services.OfflineFilePersistenceDaskConfigType, + }, + Registry: regRemote, + AuthzConfig: expectedServerOidcAuthorizConfig(), + } + Expect(repoConfig).To(Equal(testConfig)) + + // check online deployment + deploy = &appsv1.Deployment{} + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: feast.GetFeastServiceName(services.OnlineFeastType), + Namespace: resource.Namespace, + }, + deploy) + Expect(err).NotTo(HaveOccurred()) + env = getFeatureStoreYamlEnvVar(deploy.Spec.Template.Spec.Containers[0].Env) + Expect(env).NotTo(BeNil()) + + // check online config + fsYamlStr, err = feast.GetServiceFeatureStoreYamlBase64(services.OnlineFeastType) + Expect(err).NotTo(HaveOccurred()) + Expect(fsYamlStr).To(Equal(env.Value)) + + envByte, err = base64.StdEncoding.DecodeString(env.Value) + Expect(err).NotTo(HaveOccurred()) + repoConfig = &services.RepoConfig{} + err = yaml.Unmarshal(envByte, repoConfig) + Expect(err).NotTo(HaveOccurred()) + offlineRemote := services.OfflineStoreConfig{ + Host: fmt.Sprintf("feast-%s-offline.default.svc.cluster.local", resourceName), + Type: services.OfflineRemoteConfigType, + Port: services.HttpPort, + } + testConfig = &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: offlineRemote, + OnlineStore: services.OnlineStoreConfig{ + Path: services.DefaultOnlineStoreEphemeralPath, + Type: services.OnlineSqliteConfigType, + }, + Registry: regRemote, + AuthzConfig: expectedServerOidcAuthorizConfig(), + } + Expect(repoConfig).To(Equal(testConfig)) + + // check client config + cm := &corev1.ConfigMap{} + name := feast.GetFeastServiceName(services.ClientFeastType) + err = k8sClient.Get(ctx, types.NamespacedName{ + Name: name, + Namespace: resource.Namespace, + }, + cm) + Expect(err).NotTo(HaveOccurred()) + repoConfigClient := &services.RepoConfig{} + err = yaml.Unmarshal([]byte(cm.Data[services.FeatureStoreYamlCmKey]), repoConfigClient) + Expect(err).NotTo(HaveOccurred()) + clientConfig := &services.RepoConfig{ + Project: feastProject, + Provider: services.LocalProviderType, + EntityKeySerializationVersion: feastdevv1alpha1.SerializationVersion, + OfflineStore: offlineRemote, + OnlineStore: services.OnlineStoreConfig{ + Path: fmt.Sprintf("http://feast-%s-online.default.svc.cluster.local:80", resourceName), + Type: services.OnlineRemoteConfigType, + }, + Registry: services.RegistryConfig{ + RegistryType: services.RegistryRemoteConfigType, + Path: fmt.Sprintf("feast-%s-registry.default.svc.cluster.local:80", resourceName), + }, + AuthzConfig: expectedClientOidcAuthorizConfig(), + } + Expect(repoConfigClient).To(Equal(clientConfig)) + }) + + It("should fail to reconcile the resource", func() { + By("Reconciling an invalid OIDC set of properties") + controllerReconciler := &FeatureStoreReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + } + + newOidcSecretName := "invalid-secret" + newTypeNamespaceSecretdName := types.NamespacedName{ + Name: newOidcSecretName, + Namespace: "default", + } + newOidcSecret := createInvalidOidcSecret(newOidcSecretName) + err := k8sClient.Get(ctx, newTypeNamespaceSecretdName, newOidcSecret) + if err != nil && errors.IsNotFound(err) { + Expect(k8sClient.Create(ctx, newOidcSecret)).To(Succeed()) + } + + resource := &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + + resource.Spec.AuthzConfig.OidcAuthz.SecretRef.Name = newOidcSecretName + err = k8sClient.Update(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).To(HaveOccurred()) + + resource = &feastdevv1alpha1.FeatureStore{} + err = k8sClient.Get(ctx, typeNamespacedName, resource) + Expect(err).NotTo(HaveOccurred()) + Expect(resource.Status.Conditions).NotTo(BeEmpty()) + cond := apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.ReadyType) + Expect(cond).ToNot(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(feastdevv1alpha1.FailedReason)) + Expect(cond.Type).To(Equal(feastdevv1alpha1.ReadyType)) + Expect(cond.Message).To(ContainSubstring("missing OIDC")) + }) + }) +}) + +func expectedServerOidcAuthorizConfig() services.AuthzConfig { + return services.AuthzConfig{ + Type: services.OidcAuthType, + OidcParameters: map[string]interface{}{ + string(services.OidcAuthDiscoveryUrl): "auth-discovery-url", + string(services.OidcClientId): "client-id", + }, + } +} +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"}, + } +} + +func validOidcSecretMap() map[string]string { + return map[string]string{ + string(services.OidcClientId): "client-id", + string(services.OidcAuthDiscoveryUrl): "auth-discovery-url", + string(services.OidcClientSecret): "client-secret", + string(services.OidcUsername): "username", + string(services.OidcPassword): "password", + } +} + +func createValidOidcSecret(secretName string) *corev1.Secret { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: "default", + }, + StringData: validOidcSecretMap(), + } + + return secret +} + +func createInvalidOidcSecret(secretName string) *corev1.Secret { + oidcProperties := validOidcSecretMap() + delete(oidcProperties, string(services.OidcClientId)) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: secretName, + Namespace: "default", + }, + StringData: oidcProperties, + } + + return secret +} diff --git a/infra/feast-operator/internal/controller/featurestore_controller_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index a6e71934fb..44c81eca59 100644 --- a/infra/feast-operator/internal/controller/featurestore_controller_test.go +++ b/infra/feast-operator/internal/controller/featurestore_controller_test.go @@ -964,7 +964,6 @@ var _ = Describe("FeatureStore Controller", func() { }, Spec: feastdevv1alpha1.FeatureStoreSpec{ FeastProject: referencedRegistry.Spec.FeastProject, - AuthzConfig: &feastdevv1alpha1.AuthzConfig{}, Services: &feastdevv1alpha1.FeatureStoreServices{ OnlineStore: &feastdevv1alpha1.OnlineStore{}, OfflineStore: &feastdevv1alpha1.OfflineStore{}, diff --git a/infra/feast-operator/internal/controller/services/client.go b/infra/feast-operator/internal/controller/services/client.go index 48ca0751ce..d4b78e2611 100644 --- a/infra/feast-operator/internal/controller/services/client.go +++ b/infra/feast-operator/internal/controller/services/client.go @@ -47,7 +47,7 @@ func (feast *FeastServices) createClientConfigMap() error { func (feast *FeastServices) setClientConfigMap(cm *corev1.ConfigMap) error { cm.Labels = feast.getLabels(ClientFeastType) - clientYaml, err := feast.getClientFeatureStoreYaml() + clientYaml, err := feast.getClientFeatureStoreYaml(feast.extractConfigFromSecret) if err != nil { return err } diff --git a/infra/feast-operator/internal/controller/services/repo_config.go b/infra/feast-operator/internal/controller/services/repo_config.go index 8b52296160..22052aa724 100644 --- a/infra/feast-operator/internal/controller/services/repo_config.go +++ b/infra/feast-operator/internal/controller/services/repo_config.go @@ -47,10 +47,34 @@ func (feast *FeastServices) getServiceRepoConfig(feastType FeastServiceType) (Re return getServiceRepoConfig(feastType, feast.Handler.FeatureStore, feast.extractConfigFromSecret) } -func getServiceRepoConfig(feastType FeastServiceType, featureStore *feastdevv1alpha1.FeatureStore, secretExtractionFunc func(secretRef string, secretKeyName string) (map[string]interface{}, error)) (RepoConfig, error) { +func getServiceRepoConfig( + feastType FeastServiceType, + featureStore *feastdevv1alpha1.FeatureStore, + secretExtractionFunc func(secretRef string, secretKeyName string) (map[string]interface{}, error)) (RepoConfig, error) { appliedSpec := featureStore.Status.Applied - repoConfig := getClientRepoConfig(featureStore) + repoConfig, err := getClientRepoConfig(featureStore, secretExtractionFunc) + if err != nil { + return repoConfig, err + } + + if appliedSpec.AuthzConfig != nil && appliedSpec.AuthzConfig.OidcAuthz != nil { + propertiesMap, err := secretExtractionFunc(appliedSpec.AuthzConfig.OidcAuthz.SecretRef.Name, "") + if err != nil { + return repoConfig, err + } + + oidcServerProperties := map[string]interface{}{} + for _, oidcServerProperty := range OidcServerProperties { + if val, exists := propertiesMap[string(oidcServerProperty)]; exists { + oidcServerProperties[string(oidcServerProperty)] = val + } else { + return repoConfig, missingOidcSecretProperty(oidcServerProperty) + } + } + repoConfig.AuthzConfig.OidcParameters = oidcServerProperties + } + if appliedSpec.Services != nil { services := appliedSpec.Services @@ -200,11 +224,17 @@ func setRepoConfigOffline(services *feastdevv1alpha1.FeatureStoreServices, secre return nil } -func (feast *FeastServices) getClientFeatureStoreYaml() ([]byte, error) { - return yaml.Marshal(getClientRepoConfig(feast.Handler.FeatureStore)) +func (feast *FeastServices) getClientFeatureStoreYaml(secretExtractionFunc func(secretRef string, secretKeyName string) (map[string]interface{}, error)) ([]byte, error) { + clientRepo, err := getClientRepoConfig(feast.Handler.FeatureStore, secretExtractionFunc) + if err != nil { + return []byte{}, err + } + return yaml.Marshal(clientRepo) } -func getClientRepoConfig(featureStore *feastdevv1alpha1.FeatureStore) RepoConfig { +func getClientRepoConfig( + featureStore *feastdevv1alpha1.FeatureStore, + secretExtractionFunc func(secretRef string, secretKeyName string) (map[string]interface{}, error)) (RepoConfig, error) { status := featureStore.Status appliedServices := status.Applied.Services clientRepoConfig := RepoConfig{ @@ -248,13 +278,37 @@ func getClientRepoConfig(featureStore *feastdevv1alpha1.FeatureStore) RepoConfig } } - clientRepoConfig.AuthzConfig = AuthzConfig{ - Type: NoAuthAuthType, - } - if status.Applied.AuthzConfig != nil && status.Applied.AuthzConfig.KubernetesAuthz != nil { - clientRepoConfig.AuthzConfig.Type = KubernetesAuthType + if status.Applied.AuthzConfig == nil { + clientRepoConfig.AuthzConfig = AuthzConfig{ + Type: NoAuthAuthType, + } + } else { + if status.Applied.AuthzConfig.KubernetesAuthz != nil { + clientRepoConfig.AuthzConfig = AuthzConfig{ + Type: KubernetesAuthType, + } + } else if status.Applied.AuthzConfig.OidcAuthz != nil { + clientRepoConfig.AuthzConfig = AuthzConfig{ + Type: OidcAuthType, + } + + propertiesMap, err := secretExtractionFunc(status.Applied.AuthzConfig.OidcAuthz.SecretRef.Name, "") + if err != nil { + return clientRepoConfig, err + } + + oidcClientProperties := map[string]interface{}{} + for _, oidcClientProperty := range OidcClientProperties { + if val, exists := propertiesMap[string(oidcClientProperty)]; exists { + oidcClientProperties[string(oidcClientProperty)] = val + } else { + return clientRepoConfig, missingOidcSecretProperty(oidcClientProperty) + } + } + clientRepoConfig.AuthzConfig.OidcParameters = oidcClientProperties + } } - return clientRepoConfig + return clientRepoConfig, nil } func getActualPath(filePath string, pvcConfig *feastdevv1alpha1.PvcConfig) string { @@ -271,24 +325,33 @@ func (feast *FeastServices) extractConfigFromSecret(secretRef string, secretKeyN } parameters := map[string]interface{}{} - val, exists := secret.Data[secretKeyName] - if !exists { - return nil, fmt.Errorf("secret key %s doesn't exist in secret %s", secretKeyName, secretRef) - } - - err = yaml.Unmarshal(val, ¶meters) - if err != nil { - return nil, fmt.Errorf("secret %s contains invalid value", secretKeyName) - } - - _, exists = parameters["type"] - if exists { - return nil, fmt.Errorf("secret key %s in secret %s contains invalid tag named type", secretKeyName, secretRef) - } + if secretKeyName != "" { + val, exists := secret.Data[secretKeyName] + if !exists { + return nil, fmt.Errorf("secret key %s doesn't exist in secret %s", secretKeyName, secretRef) + } + err = yaml.Unmarshal(val, ¶meters) + if err != nil { + return nil, fmt.Errorf("secret %s contains invalid value", secretKeyName) + } + _, exists = parameters["type"] + if exists { + return nil, fmt.Errorf("secret key %s in secret %s contains invalid tag named type", secretKeyName, secretRef) + } - _, exists = parameters["registry_type"] - if exists { - return nil, fmt.Errorf("secret key %s in secret %s contains invalid tag named registry_type", secretKeyName, secretRef) + _, exists = parameters["registry_type"] + if exists { + return nil, fmt.Errorf("secret key %s in secret %s contains invalid tag named registry_type", secretKeyName, secretRef) + } + } else { + for k, v := range secret.Data { + var val interface{} + err := yaml.Unmarshal(v, &val) + if err != nil { + return nil, fmt.Errorf("secret %s contains invalid value %v", k, v) + } + parameters[k] = val + } } return parameters, nil 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 18cf104eb5..cb3f96c7ba 100644 --- a/infra/feast-operator/internal/controller/services/repo_config_test.go +++ b/infra/feast-operator/internal/controller/services/repo_config_test.go @@ -252,6 +252,71 @@ var _ = Describe("Repo Config", func() { } Expect(repoConfig.Registry).To(Equal(expectedRegistryConfig)) + By("Having oidc authorization") + featureStore.Spec.AuthzConfig = &feastdevv1alpha1.AuthzConfig{ + OidcAuthz: &feastdevv1alpha1.OidcAuthz{ + SecretRef: corev1.LocalObjectReference{ + Name: "oidc-secret", + }, + }, + } + ApplyDefaultsToStatus(featureStore) + + secretExtractionFunc := mockOidcConfigFromSecret(map[string]interface{}{ + string(OidcAuthDiscoveryUrl): "discovery-url", + string(OidcClientId): "client-id", + string(OidcClientSecret): "client-secret", + string(OidcUsername): "username", + string(OidcPassword): "password"}) + repoConfig, err = getServiceRepoConfig(OfflineFeastType, 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(HaveKey(string(OidcClientId))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcAuthDiscoveryUrl))) + expectedOfflineConfig = OfflineStoreConfig{ + Type: "dask", + } + Expect(repoConfig.OfflineStore).To(Equal(expectedOfflineConfig)) + Expect(repoConfig.OnlineStore).To(Equal(emptyOnlineStoreConfig())) + Expect(repoConfig.Registry).To(Equal(emptyRegistryConfig())) + + repoConfig, err = getServiceRepoConfig(OnlineFeastType, 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(HaveKey(string(OidcClientId))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcAuthDiscoveryUrl))) + Expect(repoConfig.OfflineStore).To(Equal(emptyOfflineStoreConfig())) + expectedOnlineConfig = OnlineStoreConfig{ + Type: "sqlite", + Path: DefaultOnlineStoreEphemeralPath, + } + Expect(repoConfig.OnlineStore).To(Equal(expectedOnlineConfig)) + Expect(repoConfig.Registry).To(Equal(emptyRegistryConfig())) + + repoConfig, err = getServiceRepoConfig(RegistryFeastType, 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(HaveKey(string(OidcClientId))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcAuthDiscoveryUrl))) + Expect(repoConfig.OfflineStore).To(Equal(emptyOfflineStoreConfig())) + Expect(repoConfig.OnlineStore).To(Equal(emptyOnlineStoreConfig())) + expectedRegistryConfig = RegistryConfig{ + RegistryType: "file", + Path: DefaultRegistryEphemeralPath, + } + Expect(repoConfig.Registry).To(Equal(expectedRegistryConfig)) + + repoConfig, err = getClientRepoConfig(featureStore, secretExtractionFunc) + Expect(err).NotTo(HaveOccurred()) + Expect(repoConfig.AuthzConfig.Type).To(Equal(OidcAuthType)) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveLen(3)) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcClientSecret))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcUsername))) + Expect(repoConfig.AuthzConfig.OidcParameters).To(HaveKey(string(OidcPassword))) + By("Having the all the db services") featureStore = minimalFeatureStore() featureStore.Spec.Services = &feastdevv1alpha1.FeatureStoreServices{ @@ -329,6 +394,64 @@ var _ = Describe("Repo Config", func() { Expect(repoConfig.Registry).To(Equal(expectedRegistryConfig)) }) }) + It("should fail to create the repo configs", func() { + featureStore := minimalFeatureStore() + + By("Having invalid server oidc authorization") + featureStore.Spec.AuthzConfig = &feastdevv1alpha1.AuthzConfig{ + OidcAuthz: &feastdevv1alpha1.OidcAuthz{ + SecretRef: corev1.LocalObjectReference{ + Name: "oidc-secret", + }, + }, + } + ApplyDefaultsToStatus(featureStore) + + secretExtractionFunc := mockOidcConfigFromSecret(map[string]interface{}{ + string(OidcClientId): "client-id", + string(OidcClientSecret): "client-secret", + string(OidcUsername): "username", + string(OidcPassword): "password"}) + _, err := getServiceRepoConfig(OfflineFeastType, featureStore, secretExtractionFunc) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) + _, err = getServiceRepoConfig(OnlineFeastType, featureStore, secretExtractionFunc) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) + _, err = getServiceRepoConfig(RegistryFeastType, featureStore, secretExtractionFunc) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) + _, err = getClientRepoConfig(featureStore, secretExtractionFunc) + Expect(err).ToNot(HaveOccurred()) + + By("Having invalid client oidc authorization") + featureStore.Spec.AuthzConfig = &feastdevv1alpha1.AuthzConfig{ + OidcAuthz: &feastdevv1alpha1.OidcAuthz{ + SecretRef: corev1.LocalObjectReference{ + Name: "oidc-secret", + }, + }, + } + ApplyDefaultsToStatus(featureStore) + + secretExtractionFunc = mockOidcConfigFromSecret(map[string]interface{}{ + string(OidcAuthDiscoveryUrl): "discovery-url", + string(OidcClientId): "client-id", + string(OidcUsername): "username", + string(OidcPassword): "password"}) + _, err = getServiceRepoConfig(OfflineFeastType, featureStore, secretExtractionFunc) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) + _, err = getServiceRepoConfig(OnlineFeastType, featureStore, secretExtractionFunc) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) + _, err = getServiceRepoConfig(RegistryFeastType, featureStore, secretExtractionFunc) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) + _, err = getClientRepoConfig(featureStore, secretExtractionFunc) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("missing OIDC secret")) + }) }) func emptyOnlineStoreConfig() OnlineStoreConfig { @@ -369,6 +492,13 @@ func mockExtractConfigFromSecret(secretRef string, secretKeyName string) (map[st return createParameterMap(), nil } +func mockOidcConfigFromSecret( + oidcProperties map[string]interface{}) func(secretRef string, secretKeyName string) (map[string]interface{}, error) { + return func(secretRef string, secretKeyName string) (map[string]interface{}, error) { + return oidcProperties, nil + } +} + func createParameterMap() map[string]interface{} { yamlString := ` hosts: diff --git a/infra/feast-operator/internal/controller/services/services_types.go b/infra/feast-operator/internal/controller/services/services_types.go index 65e50b3681..2c454459d8 100644 --- a/infra/feast-operator/internal/controller/services/services_types.go +++ b/infra/feast-operator/internal/controller/services/services_types.go @@ -68,6 +68,15 @@ const ( NoAuthAuthType AuthzType = "no_auth" KubernetesAuthType AuthzType = "kubernetes" + OidcAuthType AuthzType = "oidc" + + OidcClientId OidcPropertyType = "client_id" + OidcAuthDiscoveryUrl OidcPropertyType = "auth_discovery_url" + OidcClientSecret OidcPropertyType = "client_secret" + OidcUsername OidcPropertyType = "username" + OidcPassword OidcPropertyType = "password" + + OidcMissingSecretError string = "missing OIDC secret: %s" ) var ( @@ -148,11 +157,17 @@ var ( }, }, } + + OidcServerProperties = []OidcPropertyType{OidcClientId, OidcAuthDiscoveryUrl} + OidcClientProperties = []OidcPropertyType{OidcClientSecret, OidcUsername, OidcPassword} ) // AuthzType defines the authorization type type AuthzType string +// OidcPropertyType defines the OIDC property type +type OidcPropertyType string + // FeastServiceType is the type of feast service type FeastServiceType string @@ -214,7 +229,8 @@ type RegistryConfig struct { // AuthzConfig is the RBAC authorization configuration. type AuthzConfig struct { - Type AuthzType `yaml:"type,omitempty"` + Type AuthzType `yaml:"type,omitempty"` + OidcParameters map[string]interface{} `yaml:",inline,omitempty"` } type deploymentSettings struct { diff --git a/infra/feast-operator/internal/controller/services/tls_test.go b/infra/feast-operator/internal/controller/services/tls_test.go index 28a3a49ec2..a0a6cbe310 100644 --- a/infra/feast-operator/internal/controller/services/tls_test.go +++ b/infra/feast-operator/internal/controller/services/tls_test.go @@ -102,7 +102,8 @@ var _ = Describe("TLS Config", func() { err = feast.ApplyDefaults() Expect(err).To(BeNil()) - repoConfig := getClientRepoConfig(feast.Handler.FeatureStore) + repoConfig, err := getClientRepoConfig(feast.Handler.FeatureStore, emptyMockExtractConfigFromSecret) + Expect(err).NotTo(HaveOccurred()) Expect(repoConfig.OfflineStore.Port).To(Equal(HttpsPort)) Expect(repoConfig.OfflineStore.Scheme).To(Equal(HttpsScheme)) Expect(repoConfig.OfflineStore.Cert).To(ContainSubstring(string(OfflineFeastType))) @@ -208,7 +209,8 @@ var _ = Describe("TLS Config", func() { err = feast.ApplyDefaults() Expect(err).To(BeNil()) - repoConfig = getClientRepoConfig(feast.Handler.FeatureStore) + repoConfig, err = getClientRepoConfig(feast.Handler.FeatureStore, emptyMockExtractConfigFromSecret) + Expect(err).NotTo(HaveOccurred()) Expect(repoConfig.OfflineStore.Port).To(Equal(HttpsPort)) Expect(repoConfig.OfflineStore.Scheme).To(Equal(HttpsScheme)) Expect(repoConfig.OfflineStore.Cert).To(ContainSubstring(string(OfflineFeastType))) diff --git a/infra/feast-operator/internal/controller/services/util.go b/infra/feast-operator/internal/controller/services/util.go index 8f871cdd15..323f87119e 100644 --- a/infra/feast-operator/internal/controller/services/util.go +++ b/infra/feast-operator/internal/controller/services/util.go @@ -311,3 +311,7 @@ func SetIsOpenShift(cfg *rest.Config) { } } } + +func missingOidcSecretProperty(property OidcPropertyType) error { + return fmt.Errorf(OidcMissingSecretError, property) +} diff --git a/infra/feast-operator/test/api/featurestore_types_test.go b/infra/feast-operator/test/api/featurestore_types_test.go index b24973ec86..0a7f8fd53a 100644 --- a/infra/feast-operator/test/api/featurestore_types_test.go +++ b/infra/feast-operator/test/api/featurestore_types_test.go @@ -16,7 +16,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// Function to create invalid OnlineStore resource func createFeatureStore() *feastdevv1alpha1.FeatureStore { return &feastdevv1alpha1.FeatureStore{ ObjectMeta: metav1.ObjectMeta{ @@ -272,6 +271,25 @@ func pvcConfigWithResources(featureStore *feastdevv1alpha1.FeatureStore) *feastd return fsCopy } +func authzConfigWithKubernetes(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + if fsCopy.Spec.AuthzConfig == nil { + fsCopy.Spec.AuthzConfig = &feastdevv1alpha1.AuthzConfig{} + } + fsCopy.Spec.AuthzConfig.KubernetesAuthz = &feastdevv1alpha1.KubernetesAuthz{ + Roles: []string{}, + } + return fsCopy +} +func authzConfigWithOidc(featureStore *feastdevv1alpha1.FeatureStore) *feastdevv1alpha1.FeatureStore { + fsCopy := featureStore.DeepCopy() + if fsCopy.Spec.AuthzConfig == nil { + fsCopy.Spec.AuthzConfig = &feastdevv1alpha1.AuthzConfig{} + } + fsCopy.Spec.AuthzConfig.OidcAuthz = &feastdevv1alpha1.OidcAuthz{} + return fsCopy +} + const resourceName = "test-resource" const namespaceName = "default" @@ -377,10 +395,19 @@ var _ = Describe("FeatureStore API", func() { storage = resource.Status.Applied.Services.Registry.Local.Persistence.FilePersistence.PvcConfig.Create.Resources.Requests.Storage().String() Expect(storage).To(Equal("500Mi")) }) - It("should set the default AuthzConfig", func() { + }) + Context("When omitting the AuthzConfig PvcConfig", func() { + _, featurestore := initContext() + It("should keep an empty AuthzConfig", func() { resource := featurestore services.ApplyDefaultsToStatus(resource) Expect(resource.Status.Applied.AuthzConfig).To(BeNil()) }) }) + Context("When configuring the AuthzConfig", func() { + ctx, featurestore := initContext() + It("should fail when both kubernetes and oidc settings are given", func() { + attemptInvalidCreationAndAsserts(ctx, authzConfigWithOidc(authzConfigWithKubernetes(featurestore)), "One selection required between kubernetes or oidc") + }) + }) })