Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new operator capability to check if it has access to do an operation #2467

Merged
merged 13 commits into from
Jan 4, 2024

Conversation

jaronoff97
Copy link
Contributor

Description:
Adds a new rbac package for checking if a given serviceaccount is able to do an RBAC gated action. This includes a warning for an upcoming target allocator capability (#2328) as an example ahead of that PRs merging.

Link to tracking Issue: closes #2426

Testing: unit tests were added, as well as manual testing in a kind cluster

Documentation: new admissions warnings!

@jaronoff97 jaronoff97 requested a review from a team December 20, 2023 19:43
@jaronoff97
Copy link
Contributor Author

cc @rashmichandrashekar :)

Makefile Show resolved Hide resolved
main.go Show resolved Hide resolved
pkg/rbac/access.go Outdated Show resolved Hide resolved
pkg/rbac/access_test.go Outdated Show resolved Hide resolved
@jaronoff97
Copy link
Contributor Author

jaronoff97 commented Jan 2, 2024

Screenshot 2024-01-02 at 3 34 48 PM

this is how it looks in practice. I'm returning the go struct, lmk if you think i should do some work to return this in a better way. I didn't feel like we should be in the business of pretty printing a non-operator struct, but i can be swayed.

@swiatekm
Copy link
Contributor

swiatekm commented Jan 3, 2024

I didn't feel like we should be in the business of pretty printing a non-operator struct, but i can be swayed.

This struct looks pretty small, I think we can do better. Maybe marshal it to yaml or json with an indent? Or at least display a single message with missing permissions in a list?

@iblancasa
Copy link
Contributor

I didn't feel like we should be in the business of pretty printing a non-operator struct, but i can be swayed.

This struct looks pretty small, I think we can do better. Maybe marshal it to yaml or json with an indent? Or at least display a single message with missing permissions in a list?

I think the last option would be the most useful.

@jaronoff97
Copy link
Contributor Author

Screenshot 2024-01-04 at 11 00 17 AM
is how it looks now

@jaronoff97 jaronoff97 requested a review from swiatekm January 4, 2024 16:05
Copy link
Contributor

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

internal/rbac/access.go Outdated Show resolved Hide resolved
Copy link
Contributor

@iblancasa iblancasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great job!

@jaronoff97 jaronoff97 enabled auto-merge (squash) January 4, 2024 17:06
@jaronoff97 jaronoff97 merged commit 79f5a0e into open-telemetry:main Jan 4, 2024
27 checks passed
@jaronoff97 jaronoff97 deleted the mandate-rbac branch January 4, 2024 17:15
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
…ion (open-telemetry#2467)

* rbac pr testing

* makefile convenience

* Add test

* add chlog

* Add a comment

* update to allow for policy rule checking

* change package

* lint fail

* better formatting

* don't use leading slash for empty group

* add more detail for comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate if we can mandate RBAC rules for the TA in webhooks
5 participants