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

Circumvent insecure secrets listing #138

Conversation

mowoe
Copy link
Contributor

@mowoe mowoe commented Feb 19, 2024

This is a workaround for the issue described in #7 which allows the exporter to run when no secret access is present in the ClusterRole.
It just prints a warning about this, but continues to work other than that.

@nabokihms
Copy link
Member

@mowoe Thanks, it seems like a valid solution for a workaround. I will review the PR till the end of the day.

Could you please sign your commits using the git commit --ammend --sign-off so satisfy the DCO check?

@mowoe mowoe force-pushed the feature/circumvent-insecure-secrets-listing branch from 9885736 to 543976f Compare February 19, 2024 09:42
mowoe and others added 5 commits February 19, 2024 10:57
Co-authored-by: Stephan Krull <[email protected]>
Signed-off-by: Moritz Wörmann <[email protected]>
Signed-off-by: Moritz Woermann <[email protected]>
Co-authored-by: Stephan Krull <[email protected]>
Signed-off-by: Moritz Wörmann <[email protected]>
Signed-off-by: Moritz Woermann <[email protected]>
@mowoe mowoe force-pushed the feature/circumvent-insecure-secrets-listing branch from 543976f to b9608e1 Compare February 19, 2024 09:57
@mowoe
Copy link
Contributor Author

mowoe commented Feb 19, 2024

Thanks @nabokihms for the lightning fast response, i signed the commits now.

pkg/registry_checker/checker.go Outdated Show resolved Hide resolved
pkg/registry_checker/checker.go Outdated Show resolved Hide resolved
pkg/registry_checker/checker.go Outdated Show resolved Hide resolved
@mowoe mowoe force-pushed the feature/circumvent-insecure-secrets-listing branch from 0a1c587 to aef782b Compare February 20, 2024 09:53
@nabokihms nabokihms self-requested a review February 26, 2024 09:21
Copy link
Member

@nabokihms nabokihms left a comment

Choose a reason for hiding this comment

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

Two NITs left. The more important thing is that the branch is too far behind the master. Could you rebase it on the actual master to trigger CI?

pkg/registry_checker/checker.go Outdated Show resolved Hide resolved
pkg/registry_checker/checker.go Outdated Show resolved Hide resolved
@nabokihms
Copy link
Member

@mowoe don't mind chart testing, they should fail 🙂

mowoe and others added 4 commits February 27, 2024 07:59
Co-authored-by: Maksim Nabokikh <[email protected]>
Signed-off-by: Moritz Wörmann <[email protected]>
Signed-off-by: Moritz Woermann <[email protected]>
Co-authored-by: Maksim Nabokikh <[email protected]>
Signed-off-by: Moritz Wörmann <[email protected]>
Signed-off-by: Moritz Woermann <[email protected]>
…exporter into feature/circumvent-insecure-secrets-listing

Signed-off-by: Moritz Woermann <[email protected]>
@mowoe mowoe force-pushed the feature/circumvent-insecure-secrets-listing branch from fef7066 to f4fa9a3 Compare February 27, 2024 07:02
Signed-off-by: Moritz Woermann <[email protected]>
@mowoe
Copy link
Contributor Author

mowoe commented Feb 27, 2024

@nabokihms should finally be okay now, can you approve the CI runs again? 🙂

@nabokihms
Copy link
Member

@mowoe thanks for the contribution. LGTM!

@nabokihms nabokihms merged commit c91c704 into deckhouse:master Feb 29, 2024
8 checks passed
@nabokihms nabokihms added the enhancement New feature or request label Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants