From 7a92947588070a8eedd0bf50edcfbf0fcc1d4096 Mon Sep 17 00:00:00 2001 From: p0lyn0mial Date: Thu, 24 Aug 2017 21:36:39 +0200 Subject: [PATCH] adds two new fields to AdmissionOption. The first one being RecommendedPluginOrder the second one being DefaultOffPlugins. In case a cluster-admin did not provide plugin names they will be derived from these fields. --- .../app/options/options_test.go | 7 +- .../k8s.io/apiserver/pkg/server/options/BUILD | 51 +++++++------- .../apiserver/pkg/server/options/admission.go | 68 ++++++++++++++++--- .../pkg/server/options/admission_test.go | 68 +++++++++++++++++++ 4 files changed, 152 insertions(+), 42 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index 93a5d52ed6..64aee34198 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/cmd/kube-apiserver/app/options/options_test.go @@ -100,9 +100,10 @@ func TestAddFlags(t *testing.T) { MinRequestTimeout: 1800, }, Admission: &apiserveroptions.AdmissionOptions{ - PluginNames: []string{"AlwaysDeny"}, - ConfigFile: "/admission-control-config", - Plugins: s.Admission.Plugins, + RecommendedPluginOrder: []string{"NamespaceLifecycle"}, + PluginNames: []string{"AlwaysDeny"}, + ConfigFile: "/admission-control-config", + Plugins: s.Admission.Plugins, }, Etcd: &apiserveroptions.EtcdOptions{ StorageConfig: storagebackend.Config{ diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/BUILD b/staging/src/k8s.io/apiserver/pkg/server/options/BUILD index c7d7c2d5af..ed95665e7e 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/server/options/BUILD @@ -1,28 +1,4 @@ -package(default_visibility = ["//visibility:public"]) - -load( - "@io_bazel_rules_go//go:def.bzl", - "go_library", - "go_test", -) - -go_test( - name = "go_default_test", - srcs = ["serving_test.go"], - data = glob(["testdata/**"]), - library = ":go_default_library", - deps = [ - "//vendor/github.com/stretchr/testify/assert:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/version:go_default_library", - "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", - "//vendor/k8s.io/apiserver/pkg/server:go_default_library", - "//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library", - "//vendor/k8s.io/client-go/discovery:go_default_library", - "//vendor/k8s.io/client-go/rest:go_default_library", - ], -) +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -39,6 +15,7 @@ go_library( "server_run_options.go", "serving.go", ], + visibility = ["//visibility:public"], deps = [ "//vendor/github.com/golang/glog:go_default_library", "//vendor/github.com/pborman/uuid:go_default_library", @@ -52,6 +29,7 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library", + "//vendor/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle:go_default_library", "//vendor/k8s.io/apiserver/pkg/apis/audit/v1beta1:go_default_library", "//vendor/k8s.io/apiserver/pkg/audit:go_default_library", "//vendor/k8s.io/apiserver/pkg/audit/policy:go_default_library", @@ -80,9 +58,25 @@ go_library( ], ) -filegroup( - name = "testdata", - srcs = glob(["testdata/*"]), +go_test( + name = "go_default_test", + srcs = [ + "admission_test.go", + "serving_test.go", + ], + data = glob(["testdata/**"]), + library = ":go_default_library", + deps = [ + "//vendor/github.com/stretchr/testify/assert:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/version:go_default_library", + "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", + "//vendor/k8s.io/apiserver/pkg/server:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library", + "//vendor/k8s.io/client-go/discovery:go_default_library", + "//vendor/k8s.io/client-go/rest:go_default_library", + ], ) filegroup( @@ -99,4 +93,5 @@ filegroup( "//staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig:all-srcs", ], tags = ["automanaged"], + visibility = ["//visibility:public"], ) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/admission.go b/staging/src/k8s.io/apiserver/pkg/server/options/admission.go index 40c34ce3d3..8f385f52a9 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/admission.go @@ -23,6 +23,7 @@ import ( "github.com/spf13/pflag" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/initializer" + "k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle" "k8s.io/apiserver/pkg/server" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" @@ -30,19 +31,28 @@ import ( // AdmissionOptions holds the admission options type AdmissionOptions struct { - PluginNames []string - ConfigFile string - Plugins *admission.Plugins + // RecommendedPluginOrder holds an ordered list of plugin names we recommend to use by default + RecommendedPluginOrder []string + // DefaultOffPlugins a list of plugin names that should be disabled by default + DefaultOffPlugins []string + PluginNames []string + ConfigFile string + Plugins *admission.Plugins } // NewAdmissionOptions creates a new instance of AdmissionOptions // Note: -// In addition it calls RegisterAllAdmissionPlugins to register -// all generic admission plugins. +// In addition it calls RegisterAllAdmissionPlugins to register +// all generic admission plugins. +// +// Provides the list of RecommendedPluginOrder that holds sane values +// that can be used by servers that don't care about admission chain. +// Servers that do care can overwrite/append that field after creation. func NewAdmissionOptions() *AdmissionOptions { options := &AdmissionOptions{ - Plugins: &admission.Plugins{}, - PluginNames: []string{}, + Plugins: &admission.Plugins{}, + PluginNames: []string{}, + RecommendedPluginOrder: []string{lifecycle.PluginName}, } server.RegisterAllAdmissionPlugins(options.Plugins) return options @@ -58,14 +68,20 @@ func (a *AdmissionOptions) AddFlags(fs *pflag.FlagSet) { "File with admission control configuration.") } -// ApplyTo adds the admission chain to the server configuration -// the method lazily initializes a generic plugin that is appended to the list of pluginInitializers +// ApplyTo adds the admission chain to the server configuration. +// In case admission plugin names were not provided by a custer-admin they will be prepared from the recommended/default values. +// In addition the method lazily initializes a generic plugin that is appended to the list of pluginInitializers // note this method uses: // genericconfig.LoopbackClientConfig // genericconfig.SharedInformerFactory // genericconfig.Authorizer func (a *AdmissionOptions) ApplyTo(c *server.Config, informers informers.SharedInformerFactory, pluginInitializers ...admission.PluginInitializer) error { - pluginsConfigProvider, err := admission.ReadAdmissionConfiguration(a.PluginNames, a.ConfigFile) + pluginNames := a.PluginNames + if len(a.PluginNames) == 0 { + pluginNames = a.enabledPluginNames() + } + + pluginsConfigProvider, err := admission.ReadAdmissionConfiguration(pluginNames, a.ConfigFile) if err != nil { return fmt.Errorf("failed to read plugin config: %v", err) } @@ -82,7 +98,7 @@ func (a *AdmissionOptions) ApplyTo(c *server.Config, informers informers.SharedI pluginInitializers = append(pluginInitializers, genericInitializer) initializersChain = append(initializersChain, pluginInitializers...) - admissionChain, err := a.Plugins.NewFromPlugins(a.PluginNames, pluginsConfigProvider, initializersChain) + admissionChain, err := a.Plugins.NewFromPlugins(pluginNames, pluginsConfigProvider, initializersChain) if err != nil { return err } @@ -95,3 +111,33 @@ func (a *AdmissionOptions) Validate() []error { errs := []error{} return errs } + +// enabledPluginNames makes use of RecommendedPluginOrder and DefaultOffPlugins fields +// to prepare a list of plugin names that are enabled. +// +// TODO(p0lyn0mial): In the end we will introduce two new flags: +// --disable-admission-plugin this would be a list of admission plugins that a cluster-admin wants to explicitly disable. +// --enable-admission-plugin this would be a list of admission plugins that a cluster-admin wants to explicitly enable. +// both flags are going to be handled by this method +func (a *AdmissionOptions) enabledPluginNames() []string { + //TODO(p0lyn0mial): first subtract plugins that a user wants to explicitly enable from allOffPlugins (DefaultOffPlugins) + //TODO(p0lyn0miial): then add/append plugins that a user wants to explicitly disable to allOffPlugins + //TODO(p0lyn0mial): so that --off=three --on=one,three default-off=one,two results in "one" being enabled. + allOffPlugins := a.DefaultOffPlugins + onlyEnabledPluginNames := []string{} + for _, pluginName := range a.RecommendedPluginOrder { + disablePlugin := false + for _, disabledPluginName := range allOffPlugins { + if pluginName == disabledPluginName { + disablePlugin = true + break + } + } + if disablePlugin { + continue + } + onlyEnabledPluginNames = append(onlyEnabledPluginNames, pluginName) + } + + return onlyEnabledPluginNames +} diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go new file mode 100644 index 0000000000..0dfcbccce0 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/server/options/admission_test.go @@ -0,0 +1,68 @@ +/* +Copyright 2017 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 options + +import ( + "fmt" + "testing" +) + +func TestEnabledPluginNamesMethod(t *testing.T) { + scenarios := []struct { + expectedPluginNames []string + setDefaultOffPluginNames []string + setRecommendedPluginOrder []string + }{ + // scenario 1: check if a call to enabledPluginNames sets expected values. + { + expectedPluginNames: []string{"NamespaceLifecycle"}, + }, + + // scenario 2: overwrite RecommendedPluginOrder and set DefaultOffPluginNames + // make sure that plugins which are on DefaultOffPluginNames list do not get to PluginNames list. + { + expectedPluginNames: []string{"pluginA"}, + setRecommendedPluginOrder: []string{"pluginA", "pluginB"}, + setDefaultOffPluginNames: []string{"pluginB"}, + }, + } + + // act + for index, scenario := range scenarios { + t.Run(fmt.Sprintf("scenario %d", index), func(t *testing.T) { + target := NewAdmissionOptions() + + if len(scenario.setDefaultOffPluginNames) > 0 { + target.DefaultOffPlugins = scenario.setDefaultOffPluginNames + } + if len(scenario.setRecommendedPluginOrder) > 0 { + target.RecommendedPluginOrder = scenario.setRecommendedPluginOrder + } + + actualPluginNames := target.enabledPluginNames() + + if len(actualPluginNames) != len(scenario.expectedPluginNames) { + t.Errorf("incorrect number of items, got %d, expected = %d", len(actualPluginNames), len(scenario.expectedPluginNames)) + } + for i := range actualPluginNames { + if scenario.expectedPluginNames[i] != actualPluginNames[i] { + t.Errorf("missmatch at index = %d, got = %s, expected = %s", i, actualPluginNames[i], scenario.expectedPluginNames[i]) + } + } + }) + } +}