Add an option to specify the selector for psbinding webhook. (#1123)

This commit is contained in:
Cong Liu 2020-02-24 13:24:08 -08:00 committed by GitHub
parent d771641c91
commit 55831d9ef7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 107 additions and 152 deletions

View File

@ -1,36 +0,0 @@
/*
Copyright 2020 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package psbinding
import "context"
// optOutSelector is used as the key for associating information
// with a context.Context relating to whether we're going to label
// namespaces/objects for inclusion/exclusion for bindings.
type optOutSelector struct{}
// WithOptOutSelector notes on the context that we want opt-out
// behaviour for bindings.
func WithOptOutSelector(ctx context.Context) context.Context {
return context.WithValue(ctx, optOutSelector{}, struct{}{})
}
// HasOptOutSelector checks to see whether the given context has
// been marked as having opted-out behaviour for bindings webhook.
func HasOptOutSelector(ctx context.Context) bool {
return ctx.Value(optOutSelector{}) != nil
}

View File

@ -1,44 +0,0 @@
/*
Copyright 2020 The Knative Authors
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package psbinding
import (
"context"
"testing"
)
func TestDefault(t *testing.T) {
table := []struct {
name string
in context.Context
optout bool
}{{
name: "default",
in: context.Background(),
optout: false,
}, {
name: "default",
in: WithOptOutSelector(context.Background()),
optout: true,
}}
for _, tc := range table {
if want, got := tc.optout, HasOptOutSelector(tc.in); want != got {
t.Errorf("Unexpected optout (-want, +got): %v, %v", want, got)
}
}
}

View File

@ -72,7 +72,8 @@ func NewAdmissionController(
ctx context.Context,
name, path string,
gla GetListAll,
WithContext BindableContext,
withContext BindableContext,
reconcilerOptions ...ReconcilerOption,
) *controller.Impl {
// Extract the assorted things from our context.
@ -82,19 +83,7 @@ func NewAdmissionController(
options := webhook.GetOptions(ctx)
// Construct the reconciler for the mutating webhook configuration.
wh := &Reconciler{
Name: name,
HandlerPath: path,
SecretName: options.SecretName,
// This is the user-provided context-decorator, which allows
// them to infuse the context passed to Do/Undo.
WithContext: WithContext,
Client: client,
MWHLister: mwhInformer.Lister(),
SecretLister: secretInformer.Lister(),
}
wh := NewReconciler(name, path, options.SecretName, client, mwhInformer.Lister(), secretInformer.Lister(), withContext, reconcilerOptions...)
c := controller.NewImpl(wh, logging.FromContext(ctx), name)
// It doesn't matter what we enqueue because we will always Reconcile

View File

@ -46,6 +46,47 @@ import (
certresources "knative.dev/pkg/webhook/certificates/resources"
)
// ReconcilerOptions is a function to modify the Reconciler.
type ReconcilerOption func(*Reconciler)
// WithSelector specifies the selector for the webhook.
func WithSelector(s metav1.LabelSelector) ReconcilerOption {
return func(r *Reconciler) {
r.selector = s
}
}
func NewReconciler(
name, path, secretName string,
client kubernetes.Interface,
mwhLister admissionlisters.MutatingWebhookConfigurationLister,
secretLister corelisters.SecretLister,
withContext BindableContext,
options ...ReconcilerOption,
) *Reconciler {
r := &Reconciler{
Name: name,
HandlerPath: path,
SecretName: secretName,
// This is the user-provided context-decorator, which allows
// them to infuse the context passed to Do/Undo.
WithContext: withContext,
Client: client,
MWHLister: mwhLister,
SecretLister: secretLister,
selector: ExclusionSelector, // Use ExclusionSelector by default.
}
// Apply options.
for _, opt := range options {
opt(r)
}
return r
}
// Reconciler implements an AdmissionController for altering PodSpecable
// resources that are the subject of a particular type of Binding.
// The two key methods are:
@ -70,6 +111,8 @@ type Reconciler struct {
// respective tasks.
WithContext BindableContext
selector metav1.LabelSelector
// lock protects access to exact and inexact
lock sync.RWMutex
exact exactMatcher
@ -298,20 +341,14 @@ func (ac *Reconciler) reconcileMutatingWebhook(ctx context.Context, caCert []byt
// This is only supported by 1.15+ clusters.
matchPolicy := admissionregistrationv1beta1.Equivalent
// See if the opt-out behaviour has been specified and specify the Inclusion Selector.
selector := ExclusionSelector
if HasOptOutSelector(ctx) {
selector = InclusionSelector
}
for i, wh := range webhook.Webhooks {
if wh.Name != webhook.Name {
continue
}
webhook.Webhooks[i].MatchPolicy = &matchPolicy
webhook.Webhooks[i].Rules = rules
webhook.Webhooks[i].NamespaceSelector = &selector
webhook.Webhooks[i].ObjectSelector = &selector // 1.15+ only
webhook.Webhooks[i].NamespaceSelector = &ac.selector
webhook.Webhooks[i].ObjectSelector = &ac.selector // 1.15+ only
webhook.Webhooks[i].ClientConfig.CABundle = caCert
if webhook.Webhooks[i].ClientConfig.Service == nil {
return fmt.Errorf("missing service reference for webhook: %s", wh.Name)

View File

@ -24,11 +24,14 @@ import (
"testing"
"time"
"github.com/google/go-cmp/cmp"
jsonpatch "gomodules.xyz/jsonpatch/v2"
"knative.dev/pkg/apis"
"knative.dev/pkg/client/injection/ducks/duck/v1/podspecable"
kubeclient "knative.dev/pkg/client/injection/kube/client/fake"
mwhinformer "knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/mutatingwebhookconfiguration"
_ "knative.dev/pkg/client/injection/kube/informers/admissionregistration/v1beta1/mutatingwebhookconfiguration/fake"
secretinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/secret"
_ "knative.dev/pkg/client/injection/kube/informers/core/v1/secret/fake"
dynamicclient "knative.dev/pkg/injection/clients/dynamicclient/fake"
"knative.dev/pkg/tracker"
@ -512,37 +515,6 @@ func TestWebhookReconcile(t *testing.T) {
}},
},
},
}, {
Name: ":fire: everything is fine, using opt-out (inclusion) :fire:",
Key: key,
Ctx: WithOptOutSelector(context.Background()),
Objects: []runtime.Object{secret,
&admissionregistrationv1beta1.MutatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: name,
},
Webhooks: []admissionregistrationv1beta1.MutatingWebhook{{
Name: name,
ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
Service: &admissionregistrationv1beta1.ServiceReference{
Namespace: system.Namespace(),
Name: "webhook",
// Path is fine.
Path: ptr.String(path),
},
// CABundle is fine.
CABundle: []byte("present"),
},
// Rules are fine.
Rules: nil,
// MatchPolicy is fine.
MatchPolicy: &equivalent,
// Selectors are fine.
NamespaceSelector: &InclusionSelector,
ObjectSelector: &InclusionSelector,
}},
},
},
}, {
Name: "a new binding has entered the match",
Key: key,
@ -1031,27 +1003,19 @@ func TestWebhookReconcile(t *testing.T) {
}}
table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {
return &Reconciler{
Name: name,
HandlerPath: path,
SecretName: secretName,
Client: kubeclient.Get(ctx),
MWHLister: listers.GetMutatingWebhookConfigurationLister(),
SecretLister: listers.GetSecretLister(),
ListAll: func() ([]Bindable, error) {
bl := make([]Bindable, 0)
for _, elt := range listers.GetDuckObjects() {
b, ok := elt.(Bindable)
if !ok {
continue
}
bl = append(bl, b)
r := NewReconciler(name, path, secretName, kubeclient.Get(ctx), listers.GetMutatingWebhookConfigurationLister(), listers.GetSecretLister(), nil)
r.ListAll = func() ([]Bindable, error) {
bl := make([]Bindable, 0)
for _, elt := range listers.GetDuckObjects() {
b, ok := elt.(Bindable)
if !ok {
continue
}
return bl, nil
},
bl = append(bl, b)
}
return bl, nil
}
return r
}))
}
@ -1073,6 +1037,51 @@ func TestNew(t *testing.T) {
}
}
func TestNewReconciler(t *testing.T) {
ctx, _ := SetupFakeContext(t)
ctx = webhook.WithOptions(ctx, webhook.Options{})
tests := []struct {
name string
selectorOption ReconcilerOption
wantSelector metav1.LabelSelector
}{
{
name: "no selector, use default",
wantSelector: ExclusionSelector,
},
{
name: "ExclusionSelector option",
selectorOption: WithSelector(ExclusionSelector),
wantSelector: ExclusionSelector,
},
{
name: "InclusionSelector option",
selectorOption: WithSelector(InclusionSelector),
wantSelector: InclusionSelector,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := kubeclient.Get(ctx)
mwhInformer := mwhinformer.Get(ctx)
secretInformer := secretinformer.Get(ctx)
withContext := func(ctx context.Context, b Bindable) (context.Context, error) {
return ctx, nil
}
var r *Reconciler
if tt.selectorOption == nil {
r = NewReconciler("foo", "/bar", "foosec", client, mwhInformer.Lister(), secretInformer.Lister(), withContext)
} else {
r = NewReconciler("foo", "/bar", "foosec", client, mwhInformer.Lister(), secretInformer.Lister(), withContext, tt.selectorOption)
}
if diff := cmp.Diff(r.selector, tt.wantSelector); diff != "" {
t.Errorf("Wrong selector configured. Got: %+v, want: %+v, diff: %v", r.selector, tt.wantSelector, diff)
}
})
}
}
func TestBaseReconcile(t *testing.T) {
table := TableTest{{
Name: "bad key",