Add a warning message when one created collector needs extra RBAC per…

…missions and the service account doesn't have them (#3433) * Add a warning message when one created collector needs extra RBAC permissions and the service account doesn't have them Signed-off-by: Israel Blancas <[email protected]> * Fix nil Signed-off-by: Israel Blancas <[email protected]> * Show an admission warning Signed-off-by: Israel Blancas <[email protected]> * Apply changes requested in code review Signed-off-by: Israel Blancas <[email protected]> --------- Signed-off-by: Israel Blancas <[email protected]>
open-telemetry · Nov 23, 2024 · bf7cdd1 · bf7cdd1
1 parent 5e35dbb
commit bf7cdd1
Show file tree

Hide file tree

Showing 8 changed files with 428 additions and 0 deletions.
diff --git a/.chloggen/3432.yaml b/.chloggen/3432.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: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Add a warning message when one created collector needs extra RBAC permissions and the service account doesn't have them.
+
+# One or more tracking issues related to the change
+issues: [3432]
+
+# (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:
diff --git a/controllers/opentelemetrycollector_controller.go b/controllers/opentelemetrycollector_controller.go
@@ -47,6 +47,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector"
 	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/manifestutils"
+	internalRbac "github.com/open-telemetry/opentelemetry-operator/internal/rbac"
 	collectorStatus "github.com/open-telemetry/opentelemetry-operator/internal/status/collector"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/constants"
 	"github.com/open-telemetry/opentelemetry-operator/pkg/featuregate"
@@ -66,6 +67,7 @@ type OpenTelemetryCollectorReconciler struct {
 	scheme   *runtime.Scheme
 	log      logr.Logger
 	config   config.Config
+	reviewer *internalRbac.Reviewer
 }
 
 // Params is the set of options to build a new OpenTelemetryCollectorReconciler.
@@ -75,6 +77,7 @@ type Params struct {
 	Scheme   *runtime.Scheme
 	Log      logr.Logger
 	Config   config.Config
+	Reviewer *internalRbac.Reviewer
 }
 
 func (r *OpenTelemetryCollectorReconciler) findOtelOwnedObjects(ctx context.Context, params manifests.Params) (map[types.UID]client.Object, error) {
@@ -178,6 +181,7 @@ func (r *OpenTelemetryCollectorReconciler) GetParams(ctx context.Context, instan
 		Log:      r.log,
 		Scheme:   r.scheme,
 		Recorder: r.recorder,
+		Reviewer: r.reviewer,
 	}
 
 	// generate the target allocator CR from the collector CR
@@ -210,6 +214,7 @@ func NewReconciler(p Params) *OpenTelemetryCollectorReconciler {
 		scheme:   p.Scheme,
 		config:   p.Config,
 		recorder: p.Recorder,
+		reviewer: p.Reviewer,
 	}
 	return r
 }

diff --git a/internal/manifests/collector/collector.go b/internal/manifests/collector/collector.go
@@ -15,6 +15,9 @@
 package collector
 
 import (
+	"errors"
+	"fmt"
+
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/open-telemetry/opentelemetry-operator/apis/v1beta1"
@@ -80,6 +83,20 @@ func Build(params manifests.Params) ([]client.Object, error) {
 			resourceManifests = append(resourceManifests, res)
 		}
 	}
+
+	if needsCheckSaPermissions(params) {
+		warnings, err := CheckRbacRules(params, params.OtelCol.Spec.ServiceAccount)
+		if err != nil {
+			return nil, fmt.Errorf("error checking RBAC rules for serviceAccount %s: %w", params.OtelCol.Spec.ServiceAccount, err)
+		}
+
+		var w []error
+		for _, warning := range warnings {
+			w = append(w, fmt.Errorf("RBAC rules are missing: %s", warning))
+		}
+		return nil, errors.Join(w...)
+	}
+
 	routes, err := Routes(params)
 	if err != nil {
 		return nil, err
@@ -90,3 +107,10 @@ func Build(params manifests.Params) ([]client.Object, error) {
 	}
 	return resourceManifests, nil
 }
+
+func needsCheckSaPermissions(params manifests.Params) bool {
+	return params.ErrorAsWarning &&
+		params.Config.CreateRBACPermissions() == rbac.NotAvailable &&
+		params.Reviewer != nil &&
+		params.OtelCol.Spec.ServiceAccount != ""
+}