-
Notifications
You must be signed in to change notification settings - Fork 630
Use knative/pkg/webhooks #352
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
Changes from all commits
1c9a1bb
aae9bf9
53c45d0
c20432d
6edb9b4
9775e21
8f91a32
74e59ce
dd42d7d
6134284
a9bb0c7
103cf73
ba6ed98
a39a548
2918805
27bd46a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,12 @@ apiVersion: v1 | |
| kind: Service | ||
| metadata: | ||
| labels: | ||
| role: eventing-webhook | ||
| name: eventing-webhook | ||
| role: webhook | ||
| name: webhook | ||
| namespace: knative-eventing | ||
| spec: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are the Service and the Deployment for the webhook in separate files? Can we combine these at the 400 level?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (But don't do this in this PR) |
||
| ports: | ||
| - port: 443 | ||
| targetPort: 443 | ||
| selector: | ||
| role: eventing-webhook | ||
| role: webhook | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,10 @@ import ( | |
| "encoding/json" | ||
|
|
||
| "github.com/knative/pkg/apis" | ||
| "github.com/knative/pkg/webhook" | ||
| "k8s.io/api/core/v1" | ||
| meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| ) | ||
|
|
||
| // +genclient | ||
|
|
@@ -44,10 +46,19 @@ type Channel struct { | |
| var _ apis.Validatable = (*Channel)(nil) | ||
| var _ apis.Defaultable = (*Channel)(nil) | ||
| var _ apis.Immutable = (*Channel)(nil) | ||
| var _ runtime.Object = (*Channel)(nil) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It sounds like you don't need this anymore? |
||
| var _ webhook.GenericCRD = (*Channel)(nil) | ||
|
|
||
| // ChannelSpec specifies the Bus backing a channel and the configuration | ||
| // arguments for the channel. | ||
| type ChannelSpec struct { | ||
| // TODO: Generation does not work correctly with CRD. They are scrubbed | ||
| // by the APIserver (https://github.com/kubernetes/kubernetes/issues/58778) | ||
| // So, we add Generation here. Once that gets fixed, remove this and use | ||
| // ObjectMeta.Generation instead. | ||
| // +optional | ||
| Generation int64 `json:"generation,omitempty"` | ||
|
|
||
| // Name of the bus backing this channel (optional) | ||
| Bus string `json:"bus,omitempty"` | ||
|
|
||
|
|
||
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.
Changing the name of the webhook service is likely to mean that anyone with the existing webhook installed is going to run with two webhooks until they delete the old one by hand.
Put this in "release notes" -- steal the format from
knative/servingif we don't have it.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.
done, and made a PR to update our template: #357