Use SelfSubjectAccessReview to check ODLM's permission on getting catalog (#1100)

Signed-off-by: Daniel Fan <[email protected]>

Author: Daniel-Fan

controllers/operator/manager.go

@@ -174,7 +174,7 @@ func (m *ODLMOperator) GetCatalogSourceAndChannelFromPackage(ctx context.Context
 				continue
 			}
 
-			hasCatalogPermission := m.CheckResAuth(ctx, namespace, "operators.coreos.com", "catalogsources", "get")
+			hasCatalogPermission := m.CheckResAuth(ctx, pm.Status.CatalogSourceNamespace, "operators.coreos.com", "catalogsources", "get")
 			if !hasCatalogPermission {
 				klog.V(2).Infof("No permission to get CatalogSource %s in the namespace %s", pm.Status.CatalogSource, pm.Status.CatalogSourceNamespace)
 				continue
@@ -230,8 +230,8 @@ func (m *ODLMOperator) GetCatalogSourceAndChannelFromPackage(ctx context.Context
 }
 
 func (m *ODLMOperator) CheckResAuth(ctx context.Context, namespace, group, resource, verb string) bool {
-	sar := &authorizationv1.SubjectAccessReview{
-		Spec: authorizationv1.SubjectAccessReviewSpec{
+	sar := &authorizationv1.SelfSubjectAccessReview{
+		Spec: authorizationv1.SelfSubjectAccessReviewSpec{
 			ResourceAttributes: &authorizationv1.ResourceAttributes{
 				Namespace: namespace,
 				Group:     group,
@@ -241,14 +241,13 @@ func (m *ODLMOperator) CheckResAuth(ctx context.Context, namespace, group, resou
 		},
 	}
 	if err := m.Create(ctx, sar); err != nil {
+		klog.Errorf("Failed to check operator permission for Kind %s in namespace %s: %v", resource, namespace, err)
 		return false
 	}
 
-	if !sar.Status.Allowed {
-		return false
-	}
+	klog.V(2).Infof("Operator %s permission in namespace %s for Kind: %s, Allowed: %t, Denied: %t, Reason: %s", verb, namespace, resource, sar.Status.Allowed, sar.Status.Denied, sar.Status.Reason)
 
-	return true
+	return sar.Status.Allowed
 }
 
 func channelCheck(channelName string, channelList []operatorsv1.PackageChannel) bool {

controllers/operator/manager_test.go

@@ -313,7 +313,7 @@ func TestGetCatalogSourceAndChannelFromPackage(t *testing.T) {
 	}
 
 	mockClient.mock.On("Create", ctx, mock.Anything, mock.Anything).Return(nil).Run(func(args mock.Arguments) {
-		sar := args.Get(1).(*authorizationv1.SubjectAccessReview)
+		sar := args.Get(1).(*authorizationv1.SelfSubjectAccessReview)
 		sar.Status.Allowed = true
 	})
 
@@ -434,20 +434,20 @@ func TestCheckResAuth(t *testing.T) {
 	resource := "test-resource"
 	verb := "get"
 
-	// Test when SubjectAccessReview is allowed
+	// Test when SelfSubjectAccessReview is allowed
 	mockClient.mock.On("Create", ctx, mock.Anything, mock.Anything).Return(nil).Run(func(args mock.Arguments) {
-		sar := args.Get(1).(*authorizationv1.SubjectAccessReview)
+		sar := args.Get(1).(*authorizationv1.SelfSubjectAccessReview)
 		sar.Status.Allowed = true
 	})
 
 	if !operator.CheckResAuth(ctx, namespace, group, resource, verb) {
 		t.Errorf("Expected CheckResAuth to return true, but got false")
 	}
 
-	// Test when SubjectAccessReview is not allowed
+	// Test when SelfSubjectAccessReview is not allowed
 	mockClient.mock.ExpectedCalls = nil
 	mockClient.mock.On("Create", ctx, mock.Anything, mock.Anything).Return(nil).Run(func(args mock.Arguments) {
-		sar := args.Get(1).(*authorizationv1.SubjectAccessReview)
+		sar := args.Get(1).(*authorizationv1.SelfSubjectAccessReview)
 		sar.Status.Allowed = false
 	})