Skip to content

Commit 9d5b7db

Browse files
committed
Flatten webhook types struct
This flattens down the webhook package structure, removing dedicated `types` packages in favor of having things in the relevant places. To do this, the inject interface definitions for admission were also moved to the admission controller location, but they actually make some amount of sense living there. It also renames and restructures a couple of the types for code clarity (e.g. WebhookTypeMutating to MutatingWebhook) or to follow the convention of other types in CR (e.g. Making wrapped types from core Kubernetes nested fields).
1 parent fd181f5 commit 9d5b7db

File tree

18 files changed

+194
-246
lines changed

18 files changed

+194
-246
lines changed

example/mutatingwebhook.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,20 @@ import (
2323

2424
corev1 "k8s.io/api/core/v1"
2525
"sigs.k8s.io/controller-runtime/pkg/client"
26-
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
2726
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
28-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
2927
)
3028

3129
// podAnnotator annotates Pods
3230
type podAnnotator struct {
3331
client client.Client
34-
decoder types.Decoder
32+
decoder admission.Decoder
3533
}
3634

3735
// Implement admission.Handler so the controller can handle admission request.
3836
var _ admission.Handler = &podAnnotator{}
3937

4038
// podAnnotator adds an annotation to every incoming pods.
41-
func (a *podAnnotator) Handle(ctx context.Context, req types.Request) types.Response {
39+
func (a *podAnnotator) Handle(ctx context.Context, req admission.Request) admission.Response {
4240
pod := &corev1.Pod{}
4341

4442
err := a.decoder.Decode(req, pod)
@@ -70,7 +68,6 @@ func (a *podAnnotator) mutatePodsFn(ctx context.Context, pod *corev1.Pod) error
7068

7169
// podAnnotator implements inject.Client.
7270
// A client will be automatically injected.
73-
var _ inject.Client = &podAnnotator{}
7471

7572
// InjectClient injects the client.
7673
func (a *podAnnotator) InjectClient(c client.Client) error {
@@ -80,10 +77,9 @@ func (a *podAnnotator) InjectClient(c client.Client) error {
8077

8178
// podAnnotator implements inject.Decoder.
8279
// A decoder will be automatically injected.
83-
var _ inject.Decoder = &podAnnotator{}
8480

8581
// InjectDecoder injects the decoder.
86-
func (a *podAnnotator) InjectDecoder(d types.Decoder) error {
82+
func (a *podAnnotator) InjectDecoder(d admission.Decoder) error {
8783
a.decoder = d
8884
return nil
8985
}

example/validatingwebhook.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,20 @@ import (
2323

2424
corev1 "k8s.io/api/core/v1"
2525
"sigs.k8s.io/controller-runtime/pkg/client"
26-
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
2726
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
28-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
2927
)
3028

3129
// podValidator validates Pods
3230
type podValidator struct {
3331
client client.Client
34-
decoder types.Decoder
32+
decoder admission.Decoder
3533
}
3634

3735
// Implement admission.Handler so the controller can handle admission request.
3836
var _ admission.Handler = &podValidator{}
3937

4038
// podValidator admits a pod iff a specific annotation exists.
41-
func (v *podValidator) Handle(ctx context.Context, req types.Request) types.Response {
39+
func (v *podValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
4240
pod := &corev1.Pod{}
4341

4442
err := v.decoder.Decode(req, pod)
@@ -70,7 +68,6 @@ func (v *podValidator) validatePodsFn(ctx context.Context, pod *corev1.Pod) (boo
7068

7169
// podValidator implements inject.Client.
7270
// A client will be automatically injected.
73-
var _ inject.Client = &podValidator{}
7471

7572
// InjectClient injects the client.
7673
func (v *podValidator) InjectClient(c client.Client) error {
@@ -80,10 +77,9 @@ func (v *podValidator) InjectClient(c client.Client) error {
8077

8178
// podValidator implements inject.Decoder.
8279
// A decoder will be automatically injected.
83-
var _ inject.Decoder = &podValidator{}
8480

8581
// InjectDecoder injects the decoder.
86-
func (v *podValidator) InjectDecoder(d types.Decoder) error {
82+
func (v *podValidator) InjectDecoder(d admission.Decoder) error {
8783
v.decoder = d
8884
return nil
8985
}

pkg/manager/internal.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ import (
3737
"sigs.k8s.io/controller-runtime/pkg/metrics"
3838
"sigs.k8s.io/controller-runtime/pkg/recorder"
3939
"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
40-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
4140
)
4241

4342
var log = logf.RuntimeLog.WithName("manager")
@@ -49,8 +48,6 @@ type controllerManager struct {
4948
// scheme is the scheme injected into Controllers, EventHandlers, Sources and Predicates. Defaults
5049
// to scheme.scheme.
5150
scheme *runtime.Scheme
52-
// admissionDecoder is used to decode an admission.Request.
53-
admissionDecoder types.Decoder
5451

5552
// runnables is the set of Controllers that the controllerManager injects deps into and Starts.
5653
runnables []Runnable
@@ -136,9 +133,6 @@ func (cm *controllerManager) SetFields(i interface{}) error {
136133
if _, err := inject.StopChannelInto(cm.internalStop, i); err != nil {
137134
return err
138135
}
139-
if _, err := inject.DecoderInto(cm.admissionDecoder, i); err != nil {
140-
return err
141-
}
142136
if _, err := inject.MapperInto(cm.mapper, i); err != nil {
143137
return err
144138
}
@@ -157,10 +151,6 @@ func (cm *controllerManager) GetScheme() *runtime.Scheme {
157151
return cm.scheme
158152
}
159153

160-
func (cm *controllerManager) GetAdmissionDecoder() types.Decoder {
161-
return cm.admissionDecoder
162-
}
163-
164154
func (cm *controllerManager) GetFieldIndexer() client.FieldIndexer {
165155
return cm.fieldIndexes
166156
}

pkg/manager/manager.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ import (
3636
"sigs.k8s.io/controller-runtime/pkg/leaderelection"
3737
"sigs.k8s.io/controller-runtime/pkg/metrics"
3838
"sigs.k8s.io/controller-runtime/pkg/recorder"
39-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
40-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
4139
)
4240

4341
// Manager initializes shared dependencies such as Caches and Clients, and provides them to Runnables.
@@ -62,9 +60,6 @@ type Manager interface {
6260
// GetScheme returns an initialized Scheme
6361
GetScheme() *runtime.Scheme
6462

65-
// GetAdmissionDecoder returns the runtime.Decoder based on the scheme.
66-
GetAdmissionDecoder() types.Decoder
67-
6863
// GetClient returns a client configured with the Config
6964
GetClient() client.Client
7065

@@ -135,7 +130,6 @@ type Options struct {
135130
// Dependency injection for testing
136131
newRecorderProvider func(config *rest.Config, scheme *runtime.Scheme, logger logr.Logger) (recorder.Provider, error)
137132
newResourceLock func(config *rest.Config, recorderProvider recorder.Provider, options leaderelection.Options) (resourcelock.Interface, error)
138-
newAdmissionDecoder func(scheme *runtime.Scheme) (types.Decoder, error)
139133
newMetricsListener func(addr string) (net.Listener, error)
140134
}
141135

@@ -210,11 +204,6 @@ func New(config *rest.Config, options Options) (Manager, error) {
210204
return nil, err
211205
}
212206

213-
admissionDecoder, err := options.newAdmissionDecoder(options.Scheme)
214-
if err != nil {
215-
return nil, err
216-
}
217-
218207
// Create the mertics listener. This will throw an error if the metrics bind
219208
// address is invalid or already in use.
220209
metricsListener, err := options.newMetricsListener(options.MetricsBindAddress)
@@ -227,7 +216,6 @@ func New(config *rest.Config, options Options) (Manager, error) {
227216
return &controllerManager{
228217
config: config,
229218
scheme: options.Scheme,
230-
admissionDecoder: admissionDecoder,
231219
errChan: make(chan error),
232220
cache: cache,
233221
fieldIndexes: cache,
@@ -290,10 +278,6 @@ func setOptionsDefaults(options Options) Options {
290278
options.newResourceLock = leaderelection.NewResourceLock
291279
}
292280

293-
if options.newAdmissionDecoder == nil {
294-
options.newAdmissionDecoder = admission.NewDecoder
295-
}
296-
297281
if options.newMetricsListener == nil {
298282
options.newMetricsListener = metrics.NewListener
299283
}

pkg/runtime/inject/inject.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"k8s.io/client-go/rest"
2323
"sigs.k8s.io/controller-runtime/pkg/cache"
2424
"sigs.k8s.io/controller-runtime/pkg/client"
25-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
2625
)
2726

2827
// Cache is used by the ControllerManager to inject Cache into Sources, EventHandlers, Predicates, and
@@ -70,20 +69,6 @@ func ClientInto(client client.Client, i interface{}) (bool, error) {
7069
return false, nil
7170
}
7271

73-
// Decoder is used by the ControllerManager to inject decoder into webhook handlers.
74-
type Decoder interface {
75-
InjectDecoder(types.Decoder) error
76-
}
77-
78-
// DecoderInto will set decoder on i and return the result if it implements Decoder. Returns
79-
// false if i does not implement Decoder.
80-
func DecoderInto(decoder types.Decoder, i interface{}) (bool, error) {
81-
if s, ok := i.(Decoder); ok {
82-
return true, s.InjectDecoder(decoder)
83-
}
84-
return false, nil
85-
}
86-
8772
// Scheme is used by the ControllerManager to inject Scheme into Sources, EventHandlers, Predicates, and
8873
// Reconciles
8974
type Scheme interface {

pkg/webhook/admission/builder/builder.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package builder
1818

1919
import (
2020
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
21-
"sigs.k8s.io/controller-runtime/pkg/webhook/types"
2221
)
2322

2423
// WebhookBuilder builds a webhook based on the provided options.
@@ -37,7 +36,7 @@ type WebhookBuilder struct {
3736

3837
// t specifies the type of the webhook.
3938
// Currently, Mutating and Validating are supported.
40-
t *types.WebhookType
39+
t *admission.WebhookType
4140
}
4241

4342
// NewWebhookBuilder creates an empty WebhookBuilder.
@@ -55,15 +54,15 @@ func (b *WebhookBuilder) Name(name string) *WebhookBuilder {
5554
// Mutating sets the type to mutating admission webhook
5655
// Only one of Mutating and Validating can be invoked.
5756
func (b *WebhookBuilder) Mutating() *WebhookBuilder {
58-
m := types.WebhookTypeMutating
57+
m := admission.MutatingWebhook
5958
b.t = &m
6059
return b
6160
}
6261

6362
// Validating sets the type to validating admission webhook
6463
// Only one of Mutating and Validating can be invoked.
6564
func (b *WebhookBuilder) Validating() *WebhookBuilder {
66-
m := types.WebhookTypeValidating
65+
m := admission.ValidatingWebhook
6766
b.t = &m
6867
return b
6968
}

pkg/webhook/admission/decode.go

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,15 @@ package admission
1919
import (
2020
"k8s.io/apimachinery/pkg/runtime"
2121
"k8s.io/apimachinery/pkg/runtime/serializer"
22-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
2322
)
2423

25-
// DecodeFunc is a function that implements the Decoder interface.
26-
type DecodeFunc func(types.Request, runtime.Object) error
24+
// DecodeFunc is a function that implements an admission decoder in a single function.
25+
type DecodeFunc func(Request, runtime.Object) error
2726

28-
var _ types.Decoder = DecodeFunc(nil)
27+
var _ Decoder = DecodeFunc(nil)
2928

3029
// Decode implements the Decoder interface.
31-
func (f DecodeFunc) Decode(req types.Request, obj runtime.Object) error {
30+
func (f DecodeFunc) Decode(req Request, obj runtime.Object) error {
3231
return f(req, obj)
3332
}
3433

@@ -37,12 +36,12 @@ type decoder struct {
3736
}
3837

3938
// NewDecoder creates a Decoder given the runtime.Scheme
40-
func NewDecoder(scheme *runtime.Scheme) (types.Decoder, error) {
39+
func NewDecoder(scheme *runtime.Scheme) (Decoder, error) {
4140
return decoder{codecs: serializer.NewCodecFactory(scheme)}, nil
4241
}
4342

4443
// Decode decodes the inlined object in the AdmissionRequest into the passed-in runtime.Object.
45-
func (d decoder) Decode(req types.Request, into runtime.Object) error {
44+
func (d decoder) Decode(req Request, into runtime.Object) error {
4645
deserializer := d.codecs.UniversalDeserializer()
4746
return runtime.DecodeInto(deserializer, req.AdmissionRequest.Object.Raw, into)
4847
}

pkg/webhook/admission/decode_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,10 @@ import (
2424
corev1 "k8s.io/api/core/v1"
2525
"k8s.io/apimachinery/pkg/runtime"
2626
"k8s.io/client-go/kubernetes/scheme"
27-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
2827
)
2928

3029
var _ = Describe("admission webhook decoder", func() {
31-
var decoder types.Decoder
30+
var decoder Decoder
3231
BeforeEach(func(done Done) {
3332
var err error
3433
decoder, err = NewDecoder(scheme.Scheme)
@@ -46,8 +45,8 @@ var _ = Describe("admission webhook decoder", func() {
4645
})
4746

4847
Describe("Decode", func() {
49-
req := types.Request{
50-
AdmissionRequest: &admissionv1beta1.AdmissionRequest{
48+
req := Request{
49+
AdmissionRequest: admissionv1beta1.AdmissionRequest{
5150
Object: runtime.RawExtension{
5251
Raw: []byte(`{
5352
"apiVersion": "v1",

pkg/webhook/admission/http.go

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
"k8s.io/apimachinery/pkg/runtime"
3232
"k8s.io/apimachinery/pkg/runtime/serializer"
3333
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
34-
"sigs.k8s.io/controller-runtime/pkg/webhook/admission/types"
3534
"sigs.k8s.io/controller-runtime/pkg/webhook/internal/metrics"
3635
)
3736

@@ -55,7 +54,7 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
5554
var body []byte
5655
var err error
5756

58-
var reviewResponse types.Response
57+
var reviewResponse Response
5958
if r.Body != nil {
6059
if body, err = ioutil.ReadAll(r.Body); err != nil {
6160
log.Error(err, "unable to read the body from the incoming request")
@@ -81,7 +80,11 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
8180
return
8281
}
8382

84-
ar := v1beta1.AdmissionReview{}
83+
req := Request{}
84+
ar := v1beta1.AdmissionReview{
85+
// avoid an extra copy
86+
Request: &req.AdmissionRequest,
87+
}
8588
if _, _, err := admissionv1beta1schemecodecs.UniversalDeserializer().Decode(body, nil, &ar); err != nil {
8689
log.Error(err, "unable to decode the request")
8790
reviewResponse = ErrorResponse(http.StatusBadRequest, err)
@@ -90,13 +93,13 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
9093
}
9194

9295
// TODO: add panic-recovery for Handle
93-
reviewResponse = wh.Handle(context.Background(), types.Request{AdmissionRequest: ar.Request})
96+
reviewResponse = wh.Handle(context.Background(), req)
9497
wh.writeResponse(w, reviewResponse)
9598
}
9699

97-
func (wh *Webhook) writeResponse(w io.Writer, response types.Response) {
98-
if response.Response.Result.Code != 0 {
99-
if response.Response.Result.Code == http.StatusOK {
100+
func (wh *Webhook) writeResponse(w io.Writer, response Response) {
101+
if response.Result.Code != 0 {
102+
if response.Result.Code == http.StatusOK {
100103
metrics.TotalRequests.WithLabelValues(wh.Name, "true").Inc()
101104
} else {
102105
metrics.TotalRequests.WithLabelValues(wh.Name, "false").Inc()
@@ -105,7 +108,7 @@ func (wh *Webhook) writeResponse(w io.Writer, response types.Response) {
105108

106109
encoder := json.NewEncoder(w)
107110
responseAdmissionReview := v1beta1.AdmissionReview{
108-
Response: response.Response,
111+
Response: &response.AdmissionResponse,
109112
}
110113
err := encoder.Encode(responseAdmissionReview)
111114
if err != nil {

0 commit comments

Comments
 (0)