✨ Check known required permissions for install before installing with the helm applier

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"
@@ -59,6 +61,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/operator-controller/action"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/cache"
 	catalogclient "github.com/operator-framework/operator-controller/internal/operator-controller/catalogmetadata/client"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/contentmanager"
@@ -407,9 +410,16 @@ func run() error {
 		crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()),
 	}
 
+	acm := authorization.NewAuthorizationClientMapper(clientRestConfigMapper, mgr.GetConfig())
+	acm.NewForConfig = func(cfg *rest.Config) (authorizationv1client.AuthorizationV1Interface, error) {
+		// *AuthorizationV1Client implements AuthorizationV1Interface
+		return authorizationv1client.NewForConfig(cfg)
+	}
+
 	helmApplier := &applier.Helm{
-		ActionClientGetter: acg,
-		Preflights:         preflights,
+		ActionClientGetter:        acg,
+		Preflights:                preflights,
+		AuthorizationClientMapper: acm,
 	}
 
 	cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper())

internal/operator-controller/applier/helm.go

@@ -24,6 +24,7 @@ import (
 	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/convert"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/preflights/crdupgradesafety"
@@ -54,8 +55,9 @@ type Preflight interface {
 }
 
 type Helm struct {
-	ActionClientGetter helmclient.ActionClientGetter
-	Preflights         []Preflight
+	ActionClientGetter        helmclient.ActionClientGetter
+	Preflights                []Preflight
+	AuthorizationClientMapper authorization.AuthorizationClientMapper
 }
 
 // shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
@@ -79,15 +81,28 @@ func shouldSkipPreflight(ctx context.Context, preflight Preflight, ext *ocv1.Clu
 }
 
 func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExtension, objectLabels map[string]string, storageLabels map[string]string) ([]client.Object, string, error) {
+	if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) {
+		rawAuthClient, err := h.AuthorizationClientMapper.GetAuthorizationClient(ctx, ext)
+		if err != nil {
+			return nil, "", fmt.Errorf("failed to get authorization client: %w", err)
+		}
+
+		authClient := authorization.NewClient(rawAuthClient)
+		if err := h.checkContentPermissions(ctx, contentFS, authClient, ext); err != nil {
+			return nil, "", fmt.Errorf("failed checking content permissions: %w", err)
+		}
+	}
+
 	chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Namespace, []string{corev1.NamespaceAll})
+
 	if err != nil {
 		return nil, "", err
 	}
 	values := chartutil.Values{}
 
 	ac, err := h.ActionClientGetter.ActionClientFor(ctx, ext)
 	if err != nil {
-		return nil, "", err
+		return nil, "", fmt.Errorf("failed to get action client: %w", err)
 	}
 
 	post := &postrenderer{
@@ -105,14 +120,12 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
 		}
 		switch state {
 		case StateNeedsInstall:
-			err := preflight.Install(ctx, desiredRel)
-			if err != nil {
-				return nil, state, err
+			if err := preflight.Install(ctx, desiredRel); err != nil {
+				return nil, state, fmt.Errorf("preflight install check failed: %w", err)
 			}
 		case StateNeedsUpgrade:
-			err := preflight.Upgrade(ctx, desiredRel)
-			if err != nil {
-				return nil, state, err
+			if err := preflight.Upgrade(ctx, desiredRel); err != nil {
+				return nil, state, fmt.Errorf("preflight upgrade check failed: %w", err)
 			}
 		}
 	}
@@ -125,7 +138,7 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
 			return nil
 		}, helmclient.AppendInstallPostRenderer(post))
 		if err != nil {
-			return nil, state, err
+			return nil, state, fmt.Errorf("failed to install release: %w", err)
 		}
 	case StateNeedsUpgrade:
 		rel, err = ac.Upgrade(ext.GetName(), ext.Spec.Namespace, chrt, values, func(upgrade *action.Upgrade) error {
@@ -134,43 +147,55 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
 			return nil
 		}, helmclient.AppendUpgradePostRenderer(post))
 		if err != nil {
-			return nil, state, err
+			return nil, state, fmt.Errorf("failed to upgrade release: %w", err)
 		}
 	case StateUnchanged:
 		if err := ac.Reconcile(rel); err != nil {
-			return nil, state, err
+			return nil, state, fmt.Errorf("failed to reconcile release: %w", err)
 		}
 	default:
 		return nil, state, fmt.Errorf("unexpected release state %q", state)
 	}
 
 	relObjects, err := util.ManifestObjects(strings.NewReader(rel.Manifest), fmt.Sprintf("%s-release-manifest", rel.Name))
 	if err != nil {
-		return nil, state, err
+		return nil, state, fmt.Errorf("failed to convert manifest to objects: %w", err)
 	}
 
 	return relObjects, state, nil
 }
 
