Apply changes requested in code review

iblancasa · iblancasa · commit b548a41999aa · 2024-04-16T06:50:38.000+02:00
Signed-off-by: Israel Blancas &lt;iblancasa@gmail.com&gt;
diff --git a/controllers/suite_test.go b/controllers/suite_test.go
@@ -56,7 +56,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"
+	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"
@@ -96,7 +96,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)
+	RBACPermissionsFunc             func(ctx context.Context) (autoRBAC.Availability, error)
 }
 
 func (m *mockAutoDetect) PrometheusCRsAvailability() (prometheus.Availability, error) {
@@ -113,11 +113,11 @@ func (m *mockAutoDetect) OpenShiftRoutesAvailability() (openshift.RoutesAvailabi
 	return openshift.RoutesNotAvailable, nil
 }
 
-func (m *mockAutoDetect) RBACPermissions(ctx context.Context) (autoRbac.Availability, error) {
+func (m *mockAutoDetect) RBACPermissions(ctx context.Context) (autoRBAC.Availability, error) {
 	if m.RBACPermissionsFunc != nil {
 		return m.RBACPermissionsFunc(ctx)
 	}
-	return autoRbac.NotAvailable, nil
+	return autoRBAC.NotAvailable, nil
 }
 
 func TestMain(m *testing.M) {
diff --git a/internal/autodetect/main.go b/internal/autodetect/main.go
@@ -24,7 +24,7 @@ import (
 
 	"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"
+	autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
 	"github.com/open-telemetry/opentelemetry-operator/internal/rbac"
 )
 
@@ -34,7 +34,7 @@ var _ AutoDetect = (*autoDetect)(nil)
 type AutoDetect interface {
 	OpenShiftRoutesAvailability() (openshift.RoutesAvailability, error)
 	PrometheusCRsAvailability() (prometheus.Availability, error)
-	RBACPermissions(ctx context.Context) (autoRbac.Availability, error)
+	RBACPermissions(ctx context.Context) (autoRBAC.Availability, error)
 }
 
 type autoDetect struct {
@@ -92,14 +92,14 @@ 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)
+func (a *autoDetect) RBACPermissions(ctx context.Context) (autoRBAC.Availability, error) {
+	w, err := autoRBAC.CheckRBACPermissions(ctx, a.reviewer)
 	if err != nil {
-		return autoRbac.NotAvailable, err
+		return autoRBAC.NotAvailable, err
 	}
 	if w != nil {
-		return autoRbac.NotAvailable, fmt.Errorf("missing permissions: %s", w)
+		return autoRBAC.NotAvailable, fmt.Errorf("missing permissions: %s", w)
 	}
 
-	return autoRbac.Available, nil
+	return autoRBAC.Available, nil
 }
diff --git a/internal/autodetect/main_test.go b/internal/autodetect/main_test.go
@@ -35,7 +35,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"
+	autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
 	"github.com/open-telemetry/opentelemetry-operator/internal/rbac"
 )
 
@@ -155,7 +155,7 @@ func TestDetectRBACPermissionsBasedOnAvailableClusterRoles(t *testing.T) {
 
 	for _, tt := range []struct {
 		description          string
-		expectedAvailability autoRbac.Availability
+		expectedAvailability autoRBAC.Availability
 		shouldError          bool
 		namespace            string
 		serviceAccount       string
@@ -165,7 +165,7 @@ func TestDetectRBACPermissionsBasedOnAvailableClusterRoles(t *testing.T) {
 			description:          "Not possible to read the namespace",
 			namespace:            "default",
 			shouldError:          true,
-			expectedAvailability: autoRbac.NotAvailable,
+			expectedAvailability: autoRBAC.NotAvailable,
 			clientGenerator: reactorFactory(v1.SubjectAccessReviewStatus{
 				Allowed: true,
 			}),
@@ -187,7 +187,7 @@ func TestDetectRBACPermissionsBasedOnAvailableClusterRoles(t *testing.T) {
 			clientGenerator: reactorFactory(v1.SubjectAccessReviewStatus{
 				Allowed: false,
 			}),
-			expectedAvailability: autoRbac.NotAvailable,
+			expectedAvailability: autoRBAC.NotAvailable,
 		},
 		{
 			description: "RBAC resources are there",
@@ -198,13 +198,13 @@ func TestDetectRBACPermissionsBasedOnAvailableClusterRoles(t *testing.T) {
 			clientGenerator: reactorFactory(v1.SubjectAccessReviewStatus{
 				Allowed: true,
 			}),
-			expectedAvailability: autoRbac.Available,
+			expectedAvailability: autoRBAC.Available,
 		},
 	} {
 		t.Run(tt.description, func(t *testing.T) {
 			// These settings can be get from env vars
-			t.Setenv(autoRbac.NAMESPACE_ENV_VAR, tt.namespace)
-			t.Setenv(autoRbac.SA_ENV_VAR, tt.serviceAccount)
+			t.Setenv(autoRBAC.NAMESPACE_ENV_VAR, tt.namespace)
+			t.Setenv(autoRBAC.SA_ENV_VAR, tt.serviceAccount)
 
 			server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {}))
 			defer server.Close()
diff --git a/internal/autodetect/rbac/check.go b/internal/autodetect/rbac/check.go
@@ -52,9 +52,9 @@ func getOperatorServiceAccount() (string, error) {
 	return sa, nil
 }
 
-// CheckRbacPermissions checks if the operator has the needed permissions to create RBAC resources automatically.
+// CheckRBACPermissions checks if the operator has the needed permissions to create RBAC resources automatically.
 // If the RBAC is there, no errors nor warnings are returned.
-func CheckRbacPermissions(ctx context.Context, reviewer *rbac.Reviewer) (admission.Warnings, error) {
+func CheckRBACPermissions(ctx context.Context, reviewer *rbac.Reviewer) (admission.Warnings, error) {
 	namespace, err := getOperatorNamespace()
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", "not possible to check RBAC rules", err)
diff --git a/internal/config/main.go b/internal/config/main.go
@@ -25,7 +25,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"
+	autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
 	"github.com/open-telemetry/opentelemetry-operator/internal/version"
 )
 
@@ -45,7 +45,7 @@ type Config struct {
 	autoInstrumentationPythonImage      string
 	collectorImage                      string
 	collectorConfigMapEntry             string
-	createRBACPermissions               autoRbac.Availability
+	createRBACPermissions               autoRBAC.Availability
 	enableMultiInstrumentation          bool
 	enableApacheHttpdInstrumentation    bool
 	enableDotNetInstrumentation         bool
@@ -72,7 +72,7 @@ func New(opts ...Option) Config {
 	o := options{
 		prometheusCRAvailability:          prometheus.NotAvailable,
 		openshiftRoutesAvailability:       openshift.RoutesNotAvailable,
-		createRBACPermissions:             autoRbac.NotAvailable,
+		createRBACPermissions:             autoRBAC.NotAvailable,
 		collectorConfigMapEntry:           defaultCollectorConfigMapEntry,
 		targetAllocatorConfigMapEntry:     defaultTargetAllocatorConfigMapEntry,
 		operatorOpAMPBridgeConfigMapEntry: defaultOperatorOpAMPBridgeConfigMapEntry,
@@ -176,7 +176,7 @@ func (c *Config) CollectorConfigMapEntry() string {
 }
 
 // CreateRBACPermissions is true when the operator can create RBAC permissions for SAs running a collector instance. Immutable.
-func (c *Config) CreateRBACPermissions() autoRbac.Availability {
+func (c *Config) CreateRBACPermissions() autoRBAC.Availability {
 	return c.createRBACPermissions
 }
 
diff --git a/internal/config/options.go b/internal/config/options.go
@@ -23,7 +23,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"
+	autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"
 	"github.com/open-telemetry/opentelemetry-operator/internal/version"
 )
 
@@ -43,7 +43,7 @@ type options struct {
 	autoInstrumentationNginxImage       string
 	collectorImage                      string
 	collectorConfigMapEntry             string
-	createRBACPermissions               autoRbac.Availability
+	createRBACPermissions               autoRBAC.Availability
 	enableMultiInstrumentation          bool
 	enableApacheHttpdInstrumentation    bool
 	enableDotNetInstrumentation         bool
@@ -184,7 +184,7 @@ func WithPrometheusCRAvailability(pcrd prometheus.Availability) Option {
 	}
 }
 
-func WithRBACPermissions(rAuto autoRbac.Availability) Option {
+func WithRBACPermissions(rAuto autoRBAC.Availability) Option {
 	return func(o *options) {
 		o.createRBACPermissions = rAuto
 	}
diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/01-assert.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/01-assert.yaml
@@ -19,4 +19,21 @@ rules:
   verbs:
   - get
   - watch
-  - list
+  - list
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  labels:
+    app.kubernetes.io/component: opentelemetry-collector
+    app.kubernetes.io/instance: chainsaw-k8sattributes.simplest
+    app.kubernetes.io/managed-by: opentelemetry-operator
+    app.kubernetes.io/name: simplest-collector
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: simplest-chainsaw-resourcedetection-cluster-role
+subjects:
+- kind: ServiceAccount
+  name: simplest-collector
+  namespace: chainsaw-k8sattributes
diff --git a/tests/e2e-automatic-rbac/processor-k8sattributes/02-assert.yaml b/tests/e2e-automatic-rbac/processor-k8sattributes/02-assert.yaml
@@ -19,4 +19,21 @@ rules:
   verbs:
   - get
   - watch
-  - list
+  - list
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  labels:
+    app.kubernetes.io/component: opentelemetry-collector
+    app.kubernetes.io/instance: chainsaw-k8sattributes.simplest
+    app.kubernetes.io/managed-by: opentelemetry-operator
+    app.kubernetes.io/name: simplest-collector
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: simplest-chainsaw-resourcedetection-cluster-role
+subjects:
+- kind: ServiceAccount
+  name: simplest-collector
+  namespace: chainsaw-k8sattributes
diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/01-assert.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/01-assert.yaml
@@ -11,4 +11,21 @@ rules:
   verbs:
   - get
   - watch
-  - list
+  - list
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  labels:
+    app.kubernetes.io/component: opentelemetry-collector
+    app.kubernetes.io/instance: chainsaw-resourcedetection.simplest
+    app.kubernetes.io/managed-by: opentelemetry-operator
+    app.kubernetes.io/name: simplest-collector
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: simplest-chainsaw-resourcedetection-cluster-role
+subjects:
+- kind: ServiceAccount
+  name: simplest-collector
+  namespace: chainsaw-resourcedetection
diff --git a/tests/e2e-automatic-rbac/processor-resourcedetection/02-assert.yaml b/tests/e2e-automatic-rbac/processor-resourcedetection/02-assert.yaml
@@ -10,3 +10,20 @@ rules:
   verbs:
   - get
   - list
+---
+apiVersion: rbac.authorization.k8s.io/v1
+kind: ClusterRoleBinding
+metadata:
+  labels:
+    app.kubernetes.io/component: opentelemetry-collector
+    app.kubernetes.io/instance: chainsaw-resourcedetection.simplest
+    app.kubernetes.io/managed-by: opentelemetry-operator
+    app.kubernetes.io/name: simplest-collector
+roleRef:
+  apiGroup: rbac.authorization.k8s.io
+  kind: ClusterRole
+  name: simplest-chainsaw-resourcedetection-cluster-role
+subjects:
+- kind: ServiceAccount
+  name: simplest-collector
+  namespace: chainsaw-resourcedetection

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ import (`
`24`	`24`
`25`	`25`	`"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift"`
`26`	`26`	`"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus"`
`27`		`- autoRbac "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"`
	`27`	`+ autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"`
`28`	`28`	`"github.com/open-telemetry/opentelemetry-operator/internal/rbac"`
`29`	`29`	`)`
`30`	`30`
`@@ -34,7 +34,7 @@ var _ AutoDetect = (*autoDetect)(nil)`
`34`	`34`	`type AutoDetect interface {`
`35`	`35`	`OpenShiftRoutesAvailability() (openshift.RoutesAvailability, error)`
`36`	`36`	`PrometheusCRsAvailability() (prometheus.Availability, error)`
`37`		`- RBACPermissions(ctx context.Context) (autoRbac.Availability, error)`
	`37`	`+ RBACPermissions(ctx context.Context) (autoRBAC.Availability, error)`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`type autoDetect struct {`
`@@ -92,14 +92,14 @@ func (a *autoDetect) OpenShiftRoutesAvailability() (openshift.RoutesAvailability`
`92`	`92`	`return openshift.RoutesNotAvailable, nil`
`93`	`93`	`}`
`94`	`94`
`95`		`-func (a *autoDetect) RBACPermissions(ctx context.Context) (autoRbac.Availability, error) {`
`96`		`- w, err := autoRbac.CheckRbacPermissions(ctx, a.reviewer)`
	`95`	`+func (a *autoDetect) RBACPermissions(ctx context.Context) (autoRBAC.Availability, error) {`
	`96`	`+ w, err := autoRBAC.CheckRBACPermissions(ctx, a.reviewer)`
`97`	`97`	`if err != nil {`
`98`		`- return autoRbac.NotAvailable, err`
	`98`	`+ return autoRBAC.NotAvailable, err`
`99`	`99`	`}`
`100`	`100`	`if w != nil {`
`101`		`- return autoRbac.NotAvailable, fmt.Errorf("missing permissions: %s", w)`
	`101`	`+ return autoRBAC.NotAvailable, fmt.Errorf("missing permissions: %s", w)`
`102`	`102`	`}`
`103`	`103`
`104`		`- return autoRbac.Available, nil`
	`104`	`+ return autoRBAC.Available, nil`
`105`	`105`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ import (`
`23`	`23`	`"github.com/open-telemetry/opentelemetry-operator/internal/autodetect"`
`24`	`24`	`"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/openshift"`
`25`	`25`	`"github.com/open-telemetry/opentelemetry-operator/internal/autodetect/prometheus"`
`26`		`- autoRbac "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"`
	`26`	`+ autoRBAC "github.com/open-telemetry/opentelemetry-operator/internal/autodetect/rbac"`
`27`	`27`	`"github.com/open-telemetry/opentelemetry-operator/internal/version"`
`28`	`28`	`)`
`29`	`29`
`@@ -43,7 +43,7 @@ type options struct {`
`43`	`43`	`autoInstrumentationNginxImage string`
`44`	`44`	`collectorImage string`
`45`	`45`	`collectorConfigMapEntry string`
`46`		`- createRBACPermissions autoRbac.Availability`
	`46`	`+ createRBACPermissions autoRBAC.Availability`
`47`	`47`	`enableMultiInstrumentation bool`
`48`	`48`	`enableApacheHttpdInstrumentation bool`
`49`	`49`	`enableDotNetInstrumentation bool`
`@@ -184,7 +184,7 @@ func WithPrometheusCRAvailability(pcrd prometheus.Availability) Option {`
`184`	`184`	`}`
`185`	`185`	`}`
`186`	`186`
`187`		`-func WithRBACPermissions(rAuto autoRbac.Availability) Option {`
	`187`	`+func WithRBACPermissions(rAuto autoRBAC.Availability) Option {`
`188`	`188`	`return func(o *options) {`
`189`	`189`	`o.createRBACPermissions = rAuto`
`190`	`190`	`}`