diff --git a/hack/.golint_failures b/hack/.golint_failures index 9038c00c361..271b6b398df 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -353,6 +353,7 @@ staging/src/k8s.io/apiserver/pkg/apis/audit staging/src/k8s.io/apiserver/pkg/apis/audit/v1 staging/src/k8s.io/apiserver/pkg/apis/audit/v1alpha1 staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1 +staging/src/k8s.io/apiserver/pkg/apis/config/v1 staging/src/k8s.io/apiserver/pkg/apis/example staging/src/k8s.io/apiserver/pkg/apis/example/v1 staging/src/k8s.io/apiserver/pkg/apis/example2 diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/BUILD b/staging/src/k8s.io/apiserver/pkg/apis/config/BUILD index ce25409aaa8..a39a7e1fd0c 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/BUILD @@ -30,6 +30,7 @@ filegroup( srcs = [ ":package-srcs", "//staging/src/k8s.io/apiserver/pkg/apis/config/v1:all-srcs", + "//staging/src/k8s.io/apiserver/pkg/apis/config/validation:all-srcs", ], tags = ["automanaged"], visibility = ["//visibility:public"], diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/types.go b/staging/src/k8s.io/apiserver/pkg/apis/config/types.go index 5d4caaa5da0..4277c3da769 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/types.go @@ -17,6 +17,8 @@ limitations under the License. package config import ( + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -74,6 +76,11 @@ type Key struct { Secret string } +// String implements Stringer interface in a log safe way. +func (k Key) String() string { + return fmt.Sprintf("Name: %s, Secret: [REDACTED]", k.Name) +} + // IdentityConfiguration is an empty struct to allow identity transformer in provider configuration. type IdentityConfiguration struct{} @@ -83,7 +90,7 @@ type KMSConfiguration struct { Name string // cacheSize is the maximum number of secrets which are cached in memory. The default value is 1000. // +optional - CacheSize int32 + CacheSize *int32 // endpoint is the gRPC server listening address, for example "unix:///var/run/kms-provider.sock". Endpoint string // Timeout for gRPC calls to kms-plugin (ex. 5s). The default is 3 seconds. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/BUILD b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/BUILD index ca9eea027c4..13593dd7b0c 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/BUILD @@ -1,8 +1,9 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", srcs = [ + "defaults.go", "doc.go", "register.go", "types.go", @@ -35,3 +36,13 @@ filegroup( tags = ["automanaged"], visibility = ["//visibility:public"], ) + +go_test( + name = "go_default_test", + srcs = ["defaults_test.go"], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/github.com/google/go-cmp/cmp:go_default_library", + ], +) diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/defaults.go b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/defaults.go new file mode 100644 index 00000000000..2d529651a9f --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/defaults.go @@ -0,0 +1,44 @@ +/* +Copyright 2019 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 v1 + +import ( + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +var ( + defaultTimeout = &metav1.Duration{Duration: 3 * time.Second} + defaultCacheSize int32 = 1000 +) + +func addDefaultingFuncs(scheme *runtime.Scheme) error { + return RegisterDefaults(scheme) +} + +// SetDefaults_KMSConfiguration applies defaults to KMSConfiguration. +func SetDefaults_KMSConfiguration(obj *KMSConfiguration) { + if obj.Timeout == nil { + obj.Timeout = defaultTimeout + } + + if obj.CacheSize == nil { + obj.CacheSize = &defaultCacheSize + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/defaults_test.go b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/defaults_test.go new file mode 100644 index 00000000000..0b1909fe57e --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/defaults_test.go @@ -0,0 +1,92 @@ +/* +Copyright 2019 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 v1 + +import ( + "testing" + "time" + + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/google/go-cmp/cmp" +) + +func TestKMSProviderTimeoutDefaults(t *testing.T) { + testCases := []struct { + desc string + in *KMSConfiguration + want *KMSConfiguration + }{ + { + desc: "timeout not supplied", + in: &KMSConfiguration{}, + want: &KMSConfiguration{Timeout: defaultTimeout, CacheSize: &defaultCacheSize}, + }, + { + desc: "timeout supplied", + in: &KMSConfiguration{Timeout: &v1.Duration{Duration: 1 * time.Minute}}, + want: &KMSConfiguration{Timeout: &v1.Duration{Duration: 1 * time.Minute}, CacheSize: &defaultCacheSize}, + }, + } + + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + SetDefaults_KMSConfiguration(tt.in) + if d := cmp.Diff(tt.want, tt.in); d != "" { + t.Fatalf("KMS Provider mismatch (-want +got):\n%s", d) + } + }) + } +} + +func TestKMSProviderCacheDefaults(t *testing.T) { + var ( + zero int32 = 0 + ten int32 = 10 + ) + + testCases := []struct { + desc string + in *KMSConfiguration + want *KMSConfiguration + }{ + { + desc: "cache size not supplied", + in: &KMSConfiguration{}, + want: &KMSConfiguration{Timeout: defaultTimeout, CacheSize: &defaultCacheSize}, + }, + { + desc: "cache of zero size supplied", + in: &KMSConfiguration{CacheSize: &zero}, + want: &KMSConfiguration{Timeout: defaultTimeout, CacheSize: &zero}, + }, + { + desc: "positive cache size supplied", + in: &KMSConfiguration{CacheSize: &ten}, + want: &KMSConfiguration{Timeout: defaultTimeout, CacheSize: &ten}, + }, + } + + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + SetDefaults_KMSConfiguration(tt.in) + if d := cmp.Diff(tt.want, tt.in); d != "" { + t.Fatalf("KMS Provider mismatch (-want +got):\n%s", d) + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/register.go b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/register.go index 2e3ecfff2ea..32b5634c44e 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/register.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/register.go @@ -40,6 +40,7 @@ func init() { // generated functions takes place in the generated files. The separation // makes the code compile even when the generated files are missing. localSchemeBuilder.Register(addKnownTypes) + localSchemeBuilder.Register(addDefaultingFuncs) } func addKnownTypes(scheme *runtime.Scheme) error { diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/types.go b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/types.go index 1ac701bbad2..7ee20ba2419 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/types.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/types.go @@ -17,6 +17,8 @@ limitations under the License. package v1 import ( + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -74,6 +76,11 @@ type Key struct { Secret string `json:"secret"` } +// String implements Stringer interface in a log safe way. +func (k Key) String() string { + return fmt.Sprintf("Name: %s, Secret: [REDACTED]", k.Name) +} + // IdentityConfiguration is an empty struct to allow identity transformer in provider configuration. type IdentityConfiguration struct{} @@ -83,7 +90,7 @@ type KMSConfiguration struct { Name string `json:"name"` // cacheSize is the maximum number of secrets which are cached in memory. The default value is 1000. // +optional - CacheSize int32 `json:"cachesize,omitempty"` + CacheSize *int32 `json:"cachesize,omitempty"` // endpoint is the gRPC server listening address, for example "unix:///var/run/kms-provider.sock". Endpoint string `json:"endpoint"` // Timeout for gRPC calls to kms-plugin (ex. 5s). The default is 3 seconds. diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.conversion.go b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.conversion.go index 4a6843df048..c7de6539d85 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.conversion.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.conversion.go @@ -179,7 +179,7 @@ func Convert_config_IdentityConfiguration_To_v1_IdentityConfiguration(in *config func autoConvert_v1_KMSConfiguration_To_config_KMSConfiguration(in *KMSConfiguration, out *config.KMSConfiguration, s conversion.Scope) error { out.Name = in.Name - out.CacheSize = in.CacheSize + out.CacheSize = (*int32)(unsafe.Pointer(in.CacheSize)) out.Endpoint = in.Endpoint out.Timeout = (*metav1.Duration)(unsafe.Pointer(in.Timeout)) return nil @@ -192,7 +192,7 @@ func Convert_v1_KMSConfiguration_To_config_KMSConfiguration(in *KMSConfiguration func autoConvert_config_KMSConfiguration_To_v1_KMSConfiguration(in *config.KMSConfiguration, out *KMSConfiguration, s conversion.Scope) error { out.Name = in.Name - out.CacheSize = in.CacheSize + out.CacheSize = (*int32)(unsafe.Pointer(in.CacheSize)) out.Endpoint = in.Endpoint out.Timeout = (*metav1.Duration)(unsafe.Pointer(in.Timeout)) return nil diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.deepcopy.go b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.deepcopy.go index 9bd7732b062..dcb4e855297 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.deepcopy.go @@ -97,6 +97,11 @@ func (in *IdentityConfiguration) DeepCopy() *IdentityConfiguration { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KMSConfiguration) DeepCopyInto(out *KMSConfiguration) { *out = *in + if in.CacheSize != nil { + in, out := &in.CacheSize, &out.CacheSize + *out = new(int32) + **out = **in + } if in.Timeout != nil { in, out := &in.Timeout, &out.Timeout *out = new(metav1.Duration) diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.defaults.go b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.defaults.go index cce2e603a69..1c8db8d04f6 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.defaults.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/v1/zz_generated.defaults.go @@ -28,5 +28,18 @@ import ( // Public to allow building arbitrary schemes. // All generated defaulters are covering - they call all nested defaulters. func RegisterDefaults(scheme *runtime.Scheme) error { + scheme.AddTypeDefaultingFunc(&EncryptionConfiguration{}, func(obj interface{}) { SetObjectDefaults_EncryptionConfiguration(obj.(*EncryptionConfiguration)) }) return nil } + +func SetObjectDefaults_EncryptionConfiguration(in *EncryptionConfiguration) { + for i := range in.Resources { + a := &in.Resources[i] + for j := range a.Providers { + b := &a.Providers[j] + if b.KMS != nil { + SetDefaults_KMSConfiguration(b.KMS) + } + } + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/validation/BUILD b/staging/src/k8s.io/apiserver/pkg/apis/config/validation/BUILD new file mode 100644 index 00000000000..02aba21384d --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/validation/BUILD @@ -0,0 +1,39 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["validation.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apiserver/pkg/apis/config/validation", + importpath = "k8s.io/apiserver/pkg/apis/config/validation", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/config:go_default_library", + ], +) + +go_test( + name = "go_default_test", + srcs = ["validation_test.go"], + embed = [":go_default_library"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/config:go_default_library", + "//vendor/github.com/google/go-cmp/cmp: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/staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation.go new file mode 100644 index 00000000000..ec22f9ae958 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation.go @@ -0,0 +1,219 @@ +/* +Copyright 2019 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 validation validates EncryptionConfiguration. +package validation + +import ( + "encoding/base64" + "fmt" + "net/url" + + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/apis/config" +) + +const ( + moreThanOneElementErr = "more than one provider specified in a single element, should split into different list elements" + keyLenErrFmt = "secret is not of the expected length, got %d, expected one of %v" + unsupportedSchemeErrFmt = "unsupported scheme %q for KMS provider, only unix is supported" + atLeastOneRequiredErrFmt = "at least one %s is required" + mandatoryFieldErrFmt = "%s is a mandatory field for a %s" + base64EncodingErr = "secrets must be base64 encoded" + zeroOrNegativeErrFmt = "%s should be a positive value" + negativeValueErrFmt = "%s can't be negative" + encryptionConfigNilErr = "EncryptionConfiguration can't be nil" +) + +var ( + aesKeySizes = []int{16, 24, 32} + // See https://golang.org/pkg/crypto/aes/#NewCipher for details on supported key sizes for AES. + secretBoxKeySizes = []int{32} + // See https://godoc.org/golang.org/x/crypto/nacl/secretbox#Open for details on the supported key sizes for Secretbox. + root = field.NewPath("resources") +) + +// ValidateEncryptionConfiguration validates a v1.EncryptionConfiguration. +func ValidateEncryptionConfiguration(c *config.EncryptionConfiguration) field.ErrorList { + allErrs := field.ErrorList{} + + if c == nil { + allErrs = append(allErrs, field.Required(root, "EncryptionConfiguration can't be nil")) + return allErrs + } + + if len(c.Resources) == 0 { + allErrs = append(allErrs, field.Required(root, fmt.Sprintf(atLeastOneRequiredErrFmt, root))) + return allErrs + } + + for i, conf := range c.Resources { + r := root.Index(i).Child("resources") + p := root.Index(i).Child("providers") + + if len(conf.Resources) == 0 { + allErrs = append(allErrs, field.Required(r, fmt.Sprintf(atLeastOneRequiredErrFmt, r))) + } + + if len(conf.Providers) == 0 { + allErrs = append(allErrs, field.Required(p, fmt.Sprintf(atLeastOneRequiredErrFmt, p))) + } + + for j, provider := range conf.Providers { + path := p.Index(j) + allErrs = append(allErrs, validateSingleProvider(provider, path)...) + + switch { + case provider.KMS != nil: + allErrs = append(allErrs, validateKMSConfiguration(provider.KMS, path.Child("kms"))...) + case provider.AESGCM != nil: + allErrs = append(allErrs, validateKeys(provider.AESGCM.Keys, path.Child("aesgcm").Child("keys"), aesKeySizes)...) + case provider.AESCBC != nil: + allErrs = append(allErrs, validateKeys(provider.AESCBC.Keys, path.Child("aescbc").Child("keys"), aesKeySizes)...) + case provider.Secretbox != nil: + allErrs = append(allErrs, validateKeys(provider.Secretbox.Keys, path.Child("secretbox").Child("keys"), secretBoxKeySizes)...) + } + } + } + + return allErrs +} + +func validateSingleProvider(provider config.ProviderConfiguration, filedPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + found := 0 + + if provider.KMS != nil { + found++ + } + if provider.AESGCM != nil { + found++ + } + if provider.AESCBC != nil { + found++ + } + if provider.Secretbox != nil { + found++ + } + if provider.Identity != nil { + found++ + } + + if found == 0 { + return append(allErrs, field.Invalid(filedPath, provider, "provider does not contain any of the expected providers: KMS, AESGCM, AESCBC, Secretbox, Identity")) + } + + if found > 1 { + return append(allErrs, field.Invalid(filedPath, provider, moreThanOneElementErr)) + } + + return allErrs +} + +func validateKeys(keys []config.Key, fieldPath *field.Path, expectedLen []int) field.ErrorList { + allErrs := field.ErrorList{} + + if len(keys) == 0 { + allErrs = append(allErrs, field.Required(fieldPath, fmt.Sprintf(atLeastOneRequiredErrFmt, "keys"))) + return allErrs + } + + for i, key := range keys { + allErrs = append(allErrs, validateKey(key, fieldPath.Index(i), expectedLen)...) + } + + return allErrs +} + +func validateKey(key config.Key, fieldPath *field.Path, expectedLen []int) field.ErrorList { + allErrs := field.ErrorList{} + + if key.Name == "" { + allErrs = append(allErrs, field.Required(fieldPath.Child("name"), fmt.Sprintf(mandatoryFieldErrFmt, "name", "key"))) + } + + if key.Secret == "" { + allErrs = append(allErrs, field.Required(fieldPath.Child("secret"), fmt.Sprintf(mandatoryFieldErrFmt, "secret", "key"))) + return allErrs + } + + secret, err := base64.StdEncoding.DecodeString(key.Secret) + if err != nil { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("secret"), "REDACTED", base64EncodingErr)) + return allErrs + } + + lenMatched := false + for _, l := range expectedLen { + if len(secret) == l { + lenMatched = true + break + } + } + + if !lenMatched { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("secret"), "REDACTED", fmt.Sprintf(keyLenErrFmt, len(secret), expectedLen))) + } + + return allErrs +} + +func validateKMSConfiguration(c *config.KMSConfiguration, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if c.Name == "" { + allErrs = append(allErrs, field.Required(fieldPath.Child("name"), fmt.Sprintf(mandatoryFieldErrFmt, "name", "provider"))) + } + allErrs = append(allErrs, validateKMSTimeout(c, fieldPath.Child("timeout"))...) + allErrs = append(allErrs, validateKMSEndpoint(c, fieldPath.Child("endpoint"))...) + allErrs = append(allErrs, validateKMSCacheSize(c, fieldPath.Child("cachesize"))...) + return allErrs +} + +func validateKMSCacheSize(c *config.KMSConfiguration, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if *c.CacheSize <= 0 { + allErrs = append(allErrs, field.Invalid(fieldPath, *c.CacheSize, fmt.Sprintf(zeroOrNegativeErrFmt, "cachesize"))) + } + + return allErrs +} + +func validateKMSTimeout(c *config.KMSConfiguration, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if c.Timeout.Duration <= 0 { + allErrs = append(allErrs, field.Invalid(fieldPath, c.Timeout, fmt.Sprintf(zeroOrNegativeErrFmt, "timeout"))) + } + + return allErrs +} + +func validateKMSEndpoint(c *config.KMSConfiguration, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if len(c.Endpoint) == 0 { + return append(allErrs, field.Invalid(fieldPath, "", fmt.Sprintf(mandatoryFieldErrFmt, "endpoint", "kms"))) + } + + u, err := url.Parse(c.Endpoint) + if err != nil { + return append(allErrs, field.Invalid(fieldPath, c.Endpoint, fmt.Sprintf("invalid endpoint for kms provider, error: %v", err))) + } + + if u.Scheme != "unix" { + return append(allErrs, field.Invalid(fieldPath, c.Endpoint, fmt.Sprintf(unsupportedSchemeErrFmt, u.Scheme))) + } + + return allErrs +} diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation_test.go new file mode 100644 index 00000000000..0cf3fa25130 --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/validation/validation_test.go @@ -0,0 +1,354 @@ +/* +Copyright 2019 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 validation + +import ( + "fmt" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/apis/config" +) + +func TestStructure(t *testing.T) { + firstResourcePath := root.Index(0) + testCases := []struct { + desc string + in *config.EncryptionConfiguration + want field.ErrorList + }{ + { + desc: "nil encryption config", + in: nil, + want: field.ErrorList{ + field.Required(root, encryptionConfigNilErr), + }, + }, + { + desc: "empty encryption config", + in: &config.EncryptionConfiguration{}, + want: field.ErrorList{ + field.Required(root, fmt.Sprintf(atLeastOneRequiredErrFmt, root)), + }, + }, + { + desc: "no k8s resources", + in: &config.EncryptionConfiguration{ + Resources: []config.ResourceConfiguration{ + { + Providers: []config.ProviderConfiguration{ + { + AESCBC: &config.AESConfiguration{ + Keys: []config.Key{ + { + Name: "foo", + Secret: "A/j5CnrWGB83ylcPkuUhm/6TSyrQtsNJtDPwPHNOj4Q=", + }, + }, + }, + }, + }, + }, + }, + }, + want: field.ErrorList{ + field.Required(firstResourcePath.Child("resources"), fmt.Sprintf(atLeastOneRequiredErrFmt, root.Index(0).Child("resources"))), + }, + }, + { + desc: "no providers", + in: &config.EncryptionConfiguration{ + Resources: []config.ResourceConfiguration{ + { + Resources: []string{"secrets"}, + }, + }, + }, + want: field.ErrorList{ + field.Required(firstResourcePath.Child("providers"), fmt.Sprintf(atLeastOneRequiredErrFmt, root.Index(0).Child("providers"))), + }, + }, + { + desc: "multiple providers", + in: &config.EncryptionConfiguration{ + Resources: []config.ResourceConfiguration{ + { + Resources: []string{"secrets"}, + Providers: []config.ProviderConfiguration{ + { + AESGCM: &config.AESConfiguration{ + Keys: []config.Key{ + { + Name: "foo", + Secret: "A/j5CnrWGB83ylcPkuUhm/6TSyrQtsNJtDPwPHNOj4Q=", + }, + }, + }, + AESCBC: &config.AESConfiguration{ + Keys: []config.Key{ + { + Name: "foo", + Secret: "A/j5CnrWGB83ylcPkuUhm/6TSyrQtsNJtDPwPHNOj4Q=", + }, + }, + }, + }, + }, + }, + }, + }, + want: field.ErrorList{ + field.Invalid( + firstResourcePath.Child("providers").Index(0), + config.ProviderConfiguration{ + AESGCM: &config.AESConfiguration{ + Keys: []config.Key{ + { + Name: "foo", + Secret: "A/j5CnrWGB83ylcPkuUhm/6TSyrQtsNJtDPwPHNOj4Q=", + }, + }, + }, + AESCBC: &config.AESConfiguration{ + Keys: []config.Key{ + { + Name: "foo", + Secret: "A/j5CnrWGB83ylcPkuUhm/6TSyrQtsNJtDPwPHNOj4Q=", + }, + }, + }, + }, + moreThanOneElementErr), + }, + }, + { + desc: "valid config", + in: &config.EncryptionConfiguration{ + Resources: []config.ResourceConfiguration{ + { + Resources: []string{"secrets"}, + Providers: []config.ProviderConfiguration{ + { + AESGCM: &config.AESConfiguration{ + Keys: []config.Key{ + { + Name: "foo", + Secret: "A/j5CnrWGB83ylcPkuUhm/6TSyrQtsNJtDPwPHNOj4Q=", + }, + }, + }, + }, + }, + }, + }, + }, + want: field.ErrorList{}, + }, + } + + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + got := ValidateEncryptionConfiguration(tt.in) + if d := cmp.Diff(tt.want, got); d != "" { + t.Fatalf("EncryptionConfiguratoin validation results mismatch (-want +got):\n%s", d) + } + }) + } +} + +func TestKey(t *testing.T) { + path := root.Index(0).Child("provider").Index(0).Child("key").Index(0) + testCases := []struct { + desc string + in config.Key + want field.ErrorList + }{ + { + desc: "valid key", + in: config.Key{Name: "foo", Secret: "c2VjcmV0IGlzIHNlY3VyZQ=="}, + want: field.ErrorList{}, + }, + { + desc: "key without name", + in: config.Key{Secret: "c2VjcmV0IGlzIHNlY3VyZQ=="}, + want: field.ErrorList{ + field.Required(path.Child("name"), fmt.Sprintf(mandatoryFieldErrFmt, "name", "key")), + }, + }, + { + desc: "key without secret", + in: config.Key{Name: "foo"}, + want: field.ErrorList{ + field.Required(path.Child("secret"), fmt.Sprintf(mandatoryFieldErrFmt, "secret", "key")), + }, + }, + { + desc: "key is not base64 encoded", + in: config.Key{Name: "foo", Secret: "P@ssword"}, + want: field.ErrorList{ + field.Invalid(path.Child("secret"), "REDACTED", base64EncodingErr), + }, + }, + { + desc: "key is not of expected length", + in: config.Key{Name: "foo", Secret: "cGFzc3dvcmQK"}, + want: field.ErrorList{ + field.Invalid(path.Child("secret"), "REDACTED", fmt.Sprintf(keyLenErrFmt, 9, aesKeySizes)), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + got := validateKey(tt.in, path, aesKeySizes) + if d := cmp.Diff(tt.want, got); d != "" { + t.Fatalf("Key validation results mismatch (-want +got):\n%s", d) + } + }) + } +} + +func TestKMSProviderTimeout(t *testing.T) { + timeoutField := field.NewPath("Resource").Index(0).Child("Provider").Index(0).Child("KMS").Child("Timeout") + negativeTimeout := &metav1.Duration{Duration: -1 * time.Minute} + zeroTimeout := &metav1.Duration{Duration: 0 * time.Minute} + + testCases := []struct { + desc string + in *config.KMSConfiguration + want field.ErrorList + }{ + { + desc: "valid timeout", + in: &config.KMSConfiguration{Timeout: &metav1.Duration{Duration: 1 * time.Minute}}, + want: field.ErrorList{}, + }, + { + desc: "negative timeout", + in: &config.KMSConfiguration{Timeout: negativeTimeout}, + want: field.ErrorList{ + field.Invalid(timeoutField, negativeTimeout, fmt.Sprintf(zeroOrNegativeErrFmt, "timeout")), + }, + }, + { + desc: "zero timeout", + in: &config.KMSConfiguration{Timeout: zeroTimeout}, + want: field.ErrorList{ + field.Invalid(timeoutField, zeroTimeout, fmt.Sprintf(zeroOrNegativeErrFmt, "timeout")), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + got := validateKMSTimeout(tt.in, timeoutField) + if d := cmp.Diff(tt.want, got); d != "" { + t.Fatalf("KMS Provider validation mismatch (-want +got):\n%s", d) + } + }) + } +} + +func TestKMSEndpoint(t *testing.T) { + endpointField := field.NewPath("Resource").Index(0).Child("Provider").Index(0).Child("kms").Child("endpoint") + testCases := []struct { + desc string + in *config.KMSConfiguration + want field.ErrorList + }{ + { + desc: "valid endpoint", + in: &config.KMSConfiguration{Endpoint: "unix:///socket.sock"}, + want: field.ErrorList{}, + }, + { + desc: "empty endpoint", + in: &config.KMSConfiguration{}, + want: field.ErrorList{ + field.Invalid(endpointField, "", fmt.Sprintf(mandatoryFieldErrFmt, "endpoint", "kms")), + }, + }, + { + desc: "non unix endpoint", + in: &config.KMSConfiguration{Endpoint: "https://www.foo.com"}, + want: field.ErrorList{ + field.Invalid(endpointField, "https://www.foo.com", fmt.Sprintf(unsupportedSchemeErrFmt, "https")), + }, + }, + { + desc: "invalid url", + in: &config.KMSConfiguration{Endpoint: "unix:///foo\n.socket"}, + want: field.ErrorList{ + field.Invalid(endpointField, "unix:///foo\n.socket", "invalid endpoint for kms provider, error: parse unix:///foo\n.socket: net/url: invalid control character in URL"), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + got := validateKMSEndpoint(tt.in, endpointField) + if d := cmp.Diff(tt.want, got); d != "" { + t.Fatalf("KMS Provider validation mismatch (-want +got):\n%s", d) + } + }) + } +} + +func TestKMSProviderCacheSize(t *testing.T) { + cacheField := root.Index(0).Child("kms").Child("cachesize") + negativeCacheSize := int32(-1) + positiveCacheSize := int32(10) + zeroCacheSize := int32(0) + + testCases := []struct { + desc string + in *config.KMSConfiguration + want field.ErrorList + }{ + { + desc: "valid positive cache size", + in: &config.KMSConfiguration{CacheSize: &positiveCacheSize}, + want: field.ErrorList{}, + }, + { + desc: "invalid zero cache size", + in: &config.KMSConfiguration{CacheSize: &zeroCacheSize}, + want: field.ErrorList{ + field.Invalid(cacheField, int32(0), fmt.Sprintf(zeroOrNegativeErrFmt, "cachesize")), + }, + }, + { + desc: "negative caches size", + in: &config.KMSConfiguration{CacheSize: &negativeCacheSize}, + want: field.ErrorList{ + field.Invalid(cacheField, negativeCacheSize, fmt.Sprintf(zeroOrNegativeErrFmt, "cachesize")), + }, + }, + } + + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + got := validateKMSCacheSize(tt.in, cacheField) + if d := cmp.Diff(tt.want, got); d != "" { + t.Fatalf("KMS Provider validation mismatch (-want +got):\n%s", d) + } + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/apis/config/zz_generated.deepcopy.go b/staging/src/k8s.io/apiserver/pkg/apis/config/zz_generated.deepcopy.go index ce15176ef8c..dd66315ee71 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/config/zz_generated.deepcopy.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/config/zz_generated.deepcopy.go @@ -97,6 +97,11 @@ func (in *IdentityConfiguration) DeepCopy() *IdentityConfiguration { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KMSConfiguration) DeepCopyInto(out *KMSConfiguration) { *out = *in + if in.CacheSize != nil { + in, out := &in.CacheSize, &out.CacheSize + *out = new(int32) + **out = **in + } if in.Timeout != nil { in, out := &in.Timeout, &out.Timeout *out = new(v1.Duration) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/BUILD b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/BUILD index 65a57c94ebd..d5dbda29fa0 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/BUILD @@ -17,6 +17,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/config:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/config/v1:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/apis/config/validation:go_default_library", "//staging/src/k8s.io/apiserver/pkg/server/healthz:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/value:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/aes:go_default_library", @@ -32,8 +33,8 @@ go_test( data = glob(["testdata/**"]), embed = [":go_default_library"], deps = [ + "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", - "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//staging/src/k8s.io/apiserver/pkg/apis/config:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/value:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go index 4413ace2fce..00787c71f1c 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go @@ -20,6 +20,7 @@ import ( "crypto/aes" "crypto/cipher" "encoding/base64" + "errors" "fmt" "io" "io/ioutil" @@ -33,6 +34,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" apiserverconfig "k8s.io/apiserver/pkg/apis/config" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/config/v1" + "k8s.io/apiserver/pkg/apis/config/validation" "k8s.io/apiserver/pkg/server/healthz" "k8s.io/apiserver/pkg/storage/value" aestransformer "k8s.io/apiserver/pkg/storage/value/encrypt/aes" @@ -46,7 +48,6 @@ const ( aesGCMTransformerPrefixV1 = "k8s:enc:aesgcm:v1:" secretboxTransformerPrefixV1 = "k8s:enc:secretbox:v1:" kmsTransformerPrefixV1 = "k8s:enc:kms:v1:" - kmsPluginConnectionTimeout = 3 * time.Second kmsPluginHealthzTTL = 3 * time.Second ) @@ -104,15 +105,7 @@ func getKMSPluginProbes(reader io.Reader) ([]*kmsPluginProbe, error) { for _, r := range config.Resources { for _, p := range r.Providers { if p.KMS != nil { - timeout := kmsPluginConnectionTimeout - if p.KMS.Timeout != nil { - if p.KMS.Timeout.Duration <= 0 { - return nil, fmt.Errorf("could not configure KMS-Plugin's probe %q, timeout should be a positive value", p.KMS.Name) - } - timeout = p.KMS.Timeout.Duration - } - - s, err := envelope.NewGRPCService(p.KMS.Endpoint, timeout) + s, err := envelope.NewGRPCService(p.KMS.Endpoint, p.KMS.Timeout.Duration) if err != nil { return nil, fmt.Errorf("could not configure KMS-Plugin's probe %q, error: %v", p.KMS.Name, err) } @@ -221,7 +214,8 @@ func loadConfig(data []byte) (*apiserverconfig.EncryptionConfiguration, error) { if !ok { return nil, fmt.Errorf("got unexpected config type: %v", gvk) } - return config, nil + + return config, validation.ValidateEncryptionConfiguration(config).ToAggregate() } // The factory to create kms service. This is to make writing test easier. @@ -231,82 +225,38 @@ var envelopeServiceFactory = envelope.NewGRPCService func GetPrefixTransformers(config *apiserverconfig.ResourceConfiguration) ([]value.PrefixTransformer, error) { var result []value.PrefixTransformer for _, provider := range config.Providers { - found := false + var ( + transformer value.PrefixTransformer + err error + ) - var transformer value.PrefixTransformer - var err error - - if provider.AESGCM != nil { + switch { + case provider.AESGCM != nil: transformer, err = GetAESPrefixTransformer(provider.AESGCM, aestransformer.NewGCMTransformer, aesGCMTransformerPrefixV1) - if err != nil { - return result, err - } - found = true - } - - if provider.AESCBC != nil { - if found == true { - return result, fmt.Errorf("more than one provider specified in a single element, should split into different list elements") - } + case provider.AESCBC != nil: transformer, err = GetAESPrefixTransformer(provider.AESCBC, aestransformer.NewCBCTransformer, aesCBCTransformerPrefixV1) - found = true - } - - if provider.Secretbox != nil { - if found == true { - return result, fmt.Errorf("more than one provider specified in a single element, should split into different list elements") - } + case provider.Secretbox != nil: transformer, err = GetSecretboxPrefixTransformer(provider.Secretbox) - found = true - } - - if provider.Identity != nil { - if found == true { - return result, fmt.Errorf("more than one provider specified in a single element, should split into different list elements") - } - transformer = value.PrefixTransformer{ - Transformer: identity.NewEncryptCheckTransformer(), - Prefix: []byte{}, - } - found = true - } - - if provider.KMS != nil { - if found == true { - return nil, fmt.Errorf("more than one provider specified in a single element, should split into different list elements") - } - - // Ensure the endpoint is provided. - if len(provider.KMS.Endpoint) == 0 { - return nil, fmt.Errorf("remote KMS provider can't use empty string as endpoint") - } - - timeout := kmsPluginConnectionTimeout - if provider.KMS.Timeout != nil { - if provider.KMS.Timeout.Duration <= 0 { - return nil, fmt.Errorf("could not configure KMS plugin %q, timeout should be a positive value", provider.KMS.Name) - } - timeout = provider.KMS.Timeout.Duration - } - - // Get gRPC client service with endpoint. - envelopeService, err := envelopeServiceFactory(provider.KMS.Endpoint, timeout) + case provider.KMS != nil: + envelopeService, err := envelopeServiceFactory(provider.KMS.Endpoint, provider.KMS.Timeout.Duration) if err != nil { return nil, fmt.Errorf("could not configure KMS plugin %q, error: %v", provider.KMS.Name, err) } transformer, err = getEnvelopePrefixTransformer(provider.KMS, envelopeService, kmsTransformerPrefixV1) - found = true + case provider.Identity != nil: + transformer = value.PrefixTransformer{ + Transformer: identity.NewEncryptCheckTransformer(), + Prefix: []byte{}, + } + default: + return nil, errors.New("provider does not contain any of the expected providers: KMS, AESGCM, AESCBC, Secretbox, Identity") } if err != nil { return result, err } result = append(result, transformer) - - if found == false { - return result, fmt.Errorf("invalid provider configuration: at least one provider must be specified") - } } return result, nil } @@ -417,7 +367,7 @@ func GetSecretboxPrefixTransformer(config *apiserverconfig.SecretboxConfiguratio // getEnvelopePrefixTransformer returns a prefix transformer from the provided config. // envelopeService is used as the root of trust. func getEnvelopePrefixTransformer(config *apiserverconfig.KMSConfiguration, envelopeService envelope.Service, prefix string) (value.PrefixTransformer, error) { - envelopeTransformer, err := envelope.NewEnvelopeTransformer(envelopeService, int(config.CacheSize), aestransformer.NewCBCTransformer) + envelopeTransformer, err := envelope.NewEnvelopeTransformer(envelopeService, int(*config.CacheSize), aestransformer.NewCBCTransformer) if err != nil { return value.PrefixTransformer{}, err } diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go index 09b72c8d1e4..53124fbcddf 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go @@ -22,15 +22,12 @@ import ( "io" "io/ioutil" "os" - "reflect" - "strings" "testing" "time" "github.com/google/go-cmp/cmp" - + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/util/diff" apiserverconfig "k8s.io/apiserver/pkg/apis/config" "k8s.io/apiserver/pkg/storage/value" "k8s.io/apiserver/pkg/storage/value/encrypt/envelope" @@ -82,6 +79,7 @@ func newMockEnvelopeService(endpoint string, timeout time.Duration) (envelope.Se func TestLegacyConfig(t *testing.T) { legacyV1Config := "testdata/valid-configs/legacy.yaml" legacyConfigObject, err := loadConfig(mustReadConfig(t, legacyV1Config)) + cacheSize := int32(10) if err != nil { t.Fatalf("error while parsing configuration file: %s.\nThe file was:\n%s", err, legacyV1Config) } @@ -101,7 +99,8 @@ func TestLegacyConfig(t *testing.T) { {KMS: &apiserverconfig.KMSConfiguration{ Name: "testprovider", Endpoint: "unix:///tmp/testprovider.sock", - CacheSize: 10, + CacheSize: &cacheSize, + Timeout: &metav1.Duration{Duration: 3 * time.Second}, }}, {AESCBC: &apiserverconfig.AESConfiguration{ Keys: []apiserverconfig.Key{ @@ -118,8 +117,8 @@ func TestLegacyConfig(t *testing.T) { }, }, } - if !reflect.DeepEqual(legacyConfigObject, expected) { - t.Fatal(diff.ObjectReflectDiff(expected, legacyConfigObject)) + if d := cmp.Diff(expected, legacyConfigObject); d != "" { + t.Fatalf("EncryptionConfig mismatch (-want +got):\n%s", d) } } @@ -206,80 +205,8 @@ func TestEncryptionProviderConfigCorrect(t *testing.T) { } } -// Throw error if key has no secret -func TestEncryptionProviderConfigNoSecretForKey(t *testing.T) { - incorrectConfigNoSecretForKey := "testdata/invalid-configs/aes/no-key.yaml" - if _, err := ParseEncryptionConfiguration(mustConfigReader(t, incorrectConfigNoSecretForKey)); err == nil { - t.Fatalf("invalid configuration file (one key has no secret) got parsed:\n%s", incorrectConfigNoSecretForKey) - } -} - -// Throw error if invalid key for AES -func TestEncryptionProviderConfigInvalidKey(t *testing.T) { - incorrectConfigInvalidKey := "testdata/invalid-configs/aes/invalid-key.yaml" - if _, err := ParseEncryptionConfiguration(mustConfigReader(t, incorrectConfigInvalidKey)); err == nil { - t.Fatalf("invalid configuration file (bad AES key) got parsed:\n%s", incorrectConfigInvalidKey) - } -} - -// Throw error if kms has no endpoint -func TestEncryptionProviderConfigNoEndpointForKMS(t *testing.T) { - incorrectConfigNoEndpointForKMS := "testdata/invalid-configs/kms/no-endpoint.yaml" - if _, err := ParseEncryptionConfiguration(mustConfigReader(t, incorrectConfigNoEndpointForKMS)); err == nil { - t.Fatalf("invalid configuration file (kms has no endpoint) got parsed:\n%s", incorrectConfigNoEndpointForKMS) - } -} - -func TestKMSConfigTimeout(t *testing.T) { - testCases := []struct { - desc string - config string - want time.Duration - wantErr string - }{ - { - desc: "duration explicitly provided", - config: "testdata/valid-configs/kms/valid-timeout.yaml", - want: 15 * time.Second, - }, - { - desc: "duration explicitly provided as 0 which is an invalid value, error should be returned", - config: "testdata/invalid-configs/kms/zero-timeout.yaml", - wantErr: "timeout should be a positive value", - }, - { - desc: "duration is not provided, default will be supplied", - config: "testdata/valid-configs/kms/default-timeout.yaml", - want: kmsPluginConnectionTimeout, - }, - { - desc: "duration is invalid (negative), error should be returned", - config: "testdata/invalid-configs/kms/negative-timeout.yaml", - wantErr: "timeout should be a positive value", - }, - } - - for _, tt := range testCases { - t.Run(tt.desc, func(t *testing.T) { - // mocking envelopeServiceFactory to sense the value of the supplied timeout. - envelopeServiceFactory = func(endpoint string, callTimeout time.Duration) (envelope.Service, error) { - if callTimeout != tt.want { - t.Fatalf("got timeout: %v, want %v", callTimeout, tt.want) - } - - return newMockEnvelopeService(endpoint, callTimeout) - } - - // mocked envelopeServiceFactory is called during ParseEncryptionConfiguration. - if _, err := ParseEncryptionConfiguration(mustConfigReader(t, tt.config)); err != nil && !strings.Contains(err.Error(), tt.wantErr) { - t.Fatalf("unable to parse yaml\n%s\nerror: %v", tt.config, err) - } - }) - } -} - func TestKMSPluginHealthz(t *testing.T) { - service, err := envelope.NewGRPCService("unix:///tmp/testprovider.sock", kmsPluginConnectionTimeout) + service, err := envelope.NewGRPCService("unix:///tmp/testprovider.sock", 3*time.Second) if err != nil { t.Fatalf("Could not initialize envelopeService, error: %v", err) } diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/aes/invalid-key.yaml b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/aes/invalid-key.yaml deleted file mode 100644 index d3b8fb6716a..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/aes/invalid-key.yaml +++ /dev/null @@ -1,13 +0,0 @@ -kind: EncryptionConfiguration -apiVersion: apiserver.config.k8s.io/v1 -resources: - - resources: - - namespaces - - secrets - providers: - - aesgcm: - keys: - - name: key1 - secret: c2VjcmV0IGlzIHNlY3VyZQ== - - name: key2 - secret: YSBzZWNyZXQgYSBzZWNyZXQ= \ No newline at end of file diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/aes/no-key.yaml b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/aes/no-key.yaml deleted file mode 100644 index ba9976827cd..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/aes/no-key.yaml +++ /dev/null @@ -1,10 +0,0 @@ -kind: EncryptionConfiguration -apiVersion: apiserver.config.k8s.io/v1 -resources: - - resources: - - namespaces - - secrets - providers: - - aesgcm: - keys: - - name: key1 diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/negative-timeout.yaml b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/negative-timeout.yaml deleted file mode 100644 index 6880903fd20..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/negative-timeout.yaml +++ /dev/null @@ -1,10 +0,0 @@ -kind: EncryptionConfiguration -apiVersion: apiserver.config.k8s.io/v1 -resources: - - resources: - - secrets - providers: - - kms: - name: foo - endpoint: unix:///tmp/testprovider.sock - timeout: -15s \ No newline at end of file diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/no-endpoint.yaml b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/no-endpoint.yaml deleted file mode 100644 index dac36350ba6..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/no-endpoint.yaml +++ /dev/null @@ -1,9 +0,0 @@ -kind: EncryptionConfiguration -apiVersion: apiserver.config.k8s.io/v1 -resources: - - resources: - - secrets - providers: - - kms: - name: testprovider - cachesize: 10 \ No newline at end of file diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/zero-timeout.yaml b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/zero-timeout.yaml deleted file mode 100644 index 7690b66bde9..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/invalid-configs/kms/zero-timeout.yaml +++ /dev/null @@ -1,10 +0,0 @@ -kind: EncryptionConfiguration -apiVersion: apiserver.config.k8s.io/v1 -resources: - - resources: - - secrets - providers: - - kms: - name: foo - endpoint: unix:///tmp/testprovider.sock - timeout: 0s \ No newline at end of file diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/valid-configs/kms/valid-timeout.yaml b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/valid-configs/kms/valid-timeout.yaml deleted file mode 100644 index 32db07a1803..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/testdata/valid-configs/kms/valid-timeout.yaml +++ /dev/null @@ -1,10 +0,0 @@ - kind: EncryptionConfiguration - apiVersion: apiserver.config.k8s.io/v1 - resources: - - resources: - - secrets - providers: - - kms: - name: foo - endpoint: unix:///tmp/testprovider.sock - timeout: 15s \ No newline at end of file diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope.go b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope.go index 7abd109b2a5..e389f50768f 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope.go @@ -31,9 +31,6 @@ import ( "golang.org/x/crypto/cryptobyte" ) -// defaultCacheSize is the number of decrypted DEKs which would be cached by the transformer. -const defaultCacheSize = 1000 - func init() { value.RegisterMetrics() } @@ -54,6 +51,8 @@ type envelopeTransformer struct { // baseTransformerFunc creates a new transformer for encrypting the data with the DEK. baseTransformerFunc func(cipher.Block) value.Transformer + + cacheEnabled bool } // NewEnvelopeTransformer returns a transformer which implements a KEK-DEK based envelope encryption scheme. @@ -61,17 +60,22 @@ type envelopeTransformer struct { // the data items they encrypt. A cache (of size cacheSize) is maintained to store the most recently // used decrypted DEKs in memory. func NewEnvelopeTransformer(envelopeService Service, cacheSize int, baseTransformerFunc func(cipher.Block) value.Transformer) (value.Transformer, error) { - if cacheSize == 0 { - cacheSize = defaultCacheSize - } - cache, err := lru.New(cacheSize) - if err != nil { - return nil, err + var ( + cache *lru.Cache + err error + ) + + if cacheSize > 0 { + cache, err = lru.New(cacheSize) + if err != nil { + return nil, err + } } return &envelopeTransformer{ envelopeService: envelopeService, transformers: cache, baseTransformerFunc: baseTransformerFunc, + cacheEnabled: cacheSize > 0, }, nil } @@ -91,7 +95,9 @@ func (t *envelopeTransformer) TransformFromStorage(data []byte, context value.Co // Look up the decrypted DEK from cache or Envelope. transformer := t.getTransformer(encKey) if transformer == nil { - value.RecordCacheMiss() + if t.cacheEnabled { + value.RecordCacheMiss() + } key, err := t.envelopeService.Decrypt(encKey) if err != nil { // Do NOT wrap this err using fmt.Errorf() or similar functions @@ -99,6 +105,7 @@ func (t *envelopeTransformer) TransformFromStorage(data []byte, context value.Co // record the metric. return nil, false, err } + transformer, err = t.addTransformer(encKey, key) if err != nil { return nil, false, err @@ -153,12 +160,18 @@ func (t *envelopeTransformer) addTransformer(encKey []byte, key []byte) (value.T transformer := t.baseTransformerFunc(block) // Use base64 of encKey as the key into the cache because hashicorp/golang-lru // cannot hash []uint8. - t.transformers.Add(base64.StdEncoding.EncodeToString(encKey), transformer) + if t.cacheEnabled { + t.transformers.Add(base64.StdEncoding.EncodeToString(encKey), transformer) + } return transformer, nil } // getTransformer fetches the transformer corresponding to encKey from cache, if it exists. func (t *envelopeTransformer) getTransformer(encKey []byte) value.Transformer { + if !t.cacheEnabled { + return nil + } + _transformer, found := t.transformers.Get(base64.StdEncoding.EncodeToString(encKey)) if found { return _transformer.(value.Transformer) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope_test.go b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope_test.go index 046de9e4937..08676ddf2b4 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/envelope_test.go @@ -78,34 +78,54 @@ func newTestEnvelopeService() *testEnvelopeService { // Throw error if Envelope transformer tries to contact Envelope without hitting cache. func TestEnvelopeCaching(t *testing.T) { - envelopeService := newTestEnvelopeService() - envelopeTransformer, err := NewEnvelopeTransformer(envelopeService, testEnvelopeCacheSize, aestransformer.NewCBCTransformer) - if err != nil { - t.Fatalf("failed to initialize envelope transformer: %v", err) - } - context := value.DefaultContext([]byte(testContextText)) - originalText := []byte(testText) - - transformedData, err := envelopeTransformer.TransformToStorage(originalText, context) - if err != nil { - t.Fatalf("envelopeTransformer: error while transforming data to storage: %s", err) - } - untransformedData, _, err := envelopeTransformer.TransformFromStorage(transformedData, context) - if err != nil { - t.Fatalf("could not decrypt Envelope transformer's encrypted data even once: %v", err) - } - if !bytes.Equal(untransformedData, originalText) { - t.Fatalf("envelopeTransformer transformed data incorrectly. Expected: %v, got %v", originalText, untransformedData) + testCases := []struct { + desc string + cacheSize int + simulateKMSPluginFailure bool + }{ + { + desc: "positive cache size should withstand plugin failure", + cacheSize: 1000, + simulateKMSPluginFailure: true, + }, + { + desc: "cache disabled size should not withstand plugin failure", + cacheSize: 0, + }, } - envelopeService.SetDisabledStatus(true) - // Subsequent read for the same data should work fine due to caching. - untransformedData, _, err = envelopeTransformer.TransformFromStorage(transformedData, context) - if err != nil { - t.Fatalf("could not decrypt Envelope transformer's encrypted data using just cache: %v", err) - } - if !bytes.Equal(untransformedData, originalText) { - t.Fatalf("envelopeTransformer transformed data incorrectly using cache. Expected: %v, got %v", originalText, untransformedData) + for _, tt := range testCases { + t.Run(tt.desc, func(t *testing.T) { + envelopeService := newTestEnvelopeService() + envelopeTransformer, err := NewEnvelopeTransformer(envelopeService, tt.cacheSize, aestransformer.NewCBCTransformer) + if err != nil { + t.Fatalf("failed to initialize envelope transformer: %v", err) + } + context := value.DefaultContext([]byte(testContextText)) + originalText := []byte(testText) + + transformedData, err := envelopeTransformer.TransformToStorage(originalText, context) + if err != nil { + t.Fatalf("envelopeTransformer: error while transforming data to storage: %s", err) + } + untransformedData, _, err := envelopeTransformer.TransformFromStorage(transformedData, context) + if err != nil { + t.Fatalf("could not decrypt Envelope transformer's encrypted data even once: %v", err) + } + if !bytes.Equal(untransformedData, originalText) { + t.Fatalf("envelopeTransformer transformed data incorrectly. Expected: %v, got %v", originalText, untransformedData) + } + + envelopeService.SetDisabledStatus(tt.simulateKMSPluginFailure) + // Subsequent read for the same data should work fine due to caching. + untransformedData, _, err = envelopeTransformer.TransformFromStorage(transformedData, context) + if err != nil { + t.Fatalf("could not decrypt Envelope transformer's encrypted data using just cache: %v", err) + } + if !bytes.Equal(untransformedData, originalText) { + t.Fatalf("envelopeTransformer transformed data incorrectly using cache. Got: %v, want %v", untransformedData, originalText) + } + }) } } diff --git a/vendor/modules.txt b/vendor/modules.txt index a51796d1ed5..42124e2f252 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1289,6 +1289,7 @@ k8s.io/apiserver/pkg/apis/audit/v1beta1 k8s.io/apiserver/pkg/apis/audit/validation k8s.io/apiserver/pkg/apis/config k8s.io/apiserver/pkg/apis/config/v1 +k8s.io/apiserver/pkg/apis/config/validation k8s.io/apiserver/pkg/apis/example k8s.io/apiserver/pkg/apis/example/v1 k8s.io/apiserver/pkg/audit