add config validation as admission controller (#636)

configmap/filter.go

@@ -16,7 +16,12 @@ limitations under the License.
 
 package configmap
 
-import "reflect"
+import (
+	"fmt"
+	"reflect"
+
+	corev1 "k8s.io/api/core/v1"
+)
 
 // TypeFilter accepts instances of types to check against and returns a function transformer that would only let
 // the call to f through if value is assignable to any one of types of ts. Example:
@@ -42,3 +47,28 @@ func TypeFilter(ts ...interface{}) func(func(string, interface{})) func(string,
 		}
 	}
 }
+
+// ValidateConstructor checks the type of the constructor it evaluates
+// the constructor to be a function with correct signature.
+//
+// The expectation is for the constructor to receive a single input
+// parameter of type corev1.ConfigMap as the input and return two
+// values with the second value being of type error
+func ValidateConstructor(constructor interface{}) error {
+	cType := reflect.TypeOf(constructor)
+
+	if cType.Kind() != reflect.Func {
+		return fmt.Errorf("config constructor must be a function")
+	}
+
+	if cType.NumIn() != 1 || cType.In(0) != reflect.TypeOf(&corev1.ConfigMap{}) {
+		return fmt.Errorf("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)")
+	}
+
+	errorType := reflect.TypeOf((*error)(nil)).Elem()
+
+	if cType.NumOut() != 2 || !cType.Out(1).Implements(errorType) {
+		return fmt.Errorf("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)")
+	}
+	return nil
+}

configmap/store.go

