-
Notifications
You must be signed in to change notification settings - Fork 25
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
add yaml-based crd validation + defaults #150
Conversation
Angry unit tests. Will figure out why tomorrow |
@@ -146,7 +153,8 @@ type NginxIngressController struct { | |||
metav1.ObjectMeta `json:"metadata,omitempty"` | |||
|
|||
// +required | |||
Spec NginxIngressControllerSpec `json:"spec,omitempty"` | |||
// +kubebuilder:default:={"ingressClassName":"nginx.approuting.kubernetes.azure.com","controllerNamePrefix":"nginx"} |
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.
is there any way we can validate that this inline json string is properly formatted? it looks fine, but just wondering more from a theoretical standpoint
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.
If this string is malformed our e2e runner will fail because the crd won't be able to be applied.
CRDs validate this when they are applied against the k8s api server.
I know because this happened to me when I made a typo in an earlier iteration of this PR.
Pull Request Test Coverage Report for Build 7464140143
💛 - Coveralls |
/ok-to-test sha=7c5e5de |
Description
adds yaml-based crd validation to our crd. Reduces the need for webhooks (makes it more acceptable to not use webhooks which add lots of overhead).
Will clean up some old stuff that's not needed anymore in the next PR (and add some stuff to our e2e runner).
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Tested locally and e2e tested.
Checklist: