-
Notifications
You must be signed in to change notification settings - Fork 0
✨ Improve boxcutter experimental parity #2
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
✨ Improve boxcutter experimental parity #2
Conversation
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
…re component Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
| } | ||
|
|
||
| if len(reg.CSV.Spec.WebhookDefinitions) > 0 && r.CertificateProvider == nil { | ||
| return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported") |
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.
This looks more like an internal panic() that can only happen when you forget to setup the CertificateProvider with dependency injection. At least I find this error message to be confusing.
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.
No, this message is raised to users if they try to install an operator that OLM doesn't yet support.
We have similar messages for bundles the don't support AllNamespaces mode or bundles that define OLM dependencies.
The standard manifest does not enable a webhook feature gate, so in that case, we explicitly are not setting up a CertificateProvider.
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.
Ah, gotya.
| } | ||
|
|
||
| if len(reg.CSV.Spec.WebhookDefinitions) > 0 && r.CertificateProvider == nil { | ||
| return nil, fmt.Errorf("unsupported bundle: webhookDefinitions are not supported") |
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.
Ah, gotya.
|
Whelp... this got merged |
Description
3 separate commits that bring the boxcutter branch closer to being ready to merge into operator-controller's main branch:
Reviewer Checklist