@@ -101,20 +101,8 @@ func NewUntypedStore(
 }
 
 func (s *UntypedStore) registerConfig(name string, constructor interface{}) {
-	cType := reflect.TypeOf(constructor)
+	if err := ValidateConstructor(constructor); err != nil {
-
+		panic(err)
-	if cType.Kind() != reflect.Func {
-		panic("config constructor must be a function")
-	}
-
-	if cType.NumIn() != 1 || cType.In(0) != reflect.TypeOf(&corev1.ConfigMap{}) {
-		panic("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)")
-	}
-
-	errorType := reflect.TypeOf((*error)(nil)).Elem()
-
-	if cType.NumOut() != 2 || !cType.Out(1).Implements(errorType) {
-		panic("config constructor must be of the type func(*k8s.io/api/core/v1/ConfigMap) (..., error)")
 	}
 
 	s.storages[name] = &atomic.Value{}

webhook/config_validation_controller.go

@@ -0,0 +1,217 @@
+/*
+Copyright 2019 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 webhook
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"strings"
+
+	"github.com/markbates/inflect"
+	admissionv1beta1 "k8s.io/api/admission/v1beta1"
+	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
+	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/client-go/kubernetes"
+
+	"knative.dev/pkg/configmap"
+	"knative.dev/pkg/kmp"
+	"knative.dev/pkg/logging"
+)
+
+// ConfigValidationController implements the AdmissionController for ConfigMaps
+type ConfigValidationController struct {
+	constructors map[string]reflect.Value
+	options      ControllerOptions
+}
+
+// NewConfigValidationController constructs a ConfigValidationController
+func NewConfigValidationController(
+	constructors configmap.Constructors,
+	opts ControllerOptions) AdmissionController {
+	cfgValidations := &ConfigValidationController{
+		constructors: make(map[string]reflect.Value),
+		options:      opts,
+	}
+
+	for configName, constructor := range constructors {
+		cfgValidations.registerConfig(configName, constructor)
+	}
+
+	return cfgValidations
+}
+
+func (ac *ConfigValidationController) Admit(ctx context.Context, request *admissionv1beta1.AdmissionRequest) *admissionv1beta1.AdmissionResponse {
+	logger := logging.FromContext(ctx)
+	switch request.Operation {
+	case admissionv1beta1.Create, admissionv1beta1.Update:
+	default:
+		logger.Infof("Unhandled webhook operation, letting it through %v", request.Operation)
+		return &admissionv1beta1.AdmissionResponse{Allowed: true}
+	}
+
+	if err := ac.validate(ctx, request); err != nil {
+		return makeErrorStatus("validation failed: %v", err)
+	}
+
+	return &admissionv1beta1.AdmissionResponse{
+		Allowed: true,
+	}
+}
+
+func (ac *ConfigValidationController) Register(ctx context.Context, kubeClient kubernetes.Interface, caCert []byte) error {
+	client := kubeClient.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations()
+	logger := logging.FromContext(ctx)
+	failurePolicy := admissionregistrationv1beta1.Fail
+
+	resourceGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
+	var rules []admissionregistrationv1beta1.RuleWithOperations
+	plural := strings.ToLower(inflect.Pluralize(resourceGVK.Kind))
+
+	ruleScope := admissionregistrationv1beta1.NamespacedScope
+	rules = append(rules, admissionregistrationv1beta1.RuleWithOperations{
+		Operations: []admissionregistrationv1beta1.OperationType{
+			admissionregistrationv1beta1.Create,
+			admissionregistrationv1beta1.Update,
+		},
+		Rule: admissionregistrationv1beta1.Rule{
+			APIGroups:   []string{resourceGVK.Group},
+			APIVersions: []string{resourceGVK.Version},
+			Resources:   []string{plural + "/*"},
+			Scope:       &ruleScope,
+		},
+	})
+
+	webhook := &admissionregistrationv1beta1.ValidatingWebhookConfiguration{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: ac.options.ConfigValidationWebhookName,
+		},
+		Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
+			Name:  ac.options.ConfigValidationWebhookName,
+			Rules: rules,
+			ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{
+				Service: &admissionregistrationv1beta1.ServiceReference{
+					Namespace: ac.options.Namespace,
+					Name:      ac.options.ServiceName,
+					Path:      &ac.options.ConfigValidationControllerPath,
+				},
+				CABundle: caCert,
+			},
+			NamespaceSelector: &metav1.LabelSelector{
+				MatchExpressions: []metav1.LabelSelectorRequirement{{
+					Key:      ac.options.ConfigValidationNamespaceLabel,
+					Operator: metav1.LabelSelectorOpExists,
+				}},
+			},
+			FailurePolicy: &failurePolicy,
+		}},
+	}
+
+	// Set the owner to our deployment.
+	deployment, err := kubeClient.AppsV1().Deployments(ac.options.Namespace).Get(ac.options.DeploymentName, metav1.GetOptions{})
+	if err != nil {
+		return fmt.Errorf("failed to fetch our deployment: %v", err)
+	}
+	deploymentRef := metav1.NewControllerRef(deployment, deploymentKind)
+	webhook.OwnerReferences = append(webhook.OwnerReferences, *deploymentRef)
+
+	// Try to create the webhook and if it already exists validate webhook rules.
+	_, err = client.Create(webhook)
+	if err != nil {
+		if !apierrors.IsAlreadyExists(err) {
+			return fmt.Errorf("failed to create a webhook: %v", err)
+		}
+		logger.Info("Webhook already exists")
+		configuredWebhook, err := client.Get(ac.options.ConfigValidationWebhookName, metav1.GetOptions{})
+		if err != nil {
+			return fmt.Errorf("error retrieving webhook: %v", err)
+		}
+		if ok, err := kmp.SafeEqual(configuredWebhook.Webhooks, webhook.Webhooks); err != nil {
+			return fmt.Errorf("error diffing webhooks: %v", err)
+		} else if !ok {
+			logger.Info("Updating webhook")
+			// Set the ResourceVersion as required by update.
+			webhook.ObjectMeta.ResourceVersion = configuredWebhook.ObjectMeta.ResourceVersion
+			if _, err := client.Update(webhook); err != nil {
+				return fmt.Errorf("failed to update webhook: %s", err)
+			}
+		} else {
+			logger.Info("Webhook is already valid")
+		}
+	} else {
+		logger.Info("Created a webhook")
+	}
+
+	return nil
+}
+
+func (ac *ConfigValidationController) validate(ctx context.Context, req *admissionv1beta1.AdmissionRequest) error {
+	logger := logging.FromContext(ctx)
+	kind := req.Kind
+	newBytes := req.Object.Raw
+
+	// Why, oh why are these different types...
+	gvk := schema.GroupVersionKind{
+		Group:   kind.Group,
+		Version: kind.Version,
+		Kind:    kind.Kind,
+	}
+
+	resourceGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
+	if gvk != resourceGVK {
+		logger.Errorf("Unhandled kind: %v", gvk)
+		return fmt.Errorf("unhandled kind: %v", gvk)
+	}
+
+	var newObj corev1.ConfigMap
+	if len(newBytes) != 0 {
+		newDecoder := json.NewDecoder(bytes.NewBuffer(newBytes))
+		if err := newDecoder.Decode(&newObj); err != nil {
+			return fmt.Errorf("cannot decode incoming new object: %v", err)
+		}
+	}
+
+	var err error
+	if constructor, ok := ac.constructors[newObj.Name]; ok {
+
+		inputs := []reflect.Value{
+			reflect.ValueOf(&newObj),
+		}
+
+		outputs := constructor.Call(inputs)
+		errVal := outputs[1]
+
+		if !errVal.IsNil() {
+			err = errVal.Interface().(error)
+		}
+	}
+
+	return err
+}
+
+func (ac *ConfigValidationController) registerConfig(name string, constructor interface{}) {
+	if err := configmap.ValidateConstructor(constructor); err != nil {
+		panic(err)
+	}
+
+	ac.constructors[name] = reflect.ValueOf(constructor)
+}

webhook/config_validation_controller_test.go

@@ -0,0 +1,303 @@
+/*
+Copyright 2019 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 webhook
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"reflect"
+	"strconv"
+	"testing"
+
+	admissionv1beta1 "k8s.io/api/admission/v1beta1"
+	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
+	authenticationv1 "k8s.io/api/authentication/v1"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	fakekubeclientset "k8s.io/client-go/kubernetes/fake"
+
+	"knative.dev/pkg/apis"
+	"knative.dev/pkg/configmap"
+	. "knative.dev/pkg/logging/testing"
+)
+
+func newNonRunningTestConfigValidationController(t *testing.T, options ControllerOptions) (
+	kubeClient *fakekubeclientset.Clientset,
+	ac AdmissionController) {
+	t.Helper()
+	// Create fake clients
+	kubeClient = fakekubeclientset.NewSimpleClientset()
+
+	ac = NewTestConfigValidationController(options)
+	return
+}
+
+func NewTestConfigValidationController(options ControllerOptions) AdmissionController {
+	validations := configmap.Constructors{"test-config": newConfigFromConfigMap}
+	return NewConfigValidationController(validations, options)
+}
+
+func TestValidConfigValidationController(t *testing.T) {
+	kubeClient, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+	createDeployment(kubeClient)
+	err := ac.Register(TestContextWithLogger(t), kubeClient, []byte{})
+	if err != nil {
+		t.Fatalf("Failed to create webhook: %s", err)
+	}
+}
+
+func TestUpdatingConfigValidationController(t *testing.T) {
+	kubeClient, c := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	ac := c.(*ConfigValidationController)
+	webhook := &admissionregistrationv1beta1.ValidatingWebhookConfiguration{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: ac.options.ConfigValidationWebhookName,
+		},
+		Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{{
+			Name:         ac.options.ConfigValidationWebhookName,
+			Rules:        []admissionregistrationv1beta1.RuleWithOperations{{}},
+			ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{},
+		}},
+	}
+
+	createDeployment(kubeClient)
+	createConfigValidationWebhook(kubeClient, webhook)
+	err := ac.Register(TestContextWithLogger(t), kubeClient, []byte{})
+	if err != nil {
+		t.Fatalf("Failed to create webhook: %s", err)
+	}
+
+	currentWebhook, _ := kubeClient.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations().Get(ac.options.ConfigValidationWebhookName, metav1.GetOptions{})
+	if reflect.DeepEqual(currentWebhook.Webhooks, webhook.Webhooks) {
+		t.Fatalf("Expected webhook to be updated")
+	}
+}
+
+func TestDeleteAllowedForConfigMap(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	req := &admissionv1beta1.AdmissionRequest{
+		Operation: admissionv1beta1.Delete,
+	}
+
+	if resp := ac.Admit(TestContextWithLogger(t), req); !resp.Allowed {
+		t.Fatal("Unexpected denial of delete")
+	}
+}
+
+func TestConnectAllowedForConfigMap(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	req := &admissionv1beta1.AdmissionRequest{
+		Operation: admissionv1beta1.Connect,
+	}
+
+	resp := ac.Admit(TestContextWithLogger(t), req)
+	if !resp.Allowed {
+		t.Fatalf("Unexpected denial of connect")
+	}
+}
+
+func TestNonConfigMapKindFails(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	req := &admissionv1beta1.AdmissionRequest{
+		Operation: admissionv1beta1.Create,
+		Kind: metav1.GroupVersionKind{
+			Group:   "pkg.knative.dev",
+			Version: "v1alpha1",
+			Kind:    "Garbage",
+		},
+	}
+
+	expectFailsWith(t, ac.Admit(TestContextWithLogger(t), req), "unhandled kind")
+}
+
+func TestAdmitCreateValidConfigMap(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	r := createValidConfigMap()
+	ctx := apis.WithinCreate(apis.WithUserInfo(
+		TestContextWithLogger(t),
+		&authenticationv1.UserInfo{Username: user1}))
+
+	resp := ac.Admit(ctx, createCreateConfigMapRequest(ctx, r))
+
+	expectAllowed(t, resp)
+}
+
+func TestDenyInvalidCreateConfigMapWithWrongType(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	r := createWrongTypeConfigMap()
+	ctx := apis.WithinCreate(apis.WithUserInfo(
+		TestContextWithLogger(t),
+		&authenticationv1.UserInfo{Username: user1}))
+
+	resp := ac.Admit(ctx, createCreateConfigMapRequest(ctx, r))
+
+	expectFailsWith(t, resp, "invalid syntax")
+}
+
+func TestDenyInvalidCreateConfigMapOutOfRange(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	r := createWrongValueConfigMap()
+	ctx := apis.WithinCreate(apis.WithUserInfo(
+		TestContextWithLogger(t),
+		&authenticationv1.UserInfo{Username: user1}))
+
+	resp := ac.Admit(ctx, createCreateConfigMapRequest(ctx, r))
+
+	expectFailsWith(t, resp, "out of range")
+}
+
+func TestAdmitUpdateValidConfigMap(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	r := createValidConfigMap()
+	ctx := apis.WithinCreate(apis.WithUserInfo(
+		TestContextWithLogger(t),
+		&authenticationv1.UserInfo{Username: user1}))
+
+	resp := ac.Admit(ctx, updateCreateConfigMapRequest(ctx, r))
+
+	expectAllowed(t, resp)
+}
+
+func TestDenyInvalidUpdateConfigMapWithWrongType(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	r := createWrongTypeConfigMap()
+	ctx := apis.WithinCreate(apis.WithUserInfo(
+		TestContextWithLogger(t),
+		&authenticationv1.UserInfo{Username: user1}))
+
+	resp := ac.Admit(ctx, createCreateConfigMapRequest(ctx, r))
+
+	expectFailsWith(t, resp, "invalid syntax")
+}
+
+func TestDenyInvalidUpdateConfigMapOutOfRange(t *testing.T) {
+	_, ac := newNonRunningTestConfigValidationController(t, newDefaultOptions())
+
+	r := createWrongValueConfigMap()
+	ctx := apis.WithinCreate(apis.WithUserInfo(
+		TestContextWithLogger(t),
+		&authenticationv1.UserInfo{Username: user1}))
+
+	resp := ac.Admit(ctx, createCreateConfigMapRequest(ctx, r))
+
+	expectFailsWith(t, resp, "out of range")
+}
+
+func createConfigValidationWebhook(kubeClient kubernetes.Interface, webhook *admissionregistrationv1beta1.ValidatingWebhookConfiguration) {
+	client := kubeClient.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations()
+	_, err := client.Create(webhook)
+	if err != nil {
+		panic(fmt.Sprintf("failed to create test webhook: %s", err))
+	}
+}
+
+type config struct {
+	value float64
+}
+
+func newConfigFromConfigMap(configMap *corev1.ConfigMap) (*config, error) {
+	data := configMap.Data
+	cfg := &config{}
+	for _, b := range []struct {
+		key   string
+		field *float64
+	}{{
+		key:   "value",
+		field: &cfg.value,
+	}} {
+		if raw, ok := data[b.key]; !ok {
+			return nil, fmt.Errorf("not found")
+		} else if val, err := strconv.ParseFloat(raw, 64); err != nil {
+			return nil, err
+		} else {
+			*b.field = val
+		}
+	}
+
+	// some sample validation on the value
+	if cfg.value > 2.0 || cfg.value < 0.0 {
+		return nil, fmt.Errorf("out of range")
+	}
+
+	return cfg, nil
+}
+
+func createValidConfigMap() *corev1.ConfigMap {
+	return createConfigMap("1.5")
+}
+
+func createWrongTypeConfigMap() *corev1.ConfigMap {
+	return createConfigMap("bad")
+}
+
+func createWrongValueConfigMap() *corev1.ConfigMap {
+	return createConfigMap("2.5")
+}
+
+func createConfigMap(value string) *corev1.ConfigMap {
+	return &corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: testNamespace,
+			Name:      "test-config",
+		},
+		Data: map[string]string{
+			"value": value,
+		},
+	}
+}
+
+func createCreateConfigMapRequest(ctx context.Context, r *corev1.ConfigMap) *admissionv1beta1.AdmissionRequest {
+	return configMapRequest(r, admissionv1beta1.Create, *apis.GetUserInfo(ctx))
+}
+
+func updateCreateConfigMapRequest(ctx context.Context, r *corev1.ConfigMap) *admissionv1beta1.AdmissionRequest {
+	return configMapRequest(r, admissionv1beta1.Update, *apis.GetUserInfo(ctx))
+}
+
+func configMapRequest(
+	r *corev1.ConfigMap,
+	o admissionv1beta1.Operation,
+	u authenticationv1.UserInfo,
+) *admissionv1beta1.AdmissionRequest {
+	req := &admissionv1beta1.AdmissionRequest{
+		Operation: o,
+		Kind: metav1.GroupVersionKind{
+			Group:   "",
+			Version: "v1",
+			Kind:    "ConfigMap",
+		},
+		UserInfo: u,
+	}
+	marshaled, err := json.Marshal(r)
+	if err != nil {
+		panic("failed to marshal resource")
+	}
+	req.Object.Raw = marshaled
+	req.Resource.Group = ""
+	return req
+}

webhook/resource_admission_controller.go

@@ -30,9 +30,9 @@ import (
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 

webhook/resource_admission_controller_test.go

@@ -522,7 +522,7 @@ func createInnerDefaultResourceWithSpecAndStatus(t *testing.T, spec *InnerDefaul
 	return b
 }
 
-func TestValidWebhook(t *testing.T) {
+func TestValidResourceController(t *testing.T) {
 	kubeClient, ac := newNonRunningTestResourceAdmissionController(t, newDefaultOptions())
 	createDeployment(kubeClient)
 	err := ac.Register(TestContextWithLogger(t), kubeClient, []byte{})
@@ -531,7 +531,7 @@ func TestValidWebhook(t *testing.T) {
 	}
 }
 
-func TestUpdatingWebhook(t *testing.T) {
+func TestUpdatingResourceController(t *testing.T) {
 	kubeClient, c := newNonRunningTestResourceAdmissionController(t, newDefaultOptions())
 
 	ac := c.(*ResourceAdmissionController)

webhook/webhook.go

@@ -56,6 +56,10 @@ type ControllerOptions struct {
 	// mutations before they get stored in the storage.
 	ResourceMutatingWebhookName string
 
+	// ConfigValidationWebhookName is the name of the webhook we create to handle
+	// mutations before they get stored in the storage.
+	ConfigValidationWebhookName string
+
 	// ServiceName is the service name of the webhook.
 	ServiceName string
 
@@ -95,6 +99,13 @@ type ControllerOptions struct {
 	// Service path for ResourceAdmissionController webhook
 	// Default is "/" for backward compatibility and is set by the constructor
 	ResourceAdmissionControllerPath string
+
+	// Service path for ConfigValidationController webhook
+	// Default is "/config-validation" and is set by the constructor
+	ConfigValidationControllerPath string
+
+	// NamespaceLabel is the label for the Namespace we bind ConfigValidationController to
+	ConfigValidationNamespaceLabel string
 }
 
 // AdmissionController provides the interface for different admission controllers

webhook/webhook_integration_test.go

@@ -24,6 +24,8 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/url"
+	"path"
 	"strings"
 	"testing"
 	"time"
@@ -455,30 +457,178 @@ func TestWebhookClientAuth(t *testing.T) {
 	}
 }
 
-func testSetup(t *testing.T) (*Webhook, string, error) {
+func TestValidResponseForConfigMap(t *testing.T) {
-	t.Helper()
+	ac, serverURL, err := testSetup(t)
-	port, err := newTestPort()
 	if err != nil {
-		return nil, "", err
+		t.Fatalf("testSetup() = %v", err)
 	}
 
-	defaultOpts := newDefaultOptions()
+	stopCh := make(chan struct{})
-	defaultOpts.Port = port
+	defer close(stopCh)
-	kubeClient, ac := newNonRunningTestWebhook(t, defaultOpts)
 
-	nsErr := createNamespace(t, kubeClient, metav1.NamespaceSystem)
+	go func() {
-	if nsErr != nil {
+		err := ac.Run(stopCh)
-		return nil, "", nsErr
+		if err != nil {
+			t.Errorf("Unable to run controller: %s", err)
+		}
+	}()
+
+	pollErr := waitForServerAvailable(t, serverURL, testTimeout)
+	if pollErr != nil {
+		t.Fatalf("waitForServerAvailable() = %v", err)
+	}
+	tlsClient, err := createSecureTLSClient(t, ac.Client, &ac.Options)
+	if err != nil {
+		t.Fatalf("createSecureTLSClient() = %v", err)
 	}
 
-	cMapsErr := createTestConfigMap(t, kubeClient)
+	admissionreq := configMapRequest(
-	if cMapsErr != nil {
+		createValidConfigMap(),
-		return nil, "", cMapsErr
+		admissionv1beta1.Create,
+		authenticationv1.UserInfo{},
+	)
+	rev := &admissionv1beta1.AdmissionReview{
+		Request: admissionreq,
 	}
 
-	createDeployment(kubeClient)
+	reqBuf := new(bytes.Buffer)
-	resetMetrics()
+	err = json.NewEncoder(reqBuf).Encode(&rev)
-	return ac, fmt.Sprintf("0.0.0.0:%d", port), nil
+	if err != nil {
+		t.Fatalf("Failed to marshal admission review: %v", err)
+	}
+
+	u, err := url.Parse(fmt.Sprintf("https://%s", serverURL))
+	if err != nil {
+		t.Fatalf("bad url %v", err)
+	}
+
+	u.Path = path.Join(u.Path, ac.Options.ConfigValidationControllerPath)
+	req, err := http.NewRequest("GET", u.String(), reqBuf)
+	if err != nil {
+		t.Fatalf("http.NewRequest() = %v", err)
+	}
+	req.Header.Add("Content-Type", "application/json")
+
+	response, err := tlsClient.Do(req)
+	if err != nil {
+		t.Fatalf("Failed to get response %v", err)
+	}
+
+	if got, want := response.StatusCode, http.StatusOK; got != want {
+		t.Errorf("Response status code = %v, wanted %v", got, want)
+	}
+
+	defer response.Body.Close()
+	responseBody, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Fatalf("Failed to read response body %v", err)
+	}
+
+	reviewResponse := admissionv1beta1.AdmissionReview{}
+
+	err = json.NewDecoder(bytes.NewReader(responseBody)).Decode(&reviewResponse)
+	if err != nil {
+		t.Fatalf("Failed to decode response: %v", err)
+	}
+
+	expectAllowed(t, reviewResponse.Response)
+
+	metricstest.CheckStatsReported(t, requestCountName, requestLatenciesName)
+}
+
+func TestInvalidResponseForConfigMap(t *testing.T) {
+	ac, serverURL, err := testSetup(t)
+	if err != nil {
+		t.Fatalf("testSetup() = %v", err)
+	}
+
+	stopCh := make(chan struct{})
+	defer close(stopCh)
+
+	go func() {
+		err := ac.Run(stopCh)
+		if err != nil {
+			t.Errorf("Unable to run controller: %s", err)
+		}
+	}()
+
+	pollErr := waitForServerAvailable(t, serverURL, testTimeout)
+	if pollErr != nil {
+		t.Fatalf("waitForServerAvailable() = %v", err)
+	}
+	tlsClient, err := createSecureTLSClient(t, ac.Client, &ac.Options)
+	if err != nil {
+		t.Fatalf("createSecureTLSClient() = %v", err)
+	}
+
+	admissionreq := configMapRequest(
+		createWrongTypeConfigMap(),
+		admissionv1beta1.Create,
+		authenticationv1.UserInfo{},
+	)
+	rev := &admissionv1beta1.AdmissionReview{
+		Request: admissionreq,
+	}
+
+	reqBuf := new(bytes.Buffer)
+	err = json.NewEncoder(reqBuf).Encode(&rev)
+	if err != nil {
+		t.Fatalf("Failed to marshal admission review: %v", err)
+	}
+
+	u, err := url.Parse(fmt.Sprintf("https://%s", serverURL))
+	if err != nil {
+		t.Fatalf("bad url %v", err)
+	}
+
+	u.Path = path.Join(u.Path, ac.Options.ConfigValidationControllerPath)
+	req, err := http.NewRequest("GET", u.String(), reqBuf)
+	if err != nil {
+		t.Fatalf("http.NewRequest() = %v", err)
+	}
+	req.Header.Add("Content-Type", "application/json")
+
+	response, err := tlsClient.Do(req)
+	if err != nil {
+		t.Fatalf("Failed to receive response %v", err)
+	}
+
+	if got, want := response.StatusCode, http.StatusOK; got != want {
+		t.Errorf("Response status code = %v, wanted %v", got, want)
+	}
+
+	defer response.Body.Close()
+	respBody, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		t.Fatalf("Failed to read response body %v", err)
+	}
+
+	reviewResponse := admissionv1beta1.AdmissionReview{}
+
+	err = json.NewDecoder(bytes.NewReader(respBody)).Decode(&reviewResponse)
+	if err != nil {
+		t.Fatalf("Failed to decode response: %v", err)
+	}
+
+	var respPatch []jsonpatch.JsonPatchOperation
+	err = json.Unmarshal(reviewResponse.Response.Patch, &respPatch)
+	if err == nil {
+		t.Fatalf("Expected to fail JSON unmarshal of resposnse")
+	}
+
+	if got, want := reviewResponse.Response.Result.Status, "Failure"; got != want {
+		t.Errorf("Response status = %v, wanted %v", got, want)
+	}
+
+	if !strings.Contains(reviewResponse.Response.Result.Message, "invalid syntax") {
+		t.Errorf("Received unexpected response status message %s", reviewResponse.Response.Result.Message)
+	}
+	if !strings.Contains(reviewResponse.Response.Result.Message, "strconv.ParseFloat") {
+		t.Errorf("Received unexpected response status message %s", reviewResponse.Response.Result.Message)
+	}
+
+	// Stats should be reported for requests that have admission disallowed
+	metricstest.CheckStatsReported(t, requestCountName, requestLatenciesName)
 }
 
 func TestSetupWebhookHTTPServerError(t *testing.T) {
@@ -513,3 +663,29 @@ func TestSetupWebhookHTTPServerError(t *testing.T) {
 		}
 	}
 }
+
+func testSetup(t *testing.T) (*Webhook, string, error) {
+	t.Helper()
+	port, err := newTestPort()
+	if err != nil {
+		return nil, "", err
+	}
+
+	defaultOpts := newDefaultOptions()
+	defaultOpts.Port = port
+	kubeClient, ac := newNonRunningTestWebhook(t, defaultOpts)
+
+	nsErr := createNamespace(t, kubeClient, metav1.NamespaceSystem)
+	if nsErr != nil {
+		return nil, "", nsErr
+	}
+
+	cMapsErr := createTestConfigMap(t, kubeClient)
+	if cMapsErr != nil {
+		return nil, "", cMapsErr
+	}
+
+	createDeployment(kubeClient)
+	resetMetrics()
+	return ac, fmt.Sprintf("0.0.0.0:%d", port), nil
+}

webhook/webhook_test.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/client-go/kubernetes"
 	fakekubeclientset "k8s.io/client-go/kubernetes/fake"
 
+	"knative.dev/pkg/configmap"
 	. "knative.dev/pkg/logging/testing"
 )
 
@@ -44,6 +45,8 @@ func newDefaultOptions() ControllerOptions {
 		SecretName:                      "webhook-certs",
 		ResourceMutatingWebhookName:     "webhook.knative.dev",
 		ResourceAdmissionControllerPath: "/",
+		ConfigValidationWebhookName:     "configmap.webhook.knative.dev",
+		ConfigValidationControllerPath:  "/config-validation",
 	}
 }
 
@@ -104,7 +107,7 @@ func TestRegistrationStopChanFire(t *testing.T) {
 	}
 }
 
-func TestRegistrationForAlreadyExistingWebhook(t *testing.T) {
+func TestRegistrationForAlreadyExistingResourceController(t *testing.T) {
 	kubeClient, ac := newNonRunningTestWebhook(t, newDefaultOptions())
 	webhook := &admissionregistrationv1beta1.MutatingWebhookConfiguration{
 		ObjectMeta: metav1.ObjectMeta{
@@ -137,6 +140,39 @@ func TestRegistrationForAlreadyExistingWebhook(t *testing.T) {
 	}
 }
 
+func TestRegistrationForAlreadyExistingConfigValidationController(t *testing.T) {
+	kubeClient, ac := newNonRunningTestWebhook(t, newDefaultOptions())
+	webhook := &admissionregistrationv1beta1.ValidatingWebhookConfiguration{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: ac.Options.ConfigValidationWebhookName,
+		},
+		Webhooks: []admissionregistrationv1beta1.ValidatingWebhook{
+			{
+				Name:         ac.Options.ConfigValidationWebhookName,
+				Rules:        []admissionregistrationv1beta1.RuleWithOperations{{}},
+				ClientConfig: admissionregistrationv1beta1.WebhookClientConfig{},
+			},
+		},
+	}
+	createConfigValidationWebhook(kubeClient, webhook)
+
+	ac.Options.RegistrationDelay = 1 * time.Millisecond
+	stopCh := make(chan struct{})
+
+	var g errgroup.Group
+	g.Go(func() error {
+		return ac.Run(stopCh)
+	})
+	err := g.Wait()
+	if err == nil {
+		t.Fatal("Expected webhook controller to fail")
+	}
+
+	if ac.Options.ClientAuth >= tls.VerifyClientCertIfGiven && !strings.Contains(err.Error(), "configmaps") {
+		t.Fatal("Expected error msg to contain configmap key missing error")
+	}
+}
+
 func TestCertConfigurationForAlreadyGeneratedSecret(t *testing.T) {
 	secretName := "test-secret"
 	ns := "test-namespace"
@@ -225,8 +261,11 @@ func TestSettingWebhookClientAuth(t *testing.T) {
 
 func NewTestWebhook(client kubernetes.Interface, options ControllerOptions, logger *zap.SugaredLogger) (*Webhook, error) {
 	resourceHandlers := newResourceHandlers()
+	validations := configmap.Constructors{"test-config": newConfigFromConfigMap}
+
 	admissionControllers := map[string]AdmissionController{
 		options.ResourceAdmissionControllerPath: NewResourceAdmissionController(resourceHandlers, options, true),
+		options.ConfigValidationControllerPath:  NewConfigValidationController(validations, options),
 	}
 	return New(client, options, admissionControllers, logger, nil)
 }