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

remove-conflicting-webhook-api-versions #178

Merged
merged 2 commits into from
Feb 24, 2021

Conversation

QuentinBisson
Copy link
Contributor

@giantswarm/app-squad-nginx

@QuentinBisson QuentinBisson self-assigned this Feb 23, 2021
@QuentinBisson QuentinBisson requested a review from a team February 23, 2021 10:20
Copy link
Member

@ubergesundheit ubergesundheit left a comment

Choose a reason for hiding this comment

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

LGTM if tested

@webwurst
Copy link
Contributor

@QuentinBisson Can you explain why this conflicting? I would understand it as v1beta1 or v1. When removing v1 does this basically mean "Don't validate v1"? If this solves a current issue I am fine merging. But probably we should create a follow up issue to bring in validation for v1 again. Or I am missing something here :)

@QuentinBisson
Copy link
Contributor Author

QuentinBisson commented Feb 23, 2021

Sure, so basically, the default validation webhook matching mode is Equivalent by default since webhook v1. This policy will ensure that the tree of apiversions will be valid according the the webhook.

I am not really sure if this issue is from the nginx ingress controller or the actual validating webhook code hence I opened a kubernetes issue upstream kubernetes/kubernetes#99352

To make it worked I followed the nginx PR upstream and this removed the v1: kubernetes/ingress-nginx@703c2d6#diff-994d32c26c5c0bfa793222a8a5171ed8e4ea78fc850353bb98602b76751f4c2d
This PR defines v1beta1 until k8s 1.22. Once this is released, the webhook will migrate to v1

I think the issue with the equivalent webhook match policy is that there should be no overlap in the api versions defined hence my issue

I tested it on orion and it worked.

@QuentinBisson QuentinBisson merged commit c7e2086 into master Feb 24, 2021
@QuentinBisson QuentinBisson deleted the Remove-conflicting-apiVersions branch February 24, 2021 10:15
@webwurst
Copy link
Contributor

Thanks for the explanation! :)

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