Check for the permissions instead of using a CLI flag

.chloggen/2833-fix-detector-resourcedetectionprocessor.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Use the k8snode detector instead of kubernetes for the automatic RBAC creation for the resourcedetector"
+
+# One or more tracking issues related to the change
+issues: [2833]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:

.chloggen/replace-create-rbac-permissions-by-checking-the-sa-permissions.yaml

@@ -0,0 +1,16 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: operator
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Check the permissions for the SA running the OpenTelemetry Operator instead of using the create-rbac-permissions flag.
+
+# One or more tracking issues related to the change
+issues: [2588]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:

.github/workflows/e2e.yaml

@@ -24,6 +24,7 @@ jobs:
          - "1.29"
        group:
          - e2e
+         - e2e-automatic-rbac
          - e2e-autoscale
          - e2e-instrumentation
          - e2e-opampbridge

Makefile

@@ -207,6 +207,19 @@ generate: controller-gen
 e2e: chainsaw
 	$(CHAINSAW) test --test-dir ./tests/e2e
 
+# end-to-end-test for testing automatic RBAC creation
+.PHONY: e2e-automatic-rbac
+e2e-automatic-rbac: chainsaw
+	# Add extra needed permissions
+	kubectl apply -f hack/rbac-operator-permissions.yaml
+	kubectl rollout restart deployment opentelemetry-operator-controller-manager -n opentelemetry-operator-system
+	kubectl rollout status deployment opentelemetry-operator-controller-manager -n opentelemetry-operator-system
+	go run hack/check-operator-ready.go
+	$(CHAINSAW) test --test-dir ./tests/e2e-automatic-rbac
+	# Cleanup
+	kubectl delete -f hack/rbac-operator-permissions.yaml
+	kubectl rollout restart deployment opentelemetry-operator-controller-manager -n opentelemetry-operator-system
+
 # end-to-end-test for testing autoscale
 .PHONY: e2e-autoscale
 e2e-autoscale: chainsaw

apis/v1alpha1/collector_webhook.go

@@ -17,10 +17,8 @@ package v1alpha1
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/go-logr/logr"
-	v1 "k8s.io/api/authorization/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -378,7 +376,7 @@ func (c CollectorWebhook) validateTargetAllocatorConfig(ctx context.Context, r *
 		if subjectAccessReviews, err := c.reviewer.CheckPolicyRules(ctx, r.GetNamespace(), r.Spec.TargetAllocator.ServiceAccount, targetAllocatorCRPolicyRules...); err != nil {
 			return nil, fmt.Errorf("unable to check rbac rules %w", err)
 		} else if allowed, deniedReviews := rbac.AllSubjectAccessReviewsAllowed(subjectAccessReviews); !allowed {
-			return warningsGroupedByResource(deniedReviews), nil
+			return rbac.WarningsGroupedByResource(deniedReviews), nil
 		}
 	}
 
@@ -426,28 +424,6 @@ func checkAutoscalerSpec(autoscaler *AutoscalerSpec) error {
 	return nil
 }
 
