From 02b77861ec00ca93fa906d8f2d1f7529e5a9abb9 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 18 Dec 2020 13:33:43 -0800 Subject: [PATCH 1/2] Move defaultingressclass admission to net subdir --- pkg/kubeapiserver/options/BUILD | 2 +- pkg/kubeapiserver/options/plugins.go | 2 +- plugin/BUILD | 2 +- plugin/pkg/admission/{ => network}/defaultingressclass/BUILD | 2 +- .../admission/{ => network}/defaultingressclass/admission.go | 0 .../{ => network}/defaultingressclass/admission_test.go | 0 6 files changed, 4 insertions(+), 4 deletions(-) rename plugin/pkg/admission/{ => network}/defaultingressclass/BUILD (95%) rename plugin/pkg/admission/{ => network}/defaultingressclass/admission.go (100%) rename plugin/pkg/admission/{ => network}/defaultingressclass/admission_test.go (100%) diff --git a/pkg/kubeapiserver/options/BUILD b/pkg/kubeapiserver/options/BUILD index 5c9e9ba6fd8..e473d84a757 100644 --- a/pkg/kubeapiserver/options/BUILD +++ b/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,7 @@ 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/noderestriction:go_default_library", "//plugin/pkg/admission/nodetaint:go_default_library", "//plugin/pkg/admission/podnodeselector:go_default_library", diff --git a/pkg/kubeapiserver/options/plugins.go b/pkg/kubeapiserver/options/plugins.go index 351a8ef8d00..8b6f33a6da0 100644 --- a/pkg/kubeapiserver/options/plugins.go +++ b/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,7 @@ 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/noderestriction" "k8s.io/kubernetes/plugin/pkg/admission/nodetaint" "k8s.io/kubernetes/plugin/pkg/admission/podnodeselector" diff --git a/plugin/BUILD b/plugin/BUILD index 26f5245a107..23ebfa6f8bd 100644 --- a/plugin/BUILD +++ b/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,7 @@ 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/noderestriction:all-srcs", "//plugin/pkg/admission/nodetaint:all-srcs", "//plugin/pkg/admission/podnodeselector:all-srcs", diff --git a/plugin/pkg/admission/defaultingressclass/BUILD b/plugin/pkg/admission/network/defaultingressclass/BUILD similarity index 95% rename from plugin/pkg/admission/defaultingressclass/BUILD rename to plugin/pkg/admission/network/defaultingressclass/BUILD index 367c3cf3b3d..e11d7f292e0 100644 --- a/plugin/pkg/admission/defaultingressclass/BUILD +++ b/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", diff --git a/plugin/pkg/admission/defaultingressclass/admission.go b/plugin/pkg/admission/network/defaultingressclass/admission.go similarity index 100% rename from plugin/pkg/admission/defaultingressclass/admission.go rename to plugin/pkg/admission/network/defaultingressclass/admission.go diff --git a/plugin/pkg/admission/defaultingressclass/admission_test.go b/plugin/pkg/admission/network/defaultingressclass/admission_test.go similarity index 100% rename from plugin/pkg/admission/defaultingressclass/admission_test.go rename to plugin/pkg/admission/network/defaultingressclass/admission_test.go From a8299079a5ec6a09927367e0bda403860ff50b39 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 18 Dec 2020 14:37:16 -0800 Subject: [PATCH 2/2] Add denyserviceexternalips admission --- pkg/kubeapiserver/options/BUILD | 1 + pkg/kubeapiserver/options/plugins.go | 31 +++-- plugin/BUILD | 1 + .../network/denyserviceexternalips/BUILD | 41 ++++++ .../denyserviceexternalips/admission.go | 114 +++++++++++++++++ .../denyserviceexternalips/admission_test.go | 121 ++++++++++++++++++ 6 files changed, 295 insertions(+), 14 deletions(-) create mode 100644 plugin/pkg/admission/network/denyserviceexternalips/BUILD create mode 100644 plugin/pkg/admission/network/denyserviceexternalips/admission.go create mode 100644 plugin/pkg/admission/network/denyserviceexternalips/admission_test.go diff --git a/pkg/kubeapiserver/options/BUILD b/pkg/kubeapiserver/options/BUILD index e473d84a757..72d0691eef8 100644 --- a/pkg/kubeapiserver/options/BUILD +++ b/pkg/kubeapiserver/options/BUILD @@ -36,6 +36,7 @@ go_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", diff --git a/pkg/kubeapiserver/options/plugins.go b/pkg/kubeapiserver/options/plugins.go index 8b6f33a6da0..1c74a611487 100644 --- a/pkg/kubeapiserver/options/plugins.go +++ b/pkg/kubeapiserver/options/plugins.go @@ -38,6 +38,7 @@ import ( "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 - limitranger.PluginName, //LimitRanger - serviceaccount.PluginName, //ServiceAccount - setdefault.PluginName, //DefaultStorageClass - resize.PluginName, //PersistentVolumeClaimResize - defaulttolerationseconds.PluginName, //DefaultTolerationSeconds - mutatingwebhook.PluginName, //MutatingAdmissionWebhook - validatingwebhook.PluginName, //ValidatingAdmissionWebhook - resourcequota.PluginName, //ResourceQuota - storageobjectinuseprotection.PluginName, //StorageObjectInUseProtection - podpriority.PluginName, //PodPriority - nodetaint.PluginName, //TaintNodesByCondition - runtimeclass.PluginName, //RuntimeClass + lifecycle.PluginName, // NamespaceLifecycle + limitranger.PluginName, // LimitRanger + serviceaccount.PluginName, // ServiceAccount + setdefault.PluginName, // DefaultStorageClass + resize.PluginName, // PersistentVolumeClaimResize + defaulttolerationseconds.PluginName, // DefaultTolerationSeconds + mutatingwebhook.PluginName, // MutatingAdmissionWebhook + validatingwebhook.PluginName, // ValidatingAdmissionWebhook + resourcequota.PluginName, // ResourceQuota + storageobjectinuseprotection.PluginName, // StorageObjectInUseProtection + podpriority.PluginName, // PodPriority + nodetaint.PluginName, // TaintNodesByCondition + runtimeclass.PluginName, // RuntimeClass certapproval.PluginName, // CertificateApproval certsigning.PluginName, // CertificateSigning certsubjectrestriction.PluginName, // CertificateSubjectRestriction - defaultingressclass.PluginName, //DefaultIngressClass + defaultingressclass.PluginName, // DefaultIngressClass ) return sets.NewString(AllOrderedPlugins...).Difference(defaultOnPlugins) diff --git a/plugin/BUILD b/plugin/BUILD index 23ebfa6f8bd..2cf14284a50 100644 --- a/plugin/BUILD +++ b/plugin/BUILD @@ -26,6 +26,7 @@ filegroup( "//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", diff --git a/plugin/pkg/admission/network/denyserviceexternalips/BUILD b/plugin/pkg/admission/network/denyserviceexternalips/BUILD new file mode 100644 index 00000000000..ad507325cc8 --- /dev/null +++ b/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"], +) diff --git a/plugin/pkg/admission/network/denyserviceexternalips/admission.go b/plugin/pkg/admission/network/denyserviceexternalips/admission.go new file mode 100644 index 00000000000..0a933079911 --- /dev/null +++ b/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 +} diff --git a/plugin/pkg/admission/network/denyserviceexternalips/admission_test.go b/plugin/pkg/admission/network/denyserviceexternalips/admission_test.go new file mode 100644 index 00000000000..cfd71b5e6dd --- /dev/null +++ b/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") + } + }) + } +}