From 7e158fb4f62a36a1a81ee12eb3521ac4db0bb3e5 Mon Sep 17 00:00:00 2001 From: Lukasz Zajaczkowski Date: Fri, 23 Sep 2016 13:48:32 +0200 Subject: [PATCH] Add support for binary file in configmap --- pkg/apis/core/types.go | 13 ++++ pkg/apis/core/validation/validation.go | 14 ++++- pkg/apis/core/validation/validation_test.go | 70 +++++++++++++-------- pkg/kubectl/configmap.go | 38 ++++++++--- pkg/kubectl/configmap_test.go | 64 +++++++++++++++++-- pkg/kubectl/util/hash/hash_test.go | 2 +- pkg/volume/configmap/configmap.go | 20 ++++-- pkg/volume/configmap/configmap_test.go | 32 ++++++++++ staging/src/k8s.io/api/core/v1/types.go | 13 ++++ 9 files changed, 222 insertions(+), 44 deletions(-) diff --git a/pkg/apis/core/types.go b/pkg/apis/core/types.go index a1e2fd91763..bcf3661ba4e 100644 --- a/pkg/apis/core/types.go +++ b/pkg/apis/core/types.go @@ -4389,8 +4389,21 @@ type ConfigMap struct { // Data contains the configuration data. // Each key must consist of alphanumeric characters, '-', '_' or '.'. + // Values with non-UTF-8 byte sequences must use the BinaryData field. + // The keys stored in Data must not overlap with the keys in + // the BinaryData field, this is enforced during validation process. // +optional Data map[string]string + + // BinaryData contains the binary data. + // Each key must consist of alphanumeric characters, '-', '_' or '.'. + // BinaryData can contain byte sequences that are not in the UTF-8 range. + // The keys stored in BinaryData must not overlap with the ones in + // the Data field, this is enforced during validation process. + // Using this field will require 1.10+ apiserver and + // kubelet. + // +optional + BinaryData map[string][]byte } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index b86889a1630..5889e50cedf 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4356,10 +4356,22 @@ func ValidateConfigMap(cfg *core.ConfigMap) field.ErrorList { for _, msg := range validation.IsConfigMapKey(key) { allErrs = append(allErrs, field.Invalid(field.NewPath("data").Key(key), key, msg)) } + // check if we have a duplicate key in the other bag + if _, isValue := cfg.BinaryData[key]; isValue { + msg := "duplicate of key present in binaryData" + allErrs = append(allErrs, field.Invalid(field.NewPath("data").Key(key), key, msg)) + } + totalSize += len(value) + } + for key, value := range cfg.BinaryData { + for _, msg := range validation.IsConfigMapKey(key) { + allErrs = append(allErrs, field.Invalid(field.NewPath("binaryData").Key(key), key, msg)) + } totalSize += len(value) } if totalSize > core.MaxSecretSize { - allErrs = append(allErrs, field.TooLong(field.NewPath("data"), "", core.MaxSecretSize)) + // pass back "" to indicate that the error refers to the whole object. + allErrs = append(allErrs, field.TooLong(field.NewPath(""), cfg, core.MaxSecretSize)) } return allErrs diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 442adecfabd..523cb98138c 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -17,6 +17,7 @@ limitations under the License. package validation import ( + "bytes" "fmt" "math" "reflect" @@ -11800,48 +11801,65 @@ func TestValidPodLogOptions(t *testing.T) { } func TestValidateConfigMap(t *testing.T) { - newConfigMap := func(name, namespace string, data map[string]string) core.ConfigMap { + newConfigMap := func(name, namespace string, data map[string]string, binaryData map[string][]byte) core.ConfigMap { return core.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: name, Namespace: namespace, }, - Data: data, + Data: data, + BinaryData: binaryData, } } var ( - validConfigMap = newConfigMap("validname", "validns", map[string]string{"key": "value"}) - maxKeyLength = newConfigMap("validname", "validns", map[string]string{strings.Repeat("a", 253): "value"}) + validConfigMap = newConfigMap("validname", "validns", map[string]string{"key": "value"}, map[string][]byte{"bin": []byte("value")}) + maxKeyLength = newConfigMap("validname", "validns", map[string]string{strings.Repeat("a", 253): "value"}, nil) - emptyName = newConfigMap("", "validns", nil) - invalidName = newConfigMap("NoUppercaseOrSpecialCharsLike=Equals", "validns", nil) - emptyNs = newConfigMap("validname", "", nil) - invalidNs = newConfigMap("validname", "NoUppercaseOrSpecialCharsLike=Equals", nil) - invalidKey = newConfigMap("validname", "validns", map[string]string{"a*b": "value"}) - leadingDotKey = newConfigMap("validname", "validns", map[string]string{".ab": "value"}) - dotKey = newConfigMap("validname", "validns", map[string]string{".": "value"}) - doubleDotKey = newConfigMap("validname", "validns", map[string]string{"..": "value"}) - overMaxKeyLength = newConfigMap("validname", "validns", map[string]string{strings.Repeat("a", 254): "value"}) - overMaxSize = newConfigMap("validname", "validns", map[string]string{"key": strings.Repeat("a", core.MaxSecretSize+1)}) + emptyName = newConfigMap("", "validns", nil, nil) + invalidName = newConfigMap("NoUppercaseOrSpecialCharsLike=Equals", "validns", nil, nil) + emptyNs = newConfigMap("validname", "", nil, nil) + invalidNs = newConfigMap("validname", "NoUppercaseOrSpecialCharsLike=Equals", nil, nil) + invalidKey = newConfigMap("validname", "validns", map[string]string{"a*b": "value"}, nil) + leadingDotKey = newConfigMap("validname", "validns", map[string]string{".ab": "value"}, nil) + dotKey = newConfigMap("validname", "validns", map[string]string{".": "value"}, nil) + doubleDotKey = newConfigMap("validname", "validns", map[string]string{"..": "value"}, nil) + overMaxKeyLength = newConfigMap("validname", "validns", map[string]string{strings.Repeat("a", 254): "value"}, nil) + overMaxSize = newConfigMap("validname", "validns", map[string]string{"key": strings.Repeat("a", v1.MaxSecretSize+1)}, nil) + duplicatedKey = newConfigMap("validname", "validns", map[string]string{"key": "value1"}, map[string][]byte{"key": []byte("value2")}) + binDataInvalidKey = newConfigMap("validname", "validns", nil, map[string][]byte{"a*b": []byte("value")}) + binDataLeadingDotKey = newConfigMap("validname", "validns", nil, map[string][]byte{".ab": []byte("value")}) + binDataDotKey = newConfigMap("validname", "validns", nil, map[string][]byte{".": []byte("value")}) + binDataDoubleDotKey = newConfigMap("validname", "validns", nil, map[string][]byte{"..": []byte("value")}) + binDataOverMaxKeyLength = newConfigMap("validname", "validns", nil, map[string][]byte{strings.Repeat("a", 254): []byte("value")}) + binDataOverMaxSize = newConfigMap("validname", "validns", nil, map[string][]byte{"bin": bytes.Repeat([]byte("a"), v1.MaxSecretSize+1)}) + binNonUtf8Value = newConfigMap("validname", "validns", nil, map[string][]byte{"key": {0, 0xFE, 0, 0xFF}}) ) tests := map[string]struct { cfg core.ConfigMap isValid bool }{ - "valid": {validConfigMap, true}, - "max key length": {maxKeyLength, true}, - "leading dot key": {leadingDotKey, true}, - "empty name": {emptyName, false}, - "invalid name": {invalidName, false}, - "invalid key": {invalidKey, false}, - "empty namespace": {emptyNs, false}, - "invalid namespace": {invalidNs, false}, - "dot key": {dotKey, false}, - "double dot key": {doubleDotKey, false}, - "over max key length": {overMaxKeyLength, false}, - "over max size": {overMaxSize, false}, + "valid": {validConfigMap, true}, + "max key length": {maxKeyLength, true}, + "leading dot key": {leadingDotKey, true}, + "empty name": {emptyName, false}, + "invalid name": {invalidName, false}, + "invalid key": {invalidKey, false}, + "empty namespace": {emptyNs, false}, + "invalid namespace": {invalidNs, false}, + "dot key": {dotKey, false}, + "double dot key": {doubleDotKey, false}, + "over max key length": {overMaxKeyLength, false}, + "over max size": {overMaxSize, false}, + "duplicated key": {duplicatedKey, false}, + "binary data invalid key": {binDataInvalidKey, false}, + "binary data leading dot key": {binDataLeadingDotKey, true}, + "binary data dot key": {binDataDotKey, false}, + "binary data double dot key": {binDataDoubleDotKey, false}, + "binary data over max key length": {binDataOverMaxKeyLength, false}, + "binary data max size": {binDataOverMaxSize, false}, + "binary data non utf-8 bytes": {binNonUtf8Value, true}, } for name, tc := range tests { diff --git a/pkg/kubectl/configmap.go b/pkg/kubectl/configmap.go index 8d1ef5281ed..ba4ac1ee202 100644 --- a/pkg/kubectl/configmap.go +++ b/pkg/kubectl/configmap.go @@ -22,6 +22,7 @@ import ( "os" "path" "strings" + "unicode/utf8" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -130,6 +131,7 @@ func (s ConfigMapGeneratorV1) StructuredGenerate() (runtime.Object, error) { configMap := &v1.ConfigMap{} configMap.Name = s.Name configMap.Data = map[string]string{} + configMap.BinaryData = map[string][]byte{} if len(s.FileSources) > 0 { if err := handleConfigMapFromFileSources(configMap, s.FileSources); err != nil { return nil, err @@ -255,19 +257,41 @@ func addKeyFromFileToConfigMap(configMap *v1.ConfigMap, keyName, filePath string if err != nil { return err } - return addKeyFromLiteralToConfigMap(configMap, keyName, string(data)) + + if utf8.Valid(data) { + return addKeyFromLiteralToConfigMap(configMap, keyName, string(data)) + } + + err = validateNewConfigMap(configMap, keyName) + if err != nil { + return err + } + configMap.BinaryData[keyName] = data + return nil } // addKeyFromLiteralToConfigMap adds the given key and data to the given config map, // returning an error if the key is not valid or if the key already exists. func addKeyFromLiteralToConfigMap(configMap *v1.ConfigMap, keyName, data string) error { - // Note, the rules for ConfigMap keys are the exact same as the ones for SecretKeys. - if errs := validation.IsConfigMapKey(keyName); len(errs) != 0 { - return fmt.Errorf("%q is not a valid key name for a ConfigMap: %s", keyName, strings.Join(errs, ";")) - } - if _, entryExists := configMap.Data[keyName]; entryExists { - return fmt.Errorf("cannot add key %s, another key by that name already exists: %v.", keyName, configMap.Data) + err := validateNewConfigMap(configMap, keyName) + if err != nil { + return err } configMap.Data[keyName] = data return nil } + +func validateNewConfigMap(configMap *v1.ConfigMap, keyName string) error { + // Note, the rules for ConfigMap keys are the exact same as the ones for SecretKeys. + if errs := validation.IsConfigMapKey(keyName); len(errs) != 0 { + return fmt.Errorf("%q is not a valid key name for a ConfigMap: %s", keyName, strings.Join(errs, ";")) + } + + if _, exists := configMap.Data[keyName]; exists { + return fmt.Errorf("cannot add key %s, another key by that name already exists in data: %v", keyName, configMap.Data) + } + if _, exists := configMap.BinaryData[keyName]; exists { + return fmt.Errorf("cannot add key %s, another key by that name already exists in binaryData: %v", keyName, configMap.BinaryData) + } + return nil +} diff --git a/pkg/kubectl/configmap_test.go b/pkg/kubectl/configmap_test.go index 05007dfb4af..594607cfcee 100644 --- a/pkg/kubectl/configmap_test.go +++ b/pkg/kubectl/configmap_test.go @@ -41,7 +41,8 @@ func TestConfigMapGenerate(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "foo", }, - Data: map[string]string{}, + Data: map[string]string{}, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -54,7 +55,8 @@ func TestConfigMapGenerate(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "foo-867km9574f", }, - Data: map[string]string{}, + Data: map[string]string{}, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -67,7 +69,8 @@ func TestConfigMapGenerate(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "foo", }, - Data: map[string]string{}, + Data: map[string]string{}, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -81,7 +84,8 @@ func TestConfigMapGenerate(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "foo-867km9574f", }, - Data: map[string]string{}, + Data: map[string]string{}, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -98,6 +102,7 @@ func TestConfigMapGenerate(t *testing.T) { "key1": "value1", "key2": "value2", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -115,6 +120,7 @@ func TestConfigMapGenerate(t *testing.T) { "key1": "value1", "key2": "value2", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -139,6 +145,36 @@ func TestConfigMapGenerate(t *testing.T) { }, expectErr: true, }, + { + setup: setupBinaryFile([]byte{0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64}), + params: map[string]interface{}{ + "name": "foo", + "from-file": []string{"foo1"}, + }, + expected: &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Data: map[string]string{"foo1": "hello world"}, + BinaryData: map[string][]byte{}, + }, + expectErr: false, + }, + { + setup: setupBinaryFile([]byte{0xff, 0xfd}), + params: map[string]interface{}{ + "name": "foo", + "from-file": []string{"foo1"}, + }, + expected: &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Data: map[string]string{}, + BinaryData: map[string][]byte{"foo1": {0xff, 0xfd}}, + }, + expectErr: false, + }, { params: map[string]interface{}{ "name": "foo", @@ -151,6 +187,7 @@ func TestConfigMapGenerate(t *testing.T) { Data: map[string]string{ "key1": "=value1", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -167,6 +204,7 @@ func TestConfigMapGenerate(t *testing.T) { Data: map[string]string{ "key1": "=value1", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -184,6 +222,7 @@ func TestConfigMapGenerate(t *testing.T) { "key1": "value1", "key2": "value2", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -202,6 +241,7 @@ func TestConfigMapGenerate(t *testing.T) { "key1": "value1", "key2": "value2", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -223,6 +263,7 @@ func TestConfigMapGenerate(t *testing.T) { "g_key1": "1", "g_key2": "", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -245,6 +286,7 @@ func TestConfigMapGenerate(t *testing.T) { "g_key1": "1", "g_key2": "", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -277,6 +319,7 @@ func TestConfigMapGenerate(t *testing.T) { Data: map[string]string{ "key1": " value1", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -294,6 +337,7 @@ func TestConfigMapGenerate(t *testing.T) { Data: map[string]string{ "key1": " value1", }, + BinaryData: map[string][]byte{}, }, expectErr: false, }, @@ -335,3 +379,15 @@ func setupEnvFile(lines ...string) func(*testing.T, map[string]interface{}) func } } } + +func setupBinaryFile(data []byte) func(*testing.T, map[string]interface{}) func() { + return func(t *testing.T, params map[string]interface{}) func() { + tmp, _ := ioutil.TempDir("", "") + f := tmp + "/foo1" + ioutil.WriteFile(f, data, 0644) + params["from-file"] = []string{f} + return func() { + os.Remove(f) + } + } +} diff --git a/pkg/kubectl/util/hash/hash_test.go b/pkg/kubectl/util/hash/hash_test.go index 15a0bd20e14..b9a168a2378 100644 --- a/pkg/kubectl/util/hash/hash_test.go +++ b/pkg/kubectl/util/hash/hash_test.go @@ -148,7 +148,7 @@ not their metadata (e.g. the Data of a ConfigMap, but nothing in ObjectMeta). obj interface{} expect int }{ - {"ConfigMap", v1.ConfigMap{}, 3}, + {"ConfigMap", v1.ConfigMap{}, 4}, {"Secret", v1.Secret{}, 5}, } for _, c := range cases { diff --git a/pkg/volume/configmap/configmap.go b/pkg/volume/configmap/configmap.go index 94b83b7ed7f..476b2bff993 100644 --- a/pkg/volume/configmap/configmap.go +++ b/pkg/volume/configmap/configmap.go @@ -214,7 +214,7 @@ func (b *configMapVolumeMounter) SetUpAt(dir string, fsGroup *int64) error { glog.V(3).Infof("Received configMap %v/%v containing (%v) pieces of data, %v total bytes", b.pod.Namespace, b.source.Name, - len(configMap.Data), + len(configMap.Data)+len(configMap.BinaryData), totalBytes) payload, err := MakePayload(b.source.Items, configMap, b.source.DefaultMode, optional) @@ -250,7 +250,7 @@ func MakePayload(mappings []v1.KeyToPath, configMap *v1.ConfigMap, defaultMode * return nil, fmt.Errorf("No defaultMode used, not even the default value for it") } - payload := make(map[string]volumeutil.FileProjection, len(configMap.Data)) + payload := make(map[string]volumeutil.FileProjection, (len(configMap.Data) + len(configMap.BinaryData))) var fileProjection volumeutil.FileProjection if len(mappings) == 0 { @@ -259,17 +259,24 @@ func MakePayload(mappings []v1.KeyToPath, configMap *v1.ConfigMap, defaultMode * fileProjection.Mode = *defaultMode payload[name] = fileProjection } + for name, data := range configMap.BinaryData { + fileProjection.Data = data + fileProjection.Mode = *defaultMode + payload[name] = fileProjection + } } else { for _, ktp := range mappings { - content, ok := configMap.Data[ktp.Key] - if !ok { + if stringData, ok := configMap.Data[ktp.Key]; ok { + fileProjection.Data = []byte(stringData) + } else if binaryData, ok := configMap.BinaryData[ktp.Key]; ok { + fileProjection.Data = binaryData + } else { if optional { continue } return nil, fmt.Errorf("configmap references non-existent config key: %s", ktp.Key) } - fileProjection.Data = []byte(content) if ktp.Mode != nil { fileProjection.Mode = *ktp.Mode } else { @@ -287,6 +294,9 @@ func totalBytes(configMap *v1.ConfigMap) int { for _, value := range configMap.Data { totalSize += len(value) } + for _, value := range configMap.BinaryData { + totalSize += len(value) + } return totalSize } diff --git a/pkg/volume/configmap/configmap_test.go b/pkg/volume/configmap/configmap_test.go index dbe8b19ac91..eabf2c9d71e 100644 --- a/pkg/volume/configmap/configmap_test.go +++ b/pkg/volume/configmap/configmap_test.go @@ -62,6 +62,38 @@ func TestMakePayload(t *testing.T) { }, success: true, }, + { + name: "no overrides binary data", + configMap: &v1.ConfigMap{ + BinaryData: map[string][]byte{ + "foo": []byte("foo"), + "bar": []byte("bar"), + }, + }, + mode: 0644, + payload: map[string]util.FileProjection{ + "foo": {Data: []byte("foo"), Mode: 0644}, + "bar": {Data: []byte("bar"), Mode: 0644}, + }, + success: true, + }, + { + name: "no overrides mixed data", + configMap: &v1.ConfigMap{ + BinaryData: map[string][]byte{ + "foo": []byte("foo"), + }, + Data: map[string]string{ + "bar": "bar", + }, + }, + mode: 0644, + payload: map[string]util.FileProjection{ + "foo": {Data: []byte("foo"), Mode: 0644}, + "bar": {Data: []byte("bar"), Mode: 0644}, + }, + success: true, + }, { name: "basic 1", mappings: []v1.KeyToPath{ diff --git a/staging/src/k8s.io/api/core/v1/types.go b/staging/src/k8s.io/api/core/v1/types.go index f43e9c90c07..920afb05b6c 100644 --- a/staging/src/k8s.io/api/core/v1/types.go +++ b/staging/src/k8s.io/api/core/v1/types.go @@ -4940,8 +4940,21 @@ type ConfigMap struct { // Data contains the configuration data. // Each key must consist of alphanumeric characters, '-', '_' or '.'. + // Values with non-UTF-8 byte sequences must use the BinaryData field. + // The keys stored in Data must not overlap with the keys in + // the BinaryData field, this is enforced during validation process. // +optional Data map[string]string `json:"data,omitempty" protobuf:"bytes,2,rep,name=data"` + + // BinaryData contains the binary data. + // Each key must consist of alphanumeric characters, '-', '_' or '.'. + // BinaryData can contain byte sequences that are not in the UTF-8 range. + // The keys stored in BinaryData must not overlap with the ones in + // the Data field, this is enforced during validation process. + // Using this field will require 1.10+ apiserver and + // kubelet. + // +optional + BinaryData map[string][]byte `json:"binaryData,omitempty" protobuf:"bytes,3,rep,name=binaryData"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object