Add client-only helm dry-run to helm applier

Adds a check that makes sure the installer service account has the
necessary permissions to manage all the resources in the
ClusterExtension payload and returns errors detailing all
the missing permissions. This prevents a hung server-connected
dry-run getting caught on individual missing get permissions as well
returns many other missing required permissions needed for the actual
installation or upgrade.

Signed-off-by: Tayler Geiger <[email protected]>

Author: trgeiger

cmd/operator-controller/main.go

@@ -407,9 +407,12 @@ func run() error {
 		crdupgradesafety.NewPreflight(aeClient.CustomResourceDefinitions()),
 	}
 
+	acm := applier.NewAuthClientMapper(clientRestConfigMapper, mgr.GetConfig())
+
 	helmApplier := &applier.Helm{
 		ActionClientGetter: acg,
 		Preflights:         preflights,
+		AuthClientMapper:   acm,
 	}
 
 	cm := contentmanager.NewManager(clientRestConfigMapper, mgr.GetConfig(), mgr.GetRESTMapper())

internal/operator-controller/applier/helm.go

@@ -18,12 +18,15 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	apimachyaml "k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"
+	authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
 
 	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"
@@ -53,9 +56,38 @@ type Preflight interface {
 	Upgrade(context.Context, *release.Release) error
 }
 
+type RestConfigMapper func(context.Context, client.Object, *rest.Config) (*rest.Config, error)
+
+type AuthClientMapper struct {
+	rcm     RestConfigMapper
+	baseCfg *rest.Config
+}
+
+func (acm *AuthClientMapper) GetAuthenticationClient(ctx context.Context, ext *ocv1.ClusterExtension) (*authorizationv1client.AuthorizationV1Client, 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
+}
+
 type Helm struct {
 	ActionClientGetter helmclient.ActionClientGetter
 	Preflights         []Preflight
+	AuthClientMapper   AuthClientMapper
+}
+
+func NewAuthClientMapper(rcm RestConfigMapper, baseCfg *rest.Config) AuthClientMapper {
+	return AuthClientMapper{
+		rcm:     rcm,
+		baseCfg: baseCfg,
+	}
 }
 
 // shouldSkipPreflight is a helper to determine if the preflight check is CRDUpgradeSafety AND
@@ -79,7 +111,21 @@ 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) {
+		authclient, err := h.AuthClientMapper.GetAuthenticationClient(ctx, ext)
+		if err != nil {
+			return nil, "", err
+		}
+
+		err = h.checkContentPermissions(ctx, contentFS, authclient, ext)
+		if err != nil {
+			return nil, "", err
+		}
+	}
+
 	chrt, err := convert.RegistryV1ToHelmChart(ctx, contentFS, ext.Spec.Namespace, []string{corev1.NamespaceAll})
+
 	if err != nil {
 		return nil, "", err
 	}
@@ -152,8 +198,26 @@ func (h *Helm) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.ClusterExte
 	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, authcl *authorizationv1client.AuthorizationV1Client, ext *ocv1.ClusterExtension) error {
+	reg, err := convert.ParseFS(ctx, contentFS)
+	if err != nil {
+		return err
+	}
+
+	plain, err := convert.Convert(reg, ext.Spec.Namespace, []string{corev1.NamespaceAll})
+	if err != nil {
+		return err
+	}
+
+	err = authorization.CheckObjectPermissions(ctx, authcl, plain.Objects, ext)
+
+	return err
+}
+
 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

internal/operator-controller/authorization/authorization.go

