Merge pull request #97395 from thockin/externalips-admission

Add denyserviceexternalips admission (KEP 2200)

pkg/kubeapiserver/options/BUILD

@@ -25,7 +25,6 @@ go_library(
         "//plugin/pkg/admission/certificates/approval:go_default_library",
         "//plugin/pkg/admission/certificates/signing:go_default_library",
         "//plugin/pkg/admission/certificates/subjectrestriction:go_default_library",
-        "//plugin/pkg/admission/defaultingressclass:go_default_library",
         "//plugin/pkg/admission/defaulttolerationseconds:go_default_library",
         "//plugin/pkg/admission/deny:go_default_library",
         "//plugin/pkg/admission/eventratelimit:go_default_library",
@@ -36,6 +35,8 @@ go_library(
         "//plugin/pkg/admission/limitranger:go_default_library",
         "//plugin/pkg/admission/namespace/autoprovision:go_default_library",
         "//plugin/pkg/admission/namespace/exists:go_default_library",
+        "//plugin/pkg/admission/network/defaultingressclass:go_default_library",
+        "//plugin/pkg/admission/network/denyserviceexternalips:go_default_library",
         "//plugin/pkg/admission/noderestriction:go_default_library",
         "//plugin/pkg/admission/nodetaint:go_default_library",
         "//plugin/pkg/admission/podnodeselector:go_default_library",

pkg/kubeapiserver/options/plugins.go

@@ -27,7 +27,6 @@ import (
 	certapproval "k8s.io/kubernetes/plugin/pkg/admission/certificates/approval"
 	certsigning "k8s.io/kubernetes/plugin/pkg/admission/certificates/signing"
 	certsubjectrestriction "k8s.io/kubernetes/plugin/pkg/admission/certificates/subjectrestriction"
-	"k8s.io/kubernetes/plugin/pkg/admission/defaultingressclass"
 	"k8s.io/kubernetes/plugin/pkg/admission/defaulttolerationseconds"
 	"k8s.io/kubernetes/plugin/pkg/admission/deny"
 	"k8s.io/kubernetes/plugin/pkg/admission/eventratelimit"
@@ -38,6 +37,8 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/admission/limitranger"
 	"k8s.io/kubernetes/plugin/pkg/admission/namespace/autoprovision"
 	"k8s.io/kubernetes/plugin/pkg/admission/namespace/exists"
+	"k8s.io/kubernetes/plugin/pkg/admission/network/defaultingressclass"
+	"k8s.io/kubernetes/plugin/pkg/admission/network/denyserviceexternalips"
 	"k8s.io/kubernetes/plugin/pkg/admission/noderestriction"
 	"k8s.io/kubernetes/plugin/pkg/admission/nodetaint"
 	"k8s.io/kubernetes/plugin/pkg/admission/podnodeselector"
@@ -93,6 +94,7 @@ var AllOrderedPlugins = []string{
 	certsigning.PluginName,                  // CertificateSigning
 	certsubjectrestriction.PluginName,       // CertificateSubjectRestriction
 	defaultingressclass.PluginName,          // DefaultIngressClass
+	denyserviceexternalips.PluginName,       // DenyServiceExternalIPs
 
 	// new admission plugins should generally be inserted above here
 	// webhook, resourcequota, and deny plugins must go at the end
@@ -111,6 +113,7 @@ func RegisterAllAdmissionPlugins(plugins *admission.Plugins) {
 	antiaffinity.Register(plugins)
 	defaulttolerationseconds.Register(plugins)
 	defaultingressclass.Register(plugins)
+	denyserviceexternalips.Register(plugins)
 	deny.Register(plugins) // DEPRECATED as no real meaning
 	eventratelimit.Register(plugins)
 	exec.Register(plugins)
@@ -142,23 +145,23 @@ func RegisterAllAdmissionPlugins(plugins *admission.Plugins) {
 // DefaultOffAdmissionPlugins get admission plugins off by default for kube-apiserver.
 func DefaultOffAdmissionPlugins() sets.String {
 	defaultOnPlugins := sets.NewString(
-		lifecycle.PluginName,                    //NamespaceLifecycle
+		lifecycle.PluginName,                    // NamespaceLifecycle
-		limitranger.PluginName,                  //LimitRanger
+		limitranger.PluginName,                  // LimitRanger
-		serviceaccount.PluginName,               //ServiceAccount
+		serviceaccount.PluginName,               // ServiceAccount
-		setdefault.PluginName,                   //DefaultStorageClass
+		setdefault.PluginName,                   // DefaultStorageClass
-		resize.PluginName,                       //PersistentVolumeClaimResize
+		resize.PluginName,                       // PersistentVolumeClaimResize
-		defaulttolerationseconds.PluginName,     //DefaultTolerationSeconds
+		defaulttolerationseconds.PluginName,     // DefaultTolerationSeconds
-		mutatingwebhook.PluginName,              //MutatingAdmissionWebhook
+		mutatingwebhook.PluginName,              // MutatingAdmissionWebhook
-		validatingwebhook.PluginName,            //ValidatingAdmissionWebhook
+		validatingwebhook.PluginName,            // ValidatingAdmissionWebhook
-		resourcequota.PluginName,                //ResourceQuota
+		resourcequota.PluginName,                // ResourceQuota
-		storageobjectinuseprotection.PluginName, //StorageObjectInUseProtection
+		storageobjectinuseprotection.PluginName, // StorageObjectInUseProtection
-		podpriority.PluginName,                  //PodPriority
+		podpriority.PluginName,                  // PodPriority
-		nodetaint.PluginName,                    //TaintNodesByCondition
+		nodetaint.PluginName,                    // TaintNodesByCondition
-		runtimeclass.PluginName,                 //RuntimeClass
+		runtimeclass.PluginName,                 // RuntimeClass
 		certapproval.PluginName,                 // CertificateApproval
 		certsigning.PluginName,                  // CertificateSigning
 		certsubjectrestriction.PluginName,       // CertificateSubjectRestriction
-		defaultingressclass.PluginName,          //DefaultIngressClass
+		defaultingressclass.PluginName,          // DefaultIngressClass
 	)
 
 	return sets.NewString(AllOrderedPlugins...).Difference(defaultOnPlugins)

plugin/BUILD

@@ -15,7 +15,6 @@ filegroup(
         "//plugin/pkg/admission/alwayspullimages:all-srcs",
         "//plugin/pkg/admission/antiaffinity:all-srcs",
         "//plugin/pkg/admission/certificates:all-srcs",
-        "//plugin/pkg/admission/defaultingressclass:all-srcs",
         "//plugin/pkg/admission/defaulttolerationseconds:all-srcs",
         "//plugin/pkg/admission/deny:all-srcs",
         "//plugin/pkg/admission/eventratelimit:all-srcs",
@@ -26,6 +25,8 @@ filegroup(
         "//plugin/pkg/admission/limitranger:all-srcs",
         "//plugin/pkg/admission/namespace/autoprovision:all-srcs",
         "//plugin/pkg/admission/namespace/exists:all-srcs",
+        "//plugin/pkg/admission/network/defaultingressclass:all-srcs",
+        "//plugin/pkg/admission/network/denyserviceexternalips:all-srcs",
         "//plugin/pkg/admission/noderestriction:all-srcs",
         "//plugin/pkg/admission/nodetaint:all-srcs",
         "//plugin/pkg/admission/podnodeselector:all-srcs",

plugin/pkg/admission/network/defaultingressclass/BUILD

@@ -3,7 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 go_library(
     name = "go_default_library",
     srcs = ["admission.go"],
-    importpath = "k8s.io/kubernetes/plugin/pkg/admission/defaultingressclass",
+    importpath = "k8s.io/kubernetes/plugin/pkg/admission/network/defaultingressclass",
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/apis/networking:go_default_library",

plugin/pkg/admission/network/denyserviceexternalips/BUILD

@@ -0,0 +1,41 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "go_default_library",
+    srcs = ["admission.go"],
+    importpath = "k8s.io/kubernetes/plugin/pkg/admission/network/denyserviceexternalips",
+    visibility = ["//visibility:public"],
+    deps = [
+        "//pkg/apis/core:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
+        "//vendor/k8s.io/klog/v2:go_default_library",
+    ],
+)
+
+go_test(
+    name = "go_default_test",
+    srcs = ["admission_test.go"],
+    embed = [":go_default_library"],
+    deps = [
+        "//pkg/apis/core:go_default_library",
+        "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
+    ],
+)
+
+filegroup(
+    name = "package-srcs",
+    srcs = glob(["**"]),
+    tags = ["automanaged"],
+    visibility = ["//visibility:private"],
+)
+
+filegroup(
+    name = "all-srcs",
+    srcs = [":package-srcs"],
+    tags = ["automanaged"],
+    visibility = ["//visibility:public"],
+)

plugin/pkg/admission/network/denyserviceexternalips/admission.go

@@ -0,0 +1,114 @@
+/*
+Copyright 2020 The Kubernetes 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 denyserviceexternalips
+
+import (
+	"context"
+	"fmt"
+	"io"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/klog/v2"
+	"k8s.io/kubernetes/pkg/apis/core"
+)
+
+const (
+	// PluginName is the name of this admission controller plugin
+	PluginName = "DenyServiceExternalIPs"
+)
+
+// Register registers a plugin
+func Register(plugins *admission.Plugins) {
+	plugins.Register(PluginName, func(config io.Reader) (admission.Interface, error) {
+		plugin := newPlugin()
+		return plugin, nil
+	})
+}
+
+// externalIPsDenierPlugin holds state for and implements the admission plugin.
+type externalIPsDenierPlugin struct {
+	*admission.Handler
+}
+
+var _ admission.Interface = &externalIPsDenierPlugin{}
+var _ admission.ValidationInterface = &externalIPsDenierPlugin{}
+
+// newPlugin creates a new admission plugin.
+func newPlugin() *externalIPsDenierPlugin {
+	return &externalIPsDenierPlugin{
+		Handler: admission.NewHandler(admission.Create, admission.Update),
+	}
+}
+
+// Admit ensures that modifications of the Service.Spec.ExternalIPs field are
+// denied
+func (plug *externalIPsDenierPlugin) Validate(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) error {
+	if attr.GetResource().GroupResource() != core.Resource("services") {
+		return nil
+	}
+
+	if len(attr.GetSubresource()) != 0 {
+		return nil
+	}
+
+	// if we can't convert then we don't handle this object so just return
+	newSvc, ok := attr.GetObject().(*core.Service)
+	if !ok {
+		klog.V(3).Infof("Expected Service resource, got: %v", attr.GetKind())
+		return errors.NewInternalError(fmt.Errorf("Expected Service resource, got: %v", attr.GetKind()))
+	}
+
+	var oldSvc *core.Service
+	if old := attr.GetOldObject(); old != nil {
+		tmp, ok := old.(*core.Service)
+		if !ok {
+			klog.V(3).Infof("Expected Service resource, got: %v", attr.GetKind())
+			return errors.NewInternalError(fmt.Errorf("Expected Service resource, got: %v", attr.GetKind()))
+		}
+		oldSvc = tmp
+	}
+
+	if isSubset(newSvc, oldSvc) {
+		return nil
+	}
+
+	klog.V(4).Infof("Denying new use of ExternalIPs on Service %s/%s", newSvc.Namespace, newSvc.Name)
+	return admission.NewForbidden(attr, fmt.Errorf("Use of external IPs is denied by admission control"))
+}
+
+func isSubset(newSvc, oldSvc *core.Service) bool {
+	// If new has none, it's a subset.
+	if len(newSvc.Spec.ExternalIPs) == 0 {
+		return true
+	}
+	// If we have some but it's not an update, it's not a subset.
+	if oldSvc == nil {
+		return false
+	}
+	oldIPs := map[string]bool{}
+	for _, ip := range oldSvc.Spec.ExternalIPs {
+		oldIPs[ip] = true
+	}
+	// Every IP in newSvc must be in oldSvc
+	for _, ip := range newSvc.Spec.ExternalIPs {
+		if oldIPs[ip] == false {
+			return false
+		}
+	}
+	return true
+}

plugin/pkg/admission/network/denyserviceexternalips/admission_test.go

@@ -0,0 +1,121 @@
+/*
+Copyright 2020 The Kubernetes 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 denyserviceexternalips
+
+import (
+	"context"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/kubernetes/pkg/apis/core"
+	api "k8s.io/kubernetes/pkg/apis/core"
+)
+
+func makeSvc(externalIPs ...string) *core.Service {
+	svc := &core.Service{}
+	svc.Namespace = "test-ns"
+	svc.Name = "test-svc"
+	svc.Spec.ExternalIPs = externalIPs
+	return svc
+}
+
+func TestAdmission(t *testing.T) {
+	testCases := []struct {
+		name   string
+		newSvc *core.Service
+		oldSvc *core.Service
+		fail   bool
+	}{{
+		name:   "create: without externalIPs",
+		newSvc: makeSvc(),
+	}, {
+		name:   "create: with externalIPs",
+		newSvc: makeSvc("1.1.1.1"),
+		fail:   true,
+	}, {
+		name:   "update: same externalIPs",
+		newSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+		oldSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+	}, {
+		name:   "update: reorder externalIPs",
+		newSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+		oldSvc: makeSvc("2.2.2.2", "1.1.1.1"),
+	}, {
+		name:   "update: change externalIPs",
+		newSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+		oldSvc: makeSvc("1.1.1.1", "3.3.3.3"),
+		fail:   true,
+	}, {
+		name:   "update: add externalIPs",
+		newSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+		oldSvc: makeSvc("1.1.1.1"),
+		fail:   true,
+	}, {
+		name:   "update: erase externalIPs",
+		newSvc: makeSvc(),
+		oldSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+	}, {
+		name:   "update: reduce externalIPs from back",
+		newSvc: makeSvc("1.1.1.1"),
+		oldSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+	}, {
+		name:   "update: reduce externalIPs from front",
+		newSvc: makeSvc("2.2.2.2"),
+		oldSvc: makeSvc("1.1.1.1", "2.2.2.2"),
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ctrl := newPlugin()
+
+			var op admission.Operation
+			var opt runtime.Object
+			if tc.oldSvc == nil {
+				op = admission.Create
+				opt = &metav1.CreateOptions{}
+			} else {
+				op = admission.Update
+				opt = &metav1.UpdateOptions{}
+			}
+
+			attrs := admission.NewAttributesRecord(
+				tc.newSvc, // new object
+				tc.oldSvc, // old object
+				api.Kind("Service").WithVersion("version"),
+				tc.newSvc.Namespace,
+				tc.newSvc.Name,
+				corev1.Resource("services").WithVersion("version"),
+				"", // subresource
+				op,
+				opt,
+				false, // dryRun
+				nil,   // userInfo
+			)
+
+			err := ctrl.Validate(context.TODO(), attrs, nil)
+			if err != nil && !tc.fail {
+				t.Errorf("Unexpected failure: %v", err)
+			}
+			if err == nil && tc.fail {
+				t.Errorf("Unexpected success")
+			}
+		})
+	}
+}