Add webhook phase 1 #2072
Merged
Add webhook phase 1 #2072
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sebgl
approved these changes
Oct 29, 2019
| } | ||
|
|
||
| func (r *Elasticsearch) ValidateUpdate(old runtime.Object) error { | ||
| eslog.Info("validate update", "name", r.Name) |
| return errors.New("cannot cast old object to Elasticsearch type") | ||
| } | ||
| var errs field.ErrorList | ||
|
|
There was a problem hiding this comment.
nit: move the variable declaration one line below?
| duplicateNodeSets = "NodeSet names must be unique" | ||
| noDowngradesMsg = "Downgrades are not supported" | ||
| unsupportedVersionMsg = "Unsupported version" | ||
| unsupportedUpgradeMsg = "Unsupported version upgrade path" |
There was a problem hiding this comment.
Should we use capitalize the first word of them all for consistency?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially fixes #1769
Fixes #2068
This is just the first phase for re-adding the webhook validation for kubebuilder v2. In kubebuilder v2, webhooks are implemented with a receiver of the custom resource type. You can see the docs here. This leaves us in more or less the same position we were in before, but in a position to implement the webhook. I wanted to merge this sooner than later as it touches a lot of files. The changes in the future should mostly just be in the webhook handling code.
Unfortunately we had validation code in a few dependent packages, so this required a bit of refactoring to avoid circular dependencies. Including:
VerifySupportsExistingPodswas also moved to the driver since that was the only caller and it seemed to make more sense thereUpstream validation funcs also returns
field.ErrorLists, which are nice because it provides an easy way to point out exactly which part of the path is wrong and what the invalid value is. It meant a little rework in our validation funcs, but it seems easier to read imo.specUpdatedToBeta()and that the count of any node set is positive.Still to do
I had two things I was not sure about:
Do we want to actually generate the webhook yaml right now? It doesn't do anything right now, but it is necessary for the next phase
Do we want to double check that the nodeSet len is positive and that the nodeSet counts are positive in the validation funcs? In general I don't think we need to re-implement OpenAPI, but since we do not have OpenAPI validation right now I can see the argument for it. But hopefully we do by the time we cut a new release