@@ -0,0 +1,112 @@
+package authorization
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"slices"
+	"strings"
+
+	ocv1 "github.com/operator-framework/operator-controller/api/v1"
+	authv1 "k8s.io/api/authorization/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	authorizationv1client "k8s.io/client-go/kubernetes/typed/authorization/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const (
+	SelfSubjectRulesReview  string = "SelfSubjectRulesReview"
+	SelfSubjectAccessReview string = "SelfSubjectAccessReview"
+)
+
+func CheckObjectPermissions(ctx context.Context, authcl *authorizationv1client.AuthorizationV1Client, objects []client.Object, ext *ocv1.ClusterExtension) error {
+	ssrr := &authv1.SelfSubjectRulesReview{
+		Spec: authv1.SelfSubjectRulesReviewSpec{
+			Namespace: ext.Spec.Namespace,
+		},
+	}
+
+	ssrr, err := authcl.SelfSubjectRulesReviews().Create(ctx, ssrr, v1.CreateOptions{})
+	if err != nil {
+		return err
+	}
+
+	rules := []rbacv1.PolicyRule{}
+	for _, rule := range ssrr.Status.ResourceRules {
+		rules = append(rules, rbacv1.PolicyRule{
+			Verbs:         rule.Verbs,
+			APIGroups:     rule.APIGroups,
+			Resources:     rule.Resources,
+			ResourceNames: rule.ResourceNames,
+		})
+	}
+
+	for _, rule := range ssrr.Status.NonResourceRules {
+		rules = append(rules, rbacv1.PolicyRule{
+			Verbs:           rule.Verbs,
+			NonResourceURLs: rule.NonResourceURLs,
+		})
+	}
+
+	resAttrs := []authv1.ResourceAttributes{}
+	namespacedErrs := []error{}
+	clusterScopedErrs := []error{}
+	requiredVerbs := []string{"get", "create", "update", "list", "watch", "delete", "patch"}
+
+	for _, o := range objects {
+		for _, verb := range requiredVerbs {
+			resAttrs = append(resAttrs, authv1.ResourceAttributes{
+				Namespace: o.GetNamespace(),
+				Verb:      verb,
+				Resource:  sanitizeResourceName(o.GetObjectKind().GroupVersionKind().Kind),
+				Group:     o.GetObjectKind().GroupVersionKind().Group,
+				Name:      o.GetName(),
+			})
+		}
+	}
+
+	for _, resAttr := range resAttrs {
+		if !canI(resAttr, rules) {
+			if resAttr.Namespace != "" {
+				namespacedErrs = append(namespacedErrs, fmt.Errorf("cannot %s %s %s in namespace %s",
+					resAttr.Verb,
+					strings.TrimSuffix(resAttr.Resource, "s"),
+					resAttr.Name,
+					resAttr.Namespace))
+				continue
+			}
+			clusterScopedErrs = append(clusterScopedErrs, fmt.Errorf("cannot %s %s %s",
+				resAttr.Verb,
+				strings.TrimSuffix(resAttr.Resource, "s"),
+				resAttr.Name))
+		}
+	}
+	errs := append(namespacedErrs, clusterScopedErrs...)
+	if len(errs) > 0 {
+		errs = append([]error{fmt.Errorf("installer service account %s is missing required permissions", ext.Spec.ServiceAccount.Name)}, errs...)
+	}
+
+	return errors.Join(errs...)
+
+}
+
+// Checks if the rules allow the verb on the GroupVersionKind in resAttr
+func canI(resAttr authv1.ResourceAttributes, rules []rbacv1.PolicyRule) bool {
+	var canI bool
+	for _, rule := range rules {
+		if (slices.Contains(rule.APIGroups, resAttr.Group) || slices.Contains(rule.APIGroups, "*")) &&
+			(slices.Contains(rule.Resources, resAttr.Resource) || slices.Contains(rule.Resources, "*")) &&
+			(slices.Contains(rule.Verbs, resAttr.Verb) || slices.Contains(rule.Verbs, "*")) &&
+			(slices.Contains(rule.ResourceNames, resAttr.Name) || len(rule.ResourceNames) == 0) {
+			canI = true
+			break
+		}
+	}
+	return canI
+}
+
+// SelfSubjectRulesReview formats the resource names as lowercase and plural
+func sanitizeResourceName(resourceName string) string {
+	return strings.ToLower(resourceName) + "s"
+}