Automate the creation of the permissions needed by resourcedetection #2394

iblancasa · 2023-11-28T10:57:45Z

Description:

Add a methods to create the ClusterRoles and ClusterRoleBindings needed
Add the structs and methods to parse the processors
Add support for the resourcedetection processor

Link to tracking Issue: #2393

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

internal/manifests/collector/rbac.go

internal/naming/main.go

jaronoff97

I love the spirit of this PR, and think it would be great if we continued this for other processors like the k8sattr processor

internal/manifests/collector/adapters/config_to_ports.go

internal/manifests/collector/adapters/config_to_rbac.go

internal/manifests/collector/parser/processor/processor_resourcedetection.go

…or into task/2393

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

pavolloffay · 2023-12-04T12:15:50Z

@iblancasa CI failed

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

…or into task/2393

swiatekm

LGTM, but I'd like to see a e2e test added as well.

pavolloffay · 2023-12-18T09:38:34Z

@iblancasa the PR needs to be rebased, also @swiatekm-sumo suggested adding e2e test

…or into task/2393

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

pkg/featuregate/featuregate.go

…or into task/2393

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

…or into task/2393

controllers/opentelemetrycollector_controller.go

…or into task/2393

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

…or into task/2393

main.go

internal/manifests/collector/collector.go

controllers/opentelemetrycollector_controller.go

…or into task/2393

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

…or into task/2393

pavolloffay

LGTM 👍🏼

just few nit comments before merning.

pavolloffay · 2024-01-11T09:36:21Z

internal/manifests/collector/adapters/config_to_rbac_test.go

+	}
+
+	var logger = logf.Log.WithName("collector-unit-tests")
+	for _, test := range tests {


It's good that the test table is used but the tests should be run in a separate execution t.Run

pavolloffay · 2024-01-11T09:37:00Z

internal/manifests/collector/adapters/config_to_rbac.go

+	"github.com/open-telemetry/opentelemetry-operator/internal/manifests/collector/parser/processor"
+)
+
+func ConfigToRBAC(logger logr.Logger, config map[interface{}]interface{}) []rbacv1.PolicyRule {


nit: it's worth adding a comment to a public API

pavolloffay · 2024-01-11T09:37:40Z

internal/manifests/collector/parser/processor/processor.go

+	rbacv1 "k8s.io/api/rbac/v1"
+)
+
+type ProcessorParser interface {


nit: document public APIs

pavolloffay · 2024-01-11T09:37:47Z

internal/manifests/collector/parser/processor/processor.go

+	GetRBACRules() []rbacv1.PolicyRule
+}
+
+type Builder func(logr.Logger, string, map[interface{}]interface{}) ProcessorParser


nit: document public APIs

pavolloffay · 2024-01-11T09:40:21Z

main.go

