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 ability to set multiple namespaces to search for secrets and configMaps #119

Merged
merged 7 commits into from
Sep 22, 2022

Conversation

Krasttt
Copy link
Contributor

@Krasttt Krasttt commented Sep 14, 2022

Add ability to set multiple namespaces to search for secrets and configMaps.

Add new parameters:

  1. -secrets-list-of-namespaces - Kubernetes list of namespaces to search for secrets.
  2. -configmaps-list-of-namespaces -> Kubernetes list of namespaces to search for configmaps.

This parameters allow to set list of namespaces to search for secrets/configMaps.
Example,

-secrets-list-of-namespaces="namespace_1,namespace_2,namespace_3"
-configmaps-list-of-namespaces="namespace_1,namespace_2,namespace_3"

Open Issue - #73

@joe-elliott
Copy link
Owner

I really like this change, but I'd like to keep it backwards compatible by preserving the old CLI params.

Perhaps just concatenate any namespaces specified in both -secrets-namespace and -secrets-namespaces? (and do the same for the configmap params?)

@Krasttt
Copy link
Contributor Author

Krasttt commented Sep 15, 2022

I really like this change, but I'd like to keep it backwards compatible by preserving the old CLI params.

Perhaps just concatenate any namespaces specified in both -secrets-namespace and -secrets-namespaces? (and do the same for the configmap params?)

Done.

-secrets-namespace/-configmaps-namespace have marked as DEPRECATED

Copy link
Owner

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

A couple of small suggestions, but it's looking good.

One weird consequence of this PR is that you can no longer pass an empty string "" to the config map or secret checker. I'm honestly not sure what that does. If it searches all namespaces and its a feature that people relied on we need to make sure that we preserve it.

Maybe if getSanitizedNamespaceList() is about to return an empty slice then it returns a slice with just the empty string instead?

Also I think the StartChecking() methods should log an error, raise a metric and bail if an empty slice is passed in since they won't do anything.

docs/deploy.md Outdated
Kubernetes namespace to list secrets.
-secrets-list-of-namespaces string
Kubernetes list of namespaces to search for secrets.
Copy link
Owner

Choose a reason for hiding this comment

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

I prefer the old name -secrets-namespaces and -configmaps-namespaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These names are too similar and can be confused. But if you want, I can return the old names.

main.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
Change name of variable
docs/deploy.md Outdated Show resolved Hide resolved
docs/deploy.md Outdated Show resolved Hide resolved
Krasttt and others added 2 commits September 22, 2022 10:51
Co-authored-by: Joe Elliott <joe.elliott@grafana.com>
Co-authored-by: Joe Elliott <joe.elliott@grafana.com>
Copy link
Owner

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

This looks great. Thank you!

@joe-elliott joe-elliott merged commit 0e6cac7 into joe-elliott:master Sep 22, 2022
@danielrozenblum
Copy link

Hello
@joe-elliott
Using latest chart and version 2.10 but still didnt get it to work
tried:
-secrets-namespace="namespace_1,namespace_2"
-secrets-namespaces="namespace_1,namespace_2"
-secrets-list-of-namespaces="namespace_1,namespace_2"

but nothing works. it gives results only from the last namesapce I of the list

any fix for that?

Thank you

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.

None yet

3 participants