Merge pull request #316 from droot/configmap-impl

configmap resource implementation using loader

pkg/kinflate/resource/appresource_test.go

@@ -9,28 +9,6 @@ import (
 	"k8s.io/kubectl/pkg/loader"
 )
 
-var encoded = []byte(`apiVersion: v1
-kind: Deployment
-metadata:
-  name: dply1
----
-apiVersion: v1
-kind: Deployment
-metadata:
-  name: dply2
-`)
-
-type fakeLoader struct {
-}
-
-func (l fakeLoader) New(newRoot string) (loader.Loader, error) {
-	return l, nil
-}
-
-func (l fakeLoader) Load(location string) ([]byte, error) {
-	return encoded, nil
-}
-
 func makeUnconstructed(name string) *unstructured.Unstructured {
 	return &unstructured.Unstructured{
 		Object: map[string]interface{}{
@@ -44,7 +22,19 @@ func makeUnconstructed(name string) *unstructured.Unstructured {
 }
 
 func TestAppResourceList_Resources(t *testing.T) {
-	l := fakeLoader{}
+
+	resourceContent := `apiVersion: v1
+kind: Deployment
+metadata:
+  name: dply1
+---
+apiVersion: v1
+kind: Deployment
+metadata:
+  name: dply2
+`
+
+	l := loader.FakeLoader{Content: resourceContent}
 	expected := []*Resource{
 		{Data: makeUnconstructed("dply1")},
 		{Data: makeUnconstructed("dply2")},

pkg/kinflate/resource/configmap.go

@@ -17,17 +17,19 @@ limitations under the License.
 package resource
 
 import (
+	"fmt"
+	"strings"
+
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/validation"
 	manifest "k8s.io/kubectl/pkg/apis/manifest/v1alpha1"
 	cutil "k8s.io/kubectl/pkg/kinflate/configmapandsecret/util"
+	"k8s.io/kubectl/pkg/loader"
 )
 
-// (Note): pass in loader which has rootPath context and knows how to load
+// NewFromConfigMap returns a Resource given a configmap metadata from manifest file.
-// files given a relative path.
+func NewFromConfigMap(cm manifest.ConfigMap, l loader.Loader) (*Resource, error) {
-// NewFromConfigMap returns a Resource given a configmap metadata from manifest
+	corev1CM, err := makeConfigMap(cm, l)
-// file.
-func NewFromConfigMap(cm manifest.ConfigMap) (*Resource, error) {
-	corev1CM, err := makeConfigMap(cm)
 	if err != nil {
 		return nil, err
 	}
@@ -39,29 +41,92 @@ func NewFromConfigMap(cm manifest.ConfigMap) (*Resource, error) {
 	return &Resource{Data: data}, nil
 }
 
-func makeConfigMap(cm manifest.ConfigMap) (*corev1.ConfigMap, error) {
+func makeConfigMap(cm manifest.ConfigMap, l loader.Loader) (*corev1.ConfigMap, error) {
+	var envPairs, literalPairs, filePairs []kvPair
+	var err error
+
 	corev1cm := &corev1.ConfigMap{}
 	corev1cm.APIVersion = "v1"
 	corev1cm.Kind = "ConfigMap"
 	corev1cm.Name = cm.Name
 	corev1cm.Data = map[string]string{}
 
-	// TODO: move the configmap helpers functions in this file/package
 	if cm.EnvSource != "" {
-		if err := cutil.HandleConfigMapFromEnvFileSource(corev1cm, cm.EnvSource); err != nil {
+		envPairs, err = keyValuesFromEnvFile(l, cm.EnvSource)
-			return nil, err
+		if err != nil {
+			return nil, fmt.Errorf("error reading keys from env source file: %s %v", cm.EnvSource, err)
 		}
 	}
-	if cm.FileSources != nil {
+
-		if err := cutil.HandleConfigMapFromFileSources(corev1cm, cm.FileSources); err != nil {
+	literalPairs, err = keyValuesFromLiteralSources(cm.LiteralSources)
-			return nil, err
+	if err != nil {
-		}
+		return nil, fmt.Errorf("error reading key values from literal sources: %v", err)
 	}
-	if cm.LiteralSources != nil {
+
-		if err := cutil.HandleConfigMapFromLiteralSources(corev1cm, cm.LiteralSources); err != nil {
+	filePairs, err = keyValuesFromFileSources(l, cm.FileSources)
-			return nil, err
+	if err != nil {
+		return nil, fmt.Errorf("error reading key values from file sources: %v", err)
+	}
+
+	allPairs := append(append(envPairs, literalPairs...), filePairs...)
+
+	// merge key value pairs from all the sources
+	for _, kv := range allPairs {
+		err = addKV(corev1cm.Data, kv)
+		if err != nil {
+			return nil, fmt.Errorf("error adding key in configmap: %v", err)
 		}
 	}
 
 	return corev1cm, nil
 }
+
+func keyValuesFromEnvFile(l loader.Loader, path string) ([]kvPair, error) {
+	content, err := l.Load(path)
+	if err != nil {
+		return nil, err
+	}
+	return keyValuesFromLines(content)
+}
+
+func keyValuesFromLiteralSources(sources []string) ([]kvPair, error) {
+	var kvs []kvPair
+	for _, s := range sources {
+		// TODO: move ParseLiteralSource in this file
+		k, v, err := cutil.ParseLiteralSource(s)
+		if err != nil {
+			return nil, err
+		}
+		kvs = append(kvs, kvPair{key: k, value: v})
+	}
+	return kvs, nil
+}
+
+func keyValuesFromFileSources(l loader.Loader, sources []string) ([]kvPair, error) {
+	var kvs []kvPair
+
+	for _, s := range sources {
+		key, path, err := cutil.ParseFileSource(s)
+		if err != nil {
+			return nil, err
+		}
+		fileContent, err := l.Load(path)
+		if err != nil {
+			return nil, err
+		}
+		kvs = append(kvs, kvPair{key: key, value: string(fileContent)})
+	}
+	return kvs, nil
+}
+
+// addKV adds key-value pair to the provided map.
+func addKV(m map[string]string, kv kvPair) error {
+	if errs := validation.IsConfigMapKey(kv.key); len(errs) != 0 {
+		return fmt.Errorf("%q is not a valid key name: %s", kv.key, strings.Join(errs, ";"))
+	}
+	if _, exists := m[kv.key]; exists {
+		return fmt.Errorf("key %s already exists: %v.", kv.key, m)
+	}
+	m[kv.key] = kv.value
+	return nil
+}

pkg/kinflate/resource/configmap_test.go

@@ -0,0 +1,137 @@
+/*
+Copyright 2018 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 resource_test
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	manifest "k8s.io/kubectl/pkg/apis/manifest/v1alpha1"
+	"k8s.io/kubectl/pkg/kinflate/resource"
+	"k8s.io/kubectl/pkg/loader"
+)
+
+func TestNewFromConfigMap(t *testing.T) {
+	type testCase struct {
+		description string
+		input       manifest.ConfigMap
+		l           loader.Loader
+		expected    resource.Resource
+	}
+
+	testCases := []testCase{
+		{
+			description: "construct config map from env",
+			input: manifest.ConfigMap{
+				Name: "envConfigMap",
+				DataSources: manifest.DataSources{
+					EnvSource: "app.env",
+				},
+			},
+			l: loader.FakeLoader{
+				Content: `DB_USERNAME=admin
+DB_PASSWORD=somepw
+`,
+			},
+			expected: resource.Resource{
+				Data: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"apiVersion": "v1",
+						"kind":       "ConfigMap",
+						"metadata": map[string]interface{}{
+							"name":              "envConfigMap",
+							"creationTimestamp": nil,
+						},
+						"data": map[string]interface{}{
+							"DB_USERNAME": "admin",
+							"DB_PASSWORD": "somepw",
+						},
+					},
+				},
+			},
+		},
+		{
+			description: "construct config map from file",
+			input: manifest.ConfigMap{
+				Name: "fileConfigMap",
+				DataSources: manifest.DataSources{
+					FileSources: []string{"app-init.ini"},
+				},
+			},
+			l: loader.FakeLoader{
+				Content: `FOO=bar
+BAR=baz
+`,
+			},
+			expected: resource.Resource{
+				Data: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"apiVersion": "v1",
+						"kind":       "ConfigMap",
+						"metadata": map[string]interface{}{
+							"name":              "fileConfigMap",
+							"creationTimestamp": nil,
+						},
+						"data": map[string]interface{}{
+							"app-init.ini": `FOO=bar
+BAR=baz
+`,
+						},
+					},
+				},
+			},
+		},
+		{
+			description: "construct config map from literal",
+			input: manifest.ConfigMap{
+				Name: "literalConfigMap",
+				DataSources: manifest.DataSources{
+					LiteralSources: []string{"a=x", "b=y"},
+				},
+			},
+			expected: resource.Resource{
+				Data: &unstructured.Unstructured{
+					Object: map[string]interface{}{
+						"apiVersion": "v1",
+						"kind":       "ConfigMap",
+						"metadata": map[string]interface{}{
+							"name":              "literalConfigMap",
+							"creationTimestamp": nil,
+						},
+						"data": map[string]interface{}{
+							"a": "x",
+							"b": "y",
+						},
+					},
+				},
+			},
+		},
+		// TODO: add testcase for data coming from multiple sources like
+		// files/literal/env etc.
+	}
+
+	for _, tc := range testCases {
+		r, err := resource.NewFromConfigMap(tc.input, tc.l)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if !reflect.DeepEqual(*r, tc.expected) {
+			t.Fatalf("in testcase: %q got:\n%+v\n expected:\n%+v\n", tc.description, *r, tc.expected)
+		}
+	}
+}

pkg/kinflate/resource/kv.go

@@ -0,0 +1,102 @@
+/*
+Copyright 2018 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 resource
+
+import (
+	"bufio"
+	"bytes"
+	"fmt"
+	"os"
+	"strings"
+	"unicode"
+	"unicode/utf8"
+
+	"k8s.io/apimachinery/pkg/util/validation"
+)
+
+var utf8bom = []byte{0xEF, 0xBB, 0xBF}
+
+// kvPair represents a key value pair.
+type kvPair struct {
+	key   string
+	value string
+}
+
+// keyValuesFromLines parses given content in to a list of key-value pairs.
+func keyValuesFromLines(content []byte) ([]kvPair, error) {
+	var kvs []kvPair
+
+	scanner := bufio.NewScanner(bytes.NewReader(content))
+	currentLine := 0
+	for scanner.Scan() {
+		// Process the current line, retrieving a key/value pair if
+		// possible.
+		scannedBytes := scanner.Bytes()
+		kv, err := kvFromLine(scannedBytes, currentLine)
+		if err != nil {
+			return nil, err
+		}
+		currentLine++
+
+		if len(kv.key) == 0 {
+			// no key means line was empty or a comment
+			continue
+		}
+
+		kvs = append(kvs, kv)
+	}
+	return kvs, nil
+}
+
+// kvFromLine returns a kv with blank key if the line is empty or a comment.
+// The value will be retrieved from the environment if necessary.
+func kvFromLine(line []byte, currentLine int) (kvPair, error) {
+	kv := kvPair{}
+
+	if !utf8.Valid(line) {
+		return kv, fmt.Errorf("line %d has invalid utf8 bytes : %v", line, string(line))
+	}
+
+	// We trim UTF8 BOM from the first line of the file but no others
+	if currentLine == 0 {
+		line = bytes.TrimPrefix(line, utf8bom)
+	}
+
+	// trim the line from all leading whitespace first
+	line = bytes.TrimLeftFunc(line, unicode.IsSpace)
+
+	// If the line is empty or a comment, we return a blank key/value pair.
+	if len(line) == 0 || line[0] == '#' {
+		return kv, nil
+	}
+
+	data := strings.SplitN(string(line), "=", 2)
+	key := data[0]
+	if errs := validation.IsEnvVarName(key); len(errs) != 0 {
+		return kv, fmt.Errorf("%q is not a valid key name: %s", key, strings.Join(errs, ";"))
+	}
+
+	if len(data) == 2 {
+		kv.value = data[1]
+	} else {
+		// No value (no `=` in the line) is a signal to obtain the value
+		// from the environment.
+		kv.value = os.Getenv(key)
+	}
+	kv.key = key
+	return kv, nil
+}

pkg/kinflate/resource/kv_test.go

@@ -0,0 +1,67 @@
+/*
+Copyright 2018 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 resource
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestKeyValuesFromLines(t *testing.T) {
+	tests := []struct {
+		desc          string
+		content       string
+		expectedPairs []kvPair
+		expectedErr   bool
+	}{
+		{
+			desc: "valid kv content parse",
+			content: `
+		k1=v1
+		k2=v2
+		`,
+			expectedPairs: []kvPair{
+				{key: "k1", value: "v1"},
+				{key: "k2", value: "v2"},
+			},
+			expectedErr: false,
+		},
+		{
+			desc: "content with comments",
+			content: `
+		k1=v1
+		#k2=v2
+		`,
+			expectedPairs: []kvPair{
+				{key: "k1", value: "v1"},
+			},
+			expectedErr: false,
+		},
+		// TODO: add negative testcases
+	}
+
+	for _, test := range tests {
+		pairs, err := keyValuesFromLines([]byte(test.content))
+		if test.expectedErr && err == nil {
+			t.Fatalf("%s should not return error", test.desc)
+		}
+
+		if !reflect.DeepEqual(pairs, test.expectedPairs) {
+			t.Errorf("%s should succeed, got:%v exptected:%v", test.desc, pairs, test.expectedPairs)
+		}
+
+	}
+}

pkg/loader/fake_loader.go

@@ -0,0 +1,32 @@
+/*
+Copyright 2018 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 loader
+
+// FakeLoader implements Loader interface.
+type FakeLoader struct {
+	Content string
+}
+
+func (f FakeLoader) New(root string) (Loader, error) {
+	return f, nil
+}
+
+func (f FakeLoader) Load(location string) ([]byte, error) {
+	return []byte(f.Content), nil
+}
+
+var _ Loader = FakeLoader{}