-// warningsGroupedByResource is a helper to take the missing permissions and format them as warnings.
-func warningsGroupedByResource(reviews []*v1.SubjectAccessReview) []string {
-	fullResourceToVerbs := make(map[string][]string)
-	for _, review := range reviews {
-		if review.Spec.ResourceAttributes != nil {
-			key := fmt.Sprintf("%s/%s", review.Spec.ResourceAttributes.Group, review.Spec.ResourceAttributes.Resource)
-			if len(review.Spec.ResourceAttributes.Group) == 0 {
-				key = review.Spec.ResourceAttributes.Resource
-			}
-			fullResourceToVerbs[key] = append(fullResourceToVerbs[key], review.Spec.ResourceAttributes.Verb)
-		} else if review.Spec.NonResourceAttributes != nil {
-			key := fmt.Sprintf("nonResourceURL: %s", review.Spec.NonResourceAttributes.Path)
-			fullResourceToVerbs[key] = append(fullResourceToVerbs[key], review.Spec.NonResourceAttributes.Verb)
-		}
-	}
-	var warnings []string
-	for fullResource, verbs := range fullResourceToVerbs {
-		warnings = append(warnings, fmt.Sprintf("missing the following rules for %s: [%s]", fullResource, strings.Join(verbs, ",")))
-	}
-	return warnings
-}
-
 func SetupCollectorWebhook(mgr ctrl.Manager, cfg config.Config, reviewer *rbac.Reviewer) error {
 	cvw := &CollectorWebhook{
 		reviewer: reviewer,

bundle/manifests/opentelemetry-operator.clusterserviceversion.yaml

@@ -65,7 +65,7 @@ metadata:
     categories: Logging & Tracing,Monitoring
     certified: "false"
     containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator
-    createdAt: "2024-04-05T17:37:15Z"
+    createdAt: "2024-04-08T14:26:03Z"
     description: Provides the OpenTelemetry components, including the Collector
     operators.operatorframework.io/builder: operator-sdk-v1.29.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3
@@ -413,6 +413,11 @@ spec:
                 - --zap-time-encoding=rfc3339nano
                 - --feature-gates=+operator.autoinstrumentation.go
                 - --enable-nginx-instrumentation=true
+                env:
+                - name: SERVICE_ACCOUNT_NAME
+                  valueFrom:
+                    fieldRef:
+                      fieldPath: spec.serviceAccountName
                 image: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator:0.97.1
                 livenessProbe:
                   httpGet:

config/manager/manager.yaml

@@ -52,5 +52,10 @@ spec:
           requests:
             cpu: 100m
             memory: 64Mi
+        env:
+          - name: SERVICE_ACCOUNT_NAME
+            valueFrom:
+              fieldRef:
+                fieldPath: spec.serviceAccountName
       serviceAccountName: controller-manager
       terminationGracePeriodSeconds: 10

controllers/opampbridge_controller_test.go

@@ -37,6 +37,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/controllers"
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift"
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus"
+	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 )
 
@@ -48,6 +49,9 @@ var opampBridgeMockAutoDetector = &mockAutoDetect{
 	PrometheusCRsAvailabilityFunc: func() (prometheus.Availability, error) {
 		return prometheus.Available, nil
 	},
+	RBACPermissionsFunc: func(ctx context.Context) (rbac.Availability, error) {
+		return rbac.Available, nil
+	},
 }
 
 func TestNewObjectsOnReconciliation_OpAMPBridge(t *testing.T) {

controllers/opentelemetrycollector_controller.go

@@ -40,6 +40,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift"
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus"
+	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector"
@@ -245,7 +246,7 @@ func (r *OpenTelemetryCollectorReconciler) SetupWithManager(mgr ctrl.Manager) er
 		Owns(&autoscalingv2.HorizontalPodAutoscaler{}).
 		Owns(&policyV1.PodDisruptionBudget{})
 
-	if r.config.CreateRBACPermissions() {
+	if r.config.CreateRBACPermissions() == rbac.Available {
 		builder.Owns(&rbacv1.ClusterRoleBinding{})
 		builder.Owns(&rbacv1.ClusterRole{})
 	}

controllers/suite_test.go

@@ -55,6 +55,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect"
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift"
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus"
+	autoRbac "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
 	"github.com/open-telemetry/opentelemetry-operator/internal/config"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/testdata"
@@ -94,6 +95,7 @@ var _ autodetect.AutoDetect = (*mockAutoDetect)(nil)
 type mockAutoDetect struct {
 	OpenShiftRoutesAvailabilityFunc func() (openshift.RoutesAvailability, error)
 	PrometheusCRsAvailabilityFunc   func() (prometheus.Availability, error)
+	RBACPermissionsFunc             func(ctx context.Context) (autoRbac.Availability, error)
 }
 
 func (m *mockAutoDetect) PrometheusCRsAvailability() (prometheus.Availability, error) {
@@ -110,6 +112,13 @@ func (m *mockAutoDetect) OpenShiftRoutesAvailability() (openshift.RoutesAvailabi
 	return openshift.RoutesNotAvailable, nil
 }
 
+func (m *mockAutoDetect) RBACPermissions(ctx context.Context) (autoRbac.Availability, error) {
+	if m.RBACPermissionsFunc != nil {
+		return m.RBACPermissionsFunc(ctx)
+	}
+	return autoRbac.NotAvailable, nil
+}
+
 func TestMain(m *testing.M) {
 	ctx, cancel = context.WithCancel(context.TODO())
 	defer cancel()

hack/rbac-operator-permissions.yaml

@@ -0,0 +1,67 @@
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRole
+metadata:
+  name: generate-processors-rbac
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - namespaces
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - ""
+  resources:
+  - nodes
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+  - rbac.authorization.k8s.io
+  resources:
+  - clusterrolebindings
+  - clusterroles
+  verbs:
+  - create
+  - delete
+  - get
+  - list
+  - patch
+  - update
+  - watch
+- apiGroups:
+  - apps
+  resources:
+  - replicasets
+  verbs:
+  - get
+  - list
+  - watch
+- apiGroups:
+  - extensions
+  resources:
+  - replicasets
+  verbs:
+  - get
+  - list
+  - watch
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  name: generate-processors-rbac
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: generate-processors-rbac
+subjects:
+- kind: ServiceAccount
+  name: opentelemetry-operator-controller-manager
+  namespace: opentelemetry-operator-system

internal/autodetect/main.go

@@ -16,11 +16,16 @@
 package autodetect
 
 import (
+	"context"
+	"fmt"
+
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/rest"
 
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift"
 	"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus"
+	autoRbac "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
+	"github.com/open-telemetry/opentelemetry-operator/internal/rbac"
 )
 
 var _ AutoDetect = (*autoDetect)(nil)
@@ -29,14 +34,16 @@ var _ AutoDetect = (*autoDetect)(nil)
 type AutoDetect interface {
 	OpenShiftRoutesAvailability() (openshift.RoutesAvailability, error)
 	PrometheusCRsAvailability() (prometheus.Availability, error)
+	RBACPermissions(ctx context.Context) (autoRbac.Availability, error)
 }
 
 type autoDetect struct {
-	dcl discovery.DiscoveryInterface
+	dcl      discovery.DiscoveryInterface
+	reviewer *rbac.Reviewer
 }
 
 // New creates a new auto-detection worker, using the given client when talking to the current cluster.
-func New(restConfig *rest.Config) (AutoDetect, error) {
+func New(restConfig *rest.Config, reviewer *rbac.Reviewer) (AutoDetect, error) {
 	dcl, err := discovery.NewDiscoveryClientForConfig(restConfig)
 	if err != nil {
 		// it's pretty much impossible to get into this problem, as most of the
@@ -46,7 +53,8 @@ func New(restConfig *rest.Config) (AutoDetect, error) {
 	}
 
 	return &autoDetect{
-		dcl: dcl,
+		dcl:      dcl,
+		reviewer: reviewer,
 	}, nil
 }
 
@@ -83,3 +91,15 @@ func (a *autoDetect) OpenShiftRoutesAvailability() (openshift.RoutesAvailability
 
 	return openshift.RoutesNotAvailable, nil
 }
+
+func (a *autoDetect) RBACPermissions(ctx context.Context) (autoRbac.Availability, error) {
+	w, err := autoRbac.CheckRbacPermissions(ctx, a.reviewer)
+	if err != nil {
+		return autoRbac.NotAvailable, err
+	}
+	if w != nil {
+		return autoRbac.NotAvailable, fmt.Errorf("missing permissions: %s", w)
+	}
+
+	return autoRbac.Available, nil
+}