From ced56a6adabdd86f99455b100b1c0c7a2b4f3c55 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Tue, 17 Oct 2023 14:06:46 -0400 Subject: [PATCH 1/2] Restrict supported media types for new apiservers. This is to prevent the enablement of new data formats (CBOR) in the early stages of phased implementation. --- .../src/k8s.io/apiserver/pkg/server/config.go | 18 ++++++++++++++++++ .../apiserver/pkg/server/config_test.go | 19 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/staging/src/k8s.io/apiserver/pkg/server/config.go b/staging/src/k8s.io/apiserver/pkg/server/config.go index 2f72ef776db..0b5781f4b96 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config.go @@ -724,6 +724,12 @@ func (c *RecommendedConfig) Complete() CompletedConfig { return c.Config.Complete(c.SharedInformerFactory) } +var allowedMediaTypes = []string{ + runtime.ContentTypeJSON, + runtime.ContentTypeYAML, + runtime.ContentTypeProtobuf, +} + // New creates a new server which logically combines the handling chain with the passed server. // name is used to differentiate for logging. The handler chain in particular can be difficult as it starts delegating. // delegationTarget may not be nil. @@ -731,6 +737,18 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G if c.Serializer == nil { return nil, fmt.Errorf("Genericapiserver.New() called with config.Serializer == nil") } + for _, info := range c.Serializer.SupportedMediaTypes() { + var ok bool + for _, mt := range allowedMediaTypes { + if info.MediaType == mt { + ok = true + break + } + } + if !ok { + return nil, fmt.Errorf("refusing to create new apiserver %q with support for media type %q (allowed media types are: %s)", name, info.MediaType, strings.Join(allowedMediaTypes, ", ")) + } + } if c.LoopbackClientConfig == nil { return nil, fmt.Errorf("Genericapiserver.New() called with config.LoopbackClientConfig == nil") } diff --git a/staging/src/k8s.io/apiserver/pkg/server/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/config_test.go index 6e4ef65ef1a..a1d6d8902f9 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/config_test.go @@ -26,6 +26,7 @@ import ( "testing" "time" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/json" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" @@ -362,3 +363,21 @@ func (b *testBackend) ProcessEvents(events ...*auditinternal.Event) bool { b.events = append(b.events, events...) return true } + +func TestNewErrorForbiddenSerializer(t *testing.T) { + config := CompletedConfig{ + &completedConfig{ + Config: &Config{ + Serializer: runtime.NewSimpleNegotiatedSerializer(runtime.SerializerInfo{ + MediaType: "application/cbor", + }), + }, + }, + } + _, err := config.New("test", NewEmptyDelegate()) + if err == nil { + t.Error("successfully created a new server configured with cbor support") + } else if err.Error() != `refusing to create new apiserver "test" with support for media type "application/cbor" (allowed media types are: application/json, application/yaml, application/vnd.kubernetes.protobuf)` { + t.Errorf("unexpected error: %v", err) + } +} From cf836309dc278d8d4f046e1580649179b1531143 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Thu, 19 Oct 2023 10:54:16 -0400 Subject: [PATCH 2/2] Add validation for --storage-media-type option. --- .../apiserver/pkg/server/options/etcd.go | 11 ++++++ .../apiserver/pkg/server/options/etcd_test.go | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go b/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go index d4f75ddd721..14a96f0ca74 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/etcd.go @@ -26,6 +26,7 @@ import ( "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" @@ -87,6 +88,12 @@ func NewEtcdOptions(backendConfig *storagebackend.Config) *EtcdOptions { return options } +var storageMediaTypes = sets.New( + runtime.ContentTypeJSON, + runtime.ContentTypeYAML, + runtime.ContentTypeProtobuf, +) + func (s *EtcdOptions) Validate() []error { if s == nil { return nil @@ -120,6 +127,10 @@ func (s *EtcdOptions) Validate() []error { allErrors = append(allErrors, fmt.Errorf("--encryption-provider-config-automatic-reload must be set with --encryption-provider-config")) } + if s.DefaultStorageMediaType != "" && !storageMediaTypes.Has(s.DefaultStorageMediaType) { + allErrors = append(allErrors, fmt.Errorf("--storage-media-type %q invalid, allowed values: %s", s.DefaultStorageMediaType, strings.Join(sets.List(storageMediaTypes), ", "))) + } + return allErrors } diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go index 89a1251bc1e..8c21a3880ee 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/etcd_test.go @@ -160,6 +160,40 @@ func TestEtcdOptionsValidate(t *testing.T) { EtcdServersOverrides: []string{"/events#http://127.0.0.1:4002"}, }, }, + { + name: "empty storage-media-type", + testOptions: &EtcdOptions{ + StorageConfig: storagebackend.Config{ + Transport: storagebackend.TransportConfig{ + ServerList: []string{"http://127.0.0.1"}, + }, + }, + DefaultStorageMediaType: "", + }, + }, + { + name: "recognized storage-media-type", + testOptions: &EtcdOptions{ + StorageConfig: storagebackend.Config{ + Transport: storagebackend.TransportConfig{ + ServerList: []string{"http://127.0.0.1"}, + }, + }, + DefaultStorageMediaType: "application/json", + }, + }, + { + name: "unrecognized storage-media-type", + testOptions: &EtcdOptions{ + StorageConfig: storagebackend.Config{ + Transport: storagebackend.TransportConfig{ + ServerList: []string{"http://127.0.0.1"}, + }, + }, + DefaultStorageMediaType: "foo/bar", + }, + expectErr: `--storage-media-type "foo/bar" invalid, allowed values: application/json, application/vnd.kubernetes.protobuf, application/yaml`, + }, } for _, testcase := range testCases {