Change AuthorizationV1Client to interface for mocking

Signed-off-by: Brett Tofel <[email protected]>

Author: bentito

cmd/operator-controller/main.go

@@ -37,8 +37,10 @@ import (
 	k8slabels "k8s.io/apimachinery/pkg/labels"
 	k8stypes "k8s.io/apimachinery/pkg/types"
 	apimachineryrand "k8s.io/apimachinery/pkg/util/rand"
+	authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
+	"k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
 	"k8s.io/klog/v2/textlogger"
 	"k8s.io/utils/ptr"
@@ -408,6 +410,10 @@ func run() error {
 	}
 
 	acm := applier.NewAuthClientMapper(clientRestConfigMapper, mgr.GetConfig())
+	acm.NewForConfig = func(cfg *rest.Config) (authorizationv1client.AuthorizationV1Interface, error) {
+		// *AuthorizationV1Client implements AuthorizationV1Interface
+		return authorizationv1client.NewForConfig(cfg)
+	}
 
 	helmApplier := &applier.Helm{
 		ActionClientGetter: acg,

internal/operator-controller/applier/helm.go

@@ -58,23 +58,21 @@ type Preflight interface {
 
 type RestConfigMapper func(context.Context, client.Object, *rest.Config) (*rest.Config, error)
 
+type NewForConfigFunc func(*rest.Config) (authorizationv1client.AuthorizationV1Interface, error)
+
 type AuthClientMapper struct {
-	rcm     RestConfigMapper
-	baseCfg *rest.Config
+	rcm          RestConfigMapper
+	baseCfg      *rest.Config
+	NewForConfig NewForConfigFunc
 }
 
-func (acm *AuthClientMapper) GetAuthenticationClient(ctx context.Context, ext *ocv1.ClusterExtension) (*authorizationv1client.AuthorizationV1Client, error) {
+func (acm *AuthClientMapper) GetAuthenticationClient(ctx context.Context, ext *ocv1.ClusterExtension) (authorizationv1client.AuthorizationV1Interface, error) {
 	authcfg, err := acm.rcm(ctx, ext, acm.baseCfg)
 	if err != nil {
 		return nil, err
 	}
 
-	authclient, err := authorizationv1client.NewForConfig(authcfg)
-	if err != nil {
-		return nil, err
-	}
-
-	return authclient, nil
+	return acm.NewForConfig(authcfg)
 }
 
 type Helm struct {
@@ -198,7 +196,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
 }
 
 // Check if RBAC allows the installer service account necessary permissions on the objects in the contentFS
-func (h *Helm) checkContentPermissions(ctx context.Context, contentFS fs.FS, authcl *authorizationv1client.AuthorizationV1Client, ext *ocv1.ClusterExtension) error {
+func (h *Helm) checkContentPermissions(ctx context.Context, contentFS fs.FS, authcl authorizationv1client.AuthorizationV1Interface, ext *ocv1.ClusterExtension) error {
 	reg, err := convert.ParseFS(ctx, contentFS)
 	if err != nil {
 		return err

internal/operator-controller/applier/helm_test.go

@@ -13,6 +13,10 @@ import (
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
+	authorizationv1 "k8s.io/api/authorization/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
+	"k8s.io/client-go/rest"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -94,6 +98,83 @@ func (mag *mockActionGetter) Reconcile(rel *release.Release) error {
 	return mag.reconcileErr
 }
 
+// Helper returns an AuthorizationV1Interface where SSRR always passes.
+func newPassingSSRRAuthClient() authorizationv1client.AuthorizationV1Interface {
+	return &mockAuthorizationV1Client{
+		ssrrInterface: &mockSelfSubjectRulesReviewInterface{
+			retSSRR: &authorizationv1.SelfSubjectRulesReview{
+				Status: authorizationv1.SubjectRulesReviewStatus{
+					ResourceRules: []authorizationv1.ResourceRule{{
+						Verbs:     []string{"*"},
+						APIGroups: []string{"*"},
+						Resources: []string{"*"},
+					}},
+				},
+			},
+		},
+	}
+}
+
+// Helper builds an AuthClientMapper with the passing SSRR
+func newPassingAuthClientMapper() applier.AuthClientMapper {
+	fakeRestConfig := &rest.Config{Host: "fake-server"}
+	mockRCM := func(ctx context.Context, obj client.Object, cfg *rest.Config) (*rest.Config, error) {
+		return cfg, nil
+	}
+	acm := applier.NewAuthClientMapper(mockRCM, fakeRestConfig)
+	acm.NewForConfig = func(*rest.Config) (authorizationv1client.AuthorizationV1Interface, error) {
+		return newPassingSSRRAuthClient(), nil
+	}
+	return acm
+}
+
+// Helper builds a Helm applier with passing SSRR
+func buildHelmApplier(mockAcg *mockActionGetter, preflights []applier.Preflight) applier.Helm {
+	return applier.Helm{
+		ActionClientGetter: mockAcg,
+		AuthClientMapper:   newPassingAuthClientMapper(),
+		Preflights:         preflights,
+	}
+}
+
+type mockAuthorizationV1Client struct {
+	ssrrInterface authorizationv1client.SelfSubjectRulesReviewInterface
+}
+
+func (m *mockAuthorizationV1Client) SelfSubjectRulesReviews() authorizationv1client.SelfSubjectRulesReviewInterface {
+	return m.ssrrInterface
+}
+func (m *mockAuthorizationV1Client) RESTClient() rest.Interface {
+	return nil
+}
+
+// Mock for SelfSubjectRulesReviewInterface
+type mockSelfSubjectRulesReviewInterface struct {
+	retSSRR *authorizationv1.SelfSubjectRulesReview
+	retErr  error
+}
+
+func (m *mockSelfSubjectRulesReviewInterface) Create(
+	ctx context.Context,
+	ssrr *authorizationv1.SelfSubjectRulesReview,
+	opts metav1.CreateOptions,
+) (*authorizationv1.SelfSubjectRulesReview, error) {
+	// Return either a success or an error, depending on what you want in the test.
+	return m.retSSRR, m.retErr
+}
+
+func (m *mockAuthorizationV1Client) LocalSubjectAccessReviews(namespace string) authorizationv1client.LocalSubjectAccessReviewInterface {
+	return nil
+}
+
+func (m *mockAuthorizationV1Client) SelfSubjectAccessReviews() authorizationv1client.SelfSubjectAccessReviewInterface {
+	return nil
+}
+
+func (m *mockAuthorizationV1Client) SubjectAccessReviews() authorizationv1client.SubjectAccessReviewInterface {
+	return nil
+}
+
 var (
 	// required for unmockable call to convert.RegistryV1ToHelmChart
 	validFS = fstest.MapFS{
@@ -229,16 +310,23 @@ func TestApply_Installation(t *testing.T) {
 }
 
 func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
+	// Set feature gate ONCE at parent level
 	featuregatetesting.SetFeatureGateDuringTest(t, features.OperatorControllerFeatureGate, features.PreflightPermissions, true)
 
 	t.Run("fails during dry-run installation", func(t *testing.T) {
 		mockAcg := &mockActionGetter{
 			getClientErr:     driver.ErrReleaseNotFound,
 			dryRunInstallErr: errors.New("failed attempting to dry-run install chart"),
 		}
-		helmApplier := applier.Helm{ActionClientGetter: mockAcg}
-
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		helmApplier := buildHelmApplier(mockAcg, nil)
+
+		objs, state, err := helmApplier.Apply(
+			context.TODO(),
+			validFS,
+			testCE,
+			testObjectLabels,
+			testStorageLabels,
+		)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "attempting to dry-run install chart")
 		require.Nil(t, objs)
@@ -251,7 +339,8 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 			installErr:   errors.New("failed installing chart"),
 		}
 		mockPf := &mockPreflight{installErr: errors.New("failed during install pre-flight check")}
-		helmApplier := applier.Helm{ActionClientGetter: mockAcg, Preflights: []applier.Preflight{mockPf}}
+
+		helmApplier := buildHelmApplier(mockAcg, []applier.Preflight{mockPf})
 
 		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
 		require.Error(t, err)
@@ -265,9 +354,15 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 			getClientErr: driver.ErrReleaseNotFound,
 			installErr:   errors.New("failed installing chart"),
 		}
-		helmApplier := applier.Helm{ActionClientGetter: mockAcg}
-
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		helmApplier := buildHelmApplier(mockAcg, nil)
+
+		objs, state, err := helmApplier.Apply(
+			context.TODO(),
+			validFS,
+			testCE,
+			testObjectLabels,
+			testStorageLabels,
+		)
 		require.Error(t, err)
 		require.ErrorContains(t, err, "installing chart")
 		require.Equal(t, applier.StateNeedsInstall, state)
@@ -282,9 +377,15 @@ func TestApply_InstallationWithPreflightPermissionsEnabled(t *testing.T) {
 				Manifest: validManifest,
 			},
 		}
-		helmApplier := applier.Helm{ActionClientGetter: mockAcg}
-
-		objs, state, err := helmApplier.Apply(context.TODO(), validFS, testCE, testObjectLabels, testStorageLabels)
+		helmApplier := buildHelmApplier(mockAcg, nil)
+
+		objs, state, err := helmApplier.Apply(
+			context.TODO(),
+			validFS,
+			testCE,
+			testObjectLabels,
+			testStorageLabels,
+		)
 		require.NoError(t, err)
 		require.Equal(t, applier.StateNeedsInstall, state)
 		require.NotNil(t, objs)

internal/operator-controller/authorization/authorization.go

@@ -21,7 +21,7 @@ const (
 	SelfSubjectAccessReview string = "SelfSubjectAccessReview"
 )
 
-func CheckObjectPermissions(ctx context.Context, authcl *authorizationv1client.AuthorizationV1Client, objects []client.Object, ext *ocv1.ClusterExtension) error {
+func CheckObjectPermissions(ctx context.Context, authcl authorizationv1client.AuthorizationV1Interface, objects []client.Object, ext *ocv1.ClusterExtension) error {
 	ssrr := &authorizationv1.SelfSubjectRulesReview{
 		Spec: authorizationv1.SelfSubjectRulesReviewSpec{
 			Namespace: ext.Spec.Namespace,