From 1a552bbe149373c056ee004304d7e5abaa89f4c6 Mon Sep 17 00:00:00 2001 From: "Dr. Stefan Schimanski" Date: Mon, 27 Nov 2017 14:44:04 +0100 Subject: [PATCH] admission: do not leak admission config types outside of the plugins --- .../pkg/admission/eventratelimit/admission.go | 5 - .../podtolerationrestriction/admission.go | 4 - .../pkg/admission/resourcequota/admission.go | 5 - .../src/k8s.io/apiserver/pkg/admission/BUILD | 1 + .../k8s.io/apiserver/pkg/admission/config.go | 16 +-- .../apiserver/pkg/admission/config_test.go | 100 +++++++++++++++++- .../admission/plugin/webhook/validating/BUILD | 2 - .../plugin/webhook/validating/admission.go | 5 - .../k8s.io/apiserver/pkg/admission/plugins.go | 10 +- .../apiserver/pkg/apis/apiserver/types.go | 2 +- .../pkg/apis/apiserver/v1alpha1/conversion.go | 88 --------------- .../pkg/apis/apiserver/v1alpha1/types.go | 2 +- .../apiserver/pkg/server/options/admission.go | 11 +- vendor/github.com/jmespath/go-jmespath/BUILD | 5 +- 14 files changed, 119 insertions(+), 137 deletions(-) delete mode 100644 staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/conversion.go diff --git a/plugin/pkg/admission/eventratelimit/admission.go b/plugin/pkg/admission/eventratelimit/admission.go index 8cd64ebe58..7e02531969 100644 --- a/plugin/pkg/admission/eventratelimit/admission.go +++ b/plugin/pkg/admission/eventratelimit/admission.go @@ -23,7 +23,6 @@ import ( "k8s.io/client-go/util/flowcontrol" api "k8s.io/kubernetes/pkg/apis/core" eventratelimitapi "k8s.io/kubernetes/plugin/pkg/admission/eventratelimit/apis/eventratelimit" - eventratelimitapiv1alpha1 "k8s.io/kubernetes/plugin/pkg/admission/eventratelimit/apis/eventratelimit/v1alpha1" "k8s.io/kubernetes/plugin/pkg/admission/eventratelimit/apis/eventratelimit/validation" ) @@ -44,10 +43,6 @@ func Register(plugins *admission.Plugins) { } return newEventRateLimit(configuration, realClock{}) }) - - // add our config types - eventratelimitapi.AddToScheme(plugins.ConfigScheme) - eventratelimitapiv1alpha1.AddToScheme(plugins.ConfigScheme) } // Plugin implements an admission controller that can enforce event rate limits diff --git a/plugin/pkg/admission/podtolerationrestriction/admission.go b/plugin/pkg/admission/podtolerationrestriction/admission.go index 3318e221b2..0bfe76696b 100644 --- a/plugin/pkg/admission/podtolerationrestriction/admission.go +++ b/plugin/pkg/admission/podtolerationrestriction/admission.go @@ -38,7 +38,6 @@ import ( "k8s.io/kubernetes/pkg/scheduler/algorithm" "k8s.io/kubernetes/pkg/util/tolerations" pluginapi "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction" - pluginapiv1alpha1 "k8s.io/kubernetes/plugin/pkg/admission/podtolerationrestriction/apis/podtolerationrestriction/v1alpha1" ) // Register registers a plugin @@ -50,9 +49,6 @@ func Register(plugins *admission.Plugins) { } return NewPodTolerationsPlugin(pluginConfig), nil }) - // add our config types - pluginapi.AddToScheme(plugins.ConfigScheme) - pluginapiv1alpha1.AddToScheme(plugins.ConfigScheme) } // The annotation keys for default and whitelist of tolerations diff --git a/plugin/pkg/admission/resourcequota/admission.go b/plugin/pkg/admission/resourcequota/admission.go index 24f8b6354b..c6e89aad80 100644 --- a/plugin/pkg/admission/resourcequota/admission.go +++ b/plugin/pkg/admission/resourcequota/admission.go @@ -28,7 +28,6 @@ import ( kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" "k8s.io/kubernetes/pkg/quota" resourcequotaapi "k8s.io/kubernetes/plugin/pkg/admission/resourcequota/apis/resourcequota" - resourcequotaapiv1alpha1 "k8s.io/kubernetes/plugin/pkg/admission/resourcequota/apis/resourcequota/v1alpha1" "k8s.io/kubernetes/plugin/pkg/admission/resourcequota/apis/resourcequota/validation" ) @@ -49,10 +48,6 @@ func Register(plugins *admission.Plugins) { } return NewResourceQuota(configuration, 5, make(chan struct{})) }) - - // add our config types - resourcequotaapi.AddToScheme(plugins.ConfigScheme) - resourcequotaapiv1alpha1.AddToScheme(plugins.ConfigScheme) } // QuotaAdmission implements an admission controller that can enforce quota constraints diff --git a/staging/src/k8s.io/apiserver/pkg/admission/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/BUILD index aab87e4579..4af97de951 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/BUILD @@ -20,6 +20,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/json:go_default_library", "//vendor/k8s.io/apiserver/pkg/apis/apiserver:go_default_library", "//vendor/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1:go_default_library", ], diff --git a/staging/src/k8s.io/apiserver/pkg/admission/config.go b/staging/src/k8s.io/apiserver/pkg/admission/config.go index eb97986120..e716e62238 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/config.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/config.go @@ -126,16 +126,10 @@ type configProvider struct { } // GetAdmissionPluginConfigurationFor returns a reader that holds the admission plugin configuration. -func GetAdmissionPluginConfigurationFor(pluginCfg apiserver.AdmissionPluginConfiguration, scheme *runtime.Scheme) (io.Reader, error) { - // if there is nothing nested in the object, we return the named location - obj := pluginCfg.Configuration - if obj != nil { - // serialize the configuration and build a reader for it - content, err := writeYAML(obj, scheme) - if err != nil { - return nil, err - } - return bytes.NewBuffer(content), nil +func GetAdmissionPluginConfigurationFor(pluginCfg apiserver.AdmissionPluginConfiguration) (io.Reader, error) { + // if there is a nest object, return it directly + if pluginCfg.Configuration != nil { + return bytes.NewBuffer(pluginCfg.Configuration.Raw), nil } // there is nothing nested, so we delegate to path if pluginCfg.Path != "" { @@ -162,7 +156,7 @@ func (p configProvider) ConfigFor(pluginName string) (io.Reader, error) { if pluginName != pluginCfg.Name { continue } - pluginConfig, err := GetAdmissionPluginConfigurationFor(pluginCfg, p.scheme) + pluginConfig, err := GetAdmissionPluginConfigurationFor(pluginCfg) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/config_test.go b/staging/src/k8s.io/apiserver/pkg/admission/config_test.go index debde2463d..67d8a1a562 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/config_test.go @@ -23,6 +23,7 @@ import ( "testing" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/json" "k8s.io/apiserver/pkg/apis/apiserver" apiserverapi "k8s.io/apiserver/pkg/apis/apiserver" apiserverapiv1alpha1 "k8s.io/apiserver/pkg/apis/apiserver/v1alpha1" @@ -49,7 +50,7 @@ func TestReadAdmissionConfiguration(t *testing.T) { ExpectedAdmissionConfig *apiserver.AdmissionConfiguration PluginNames []string }{ - "v1Alpha1 configuration - path fixup": { + "v1alpha1 configuration - path fixup": { ConfigBody: `{ "apiVersion": "apiserver.k8s.io/v1alpha1", "kind": "AdmissionConfiguration", @@ -70,7 +71,7 @@ func TestReadAdmissionConfiguration(t *testing.T) { }, PluginNames: []string{}, }, - "v1Alpha1 configuration - abspath": { + "v1alpha1 configuration - abspath": { ConfigBody: `{ "apiVersion": "apiserver.k8s.io/v1alpha1", "kind": "AdmissionConfiguration", @@ -153,3 +154,98 @@ func TestReadAdmissionConfiguration(t *testing.T) { } } } + +func TestEmbeddedConfiguration(t *testing.T) { + // create a place holder file to hold per test config + configFile, err := ioutil.TempFile("", "admission-plugin-config") + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if err = configFile.Close(); err != nil { + t.Fatalf("unexpected err: %v", err) + } + configFileName := configFile.Name() + + testCases := map[string]struct { + ConfigBody string + ExpectedConfig string + }{ + "versioned configuration": { + ConfigBody: `{ + "apiVersion": "apiserver.k8s.io/v1alpha1", + "kind": "AdmissionConfiguration", + "plugins": [ + { + "name": "Foo", + "configuration": { + "apiVersion": "foo.admission.k8s.io/v1alpha1", + "kind": "Configuration", + "foo": "bar" + } + } + ]}`, + ExpectedConfig: `{ + "apiVersion": "foo.admission.k8s.io/v1alpha1", + "kind": "Configuration", + "foo": "bar" + }`, + }, + "legacy configuration": { + ConfigBody: `{ + "apiVersion": "apiserver.k8s.io/v1alpha1", + "kind": "AdmissionConfiguration", + "plugins": [ + { + "name": "Foo", + "configuration": { + "foo": "bar" + } + } + ]}`, + ExpectedConfig: `{ + "foo": "bar" + }`, + }, + } + + for desc, test := range testCases { + scheme := runtime.NewScheme() + apiserverapi.AddToScheme(scheme) + apiserverapiv1alpha1.AddToScheme(scheme) + + if err = ioutil.WriteFile(configFileName, []byte(test.ConfigBody), 0644); err != nil { + t.Errorf("[%s] unexpected err writing temp file: %v", desc, err) + continue + } + config, err := ReadAdmissionConfiguration([]string{"Foo"}, configFileName, scheme) + if err != nil { + t.Errorf("[%s] unexpected err: %v", desc, err) + continue + } + r, err := config.ConfigFor("Foo") + if err != nil { + t.Errorf("[%s] Failed to get Foo config: %v", desc, err) + continue + } + bs, err := ioutil.ReadAll(r) + if err != nil { + t.Errorf("[%s] Failed to read Foo config data: %v", desc, err) + continue + } + + if !equalJSON(test.ExpectedConfig, string(bs)) { + t.Errorf("Unexpected config: expected=%q got=%q", test.ExpectedConfig, string(bs)) + } + } +} + +func equalJSON(a, b string) bool { + var x, y interface{} + if err := json.Unmarshal([]byte(a), &x); err != nil { + return false + } + if err := json.Unmarshal([]byte(b), &y); err != nil { + return false + } + return reflect.DeepEqual(x, y) +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD index 5ab45072db..4226a13912 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD @@ -23,8 +23,6 @@ go_library( "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/metrics:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config:go_default_library", - "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config/apis/webhookadmission:go_default_library", - "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/config/apis/webhookadmission/v1alpha1:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/errors:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/namespace:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/plugin/webhook/request:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go index b88556631c..f68e46fa58 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go @@ -40,8 +40,6 @@ import ( genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer" admissionmetrics "k8s.io/apiserver/pkg/admission/metrics" "k8s.io/apiserver/pkg/admission/plugin/webhook/config" - webhookadmissionapi "k8s.io/apiserver/pkg/admission/plugin/webhook/config/apis/webhookadmission" - webhookadmissionapiv1alpha1 "k8s.io/apiserver/pkg/admission/plugin/webhook/config/apis/webhookadmission/v1alpha1" webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors" "k8s.io/apiserver/pkg/admission/plugin/webhook/namespace" "k8s.io/apiserver/pkg/admission/plugin/webhook/request" @@ -66,9 +64,6 @@ func Register(plugins *admission.Plugins) { return plugin, nil }) - // add our config types - webhookadmissionapi.AddToScheme(plugins.ConfigScheme) - webhookadmissionapiv1alpha1.AddToScheme(plugins.ConfigScheme) } // WebhookSource can list dynamic webhook plugins. diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugins.go b/staging/src/k8s.io/apiserver/pkg/admission/plugins.go index 3ede44a173..05e321ffc1 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugins.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugins.go @@ -25,8 +25,6 @@ import ( "sort" "sync" - "k8s.io/apimachinery/pkg/runtime" - "github.com/golang/glog" ) @@ -39,16 +37,10 @@ type Factory func(config io.Reader) (Interface, error) type Plugins struct { lock sync.Mutex registry map[string]Factory - - // ConfigScheme is used to parse the admission plugin config file. - // It is exposed to act as a hook for extending server providing their own config. - ConfigScheme *runtime.Scheme } func NewPlugins() *Plugins { - return &Plugins{ - ConfigScheme: runtime.NewScheme(), - } + return &Plugins{} } // All registered admission options. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go index f84fd04a34..e55da95f95 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/types.go @@ -46,5 +46,5 @@ type AdmissionPluginConfiguration struct { // Configuration is an embedded configuration object to be used as the plugin's // configuration. If present, it will be used instead of the path to the configuration file. // +optional - Configuration runtime.Object + Configuration *runtime.Unknown } diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/conversion.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/conversion.go deleted file mode 100644 index 378cc080d3..0000000000 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/conversion.go +++ /dev/null @@ -1,88 +0,0 @@ -/* -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 v1alpha1 - -import ( - runtime "k8s.io/apimachinery/pkg/runtime" -) - -var _ runtime.NestedObjectDecoder = &AdmissionConfiguration{} - -// DecodeNestedObjects handles encoding RawExtensions on the AdmissionConfiguration, ensuring the -// objects are decoded with the provided decoder. -func (c *AdmissionConfiguration) DecodeNestedObjects(d runtime.Decoder) error { - // decoding failures result in a runtime.Unknown object being created in Object and passed - // to conversion - for k, v := range c.Plugins { - decodeNestedRawExtensionOrUnknown(d, &v.Configuration) - c.Plugins[k] = v - } - return nil -} - -var _ runtime.NestedObjectEncoder = &AdmissionConfiguration{} - -// EncodeNestedObjects handles encoding RawExtensions on the AdmissionConfiguration, ensuring the -// objects are encoded with the provided encoder. -func (c *AdmissionConfiguration) EncodeNestedObjects(e runtime.Encoder) error { - for k, v := range c.Plugins { - if err := encodeNestedRawExtension(e, &v.Configuration); err != nil { - return err - } - c.Plugins[k] = v - } - return nil -} - -// decodeNestedRawExtensionOrUnknown decodes the raw extension into an object once. If called -// On a RawExtension that has already been decoded (has an object), it will not run again. -func decodeNestedRawExtensionOrUnknown(d runtime.Decoder, ext *runtime.RawExtension) { - if ext.Raw == nil || ext.Object != nil { - return - } - obj, gvk, err := d.Decode(ext.Raw, nil, nil) - if err != nil { - unk := &runtime.Unknown{Raw: ext.Raw} - if runtime.IsNotRegisteredError(err) { - if _, gvk, err := d.Decode(ext.Raw, nil, unk); err == nil { - unk.APIVersion = gvk.GroupVersion().String() - unk.Kind = gvk.Kind - ext.Object = unk - return - } - } - // TODO: record mime-type with the object - if gvk != nil { - unk.APIVersion = gvk.GroupVersion().String() - unk.Kind = gvk.Kind - } - obj = unk - } - ext.Object = obj -} - -func encodeNestedRawExtension(e runtime.Encoder, ext *runtime.RawExtension) error { - if ext.Raw != nil || ext.Object == nil { - return nil - } - data, err := runtime.Encode(e, ext.Object) - if err != nil { - return err - } - ext.Raw = data - return nil -} diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go index 522c41c414..239b8e20e0 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/v1alpha1/types.go @@ -46,5 +46,5 @@ type AdmissionPluginConfiguration struct { // Configuration is an embedded configuration object to be used as the plugin's // configuration. If present, it will be used instead of the path to the configuration file. // +optional - Configuration runtime.RawExtension `json:"configuration"` + Configuration *runtime.Unknown `json:"configuration"` } 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 aa180378dc..66b0b97ba6 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/admission.go @@ -38,6 +38,13 @@ import ( "k8s.io/client-go/rest" ) +var scheme = runtime.NewScheme() + +func init() { + apiserverapi.AddToScheme(scheme) + apiserverapiv1alpha1.AddToScheme(scheme) +} + // AdmissionOptions holds the admission options type AdmissionOptions struct { // RecommendedPluginOrder holds an ordered list of plugin names we recommend to use by default @@ -69,8 +76,6 @@ func NewAdmissionOptions() *AdmissionOptions { RecommendedPluginOrder: []string{lifecycle.PluginName, initialization.PluginName, mutatingwebhook.PluginName, validatingwebhook.PluginName}, DefaultOffPlugins: []string{initialization.PluginName, mutatingwebhook.PluginName, validatingwebhook.PluginName}, } - apiserverapi.AddToScheme(options.Plugins.ConfigScheme) - apiserverapiv1alpha1.AddToScheme(options.Plugins.ConfigScheme) server.RegisterAllAdmissionPlugins(options.Plugins) return options } @@ -120,7 +125,7 @@ func (a *AdmissionOptions) ApplyTo( pluginNames = a.enabledPluginNames() } - pluginsConfigProvider, err := admission.ReadAdmissionConfiguration(pluginNames, a.ConfigFile, a.Plugins.ConfigScheme) + pluginsConfigProvider, err := admission.ReadAdmissionConfiguration(pluginNames, a.ConfigFile, scheme) if err != nil { return fmt.Errorf("failed to read plugin config: %v", err) } diff --git a/vendor/github.com/jmespath/go-jmespath/BUILD b/vendor/github.com/jmespath/go-jmespath/BUILD index f4c95791b8..a3dbf5f5c3 100644 --- a/vendor/github.com/jmespath/go-jmespath/BUILD +++ b/vendor/github.com/jmespath/go-jmespath/BUILD @@ -25,7 +25,10 @@ filegroup( filegroup( name = "all-srcs", - srcs = [":package-srcs"], + srcs = [ + ":package-srcs", + "//vendor/github.com/jmespath/go-jmespath/cmd/jpgo:all-srcs", + ], tags = ["automanaged"], visibility = ["//visibility:public"], )