Discover private CatalogSource only if no catalog cluster permission exists (#1099)

* read catalogsource permission only for non-private ctrsource

Signed-off-by: YuChen <[email protected]>

* add test case for checkResAuth function in manager.go

Signed-off-by: YuChen <[email protected]>

* add create mock test or ODLMoperator

Signed-off-by: YuChen <[email protected]>

* update test case

Signed-off-by: YuChen <[email protected]>

---------

Signed-off-by: YuChen <[email protected]>

Author: YCShen1010

controllers/operator/manager.go

@@ -27,6 +27,7 @@ import (
 	operatorsv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1"
 	"github.com/pkg/errors"
 	"golang.org/x/mod/semver"
+	authorizationv1 "k8s.io/api/authorization/v1"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -172,11 +173,19 @@ func (m *ODLMOperator) GetCatalogSourceAndChannelFromPackage(ctx context.Context
 			if excludedCatalogSources != nil && util.Contains(excludedCatalogSources, pm.Status.CatalogSource) {
 				continue
 			}
+
+			hasCatalogPermission := m.CheckResAuth(ctx, namespace, "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
+			}
+			// Fetch the CatalogSource if cluster permission allows
 			catalogsource := &olmv1alpha1.CatalogSource{}
 			if err := m.Reader.Get(ctx, types.NamespacedName{Name: pm.Status.CatalogSource, Namespace: pm.Status.CatalogSourceNamespace}, catalogsource); err != nil {
 				klog.Warning(err)
 				continue
 			}
+
 			currentCatalog := CatalogSource{
 				Name:                 pm.Status.CatalogSource,
 				Namespace:            pm.Status.CatalogSourceNamespace,
@@ -220,6 +229,28 @@ 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{
+			ResourceAttributes: &authorizationv1.ResourceAttributes{
+				Namespace: namespace,
+				Group:     group,
+				Resource:  resource,
+				Verb:      verb,
+			},
+		},
+	}
+	if err := m.Create(ctx, sar); err != nil {
+		return false
+	}
+
+	if !sar.Status.Allowed {
+		return false
+	}
+
+	return true
+}
+
 func channelCheck(channelName string, channelList []operatorsv1.PackageChannel) bool {
 	for _, channel := range channelList {
 		if channelName == channel.Name {

controllers/operator/manager_test.go

@@ -18,11 +18,14 @@ package operator
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 	"testing"
 
 	olmv1alpha1 "github.com/operator-framework/api/pkg/operators/v1alpha1"
 	operatorsv1 "github.com/operator-framework/operator-lifecycle-manager/pkg/package-server/apis/operators/v1"
+	"github.com/stretchr/testify/mock"
+	authorizationv1 "k8s.io/api/authorization/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -302,10 +305,18 @@ func TestGetCatalogSourceAndChannelFromPackage(t *testing.T) {
 		CatalogSourceList:   CatalogSourceList,
 	}
 
+	mockClient := &MockClient{}
+
 	operator := &ODLMOperator{
 		Reader: fakeReader,
+		Client: mockClient,
 	}
 
+	mockClient.mock.On("Create", ctx, mock.Anything, mock.Anything).Return(nil).Run(func(args mock.Arguments) {
+		sar := args.Get(1).(*authorizationv1.SubjectAccessReview)
+		sar.Status.Allowed = true
+	})
+
 	registryNs := "registry-namespace"
 	odlmCatalog := "odlm-catalog"
 	odlmCatalogNs := "odlm-namespace"
@@ -400,6 +411,59 @@ func TestGetCatalogSourceAndChannelFromPackage(t *testing.T) {
 
 }
 
+type MockClient struct {
+	mock mock.Mock
+	client.Client
+}
+
+func (m *MockClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error {
+	args := m.mock.Called(ctx, obj, opts)
+	return args.Error(0)
+}
+
+func TestCheckResAuth(t *testing.T) {
+	ctx := context.TODO()
+
+	mockClient := &MockClient{}
+	operator := &ODLMOperator{
+		Client: mockClient,
+	}
+
+	namespace := "test-namespace"
+	group := "test-group"
+	resource := "test-resource"
+	verb := "get"
+
+	// Test when SubjectAccessReview 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.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
+	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.Status.Allowed = false
+	})
+
+	if operator.CheckResAuth(ctx, namespace, group, resource, verb) {
+		t.Errorf("Expected CheckResAuth to return false, but got true")
+	}
+
+	// Test when Create returns an error
+	mockClient.mock.ExpectedCalls = nil
+	mockClient.mock.On("Create", ctx, mock.Anything, mock.Anything).Return(fmt.Errorf("create error"))
+
+	if operator.CheckResAuth(ctx, namespace, group, resource, verb) {
+		t.Errorf("Expected CheckResAuth to return false, but got true")
+	}
+}
+
 func assertCatalogSourceAndChannel(t *testing.T, catalogSourceName, expectedCatalogSourceName, catalogSourceNs, expectedCatalogSourceNs, availableChannel, expectedAvailableChannel string) {
 	t.Helper()