Skip to content

Commit d4c3229

Browse files
tchughesivtchughesiv
committed
authz rbac fixes
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
1 parent 648f519 commit d4c3229

File tree

12 files changed

+44
-44
lines changed

12 files changed

+44
-44
lines changed

‎infra/feast-operator/api/v1alpha1/featurestore_types.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const (
3333
OnlineStoreReadyType = "OnlineStore"
3434
RegistryReadyType = "Registry"
3535
ReadyType = "FeatureStore"
36-
AuthorizationReadyType = "AuthorizationReadyType"
36+
AuthorizationReadyType = "Authorization"
3737

3838
// Feast condition reasons:
3939
ReadyReason = "Ready"

‎infra/feast-operator/config/rbac/role.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ rules:
6565
- apiGroups:
6666
- rbac.authorization.k8s.io
6767
resources:
68+
- rolebindings
6869
- roles
6970
verbs:
7071
- create

‎infra/feast-operator/dist/install.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2667,6 +2667,7 @@ rules:
26672667
- apiGroups:
26682668
- rbac.authorization.k8s.io
26692669
resources:
2670+
- rolebindings
26702671
- roles
26712672
verbs:
26722673
- create

‎infra/feast-operator/internal/controller/authz/authz.go‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,35 +17,40 @@ import (
1717

1818
// Deploy the feast authorization
1919
func (authz *FeastAuthorization) Deploy() error {
20-
authzConfig := authz.Handler.FeatureStore.Status.Applied.AuthzConfig
21-
if authzConfig != nil {
22-
if authzConfig.KubernetesAuthz != nil {
23-
if err := authz.deployKubernetesAuth(authzConfig.KubernetesAuthz); err != nil {
24-
return err
25-
}
26-
} else {
27-
authz.removeOrphanedRoles()
28-
_ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRole())
29-
_ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRoleBinding())
20+
if authz.isKubernetesAuth() {
21+
if err := authz.deployKubernetesAuth(); err != nil {
22+
return err
3023
}
24+
} else {
25+
authz.removeOrphanedRoles()
26+
_ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRole())
27+
_ = authz.Handler.DeleteOwnedFeastObj(authz.initFeastRoleBinding())
28+
apimeta.RemoveStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue].Type)
3129
}
3230
return nil
3331
}
3432

35-
func (authz *FeastAuthorization) deployKubernetesAuth(kubernetesAuth *feastdevv1alpha1.KubernetesAuthz) error {
36-
authz.removeOrphanedRoles()
33+
func (authz *FeastAuthorization) isKubernetesAuth() bool {
34+
authzConfig := authz.Handler.FeatureStore.Status.Applied.AuthzConfig
35+
return authzConfig != nil && authzConfig.KubernetesAuthz != nil
36+
}
3737

38-
if err := authz.createFeastRole(); err != nil {
39-
return authz.setFeastKubernetesAuthCondition(err)
40-
}
41-
if err := authz.createFeastRoleBinding(); err != nil {
42-
return authz.setFeastKubernetesAuthCondition(err)
43-
}
38+
func (authz *FeastAuthorization) deployKubernetesAuth() error {
39+
if authz.isKubernetesAuth() {
40+
authz.removeOrphanedRoles()
4441

45-
for _, roleName := range kubernetesAuth.Roles {
46-
if err := authz.createAuthRole(roleName); err != nil {
42+
if err := authz.createFeastRole(); err != nil {
43+
return authz.setFeastKubernetesAuthCondition(err)
44+
}
45+
if err := authz.createFeastRoleBinding(); err != nil {
4746
return authz.setFeastKubernetesAuthCondition(err)
4847
}
48+
49+
for _, roleName := range authz.Handler.FeatureStore.Status.Applied.AuthzConfig.KubernetesAuthz.Roles {
50+
if err := authz.createAuthRole(roleName); err != nil {
51+
return authz.setFeastKubernetesAuthCondition(err)
52+
}
53+
}
4954
}
5055
return authz.setFeastKubernetesAuthCondition(nil)
5156
}
@@ -61,7 +66,7 @@ func (authz *FeastAuthorization) removeOrphanedRoles() {
6166
}
6267

6368
desiredRoles := []string{}
64-
if authz.Handler.FeatureStore.Status.Applied.AuthzConfig.KubernetesAuthz != nil {
69+
if authz.isKubernetesAuth() {
6570
desiredRoles = authz.Handler.FeatureStore.Status.Applied.AuthzConfig.KubernetesAuthz.Roles
6671
}
6772
for _, role := range roleList.Items {

‎infra/feast-operator/internal/controller/featurestore_controller.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ type FeatureStoreReconciler struct {
5757
//+kubebuilder:rbac:groups=feast.dev,resources=featurestores/finalizers,verbs=update
5858
//+kubebuilder:rbac:groups=apps,resources=deployments,verbs=get;list;create;update;watch;delete
5959
//+kubebuilder:rbac:groups=core,resources=services;configmaps;persistentvolumeclaims;serviceaccounts,verbs=get;list;create;update;watch;delete
60-
//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles,verbs=get;list;create;update;watch;delete
60+
//+kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=roles;rolebindings,verbs=get;list;create;update;watch;delete
6161
//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list
6262

6363
// Reconcile is part of the main kubernetes reconciliation loop which aims to

‎infra/feast-operator/internal/controller/featurestore_controller_ephemeral_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() {
127127
Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion))
128128
Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType)))
129129
Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject))
130-
Expect(resource.Status.Applied.AuthzConfig).To(Equal(&feastdevv1alpha1.AuthzConfig{}))
130+
Expect(resource.Status.Applied.AuthzConfig).To(BeNil())
131131
Expect(resource.Status.Applied.Services).NotTo(BeNil())
132132
Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil())
133133
Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil())

