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

Fix conversion webhooks in operator bundle #1765

Merged
merged 4 commits into from
Aug 31, 2021

Conversation

babbageclunk
Copy link
Member

What this PR does / why we need it:
The previously released operator bundle wasn't setting up conversion webhooks properly, so any custom resource with multiple versions would encounter problems reconciling. This updates the bundle generation to use the latest release of operator-sdk, and corrects the webhook config on the multi-version CRDs so that they refer to the webhook service OLM creates, since that's the one that it sets up certificates for.

The azureoperator-webhook-service has been removed.

Since OLM manages the certificate setup for the webhooks, I've also removed the cert-manager annotations from the CRDs, and removed the cert-manager setup from the installation instructions.

Closes #1718

How does this PR make you feel:
gif

If applicable:

  • this PR contains documentation

Now that recent versions of OLM and operator-sdk support conversion
webhooks, we can remove some of the scaffolding and use of cert-manager.
These aren't needed any longer - OLM and operator-sdk set up
certificates between the webhook service, pod, crds and webhook
configs.
This has much better support for conversion webhooks.
@babbageclunk babbageclunk changed the title Fix webhook registration in operator bundle Fix conversion webhooks in operator bundle Aug 30, 2021
Copy link
Member

@matthchr matthchr left a comment

Choose a reason for hiding this comment

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

LGTM - the bit on yq'ing the files is icky but I don't have a better way to do it than that. Probably less icky once we have tests that can actually confirm it did the right thing too.

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.

Bug: Unable to deploy resources on new (1.27207) ASO deployment with error: context deadline exceeded.
2 participants