chore: refactor CRD and RBAC checks

This change is a pre-requisite for prometheus-operator#5898.

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

cmd/operator/main.go

@@ -39,6 +39,8 @@ import (
 	"github.com/prometheus/common/version"
 	"golang.org/x/sync/errgroup"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes"
 	klog "k8s.io/klog/v2"
 
 	logging "github.com/prometheus-operator/prometheus-operator/internal/log"
@@ -93,19 +95,40 @@ func (n namespaces) asSlice() []string {
 	return ns
 }
 
-// Helper function for checking CRD prerequisites
-func checkPrerequisites(ctx context.Context, logger log.Logger, cc *k8sutil.CRDChecker, allowedNamespaces []string, verbs map[string][]string, groupVersion, resourceName string) (bool, error) {
-	err := cc.CheckPrerequisites(ctx, allowedNamespaces, verbs, groupVersion, resourceName)
-	switch {
-	case errors.Is(err, k8sutil.ErrPrerequiresitesFailed):
-		level.Warn(logger).Log("msg", fmt.Sprintf("%s CRD not supported", resourceName), "reason", err)
+// checkPrerequisites verifies that the CRD is installed in the cluster and
+// that the operator has enough permissions to manage the resource.
+func checkPrerequisites(
+	ctx context.Context,
+	logger log.Logger,
+	kclient kubernetes.Interface,
+	allowedNamespaces []string,
+	groupVersion schema.GroupVersion,
+	resource string,
+	attributes ...k8sutil.ResourceAttribute,
+) (bool, error) {
+	installed, err := k8sutil.IsAPIGroupVersionResourceSupported(kclient.Discovery(), groupVersion, resource)
+	if err != nil {
+		return false, fmt.Errorf("failed to check presence of resource %q (group %q): %w", resource, groupVersion, err)
+	}
+
+	if !installed {
+		level.Warn(logger).Log("msg", fmt.Sprintf("resource %q (group: %q) not installed in the cluster", resource, groupVersion))
 		return false, nil
-	case err != nil:
-		level.Error(logger).Log("msg", fmt.Sprintf("failed to check prerequisites for the %s CRD ", resourceName), "err", err)
-		return false, err
-	default:
-		return true, nil
 	}
+
+	allowed, errs, err := k8sutil.IsAllowed(ctx, kclient.AuthorizationV1().SelfSubjectAccessReviews(), allowedNamespaces, attributes...)
+	if err != nil {
+		return false, fmt.Errorf("failed to check permissions on resource %q (group %q): %w", resource, groupVersion, err)
+	}
+
+	if !allowed {
+		for _, reason := range errs {
+			level.Warn(logger).Log("msg", fmt.Sprintf("missing permission on resource %q (group: %q)", resource, groupVersion), "reason", reason)
+		}
+		return false, nil
+	}
+
+	return true, nil
 }
 
 func serve(srv *http.Server, listener net.Listener, logger log.Logger) func() error {
@@ -265,32 +288,32 @@ func run() int {
 
 	k8sutil.MustRegisterClientGoMetrics(r)
 
-	allowedNamespaces := namespaces(cfg.Namespaces.AllowList).asSlice()
-
 	restConfig, err := k8sutil.NewClusterConfig(cfg.Host, cfg.TLSInsecure, &cfg.TLSConfig, cfg.ImpersonateUser)
 	if err != nil {
 		level.Error(logger).Log("msg", "failed to create Kubernetes client configuration", "err", err)
 		cancel()
 		return 1
 	}
 
-	cc, err := k8sutil.NewCRDChecker(restConfig)
+	kclient, err := kubernetes.NewForConfig(restConfig)
 	if err != nil {
-		level.Error(logger).Log("msg", "failed to create new CRDChecker object ", "err", err)
+		level.Error(logger).Log("msg", "failed to create Kubernetes client", "err", err)
 		cancel()
 		return 1
 	}
 
 	scrapeConfigSupported, err := checkPrerequisites(
 		ctx,
 		logger,
-		cc,
-		allowedNamespaces,
-		map[string][]string{
-			monitoringv1alpha1.ScrapeConfigName: {"get", "list", "watch"},
-		},
-		monitoringv1alpha1.SchemeGroupVersion.String(),
+		kclient,
+		namespaces(cfg.Namespaces.AllowList).asSlice(),
+		monitoringv1alpha1.SchemeGroupVersion,
 		monitoringv1alpha1.ScrapeConfigName,
+		k8sutil.ResourceAttribute{
+			Group:    monitoringv1alpha1.SchemeGroupVersion.String(),
+			Resource: monitoringv1alpha1.ScrapeConfigName,
+			Verbs:    []string{"get", "list", "watch"},
+		},
 	)
 
 	if err != nil {
@@ -308,14 +331,20 @@ func run() int {
 	prometheusAgentSupported, err := checkPrerequisites(
 		ctx,
 		logger,
-		cc,
-		allowedNamespaces,
-		map[string][]string{
-			monitoringv1alpha1.PrometheusAgentName:                           {"get", "list", "watch"},
-			fmt.Sprintf("%s/status", monitoringv1alpha1.PrometheusAgentName): {"update"},
-		},
-		monitoringv1alpha1.SchemeGroupVersion.String(),
+		kclient,
+		namespaces(cfg.Namespaces.PrometheusAllowList).asSlice(),
+		monitoringv1alpha1.SchemeGroupVersion,
 		monitoringv1alpha1.PrometheusAgentName,
+		k8sutil.ResourceAttribute{
+			Group:    monitoringv1alpha1.SchemeGroupVersion.String(),
+			Resource: monitoringv1alpha1.PrometheusAgentName,
+			Verbs:    []string{"get", "list", "watch"},
+		},
+		k8sutil.ResourceAttribute{
+			Group:    monitoringv1alpha1.SchemeGroupVersion.String(),
+			Resource: fmt.Sprintf("%s/status", monitoringv1alpha1.PrometheusAgentName),
+			Verbs:    []string{"update"},
+		},
 	)
 	if err != nil {
 		cancel()

pkg/k8sutil/k8sutil.go

@@ -33,10 +33,11 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/client-go/discovery"
-	"k8s.io/client-go/kubernetes"
 	clientappsv1 "k8s.io/client-go/kubernetes/typed/apps/v1"
+	clientauthv1 "k8s.io/client-go/kubernetes/typed/authorization/v1"
 	clientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
@@ -54,27 +55,12 @@ var invalidDNS1123Characters = regexp.MustCompile("[^-a-z0-9]+")
 
 var scheme = runtime.NewScheme()
 
-var ErrPrerequiresitesFailed = errors.New("unmet prerequisites")
-
 func init() {
 	_ = monitoringv1.SchemeBuilder.AddToScheme(scheme)
 	_ = monitoringv1alpha1.SchemeBuilder.AddToScheme(scheme)
 	_ = monitoringv1beta1.SchemeBuilder.AddToScheme(scheme)
 }
 
-type CRDChecker struct {
-	kclient kubernetes.Interface
-}
-
-func NewCRDChecker(cfg *rest.Config) (*CRDChecker, error) {
-	kclient, err := kubernetes.NewForConfig(cfg)
-	if err != nil {
-		return nil, errors.Wrap(err, "instantiating kubernetes client failed")
-	}
-
-	return &CRDChecker{kclient: kclient}, nil
-}
-
 // PodRunningAndReady returns whether a pod is running and each container has
 // passed it's ready state.
 func PodRunningAndReady(pod v1.Pod) (bool, error) {
@@ -133,79 +119,101 @@ func NewClusterConfig(host string, tlsInsecure bool, tlsConfig *rest.TLSClientCo
 	return cfg, nil
 }
 
-// CheckPrerequisites checks if given resource's CRD is installed in the cluster and the operator
-// serviceaccount has the necessary RBAC verbs in the namespace list to reconcile it.
-func (cc CRDChecker) CheckPrerequisites(ctx context.Context, nsAllowList []string, verbs map[string][]string, sgv, resource string) error {
-	if err := cc.validateCRDInstallation(sgv, resource); err != nil {
-		return err
-	}
-
-	missingPermissions, err := cc.getMissingPermissions(ctx, nsAllowList, verbs)
-	if err != nil {
-		return err
-	}
-	if len(missingPermissions) > 0 {
-		return fmt.Errorf("%w: some permissions are missing: %v", ErrPrerequiresitesFailed, missingPermissions)
-	}
-
-	return nil
+// ResourceAttribute represents authorization attributes to check on a given resource.
+type ResourceAttribute struct {
+	Resource string
+	Name     string
+	Group    string
+	Verbs    []string
 }
 
-func (cc CRDChecker) validateCRDInstallation(sgv, resource string) error {
-	crdInstalled, err := IsAPIGroupVersionResourceSupported(cc.kclient.Discovery(), sgv, resource)
-	if err != nil {
-		return fmt.Errorf("failed to check if the API supports %s resource (apiGroup: %q): %w", resource, sgv, err)
+// IsAllowed returns whether the user (e.g. the operator's service account) has
+// been granted the required RBAC attributes.
+// It returns true when the conditions are met for the namespaces (an empty
+// namespace value means "all").
+// The second return value returns the list of permissions that are missing if
+// the requirements aren't met.
+func IsAllowed(
+	ctx context.Context,
+	ssarClient clientauthv1.SelfSubjectAccessReviewInterface,
+	namespaces []string,
+	attributes ...ResourceAttribute,
+) (bool, []error, error) {
+	if len(attributes) == 0 {
+		return false, nil, fmt.Errorf("resource attributes must not be empty")
 	}
-	if !crdInstalled {
-		return fmt.Errorf("%w: %s resource (apiGroup: %q) not installed", ErrPrerequiresitesFailed, resource, sgv)
+
+	if len(namespaces) == 0 {
+		namespaces = []string{v1.NamespaceAll}
 	}
-	return nil
-}
 
-// getMissingPermissions returns the RBAC permissions that the controller would need to be
-// granted to fulfill its mission. An empty map means that everything is ok.
-func (cc CRDChecker) getMissingPermissions(ctx context.Context, nsAllowList []string, verbs map[string][]string) (map[string][]string, error) {
-	var ssar *authv1.SelfSubjectAccessReview
-	var ssarResponse *authv1.SelfSubjectAccessReview
-	var err error
-	missingPermissions := map[string][]string{}
+	var missingPermissions []error
+	for _, ns := range namespaces {
+		for _, ra := range attributes {
+			for _, verb := range ra.Verbs {
+				resourceAttributes := authv1.ResourceAttributes{
+					Verb:     verb,
+					Group:    ra.Group,
+					Resource: ra.Resource,
+					// An empty name value means "all" resources.
+					Name: ra.Name,
+					// An empty namespace value means "all" for namespace-scoped resources.
+					Namespace: ns,
+				}
+
+				// Special case for SAR on namespaces resources: Namespace and
+				// Name need to be equal.
+				if resourceAttributes.Group == "" && resourceAttributes.Resource == "namespaces" && resourceAttributes.Name != "" && resourceAttributes.Namespace == "" {
+					resourceAttributes.Namespace = resourceAttributes.Name
+				}
 
-	for _, ns := range nsAllowList {
-		for resource, verbs := range verbs {
-			for _, verb := range verbs {
-				ssar = &authv1.SelfSubjectAccessReview{
+				ssar := &authv1.SelfSubjectAccessReview{
 					Spec: authv1.SelfSubjectAccessReviewSpec{
-						ResourceAttributes: &authv1.ResourceAttributes{
-							Verb:     verb,
-							Group:    monitoringv1alpha1.SchemeGroupVersion.Group,
-							Resource: resource,
-							// If ns is empty string, it will check cluster-wide
-							Namespace: ns,
-						},
+						ResourceAttributes: &resourceAttributes,
 					},
 				}
-				ssarResponse, err = cc.kclient.AuthorizationV1().SelfSubjectAccessReviews().Create(ctx, ssar, metav1.CreateOptions{})
+
+				// FIXME(simonpasquier): retry in case of server-side errors.
+				ssarResponse, err := ssarClient.Create(ctx, ssar, metav1.CreateOptions{})
 				if err != nil {
-					return nil, err
+					return false, nil, err
 				}
+
 				if !ssarResponse.Status.Allowed {
-					missingPermissions[resource] = append(missingPermissions[resource], verb)
+					var (
+						reason   error
+						resource = ra.Resource
+					)
+					if ra.Name != "" {
+						resource += "/" + ra.Name
+					}
+
+					switch {
+					case ns == v1.NamespaceAll:
+						reason = fmt.Errorf("missing %q permission on resource %q (group: %q) for all namespaces", verb, resource, ra.Group)
+					default:
+						reason = fmt.Errorf("missing %q permission on resource %q (group: %q) for namespace %q", verb, resource, ra.Group, ns)
+					}
+
+					missingPermissions = append(missingPermissions, reason)
 				}
 			}
 		}
 	}
 
-	return missingPermissions, nil
+	return len(missingPermissions) == 0, missingPermissions, nil
 }
 
 func IsResourceNotFoundError(err error) bool {
 	se, ok := err.(*apierrors.StatusError)
 	if !ok {
 		return false
 	}
+
 	if se.Status().Code == http.StatusNotFound && se.Status().Reason == metav1.StatusReasonNotFound {
 		return true
 	}
+
 	return false
 }
 
@@ -299,14 +307,13 @@ func CreateOrUpdateSecret(ctx context.Context, secretClient clientv1.SecretInter
 }
 
 // IsAPIGroupVersionResourceSupported checks if given groupVersion and resource is supported by the cluster.
-//
-// you can exec `kubectl api-resources` to find groupVersion and resource.
-func IsAPIGroupVersionResourceSupported(discoveryCli discovery.DiscoveryInterface, groupversion string, resource string) (bool, error) {
-	apiResourceList, err := discoveryCli.ServerResourcesForGroupVersion(groupversion)
+func IsAPIGroupVersionResourceSupported(discoveryCli discovery.DiscoveryInterface, groupVersion schema.GroupVersion, resource string) (bool, error) {
+	apiResourceList, err := discoveryCli.ServerResourcesForGroupVersion(groupVersion.String())
 	if err != nil {
 		if IsResourceNotFoundError(err) {
 			return false, nil
 		}
+
 		return false, err
 	}
 
@@ -315,6 +322,7 @@ func IsAPIGroupVersionResourceSupported(discoveryCli discovery.DiscoveryInterfac
 			return true, nil
 		}
 	}
+
 	return false, nil
 }
 

pkg/prometheus/agent/operator.go

@@ -31,6 +31,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
@@ -284,7 +285,7 @@ func New(ctx context.Context, restConfig *rest.Config, conf operator.Config, log
 		c.nsPromInf = newNamespaceInformer(c, c.config.Namespaces.PrometheusAllowList)
 	}
 
-	endpointSliceSupported, err := k8sutil.IsAPIGroupVersionResourceSupported(c.kclient.Discovery(), "discovery.k8s.io/v1", "endpointslices")
+	endpointSliceSupported, err := k8sutil.IsAPIGroupVersionResourceSupported(c.kclient.Discovery(), schema.GroupVersion{Group: "discovery.k8s.io", Version: "v1"}, "endpointslices")
 	if err != nil {
 		level.Warn(c.logger).Log("msg", "failed to check if the API supports the endpointslice resources", "err ", err)
 	}

pkg/prometheus/server/operator.go

@@ -31,6 +31,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/metadata"
 	"k8s.io/client-go/rest"
@@ -345,7 +346,7 @@ func New(ctx context.Context, restConfig *rest.Config, conf operator.Config, log
 		c.nsPromInf = newNamespaceInformer(c, c.config.Namespaces.PrometheusAllowList)
 	}
 
-	endpointSliceSupported, err := k8sutil.IsAPIGroupVersionResourceSupported(c.kclient.Discovery(), "discovery.k8s.io/v1", "endpointslices")
+	endpointSliceSupported, err := k8sutil.IsAPIGroupVersionResourceSupported(c.kclient.Discovery(), schema.GroupVersion{Group: "discovery.k8s.io", Version: "v1"}, "endpointslices")
 	if err != nil {
 		level.Warn(c.logger).Log("msg", "failed to check if the API supports the endpointslice resources", "err ", err)
 	}