-
Notifications
You must be signed in to change notification settings - Fork 112
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
Reverse order of CRD init operations #155
Conversation
Thanks @schancel for the PR! Left a couple of suggestions that I think will fix the error in the build. |
@wallyqs Should be fixed. Any chance you're willing to switch to |
pkg/util/kubernetes/crd.go
Outdated
// Attempt to update the CRD by setting its spec to the expected value. | ||
d.Spec = crd.Spec | ||
if _, err := extsClient.ApiextensionsV1beta1().CustomResourceDefinitions().Update(d); err != nil { | ||
// Registration failed |
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.
Actually this is silly...
This commit reverses the order of CRD init operations. The reason for doing CRD init operations in this new order, is so that k8s admins can manually create the expected CRDs. By creating the CRDs by hand, create CRD permission does not need to be granted to the nats-operator SA. Only read permission is required.
Sorry about the inconvenience, will switch to use go modules after 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.
LGTM
This commit reverses the order of CRD init operations. The reason for
doing CRD init operations in this new order, is so that k8s admins
can manually create the expected CRDs. By creating the CRDs by hand,
create CRD permission does not need to be granted to the nats-operator SA.
Only read permission is required.