+// 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, authClient authorization.AuthorizationClient, ext *ocv1.ClusterExtension) error {
+	reg, err := convert.ParseFS(ctx, contentFS)
+	if err != nil {
+		return fmt.Errorf("failed to parse content FS: %w", err)
+	}
+
+	plain, err := convert.Convert(reg, ext.Spec.Namespace, []string{corev1.NamespaceAll})
+	if err != nil {
+		return fmt.Errorf("failed to convert registry: %w", err)
+	}
+
+	return authClient.CheckContentPermissions(ctx, plain.Objects, ext)
+}
+
 func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1.ClusterExtension, chrt *chart.Chart, values chartutil.Values, post postrender.PostRenderer) (*release.Release, *release.Release, string, error) {
 	currentRelease, err := cl.Get(ext.GetName())
+
 	if errors.Is(err, driver.ErrReleaseNotFound) {
 		desiredRelease, err := cl.Install(ext.GetName(), ext.Spec.Namespace, chrt, values, func(i *action.Install) error {
 			i.DryRun = true
 			i.DryRunOption = "server"
 			return nil
 		}, helmclient.AppendInstallPostRenderer(post))
 		if err != nil {
-			if features.OperatorControllerFeatureGate.Enabled(features.PreflightPermissions) {
-				_ = struct{}{} // minimal no-op to satisfy linter
-				// probably need to break out this error as it's the one for helm dry-run as opposed to any returned later
-			}
-			return nil, nil, StateError, err
+			return nil, nil, StateError, fmt.Errorf("failed dry-run install: %w", err)
 		}
 		return nil, desiredRelease, StateNeedsInstall, nil
 	}
 	if err != nil {
-		return nil, nil, StateError, err
+		return nil, nil, StateError, fmt.Errorf("failed to get current release: %w", err)
 	}
 
 	desiredRelease, err := cl.Upgrade(ext.GetName(), ext.Spec.Namespace, chrt, values, func(upgrade *action.Upgrade) error {
@@ -180,7 +205,7 @@ func (h *Helm) getReleaseState(cl helmclient.ActionInterface, ext *ocv1.ClusterE
 		return nil
 	}, helmclient.AppendUpgradePostRenderer(post))
 	if err != nil {
-		return currentRelease, nil, StateError, err
+		return currentRelease, nil, StateError, fmt.Errorf("failed dry-run upgrade: %w", err)
 	}
 	relState := StateUnchanged
 	if desiredRelease.Manifest != currentRelease.Manifest ||

internal/operator-controller/applier/helm_test.go

@@ -13,13 +13,18 @@
 	"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"
 
 	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
 
 	v1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
+	"github.com/operator-framework/operator-controller/internal/operator-controller/authorization"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/features"
 )
 
@@ -94,6 +99,83 @@
 	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 AuthorizationClientMapper with the passing SSRR
+func newPassingAuthorizationClientMapper() applier.AuthorizationClientMapper {
+	fakeRestConfig := &rest.Config{Host: "fake-server"}
+	mockRCM := func(ctx context.Context, obj client.Object, cfg *rest.Config) (*rest.Config, error) {
+		return cfg, nil
+	}
+	acm := authorization.NewAuthorizationClientMapper(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,
+		AuthorizationClientMapper: newPassingAuthorizationClientMapper(),
+		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 +311,23 @@
 }
 
 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 +340,8 @@
 			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 +355,15 @@
 			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 +378,15 @@
 				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)