-
Notifications
You must be signed in to change notification settings - Fork 177
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
feat: cannonical label for skipping pod identity webhook #216
feat: cannonical label for skipping pod identity webhook #216
Conversation
deploy/mutatingwebhook.yaml
Outdated
@@ -13,6 +13,11 @@ webhooks: | |||
name: pod-identity-webhook | |||
namespace: default | |||
path: "/mutate" | |||
objectSelector: | |||
matchExpressions: | |||
- key: eks.amazonaws.com/skip-identity-webhook |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with this change, would love to hear @micahhausler opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@micahhausler could you take a look at this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prateekgogia Can I have you re-review this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we change this to “eks.amazonaws.com/skip-pod-identity-webhook”. Identity webhook may be too generic if EKS adds more identity related webhooks in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielCKennedy makes sense! Fixed!
I looked into this last night, EKS users should be able to modify the mutating webhook in their cluster to skip certain pods (the change in PR). Another option to achieve this is by using this existing annotation supported by webhook Do any of these option help achieving the desired output? |
Hey @prateekgogia! 👋🏻 Thanks for looking at this! This discussion is a bit nuanced- I've tried my best to capture this in writing, but we can also discuss synchronously if that would be easier.
There are a few reasons the PR is needed to solve this problem:
Unfortunately, this isn't a workable solution because of the circular dependency issue described here. It appears |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@DerekTBrown this change is now live across EKS. Thank you for the contribution! |
Adds a condition to the mutating webhook, preventing pods from being processed with a particular label.
See: #215