@@ -124,6 +125,7 @@ func main() {
 	pflag.BoolVar(&enableLeaderElection, "enable-leader-election", false,
 		"Enable leader election for controller manager. "+
 			"Enabling this will ensure there is only one active controller manager.")
+	pflag.BoolVar(&createRBACPermissions, "create-rbac-permissions", false, "Automatically create RBAC permissions needed by the processors")


did we discuss whether it should be disabled by default?

I would prefer to ship the OSS distro with as much enabled functionality.

It is disabled by default because otherwise, it would change the current behavior and could be considered as a breaking change. I think this approach is good for now and we can set it to true in the future.

…or into task/2393

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

…pen-telemetry#2394) * Automate the creation of the permissions requested by resourcedetection Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Add changelog Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Fix merge Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Apply changes requested in code review Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Fix lint Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Add feature gate and test Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Add unit tests Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Apply feedback from pull request Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Apply changes requested as part of the Pull Request Signed-off-by: Israel Blancas <iblancasa@gmail.com> * Apply changes requested as part of the Pull Request Signed-off-by: Israel Blancas <iblancasa@gmail.com> --------- Signed-off-by: Israel Blancas <iblancasa@gmail.com>

iblancasa added 2 commits November 28, 2023 11:53

Automate the creation of the permissions requested by resourcedetection

cb4771a

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Add changelog

c566e26

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

iblancasa requested a review from a team November 28, 2023 10:57

Fix merge

d90814d

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

iblancasa changed the title ~~Automate the creation of the permissions requested by resourcedetection~~ Automate the creation of the permissions needed by resourcedetection Nov 28, 2023

iblancasa mentioned this pull request Nov 28, 2023

Automate the creation of the permissions needed by k8sattributes. #2396

Merged

swiatekm reviewed Nov 28, 2023

View reviewed changes

internal/manifests/collector/rbac.go Outdated Show resolved Hide resolved

internal/manifests/collector/rbac.go Outdated Show resolved Hide resolved

internal/manifests/collector/rbac.go Outdated Show resolved Hide resolved

internal/naming/main.go Outdated Show resolved Hide resolved

jaronoff97 reviewed Nov 28, 2023

View reviewed changes

internal/manifests/collector/adapters/config_to_ports.go Outdated Show resolved Hide resolved

internal/manifests/collector/adapters/config_to_rbac.go Outdated Show resolved Hide resolved

internal/manifests/collector/parser/processor/processor_resourcedetection.go Show resolved Hide resolved

jaronoff97 mentioned this pull request Nov 28, 2023

grant default sa permissions to access openshift infrastructure api #1387

Closed

jaronoff97 linked an issue Nov 29, 2023 that may be closed by this pull request

grant default sa permissions to access openshift infrastructure api #1387

Closed

iblancasa added 2 commits December 4, 2023 12:27

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

d6444ab

…or into task/2393

Apply changes requested in code review

c695885

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

pavolloffay approved these changes Dec 4, 2023

View reviewed changes

iblancasa added 4 commits December 4, 2023 14:23

Merge branch 'main' into task/2393

bae77d2

Fix lint

a977949

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

921b6eb

…or into task/2393

Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…

7aa647a

…or into task/2393

swiatekm reviewed Dec 5, 2023

View reviewed changes

iblancasa added 4 commits December 18, 2023 13:22

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

6bd7d5f

…or into task/2393

Add feature gate and test

10f9ea2

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch 'main' into task/2393

b3243d2

Merge branch 'main' into task/2393

23531b6

swiatekm reviewed Dec 19, 2023

View reviewed changes

pkg/featuregate/featuregate.go Outdated Show resolved Hide resolved

iblancasa added 3 commits December 20, 2023 09:47

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

ac325d2

…or into task/2393

Add unit tests

1e70aea

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…

be7042a

…or into task/2393

jaronoff97 mentioned this pull request Dec 20, 2023

Investigate if we can mandate RBAC rules for the TA in webhooks #2426

Closed

pavolloffay reviewed Dec 20, 2023

View reviewed changes

controllers/opentelemetrycollector_controller.go Outdated Show resolved Hide resolved

controllers/opentelemetrycollector_controller.go Outdated Show resolved Hide resolved

iblancasa added 5 commits January 2, 2024 15:58

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

4ef9f74

…or into task/2393

Apply feedback from pull request

d872607

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch 'main' into task/2393

461cd29

Merge branch 'main' into task/2393

af31a85

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

70badae

…or into task/2393

jaronoff97 reviewed Jan 5, 2024

View reviewed changes

main.go Outdated Show resolved Hide resolved

main.go Show resolved Hide resolved

internal/manifests/collector/collector.go Outdated Show resolved Hide resolved

controllers/opentelemetrycollector_controller.go Show resolved Hide resolved

iblancasa added 3 commits January 8, 2024 11:10

Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…

78acfa0

…or into task/2393

Apply changes requested as part of the Pull Request

ddd8194

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

d83b683

…or into task/2393

jaronoff97 approved these changes Jan 8, 2024

View reviewed changes

jaronoff97 requested review from pavolloffay and swiatekm January 8, 2024 20:20

iblancasa added 2 commits January 9, 2024 09:12

Merge branch 'main' into task/2393

29f4ca6

Merge branch 'main' into task/2393

40ee3ef

pavolloffay reviewed Jan 11, 2024

View reviewed changes

iblancasa added 4 commits January 11, 2024 11:31

Merge branch 'task/2393' of github.com:iblancasa/opentelemetry-operat…

9fd612d

…or into task/2393

Merge branch 'main' of github.com:open-telemetry/opentelemetry-operat…

c99f53f

…or into task/2393

Apply changes requested as part of the Pull Request

a3d9d95

Signed-off-by: Israel Blancas <iblancasa@gmail.com>

Merge branch 'main' into task/2393

132328b

pavolloffay approved these changes Jan 11, 2024

View reviewed changes

pavolloffay merged commit b9f9d04 into open-telemetry:main Jan 11, 2024
27 checks passed

iblancasa deleted the task/2393 branch January 11, 2024 16:19

pavolloffay mentioned this pull request Jan 11, 2024

Enable RBAC creation by default #2507

Closed

This was referenced Jan 17, 2024

[operator] Bump operator version to 0.92.1 open-telemetry/opentelemetry-helm-charts#998

Merged

Operator won't reconcile without CRUD permissions for ClusterRoles and Bindings #2525

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Automate the creation of the permissions needed by resourcedetection #2394

Automate the creation of the permissions needed by resourcedetection #2394

iblancasa commented Nov 28, 2023

jaronoff97 left a comment

pavolloffay commented Dec 4, 2023

swiatekm left a comment •

edited

Loading

pavolloffay commented Dec 18, 2023

pavolloffay left a comment

pavolloffay Jan 11, 2024

pavolloffay Jan 11, 2024

pavolloffay Jan 11, 2024

pavolloffay Jan 11, 2024

pavolloffay Jan 11, 2024

iblancasa Jan 11, 2024

Automate the creation of the permissions needed by resourcedetection #2394

Automate the creation of the permissions needed by resourcedetection #2394

Conversation

iblancasa commented Nov 28, 2023

jaronoff97 left a comment

Choose a reason for hiding this comment

pavolloffay commented Dec 4, 2023

swiatekm left a comment • edited Loading

Choose a reason for hiding this comment

pavolloffay commented Dec 18, 2023

pavolloffay left a comment

Choose a reason for hiding this comment

pavolloffay Jan 11, 2024

Choose a reason for hiding this comment

pavolloffay Jan 11, 2024

Choose a reason for hiding this comment

pavolloffay Jan 11, 2024

Choose a reason for hiding this comment

pavolloffay Jan 11, 2024

Choose a reason for hiding this comment

pavolloffay Jan 11, 2024

Choose a reason for hiding this comment

iblancasa Jan 11, 2024

Choose a reason for hiding this comment

swiatekm left a comment •

edited

Loading