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

Implement --skip-prefixes parameter #23

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

iamasmith
Copy link
Contributor

  • Allows specific image prefixes to be skipped from digest resolution
  • Implemented for webhook and kpt
  • kpt support also includes environment variable SKIP_PREFIXES bound to the same parameter
  • refactors kubeconfig parsing of multi value strings into util.StringArray for reuse

@iamasmith
Copy link
Contributor Author

First try, I tried to keep with the theme but there's always something that could be better so let me know if you would like to use it but want changes please.

* Allows specific image prefixes to be skipped from digest resolution
* Implemented for webhook and kpt
* kpt support also includes environment variable SKIP_PREFIXES bound to the same parameter
* refactors kubeconfig parsing of multi value strings into util.StringArray for reuse
* common-issues.md updated describing the use case
@costinm
Copy link

costinm commented Nov 22, 2023

Suggestion: can we also add a label ? I.e. if the pod label matches, skip ?

@iamasmith
Copy link
Contributor Author

iamasmith commented Nov 22, 2023

@costinm I think this is a separate use case. Sounds great but it's probably a separate ask on Issues to be considered with use cases. To clarify, this is about skipping selective containers within a pod, not the pod overall.
Editing again on this.. are you suggesting making this specific feature dependent upon the label rather than capable as an overall fix for this specic issue? If so as a minimum it's probably an ask for the label check to be on the namespace since if somebody required this broadly it would be quite onerious.

@halvards
Copy link
Member

Thanks @iamasmith for the PR!

The changes look good, and I also really appreciate that you've added tests and updated the documentation.

I'll merge and push out a release.

@costinm, your request sounds like another feature that could be added. Feel free to raise another issue to track it.

@halvards halvards merged commit 400181d into google:main Nov 22, 2023
2 checks passed
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.

3 participants