Remove ignored namespaces check #53
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's possible for metadata.namespace and metadata.name to be empty in the objects passed to the webhook. This was causing the ignored namespaces check to fail and the name and namespace of the pods was empty in a number of logging statements. Additionally, the name in the request may also be omitted, in which case there is unfortunately no way to know the name of the pod. I've addressed these issues by: - Moving the namespaces check into the mutate method and using the Namespace field from the request to perform the check - Removing the logging statements from `getSidecarConfigurationRequested` in favour of wrapping the errors it returns with the required information. The log statement in the mutate function can then log this information, along with the Namespace from the request. - Replacing the pod name with an informative placeholder in cases where the pod name cannot be known.
And replace it with a namespaceSelector in the example.
ribbybibby
changed the title
|
Ping - just checking if anyone has had a chance to look at this? |
lamiae27
approved these changes
Dec 12, 2020
lamiae27
approved these changes
Dec 12, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What and why?
Fixes #44 and #52.
Remove the ignored namespaces check from the code in favour of a
namespaceSelectorin the webhook example. Not only does this allow users to inject into pods in kube-system if they want to, it also provides more protection against problems with the webhook as pods from ignored namespaces won't be forwarded to the webhook at all.This PR also tweaks the logging in the
mutatefunction to account for the fact that the object metadata may not contain name or namespace information.Testing Steps
Please provide adequate testing steps (including screenshots if necessary).
Include any test fixtures or sample configurations in your commit.
make test)Reviewers
Required reviewers:
@byxornaRequest reviews from other people you want to review this PR in the "Reviewers" section on the right.