From 7f5c5bb40dd2d572f522d6a42d9b44378533c4eb Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 22 Nov 2024 22:18:01 +0100 Subject: [PATCH 01/11] Initial commit Signed-off-by: Daniele Martinoli --- .../api/v1alpha1/featurestore_types.go | 18 +- .../api/v1alpha1/zz_generated.deepcopy.go | 28 +-- .../crd/bases/feast.dev_featurestores.yaml | 28 +-- ...a1_featurestore_ephemeral_persistence.yaml | 3 + ...v1alpha1_featurestore_kubernetes_auth.yaml | 5 +- ..._featurestore_objectstore_persistence.yaml | 3 + ...v1alpha1_featurestore_pvc_persistence.yaml | 3 + infra/feast-operator/dist/install.yaml | 28 +-- .../internal/controller/auth/auth.go | 220 ++++++++++++++++++ .../internal/controller/auth/auth_types.go | 30 +++ 10 files changed, 290 insertions(+), 76 deletions(-) create mode 100644 infra/feast-operator/internal/controller/auth/auth.go create mode 100644 infra/feast-operator/internal/controller/auth/auth_types.go diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 4bc0aa7c5e0..94b19dd6e35 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -62,7 +62,7 @@ type FeatureStoreSpec struct { // FeastProject is the Feast project id. This can be any alphanumeric string with underscores, but it cannot start with an underscore. Required. FeastProject string `json:"feastProject"` Services *FeatureStoreServices `json:"services,omitempty"` - AuthzConfig *AuthzConfig `json:"authz,omitempty"` + AuthConfig *AuthConfig `json:"auth,omitempty"` } // FeatureStoreServices defines the desired feast service deployments. ephemeral registry is deployed by default. @@ -279,20 +279,12 @@ type OptionalConfigs struct { Resources *corev1.ResourceRequirements `json:"resources,omitempty"` } -// AuthzConfig defines the authorization settings for the deployed Feast services. -type AuthzConfig struct { - KubernetesAuthz *KubernetesAuthz `json:"kubernetes,omitempty"` +// AuthConfig defines the authorization settings for the deployed Feast services. +type AuthConfig struct { + KubernetesAuth *KubernetesAuth `json:"kubernetes,omitempty"` } -// KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. -// https://kubernetes.io/docs/reference/access-authn-authz/rbac/ -type KubernetesAuthz struct { - // The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. - // Roles are managed by the operator and created with an empty list of rules. - // See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission - // The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. - // This configuration option is only providing a way to automate this procedure. - // Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. +type KubernetesAuth struct { Roles []string `json:"roles,omitempty"` } diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index 8675397fee5..f9f7ccaeaff 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -27,21 +27,21 @@ import ( ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AuthzConfig) DeepCopyInto(out *AuthzConfig) { +func (in *AuthConfig) DeepCopyInto(out *AuthConfig) { *out = *in - if in.KubernetesAuthz != nil { - in, out := &in.KubernetesAuthz, &out.KubernetesAuthz - *out = new(KubernetesAuthz) + if in.KubernetesAuth != nil { + in, out := &in.KubernetesAuth, &out.KubernetesAuth + *out = new(KubernetesAuth) (*in).DeepCopyInto(*out) } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthzConfig. -func (in *AuthzConfig) DeepCopy() *AuthzConfig { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthConfig. +func (in *AuthConfig) DeepCopy() *AuthConfig { if in == nil { return nil } - out := new(AuthzConfig) + out := new(AuthConfig) in.DeepCopyInto(out) return out } @@ -178,9 +178,9 @@ func (in *FeatureStoreSpec) DeepCopyInto(out *FeatureStoreSpec) { *out = new(FeatureStoreServices) (*in).DeepCopyInto(*out) } - if in.AuthzConfig != nil { - in, out := &in.AuthzConfig, &out.AuthzConfig - *out = new(AuthzConfig) + if in.AuthConfig != nil { + in, out := &in.AuthConfig, &out.AuthConfig + *out = new(AuthConfig) (*in).DeepCopyInto(*out) } } @@ -220,7 +220,7 @@ func (in *FeatureStoreStatus) DeepCopy() *FeatureStoreStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *KubernetesAuthz) DeepCopyInto(out *KubernetesAuthz) { +func (in *KubernetesAuth) DeepCopyInto(out *KubernetesAuth) { *out = *in if in.Roles != nil { in, out := &in.Roles, &out.Roles @@ -229,12 +229,12 @@ func (in *KubernetesAuthz) DeepCopyInto(out *KubernetesAuthz) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesAuthz. -func (in *KubernetesAuthz) DeepCopy() *KubernetesAuthz { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesAuth. +func (in *KubernetesAuth) DeepCopy() *KubernetesAuth { if in == nil { return nil } - out := new(KubernetesAuthz) + out := new(KubernetesAuth) in.DeepCopyInto(out) return out } 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 958c7cdddb1..87c8e1e2b49 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -48,23 +48,13 @@ spec: spec: description: FeatureStoreSpec defines the desired state of FeatureStore properties: - authz: - description: AuthzConfig defines the authorization settings for the + auth: + description: AuthConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: - description: |- - KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. - https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: - description: |- - The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. - Roles are managed by the operator and created with an empty list of rules. - See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission - The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. - This configuration option is only providing a way to automate this procedure. - Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array @@ -1217,23 +1207,13 @@ spec: description: Shows the currently applied feast configuration, including any pertinent defaults properties: - authz: - description: AuthzConfig defines the authorization settings for + auth: + description: AuthConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: - description: |- - KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. - https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: - description: |- - The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. - Roles are managed by the operator and created with an empty list of rules. - See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission - The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. - This configuration option is only providing a way to automate this procedure. - Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml index 512fed9d4c0..8449ec1bf51 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml @@ -6,15 +6,18 @@ spec: feastProject: my_project services: onlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/online_store.db offlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: type: dask registry: local: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/registry.db diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml index ed95b41cf47..add9c4431fb 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml @@ -6,19 +6,22 @@ spec: feastProject: my_project services: onlineStore: + image: quay.io/dmartino/feature-server:0.3 persistence: file: path: /data/online_store.db offlineStore: + image: quay.io/dmartino/feature-server:0.3 persistence: file: type: dask registry: local: + image: quay.io/dmartino/feature-server:0.3 persistence: file: path: /data/registry.db - authz: + auth: kubernetes: roles: - reader diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml index 45f12a67a18..e038242da4c 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml @@ -6,15 +6,18 @@ spec: feastProject: my_project services: onlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/online_store.db offlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: type: dask registry: local: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: s3://bucket/registry.db diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml index b7c7412c0f0..fe2a5071cda 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml @@ -6,6 +6,7 @@ spec: feastProject: my_project services: onlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: online_store.db @@ -14,6 +15,7 @@ spec: name: online-pvc mountPath: /data/online offlineStore: + image: quay.io/dmartino/feature-server:0.2 persistence: file: type: duckdb @@ -26,6 +28,7 @@ spec: mountPath: /data/offline registry: local: + image: quay.io/dmartino/feature-server:0.2 persistence: file: path: registry.db diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index b5e103f9692..f60489c891d 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -56,23 +56,13 @@ spec: spec: description: FeatureStoreSpec defines the desired state of FeatureStore properties: - authz: - description: AuthzConfig defines the authorization settings for the + auth: + description: AuthConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: - description: |- - KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. - https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: - description: |- - The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. - Roles are managed by the operator and created with an empty list of rules. - See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission - The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. - This configuration option is only providing a way to automate this procedure. - Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array @@ -1225,23 +1215,13 @@ spec: description: Shows the currently applied feast configuration, including any pertinent defaults properties: - authz: - description: AuthzConfig defines the authorization settings for + auth: + description: AuthConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: - description: |- - KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. - https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: - description: |- - The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. - Roles are managed by the operator and created with an empty list of rules. - See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission - The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. - This configuration option is only providing a way to automate this procedure. - Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array diff --git a/infra/feast-operator/internal/controller/auth/auth.go b/infra/feast-operator/internal/controller/auth/auth.go new file mode 100644 index 00000000000..3be0349f97f --- /dev/null +++ b/infra/feast-operator/internal/controller/auth/auth.go @@ -0,0 +1,220 @@ +package auth + +import ( + "context" + "slices" + + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" + rbacv1 "k8s.io/api/rbac/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +// Deploy the feast services +func (auth *FeastAuth) Deploy() error { + authConfig := auth.Handler.FeatureStore.Status.Applied.AuthConfig + if authConfig != nil { + if authConfig.KubernetesAuth != nil { + if err := auth.deployKubernetesAuth(authConfig.KubernetesAuth); err != nil { + return err + } + } else { + auth.removeOrphanedRoles() + _ = auth.Handler.DeleteOwnedFeastObj(auth.initFeastRole()) + _ = auth.Handler.DeleteOwnedFeastObj(auth.initFeastRoleBinding()) + } + } + return nil +} + +func (auth *FeastAuth) deployKubernetesAuth(kubernetesAuth *feastdevv1alpha1.KubernetesAuth) error { + auth.removeOrphanedRoles() + + if err := auth.createFeastRole(); err != nil { + return auth.setFeastKubernetesAuthCondition(err) + } + if err := auth.createFeastRoleBinding(); err != nil { + return auth.setFeastKubernetesAuthCondition(err) + } + + for _, roleName := range kubernetesAuth.Roles { + if err := auth.createAuthRole(roleName); err != nil { + return auth.setFeastKubernetesAuthCondition(err) + } + } + return auth.setFeastKubernetesAuthCondition(nil) +} + +func (auth *FeastAuth) removeOrphanedRoles() { + roleList := &rbacv1.RoleList{} + err := auth.Handler.Client.List(context.TODO(), roleList, &client.ListOptions{ + Namespace: auth.Handler.FeatureStore.Namespace, + LabelSelector: labels.SelectorFromSet(auth.getLabels()), + }) + if err != nil { + return + } + + desiredRoles := []string{} + if auth.Handler.FeatureStore.Status.Applied.AuthConfig.KubernetesAuth != nil { + desiredRoles = auth.Handler.FeatureStore.Status.Applied.AuthConfig.KubernetesAuth.Roles + } + for _, role := range roleList.Items { + roleName := role.Name + if roleName != auth.getFeastRoleName() && !slices.Contains(desiredRoles, roleName) { + _ = auth.Handler.DeleteOwnedFeastObj(auth.initAuthRole(roleName)) + } + } +} + +func (auth *FeastAuth) createFeastRole() error { + logger := log.FromContext(auth.Handler.Context) + role := auth.initFeastRole() + if op, err := controllerutil.CreateOrUpdate(auth.Handler.Context, auth.Handler.Client, role, controllerutil.MutateFn(func() error { + return auth.setFeastRole(role) + })); err != nil { + return err + } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "Role", role.Name, "operation", op) + } + + return nil +} + +func (auth *FeastAuth) initFeastRole() *rbacv1.Role { + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{Name: auth.getFeastRoleName(), Namespace: auth.Handler.FeatureStore.Namespace}, + } + role.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind("Role")) + return role +} + +func (auth *FeastAuth) setFeastRole(role *rbacv1.Role) error { + role.Labels = auth.getLabels() + role.Rules = []rbacv1.PolicyRule{ + { + APIGroups: []string{rbacv1.GroupName}, + Resources: []string{"roles", "rolebindings"}, + Verbs: []string{"get", "list", "watch"}, + }, + } + + return controllerutil.SetControllerReference(auth.Handler.FeatureStore, role, auth.Handler.Scheme) +} + +func (auth *FeastAuth) createFeastRoleBinding() error { + logger := log.FromContext(auth.Handler.Context) + roleBinding := auth.initFeastRoleBinding() + if op, err := controllerutil.CreateOrUpdate(auth.Handler.Context, auth.Handler.Client, roleBinding, controllerutil.MutateFn(func() error { + return auth.setFeastRoleBinding(roleBinding) + })); err != nil { + return err + } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "RoleBinding", roleBinding.Name, "operation", op) + } + + return nil +} + +func (auth *FeastAuth) initFeastRoleBinding() *rbacv1.RoleBinding { + roleBinding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: auth.getFeastRoleName(), Namespace: auth.Handler.FeatureStore.Namespace}, + } + roleBinding.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind("RoleBinding")) + return roleBinding +} + +func (auth *FeastAuth) setFeastRoleBinding(roleBinding *rbacv1.RoleBinding) error { + roleBinding.Labels = auth.getLabels() + roleBinding.Subjects = []rbacv1.Subject{} + if auth.Handler.FeatureStore.Status.Applied.Services.OfflineStore != nil { + roleBinding.Subjects = append(roleBinding.Subjects, rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: services.GetFeastServiceName(auth.Handler.FeatureStore, services.OfflineFeastType), + Namespace: auth.Handler.FeatureStore.Namespace, + }) + } + if auth.Handler.FeatureStore.Status.Applied.Services.OnlineStore != nil { + roleBinding.Subjects = append(roleBinding.Subjects, rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: services.GetFeastServiceName(auth.Handler.FeatureStore, services.OnlineFeastType), + Namespace: auth.Handler.FeatureStore.Namespace, + }) + } + if auth.Handler.FeatureStore.Status.Applied.Services.Registry != nil { + roleBinding.Subjects = append(roleBinding.Subjects, rbacv1.Subject{ + Kind: rbacv1.ServiceAccountKind, + Name: services.GetFeastServiceName(auth.Handler.FeatureStore, services.RegistryFeastType), + Namespace: auth.Handler.FeatureStore.Namespace, + }) + } + roleBinding.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: auth.getFeastRoleName(), + } + + return controllerutil.SetControllerReference(auth.Handler.FeatureStore, roleBinding, auth.Handler.Scheme) +} + +func (auth *FeastAuth) createAuthRole(roleName string) error { + logger := log.FromContext(auth.Handler.Context) + role := auth.initAuthRole(roleName) + if op, err := controllerutil.CreateOrUpdate(auth.Handler.Context, auth.Handler.Client, role, controllerutil.MutateFn(func() error { + return auth.setAuthRole(role) + })); err != nil { + return err + } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { + logger.Info("Successfully reconciled", "Role", role.Name, "operation", op) + } + + return nil +} + +func (auth *FeastAuth) initAuthRole(roleName string) *rbacv1.Role { + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{Name: roleName, Namespace: auth.Handler.FeatureStore.Namespace}, + } + role.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind("Role")) + return role +} + +func (auth *FeastAuth) setAuthRole(role *rbacv1.Role) error { + role.Labels = auth.getLabels() + role.Rules = []rbacv1.PolicyRule{} + + return controllerutil.SetControllerReference(auth.Handler.FeatureStore, role, auth.Handler.Scheme) +} + +func (auth *FeastAuth) getLabels() map[string]string { + return map[string]string{ + nameLabelKey: auth.Handler.FeatureStore.Name, + } +} + +func (auth *FeastAuth) setFeastKubernetesAuthCondition(err error) error { + if err != nil { + logger := log.FromContext(auth.Handler.Context) + cond := feastKubernetesAuthConditions[metav1.ConditionFalse] + cond.Message = "Error: " + err.Error() + apimeta.SetStatusCondition(&auth.Handler.FeatureStore.Status.Conditions, cond) + logger.Error(err, "Error deploying the Kubernetes authorization") + return err + } else { + apimeta.SetStatusCondition(&auth.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue]) + } + return nil +} + +func (auth *FeastAuth) getFeastRoleName() string { + return GetFeastRoleName(auth.Handler.FeatureStore) +} + +func GetFeastRoleName(featureStore *feastdevv1alpha1.FeatureStore) string { + return services.GetFeastName(featureStore) +} diff --git a/infra/feast-operator/internal/controller/auth/auth_types.go b/infra/feast-operator/internal/controller/auth/auth_types.go new file mode 100644 index 00000000000..c51025abeba --- /dev/null +++ b/infra/feast-operator/internal/controller/auth/auth_types.go @@ -0,0 +1,30 @@ +package auth + +import ( + feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" + "github.com/feast-dev/feast/infra/feast-operator/internal/controller/handler" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// FeastAuth is an interface for configuring feast authorization +type FeastAuth struct { + Handler handler.FeastHandler +} + +var ( + nameLabelKey = feastdevv1alpha1.GroupVersion.Group + "/name" + + feastKubernetesAuthConditions = map[metav1.ConditionStatus]metav1.Condition{ + metav1.ConditionTrue: { + Type: feastdevv1alpha1.KubernetesAuthReadyType, + Status: metav1.ConditionTrue, + Reason: feastdevv1alpha1.ReadyReason, + Message: feastdevv1alpha1.KubernetesAuthReadyMessage, + }, + metav1.ConditionFalse: { + Type: feastdevv1alpha1.KubernetesAuthReadyType, + Status: metav1.ConditionFalse, + Reason: feastdevv1alpha1.KubernetesAuthFailedReason, + }, + } +) From 55badf93ecf8961540d9ba8654db4a044f384477 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Fri, 22 Nov 2024 22:29:06 +0100 Subject: [PATCH 02/11] no private image Signed-off-by: Daniele Martinoli --- .../samples/v1alpha1_featurestore_ephemeral_persistence.yaml | 3 --- .../config/samples/v1alpha1_featurestore_kubernetes_auth.yaml | 3 --- .../samples/v1alpha1_featurestore_objectstore_persistence.yaml | 3 --- .../config/samples/v1alpha1_featurestore_pvc_persistence.yaml | 3 --- 4 files changed, 12 deletions(-) diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml index 8449ec1bf51..512fed9d4c0 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_ephemeral_persistence.yaml @@ -6,18 +6,15 @@ spec: feastProject: my_project services: onlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/online_store.db offlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: type: dask registry: local: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/registry.db diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml index add9c4431fb..17b40e51eea 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml @@ -6,18 +6,15 @@ spec: feastProject: my_project services: onlineStore: - image: quay.io/dmartino/feature-server:0.3 persistence: file: path: /data/online_store.db offlineStore: - image: quay.io/dmartino/feature-server:0.3 persistence: file: type: dask registry: local: - image: quay.io/dmartino/feature-server:0.3 persistence: file: path: /data/registry.db diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml index e038242da4c..45f12a67a18 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_objectstore_persistence.yaml @@ -6,18 +6,15 @@ spec: feastProject: my_project services: onlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: /data/online_store.db offlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: type: dask registry: local: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: s3://bucket/registry.db diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml index fe2a5071cda..b7c7412c0f0 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_pvc_persistence.yaml @@ -6,7 +6,6 @@ spec: feastProject: my_project services: onlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: online_store.db @@ -15,7 +14,6 @@ spec: name: online-pvc mountPath: /data/online offlineStore: - image: quay.io/dmartino/feature-server:0.2 persistence: file: type: duckdb @@ -28,7 +26,6 @@ spec: mountPath: /data/offline registry: local: - image: quay.io/dmartino/feature-server:0.2 persistence: file: path: registry.db From 2453a1158cfd4ce9ec6479b6a4c99263a021ff63 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 25 Nov 2024 09:41:06 +0100 Subject: [PATCH 03/11] removed nameLabelKey, using serices.NameLabelKey Signed-off-by: Daniele Martinoli --- infra/feast-operator/internal/controller/auth/auth.go | 4 ++-- infra/feast-operator/internal/controller/auth/auth_types.go | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/infra/feast-operator/internal/controller/auth/auth.go b/infra/feast-operator/internal/controller/auth/auth.go index 3be0349f97f..bdc64c06f13 100644 --- a/infra/feast-operator/internal/controller/auth/auth.go +++ b/infra/feast-operator/internal/controller/auth/auth.go @@ -15,7 +15,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" ) -// Deploy the feast services +// Deploy the feast authorization func (auth *FeastAuth) Deploy() error { authConfig := auth.Handler.FeatureStore.Status.Applied.AuthConfig if authConfig != nil { @@ -193,7 +193,7 @@ func (auth *FeastAuth) setAuthRole(role *rbacv1.Role) error { func (auth *FeastAuth) getLabels() map[string]string { return map[string]string{ - nameLabelKey: auth.Handler.FeatureStore.Name, + services.NameLabelKey: auth.Handler.FeatureStore.Name, } } diff --git a/infra/feast-operator/internal/controller/auth/auth_types.go b/infra/feast-operator/internal/controller/auth/auth_types.go index c51025abeba..d3ee4086821 100644 --- a/infra/feast-operator/internal/controller/auth/auth_types.go +++ b/infra/feast-operator/internal/controller/auth/auth_types.go @@ -12,8 +12,6 @@ type FeastAuth struct { } var ( - nameLabelKey = feastdevv1alpha1.GroupVersion.Group + "/name" - feastKubernetesAuthConditions = map[metav1.ConditionStatus]metav1.Condition{ metav1.ConditionTrue: { Type: feastdevv1alpha1.KubernetesAuthReadyType, From cad9f3ec9bc123879e4e5e7acc953a711e130c0a Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 25 Nov 2024 16:27:58 +0100 Subject: [PATCH 04/11] improved CRD comments and using IsLocalRegistry Signed-off-by: Daniele Martinoli --- .../api/v1alpha1/featurestore_types.go | 8 ++++++++ .../crd/bases/feast.dev_featurestores.yaml | 20 +++++++++++++++++++ infra/feast-operator/dist/install.yaml | 18 +++++++++++++++++ .../internal/controller/auth/auth.go | 2 +- 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 94b19dd6e35..d3752debc29 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -284,7 +284,15 @@ type AuthConfig struct { KubernetesAuth *KubernetesAuth `json:"kubernetes,omitempty"` } +// KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. +// https://kubernetes.io/docs/reference/access-authn-authz/rbac/ type KubernetesAuth struct { + // The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. + // Roles are managed by the operator and created with an empty list of rules. + // See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission + // The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. + // This configuration option is only providing a way to automate this procedure. + // Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. Roles []string `json:"roles,omitempty"` } 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 87c8e1e2b49..717f90e5074 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -53,8 +53,18 @@ spec: deployed Feast services. properties: kubernetes: + description: |- + KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. + https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: + description: |- + The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. + Roles are managed by the operator and created with an empty list of rules. + See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission + The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. + This configuration option is only providing a way to automate this procedure. + Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array @@ -1212,8 +1222,18 @@ spec: the deployed Feast services. properties: kubernetes: + description: |- + KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. + https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: + description: |- + The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. + Roles are managed by the operator and created with an empty list of rules. + See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission + The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. + This configuration option is only providing a way to automate this procedure. + Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index f60489c891d..f1c33276db4 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -61,8 +61,17 @@ spec: deployed Feast services. properties: kubernetes: + description: |- + KubernetesAuth defines the authorization settings using Kubernetes RBAC. + https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: + description: |- + The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. + See the Feast permission model https://docs.feast.dev/getting-started/concepts/permission + Please note that the feature store admin is not obligated to manage roles using the Feast operator. + Roles can be managed independently. This configuration is only providing a way to automate this step. + Note that the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array @@ -1220,8 +1229,17 @@ spec: the deployed Feast services. properties: kubernetes: + description: |- + KubernetesAuth defines the authorization settings using Kubernetes RBAC. + https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: + description: |- + The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. + See the Feast permission model https://docs.feast.dev/getting-started/concepts/permission + Please note that the feature store admin is not obligated to manage roles using the Feast operator. + Roles can be managed independently. This configuration is only providing a way to automate this step. + Note that the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array diff --git a/infra/feast-operator/internal/controller/auth/auth.go b/infra/feast-operator/internal/controller/auth/auth.go index bdc64c06f13..9db7d7a9c95 100644 --- a/infra/feast-operator/internal/controller/auth/auth.go +++ b/infra/feast-operator/internal/controller/auth/auth.go @@ -146,7 +146,7 @@ func (auth *FeastAuth) setFeastRoleBinding(roleBinding *rbacv1.RoleBinding) erro Namespace: auth.Handler.FeatureStore.Namespace, }) } - if auth.Handler.FeatureStore.Status.Applied.Services.Registry != nil { + if services.IsLocalRegistry(auth.Handler.FeatureStore) { roleBinding.Subjects = append(roleBinding.Subjects, rbacv1.Subject{ Kind: rbacv1.ServiceAccountKind, Name: services.GetFeastServiceName(auth.Handler.FeatureStore, services.RegistryFeastType), From 25f1d90853ecb8fc7ac9544ba12d5248cbb6ccbc Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 25 Nov 2024 16:33:29 +0100 Subject: [PATCH 05/11] fixing generated code Signed-off-by: Daniele Martinoli --- infra/feast-operator/dist/install.yaml | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index f1c33276db4..4bd00400d52 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -62,16 +62,17 @@ spec: properties: kubernetes: description: |- - KubernetesAuth defines the authorization settings using Kubernetes RBAC. + KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: description: |- The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. - See the Feast permission model https://docs.feast.dev/getting-started/concepts/permission - Please note that the feature store admin is not obligated to manage roles using the Feast operator. - Roles can be managed independently. This configuration is only providing a way to automate this step. - Note that the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. + Roles are managed by the operator and created with an empty list of rules. + See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission + The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. + This configuration option is only providing a way to automate this procedure. + Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array @@ -1230,16 +1231,17 @@ spec: properties: kubernetes: description: |- - KubernetesAuth defines the authorization settings using Kubernetes RBAC. + KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: description: |- The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. - See the Feast permission model https://docs.feast.dev/getting-started/concepts/permission - Please note that the feature store admin is not obligated to manage roles using the Feast operator. - Roles can be managed independently. This configuration is only providing a way to automate this step. - Note that the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. + Roles are managed by the operator and created with an empty list of rules. + See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission + The feature store admin is not obligated to manage roles using the Feast operator, roles can be managed independently. + This configuration option is only providing a way to automate this procedure. + Important note: the operator cannot ensure that these roles will match the ones used in the configured Feast permissions. items: type: string type: array From 2579f40adee371e7c00a07ab19c26e324a477db9 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Tue, 26 Nov 2024 21:07:44 +0100 Subject: [PATCH 06/11] renamed auth condition and types Signed-off-by: Daniele Martinoli --- .../api/v1alpha1/featurestore_types.go | 6 +- .../api/v1alpha1/zz_generated.deepcopy.go | 14 +- .../crd/bases/feast.dev_featurestores.yaml | 8 +- ...v1alpha1_featurestore_kubernetes_auth.yaml | 2 +- infra/feast-operator/dist/install.yaml | 8 +- .../internal/controller/auth/auth.go | 220 ------------------ .../internal/controller/auth/auth_types.go | 28 --- 7 files changed, 19 insertions(+), 267 deletions(-) delete mode 100644 infra/feast-operator/internal/controller/auth/auth.go delete mode 100644 infra/feast-operator/internal/controller/auth/auth_types.go diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index d3752debc29..a1fcef8bb15 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -62,7 +62,7 @@ type FeatureStoreSpec struct { // FeastProject is the Feast project id. This can be any alphanumeric string with underscores, but it cannot start with an underscore. Required. FeastProject string `json:"feastProject"` Services *FeatureStoreServices `json:"services,omitempty"` - AuthConfig *AuthConfig `json:"auth,omitempty"` + AuthzConfig *AuthzConfig `json:"authz,omitempty"` } // FeatureStoreServices defines the desired feast service deployments. ephemeral registry is deployed by default. @@ -279,8 +279,8 @@ type OptionalConfigs struct { Resources *corev1.ResourceRequirements `json:"resources,omitempty"` } -// AuthConfig defines the authorization settings for the deployed Feast services. -type AuthConfig struct { +// AuthzConfig defines the authorization settings for the deployed Feast services. +type AuthzConfig struct { KubernetesAuth *KubernetesAuth `json:"kubernetes,omitempty"` } diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index f9f7ccaeaff..bceb6476851 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -27,7 +27,7 @@ import ( ) // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *AuthConfig) DeepCopyInto(out *AuthConfig) { +func (in *AuthzConfig) DeepCopyInto(out *AuthzConfig) { *out = *in if in.KubernetesAuth != nil { in, out := &in.KubernetesAuth, &out.KubernetesAuth @@ -36,12 +36,12 @@ func (in *AuthConfig) DeepCopyInto(out *AuthConfig) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthConfig. -func (in *AuthConfig) DeepCopy() *AuthConfig { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthzConfig. +func (in *AuthzConfig) DeepCopy() *AuthzConfig { if in == nil { return nil } - out := new(AuthConfig) + out := new(AuthzConfig) in.DeepCopyInto(out) return out } @@ -178,9 +178,9 @@ func (in *FeatureStoreSpec) DeepCopyInto(out *FeatureStoreSpec) { *out = new(FeatureStoreServices) (*in).DeepCopyInto(*out) } - if in.AuthConfig != nil { - in, out := &in.AuthConfig, &out.AuthConfig - *out = new(AuthConfig) + if in.AuthzConfig != nil { + in, out := &in.AuthzConfig, &out.AuthzConfig + *out = new(AuthzConfig) (*in).DeepCopyInto(*out) } } 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 717f90e5074..6d118a4bf57 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -48,8 +48,8 @@ spec: spec: description: FeatureStoreSpec defines the desired state of FeatureStore properties: - auth: - description: AuthConfig defines the authorization settings for the + authz: + description: AuthzConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: @@ -1217,8 +1217,8 @@ spec: description: Shows the currently applied feast configuration, including any pertinent defaults properties: - auth: - description: AuthConfig defines the authorization settings for + authz: + description: AuthzConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml index 17b40e51eea..ed95b41cf47 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_kubernetes_auth.yaml @@ -18,7 +18,7 @@ spec: persistence: file: path: /data/registry.db - auth: + authz: kubernetes: roles: - reader diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 4bd00400d52..23ca314c74e 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -56,8 +56,8 @@ spec: spec: description: FeatureStoreSpec defines the desired state of FeatureStore properties: - auth: - description: AuthConfig defines the authorization settings for the + authz: + description: AuthzConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: @@ -1225,8 +1225,8 @@ spec: description: Shows the currently applied feast configuration, including any pertinent defaults properties: - auth: - description: AuthConfig defines the authorization settings for + authz: + description: AuthzConfig defines the authorization settings for the deployed Feast services. properties: kubernetes: diff --git a/infra/feast-operator/internal/controller/auth/auth.go b/infra/feast-operator/internal/controller/auth/auth.go deleted file mode 100644 index 9db7d7a9c95..00000000000 --- a/infra/feast-operator/internal/controller/auth/auth.go +++ /dev/null @@ -1,220 +0,0 @@ -package auth - -import ( - "context" - "slices" - - feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" - "github.com/feast-dev/feast/infra/feast-operator/internal/controller/services" - rbacv1 "k8s.io/api/rbac/v1" - apimeta "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/log" -) - -// Deploy the feast authorization -func (auth *FeastAuth) Deploy() error { - authConfig := auth.Handler.FeatureStore.Status.Applied.AuthConfig - if authConfig != nil { - if authConfig.KubernetesAuth != nil { - if err := auth.deployKubernetesAuth(authConfig.KubernetesAuth); err != nil { - return err - } - } else { - auth.removeOrphanedRoles() - _ = auth.Handler.DeleteOwnedFeastObj(auth.initFeastRole()) - _ = auth.Handler.DeleteOwnedFeastObj(auth.initFeastRoleBinding()) - } - } - return nil -} - -func (auth *FeastAuth) deployKubernetesAuth(kubernetesAuth *feastdevv1alpha1.KubernetesAuth) error { - auth.removeOrphanedRoles() - - if err := auth.createFeastRole(); err != nil { - return auth.setFeastKubernetesAuthCondition(err) - } - if err := auth.createFeastRoleBinding(); err != nil { - return auth.setFeastKubernetesAuthCondition(err) - } - - for _, roleName := range kubernetesAuth.Roles { - if err := auth.createAuthRole(roleName); err != nil { - return auth.setFeastKubernetesAuthCondition(err) - } - } - return auth.setFeastKubernetesAuthCondition(nil) -} - -func (auth *FeastAuth) removeOrphanedRoles() { - roleList := &rbacv1.RoleList{} - err := auth.Handler.Client.List(context.TODO(), roleList, &client.ListOptions{ - Namespace: auth.Handler.FeatureStore.Namespace, - LabelSelector: labels.SelectorFromSet(auth.getLabels()), - }) - if err != nil { - return - } - - desiredRoles := []string{} - if auth.Handler.FeatureStore.Status.Applied.AuthConfig.KubernetesAuth != nil { - desiredRoles = auth.Handler.FeatureStore.Status.Applied.AuthConfig.KubernetesAuth.Roles - } - for _, role := range roleList.Items { - roleName := role.Name - if roleName != auth.getFeastRoleName() && !slices.Contains(desiredRoles, roleName) { - _ = auth.Handler.DeleteOwnedFeastObj(auth.initAuthRole(roleName)) - } - } -} - -func (auth *FeastAuth) createFeastRole() error { - logger := log.FromContext(auth.Handler.Context) - role := auth.initFeastRole() - if op, err := controllerutil.CreateOrUpdate(auth.Handler.Context, auth.Handler.Client, role, controllerutil.MutateFn(func() error { - return auth.setFeastRole(role) - })); err != nil { - return err - } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - logger.Info("Successfully reconciled", "Role", role.Name, "operation", op) - } - - return nil -} - -func (auth *FeastAuth) initFeastRole() *rbacv1.Role { - role := &rbacv1.Role{ - ObjectMeta: metav1.ObjectMeta{Name: auth.getFeastRoleName(), Namespace: auth.Handler.FeatureStore.Namespace}, - } - role.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind("Role")) - return role -} - -func (auth *FeastAuth) setFeastRole(role *rbacv1.Role) error { - role.Labels = auth.getLabels() - role.Rules = []rbacv1.PolicyRule{ - { - APIGroups: []string{rbacv1.GroupName}, - Resources: []string{"roles", "rolebindings"}, - Verbs: []string{"get", "list", "watch"}, - }, - } - - return controllerutil.SetControllerReference(auth.Handler.FeatureStore, role, auth.Handler.Scheme) -} - -func (auth *FeastAuth) createFeastRoleBinding() error { - logger := log.FromContext(auth.Handler.Context) - roleBinding := auth.initFeastRoleBinding() - if op, err := controllerutil.CreateOrUpdate(auth.Handler.Context, auth.Handler.Client, roleBinding, controllerutil.MutateFn(func() error { - return auth.setFeastRoleBinding(roleBinding) - })); err != nil { - return err - } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - logger.Info("Successfully reconciled", "RoleBinding", roleBinding.Name, "operation", op) - } - - return nil -} - -func (auth *FeastAuth) initFeastRoleBinding() *rbacv1.RoleBinding { - roleBinding := &rbacv1.RoleBinding{ - ObjectMeta: metav1.ObjectMeta{Name: auth.getFeastRoleName(), Namespace: auth.Handler.FeatureStore.Namespace}, - } - roleBinding.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind("RoleBinding")) - return roleBinding -} - -func (auth *FeastAuth) setFeastRoleBinding(roleBinding *rbacv1.RoleBinding) error { - roleBinding.Labels = auth.getLabels() - roleBinding.Subjects = []rbacv1.Subject{} - if auth.Handler.FeatureStore.Status.Applied.Services.OfflineStore != nil { - roleBinding.Subjects = append(roleBinding.Subjects, rbacv1.Subject{ - Kind: rbacv1.ServiceAccountKind, - Name: services.GetFeastServiceName(auth.Handler.FeatureStore, services.OfflineFeastType), - Namespace: auth.Handler.FeatureStore.Namespace, - }) - } - if auth.Handler.FeatureStore.Status.Applied.Services.OnlineStore != nil { - roleBinding.Subjects = append(roleBinding.Subjects, rbacv1.Subject{ - Kind: rbacv1.ServiceAccountKind, - Name: services.GetFeastServiceName(auth.Handler.FeatureStore, services.OnlineFeastType), - Namespace: auth.Handler.FeatureStore.Namespace, - }) - } - if services.IsLocalRegistry(auth.Handler.FeatureStore) { - roleBinding.Subjects = append(roleBinding.Subjects, rbacv1.Subject{ - Kind: rbacv1.ServiceAccountKind, - Name: services.GetFeastServiceName(auth.Handler.FeatureStore, services.RegistryFeastType), - Namespace: auth.Handler.FeatureStore.Namespace, - }) - } - roleBinding.RoleRef = rbacv1.RoleRef{ - APIGroup: rbacv1.GroupName, - Kind: "Role", - Name: auth.getFeastRoleName(), - } - - return controllerutil.SetControllerReference(auth.Handler.FeatureStore, roleBinding, auth.Handler.Scheme) -} - -func (auth *FeastAuth) createAuthRole(roleName string) error { - logger := log.FromContext(auth.Handler.Context) - role := auth.initAuthRole(roleName) - if op, err := controllerutil.CreateOrUpdate(auth.Handler.Context, auth.Handler.Client, role, controllerutil.MutateFn(func() error { - return auth.setAuthRole(role) - })); err != nil { - return err - } else if op == controllerutil.OperationResultCreated || op == controllerutil.OperationResultUpdated { - logger.Info("Successfully reconciled", "Role", role.Name, "operation", op) - } - - return nil -} - -func (auth *FeastAuth) initAuthRole(roleName string) *rbacv1.Role { - role := &rbacv1.Role{ - ObjectMeta: metav1.ObjectMeta{Name: roleName, Namespace: auth.Handler.FeatureStore.Namespace}, - } - role.SetGroupVersionKind(rbacv1.SchemeGroupVersion.WithKind("Role")) - return role -} - -func (auth *FeastAuth) setAuthRole(role *rbacv1.Role) error { - role.Labels = auth.getLabels() - role.Rules = []rbacv1.PolicyRule{} - - return controllerutil.SetControllerReference(auth.Handler.FeatureStore, role, auth.Handler.Scheme) -} - -func (auth *FeastAuth) getLabels() map[string]string { - return map[string]string{ - services.NameLabelKey: auth.Handler.FeatureStore.Name, - } -} - -func (auth *FeastAuth) setFeastKubernetesAuthCondition(err error) error { - if err != nil { - logger := log.FromContext(auth.Handler.Context) - cond := feastKubernetesAuthConditions[metav1.ConditionFalse] - cond.Message = "Error: " + err.Error() - apimeta.SetStatusCondition(&auth.Handler.FeatureStore.Status.Conditions, cond) - logger.Error(err, "Error deploying the Kubernetes authorization") - return err - } else { - apimeta.SetStatusCondition(&auth.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue]) - } - return nil -} - -func (auth *FeastAuth) getFeastRoleName() string { - return GetFeastRoleName(auth.Handler.FeatureStore) -} - -func GetFeastRoleName(featureStore *feastdevv1alpha1.FeatureStore) string { - return services.GetFeastName(featureStore) -} diff --git a/infra/feast-operator/internal/controller/auth/auth_types.go b/infra/feast-operator/internal/controller/auth/auth_types.go deleted file mode 100644 index d3ee4086821..00000000000 --- a/infra/feast-operator/internal/controller/auth/auth_types.go +++ /dev/null @@ -1,28 +0,0 @@ -package auth - -import ( - feastdevv1alpha1 "github.com/feast-dev/feast/infra/feast-operator/api/v1alpha1" - "github.com/feast-dev/feast/infra/feast-operator/internal/controller/handler" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// FeastAuth is an interface for configuring feast authorization -type FeastAuth struct { - Handler handler.FeastHandler -} - -var ( - feastKubernetesAuthConditions = map[metav1.ConditionStatus]metav1.Condition{ - metav1.ConditionTrue: { - Type: feastdevv1alpha1.KubernetesAuthReadyType, - Status: metav1.ConditionTrue, - Reason: feastdevv1alpha1.ReadyReason, - Message: feastdevv1alpha1.KubernetesAuthReadyMessage, - }, - metav1.ConditionFalse: { - Type: feastdevv1alpha1.KubernetesAuthReadyType, - Status: metav1.ConditionFalse, - Reason: feastdevv1alpha1.KubernetesAuthFailedReason, - }, - } -) From acde71ac165a3f5e906ccf64533135eee71f984c Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Tue, 26 Nov 2024 21:53:18 +0100 Subject: [PATCH 07/11] more renamings Signed-off-by: Daniele Martinoli --- .../api/v1alpha1/featurestore_types.go | 6 +++--- .../api/v1alpha1/zz_generated.deepcopy.go | 14 +++++++------- .../config/crd/bases/feast.dev_featurestores.yaml | 4 ++-- infra/feast-operator/dist/install.yaml | 4 ++-- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index a1fcef8bb15..4bc0aa7c5e0 100644 --- a/infra/feast-operator/api/v1alpha1/featurestore_types.go +++ b/infra/feast-operator/api/v1alpha1/featurestore_types.go @@ -281,12 +281,12 @@ type OptionalConfigs struct { // AuthzConfig defines the authorization settings for the deployed Feast services. type AuthzConfig struct { - KubernetesAuth *KubernetesAuth `json:"kubernetes,omitempty"` + KubernetesAuthz *KubernetesAuthz `json:"kubernetes,omitempty"` } -// KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. +// KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. // https://kubernetes.io/docs/reference/access-authn-authz/rbac/ -type KubernetesAuth struct { +type KubernetesAuthz struct { // The Kubernetes RBAC roles to be deployed in the same namespace of the FeatureStore. // Roles are managed by the operator and created with an empty list of rules. // See the Feast permission model at https://docs.feast.dev/getting-started/concepts/permission diff --git a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go index bceb6476851..8675397fee5 100644 --- a/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/infra/feast-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -29,9 +29,9 @@ import ( // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AuthzConfig) DeepCopyInto(out *AuthzConfig) { *out = *in - if in.KubernetesAuth != nil { - in, out := &in.KubernetesAuth, &out.KubernetesAuth - *out = new(KubernetesAuth) + if in.KubernetesAuthz != nil { + in, out := &in.KubernetesAuthz, &out.KubernetesAuthz + *out = new(KubernetesAuthz) (*in).DeepCopyInto(*out) } } @@ -220,7 +220,7 @@ func (in *FeatureStoreStatus) DeepCopy() *FeatureStoreStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *KubernetesAuth) DeepCopyInto(out *KubernetesAuth) { +func (in *KubernetesAuthz) DeepCopyInto(out *KubernetesAuthz) { *out = *in if in.Roles != nil { in, out := &in.Roles, &out.Roles @@ -229,12 +229,12 @@ func (in *KubernetesAuth) DeepCopyInto(out *KubernetesAuth) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesAuth. -func (in *KubernetesAuth) DeepCopy() *KubernetesAuth { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesAuthz. +func (in *KubernetesAuthz) DeepCopy() *KubernetesAuthz { if in == nil { return nil } - out := new(KubernetesAuth) + out := new(KubernetesAuthz) in.DeepCopyInto(out) return out } 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 6d118a4bf57..958c7cdddb1 100644 --- a/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml +++ b/infra/feast-operator/config/crd/bases/feast.dev_featurestores.yaml @@ -54,7 +54,7 @@ spec: properties: kubernetes: description: |- - KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. + KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: @@ -1223,7 +1223,7 @@ spec: properties: kubernetes: description: |- - KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. + KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: diff --git a/infra/feast-operator/dist/install.yaml b/infra/feast-operator/dist/install.yaml index 23ca314c74e..b5e103f9692 100644 --- a/infra/feast-operator/dist/install.yaml +++ b/infra/feast-operator/dist/install.yaml @@ -62,7 +62,7 @@ spec: properties: kubernetes: description: |- - KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. + KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: @@ -1231,7 +1231,7 @@ spec: properties: kubernetes: description: |- - KubernetesAuth provides a way to define the authorization settings using Kubernetes RBAC resources. + KubernetesAuthz provides a way to define the authorization settings using Kubernetes RBAC resources. https://kubernetes.io/docs/reference/access-authn-authz/rbac/ properties: roles: From 82085d66e95b6bf75280869bb5681a5550ba0b76 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Thu, 28 Nov 2024 15:49:14 +0100 Subject: [PATCH 08/11] initial commit Signed-off-by: Daniele Martinoli --- .../api/v1alpha1/featurestore_types.go | 8 ++ .../api/v1alpha1/zz_generated.deepcopy.go | 21 +++ .../crd/bases/feast.dev_featurestores.yaml | 49 +++++++ infra/feast-operator/dist/install.yaml | 49 +++++++ .../internal/controller/authz/authz.go | 14 +- ...restore_controller_kubernetes_auth_test.go | 4 +- .../featurestore_controller_test.go | 1 - .../internal/controller/services/client.go | 2 +- .../controller/services/repo_config.go | 119 ++++++++++++---- .../controller/services/repo_config_test.go | 130 ++++++++++++++++++ .../controller/services/services_types.go | 18 ++- .../internal/controller/services/tls_test.go | 6 +- .../internal/controller/services/util.go | 4 + .../test/api/featurestore_types_test.go | 31 ++++- 14 files changed, 411 insertions(+), 45 deletions(-) diff --git a/infra/feast-operator/api/v1alpha1/featurestore_types.go b/infra/feast-operator/api/v1alpha1/featurestore_types.go index 4bc0aa7c5e0..bdeecd570d1 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 8675397fee5..3f317c650e9 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 958c7cdddb1..fddac5458fc 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/dist/install.yaml b/infra/feast-operator/dist/install.yaml index b5e103f9692..59ca5505b92 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 c7d4b896063..efcae23a4b0 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 6589e181af7..4930f3fc590 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_test.go b/infra/feast-operator/internal/controller/featurestore_controller_test.go index a6e71934fb0..44c81eca59a 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 48ca0751cee..d4b78e2611e 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 8b52296160f..22052aa724d 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 18cf104eb52..cb3f96c7ba7 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 65e50b36818..2c454459d88 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 28a3a49ec2f..a0a6cbe3101 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 8f871cdd15f..323f87119e3 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 b24973ec86c..0a7f8fd53a2 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") + }) + }) }) From 99455e51db3376e286057ba32c6e26d8e41a6a6c Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 2 Dec 2024 09:54:24 +0100 Subject: [PATCH 09/11] oidc IT Signed-off-by: Daniele Martinoli --- .../featurestore_controller_oidc_auth_test.go | 589 ++++++++++++++++++ 1 file changed, 589 insertions(+) create mode 100644 infra/feast-operator/internal/controller/featurestore_controller_oidc_auth_test.go 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 00000000000..c062a573df2 --- /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 +} From 5f2fd3333edfa7f3ff60e04cb536d41dc9c78c63 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 2 Dec 2024 10:43:05 +0100 Subject: [PATCH 10/11] with sample Signed-off-by: Daniele Martinoli --- .../v1alpha1_featurestore_oidc_auth.yaml | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml 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 00000000000..7e7e5ca1d85 --- /dev/null +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml @@ -0,0 +1,38 @@ +apiVersion: feast.dev/v1alpha1 +kind: FeatureStore +metadata: + name: sample-oidc-auth +spec: + feastProject: my_project + services: + onlineStore: + image: quay.io/dmartino/feature-server:0.3 + persistence: + file: + path: /data/online_store.db + offlineStore: + image: quay.io/dmartino/feature-server:0.3 + persistence: + file: + type: dask + registry: + local: + image: quay.io/dmartino/feature-server:0.3 + 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 From e3de29cd88ed2567fd5c81c2b52b45f9052d8b09 Mon Sep 17 00:00:00 2001 From: Daniele Martinoli Date: Mon, 2 Dec 2024 10:46:13 +0100 Subject: [PATCH 11/11] no private image Signed-off-by: Daniele Martinoli --- .../config/samples/v1alpha1_featurestore_oidc_auth.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml b/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml index 7e7e5ca1d85..c70f172ded9 100644 --- a/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml +++ b/infra/feast-operator/config/samples/v1alpha1_featurestore_oidc_auth.yaml @@ -6,18 +6,15 @@ spec: feastProject: my_project services: onlineStore: - image: quay.io/dmartino/feature-server:0.3 persistence: file: path: /data/online_store.db offlineStore: - image: quay.io/dmartino/feature-server:0.3 persistence: file: type: dask registry: local: - image: quay.io/dmartino/feature-server:0.3 persistence: file: path: /data/registry.db