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

Refactored instance validation webhook #1207

Merged
merged 2 commits into from
Dec 19, 2019
Merged

Conversation

zen-dog
Copy link
Contributor

@zen-dog zen-dog commented Dec 18, 2019

Summary:
existing Validator interface was removed, we're now using the underlying http.Handler interface to implement webhooks. This reduces glue code and prepares the foundation for upcoming KEP-20 admission webhook which will use the same interface (but will be a mutating instead of validating webhook)

Summary:
existing `Validator` interface was removed, we're now using the underlying `http.Handler` interface to register webhooks. This reduces glue code and prepares the foundation for upcoming KEP-20 admission webhook which will use the same interface (but will be a mutating instead of validating webhook)
// A client will be automatically injected.

// InjectClient injects the client.
func (v *InstanceValidator) InjectClient(c client.Client) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strictly speaking, this webhook doesn't need the client but I intend for this webhook to be a template for future ones so I kept it here.

Copy link
Contributor

@alenkacz alenkacz left a comment

Choose a reason for hiding this comment

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

LGTM

// registerWebhook method registers passed webhook using a give prefix (e.g. "/validate") and runtime object
// (e.g. v1beta1.Instance) to generate a webhook path e.g. "/validate-kudo-dev-v1beta1-instances". Webhook
// has to implement http.Handler interface (see v1beta1.InstanceValidator for an example)
func registerWebhook(prefix string, obj runtime.Object, hook http.Handler, mgr manager.Manager) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

even nicer would be to have a webhook type that would then map to this prefix :) but we can improve on that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep. let's see what abstractions will prove themselves useful once we add more hooks.

@zen-dog zen-dog merged commit 366222d into master Dec 19, 2019
@zen-dog zen-dog deleted the fb/validating-webhook-2.0 branch December 19, 2019 13:58
kensipe added a commit that referenced this pull request Dec 27, 2019
kensipe added a commit that referenced this pull request Dec 27, 2019
* Revert "Refactored instance validation webhook (#1207)"

This reverts commit 366222d.

Signed-off-by: Ken Sipe <kensipe@gmail.com>
zen-dog pushed a commit that referenced this pull request Dec 28, 2019
Summary:
existing `Validator` interface was removed, we're now using the underlying `http.Handler` interface to register webhooks. This reduces glue code and prepares the foundation for upcoming KEP-20 admission webhook which will use the same interface (but will be a mutating instead of validating webhook)

(cherry picked from commit 366222d)
zen-dog added a commit that referenced this pull request Dec 28, 2019
Additionally:
- excluded `instance_validator.go` from code generation
- and fixed an erroneous client import
kensipe pushed a commit that referenced this pull request Dec 30, 2019
* Refactored instance validation webhook (#1207)

Summary:
existing `Validator` interface was removed, we're now using the underlying `http.Handler` interface to register webhooks. This reduces glue code and prepares the foundation for upcoming KEP-20 admission webhook which will use the same interface (but will be a mutating instead of validating webhook)

(cherry picked from commit 366222d)

* Reintroduce 366222d (#1207)

Additionally:
- excluded `instance_validator.go` from code generation
- and fixed an erroneous client import
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
Summary:
existing `Validator` interface was removed, we're now using the underlying `http.Handler` interface to register webhooks. This reduces glue code and prepares the foundation for upcoming KEP-20 admission webhook which will use the same interface (but will be a mutating instead of validating webhook)

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
* Revert "Refactored instance validation webhook (#1207)"

This reverts commit 366222d.

Signed-off-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
ANeumann82 pushed a commit that referenced this pull request Feb 13, 2020
* Refactored instance validation webhook (#1207)

Summary:
existing `Validator` interface was removed, we're now using the underlying `http.Handler` interface to register webhooks. This reduces glue code and prepares the foundation for upcoming KEP-20 admission webhook which will use the same interface (but will be a mutating instead of validating webhook)

(cherry picked from commit 366222d)

* Reintroduce 366222d (#1207)

Additionally:
- excluded `instance_validator.go` from code generation
- and fixed an erroneous client import

Signed-off-by: Andreas Neumann <aneumann@mesosphere.com>
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.

None yet

2 participants