‎infra/feast-operator/internal/controller/featurestore_controller_objectstore_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() {
122122
Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion))
123123
Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType)))
124124
Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject))
125-
Expect(resource.Status.Applied.AuthzConfig).To(Equal(&feastdevv1alpha1.AuthzConfig{}))
125+
Expect(resource.Status.Applied.AuthzConfig).To(BeNil())
126126
Expect(resource.Status.Applied.Services).NotTo(BeNil())
127127
Expect(resource.Status.Applied.Services.OfflineStore).To(BeNil())
128128
Expect(resource.Status.Applied.Services.OnlineStore).To(BeNil())

‎infra/feast-operator/internal/controller/featurestore_controller_pvc_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ var _ = Describe("FeatureStore Controller-Ephemeral services", func() {
153153
Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion))
154154
Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType)))
155155
Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject))
156-
Expect(resource.Status.Applied.AuthzConfig).To(Equal(&feastdevv1alpha1.AuthzConfig{}))
156+
Expect(resource.Status.Applied.AuthzConfig).To(BeNil())
157157
Expect(resource.Status.Applied.Services).NotTo(BeNil())
158158
Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil())
159159
Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil())

‎infra/feast-operator/internal/controller/featurestore_controller_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ var _ = Describe("FeatureStore Controller", func() {
134134
Expect(resource.Status.ServiceHostnames.OnlineStore).To(BeEmpty())
135135
Expect(resource.Status.ServiceHostnames.Registry).To(Equal(feast.GetFeastServiceName(services.RegistryFeastType) + "." + resource.Namespace + ".svc.cluster.local:80"))
136136
Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject))
137-
Expect(resource.Status.Applied.AuthzConfig).To(Equal(&feastdevv1alpha1.AuthzConfig{}))
137+
Expect(resource.Status.Applied.AuthzConfig).To(BeNil())
138138
Expect(resource.Status.Applied.Services).NotTo(BeNil())
139139
Expect(resource.Status.Applied.Services.OfflineStore).To(BeNil())
140140
Expect(resource.Status.Applied.Services.OnlineStore).To(BeNil())
@@ -152,6 +152,8 @@ var _ = Describe("FeatureStore Controller", func() {
152152
Expect(cond.Reason).To(Equal(feastdevv1alpha1.ReadyReason))
153153
Expect(cond.Type).To(Equal(feastdevv1alpha1.ReadyType))
154154
Expect(cond.Message).To(Equal(feastdevv1alpha1.ReadyMessage))
155+
cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.AuthorizationReadyType)
156+
Expect(cond).To(BeNil())
155157

156158
cond = apimeta.FindStatusCondition(resource.Status.Conditions, feastdevv1alpha1.RegistryReadyType)
157159
Expect(cond).ToNot(BeNil())
@@ -459,7 +461,7 @@ var _ = Describe("FeatureStore Controller", func() {
459461
Expect(resource.Status.FeastVersion).To(Equal(feastversion.FeastVersion))
460462
Expect(resource.Status.ClientConfigMap).To(Equal(feast.GetFeastServiceName(services.ClientFeastType)))
461463
Expect(resource.Status.Applied.FeastProject).To(Equal(resource.Spec.FeastProject))
462-
Expect(resource.Status.Applied.AuthzConfig).To(Equal(&feastdevv1alpha1.AuthzConfig{}))
464+
Expect(resource.Status.Applied.AuthzConfig).To(BeNil())
463465
Expect(resource.Status.Applied.Services).NotTo(BeNil())
464466
Expect(resource.Status.Applied.Services.OfflineStore).NotTo(BeNil())
465467
Expect(resource.Status.Applied.Services.OfflineStore.Persistence).NotTo(BeNil())
@@ -962,6 +964,7 @@ var _ = Describe("FeatureStore Controller", func() {
962964
},
963965
Spec: feastdevv1alpha1.FeatureStoreSpec{
964966
FeastProject: referencedRegistry.Spec.FeastProject,
967+
AuthzConfig: &feastdevv1alpha1.AuthzConfig{},
965968
Services: &feastdevv1alpha1.FeatureStoreServices{
966969
OnlineStore: &feastdevv1alpha1.OnlineStore{},
967970
OfflineStore: &feastdevv1alpha1.OfflineStore{},

‎infra/feast-operator/internal/controller/services/repo_config.go‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -248,16 +248,11 @@ func getClientRepoConfig(featureStore *feastdevv1alpha1.FeatureStore) RepoConfig
248248
}
249249
}
250250

251-
if status.Applied.AuthzConfig.KubernetesAuthz == nil {
252-
clientRepoConfig.AuthzConfig = AuthzConfig{
253-
Type: NoAuthAuthType,
254-
}
255-
} else {
256-
if status.Applied.AuthzConfig.KubernetesAuthz != nil {
257-
clientRepoConfig.AuthzConfig = AuthzConfig{
258-
Type: KubernetesAuthType,
259-
}
260-
}
251+
clientRepoConfig.AuthzConfig = AuthzConfig{
252+
Type: NoAuthAuthType,
253+
}
254+
if status.Applied.AuthzConfig != nil && status.Applied.AuthzConfig.KubernetesAuthz != nil {
255+
clientRepoConfig.AuthzConfig.Type = KubernetesAuthType
261256
}
262257
return clientRepoConfig
263258
}

0 commit comments

Comments
 (0)