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.

Sign up for GitHub

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

Webhook proposal #1455

Closed
wants to merge 4 commits into from
Closed

Webhook proposal #1455

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2eda33f
Initial Proposal
May 10, 2019
6028d83
add example
May 10, 2019
2e293ee
updated content and formatting
Jun 24, 2019
a17b077
formatting and flags
Jul 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions doc/proposals/webhook-proposal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## Webhook Support
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we change it to use the template? WDYT?


Implementation Owner: theishshah
Status: Draft

[Background](#Background)
[Goals](#Goals)
[Design overview](#Design_overview)
[User facing usage](#User_facing_usage)
[Observations and open questions](#Observations_and_open_questions)

### Background

The upcoming stable version of controller runtime has support for running a webhook server and having webhooks to mutate or validate pods. The mutation webhook can change various attributes of a pod, and the validation webhook can read pod attributes and allow/deny a pod to run based on this information.
Copy link
Member

Choose a reason for hiding this comment

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

Can we specify the version here, e.g.:

Suggested change
The upcoming stable version of controller runtime has support for running a webhook server and having webhooks to mutate or validate pods. The mutation webhook can change various attributes of a pod, and the validation webhook can read pod attributes and allow/deny a pod to run based on this information.
The `v0.2.0` of controller runtime has support for running a webhook server and having webhooks to mutate or validate pods. The mutation webhook can change various attributes of a pod, and the validation webhook can read pod attributes and allow/deny a pod to run based on this information.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The upcoming stable version of controller runtime has support for running a webhook server and having webhooks to mutate or validate pods. The mutation webhook can change various attributes of a pod, and the validation webhook can read pod attributes and allow/deny a pod to run based on this information.
The upcoming stable version of controller runtime has support for running a webhook server and having webhooks to mutate or validate Kubernetes resources. The mutation webhook can change various attributes of a resource, and the validation webhook can read resource attributes and allow/deny changes based on this information.


### Goals

The goal of this proposal is to add the abilty for users to quickly add a webhook server and only implement the validation/mutation logic for their webhooks.

### Design overview

All of the necesary files and changes for the generated operator occur in the cmd/manager/ directory. The code to create and register the server is in the main.go file and can be completely generated with no additional input needed from the user. In addition the osdk will provide 2 files, 1 each for validation and mutation webhooks. These will have a template Handle function in which the user can define the desired behavior for their pod validation/mutation logic.

### User facing usage (if needed)

My suggested method for interacting with this feature is to have a command in the osdk which can be run after generating the base operator. The new command `generate webhook` will write the files cmd/manager/main.go, cmd/manager/mutationwebhook.go, and cmd/manager/validationwebhook.go
Copy link
Member

Choose a reason for hiding this comment

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

@theishshah Have you had a chance to think about some of the improvements we discussed offline? If I recall, I think we talked about:

  • Supporting a webhook per API
  • Designing the webhook support such that the user's main.go file does not need to be changed to add new webhook servers.

Also, would you mind adding more detail to this proposal doc?

  • Will there be a new SDK subcommand to scaffold webhooks and if so, what flags will it have. Descriptions of all of those would be nice to have documented here for discussion (and will make it easy to drop into the real docs when the proposal is approved)
  • Can you embed example project layout changes and scaffold snippets for the major places we're proposing to change? Basically, when someone runs the above subcommand, what will we create, and how will it hook into the user's main.go?

Copy link
Member

Choose a reason for hiding this comment

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

Agree with @joelanford on the last two points ^


### Observations and open questions

[Here](https://github.com/operator-framework/operator-sdk-samples/pull/63) is an example of the planned implementation on the alpha version of controller runtime.
Copy link
Member

Choose a reason for hiding this comment

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

While I like having a working example, can you include the code parts roughly in this proposal?