Fix ConfigMap name clash if env_file name is equal (#2001)

* Fix ConfigMap name clash if env_file name is equal #2000

* Generate envFrom instead of single env entries from env_file entries #1999

* Fix failing test fixture outputs

pkg/transformer/kubernetes/k8sutils.go

@@ -547,7 +547,7 @@ func (k *Kubernetes) UpdateKubernetesObjectsMultipleContainers(name string, serv
 // UpdateKubernetesObjects loads configurations to k8s objects
 func (k *Kubernetes) UpdateKubernetesObjects(name string, service kobject.ServiceConfig, opt kobject.ConvertOptions, objects *[]runtime.Object) error {
 	// Configure the environment variables.
-	envs, err := ConfigEnvs(service, opt)
+	envs, envsFrom, err := ConfigEnvs(service, opt)
 	if err != nil {
 		return errors.Wrap(err, "Unable to load env variables")
 	}
@@ -589,6 +589,7 @@ func (k *Kubernetes) UpdateKubernetesObjects(name string, service kobject.Servic
 	fillTemplate := func(template *api.PodTemplateSpec) error {
 		template.Spec.Containers[0].Name = GetContainerName(service)
 		template.Spec.Containers[0].Env = envs
+		template.Spec.Containers[0].EnvFrom = envsFrom
 		template.Spec.Containers[0].Command = service.Command
 		template.Spec.Containers[0].Args = GetContainerArgs(service)
 		template.Spec.Containers[0].WorkingDir = service.WorkingDir
@@ -973,12 +974,9 @@ func GetContentFromFile(file string) (string, error) {
 // FormatEnvName format env name
 func FormatEnvName(name string, serviceName string) string {
 	envName := strings.Trim(name, "./")
-	// only take string after the last slash only if the string contains a slash
-	if strings.Contains(envName, "/") {
-		envName = envName[strings.LastIndex(envName, "/")+1:]
-	}
 
-	envName = strings.Replace(envName, ".", "-", -1)
+	// replace all non-alphanumerical characters with dashes to have a unique envName (env filename could be used multiple times)
+	envName = regexp.MustCompile(`[^a-zA-Z0-9]`).ReplaceAllString(envName, "-")
 	envName = getUsableNameEnvFile(envName, serviceName)
 	return envName
 }

pkg/transformer/kubernetes/k8sutils_test.go

@@ -875,7 +875,7 @@ func TestFormatEnvName(t *testing.T) {
 			args: args{
 				name: "random/test/v1",
 			},
-			want: "v1",
+			want: "random-test-v1",
 		},
 		{
 			name: "check that ./ is removed",
@@ -885,7 +885,7 @@ func TestFormatEnvName(t *testing.T) {
 			want: "random",
 		},
 		{
-			name: "check that ./ is removed",
+			name: "check that everything after $ is removed",
 			args: args{
 				name: "abcdefghijklnmopqrstuvxyzabcdefghijklmnopqrstuvwxyzabcdejghijkl$Hereisadditional",
 			},
@@ -897,7 +897,7 @@ func TestFormatEnvName(t *testing.T) {
 				name:        "src/app/.env",
 				serviceName: "app",
 			},
-			want: "app-env",
+			want: "src-app--env",
 		},
 	}
 	for _, tt := range tests {

pkg/transformer/kubernetes/kubernetes.go

@@ -1153,19 +1153,26 @@ func (k *Kubernetes) ConfigPVCVolumeSource(name string, readonly bool) *api.Volu
 }
 
 // ConfigEnvs configures the environment variables.
-func ConfigEnvs(service kobject.ServiceConfig, opt kobject.ConvertOptions) ([]api.EnvVar, error) {
+func ConfigEnvs(service kobject.ServiceConfig, opt kobject.ConvertOptions) ([]api.EnvVar, []api.EnvFromSource, error) {
 	envs := transformer.EnvSort{}
+	envsFrom := []api.EnvFromSource{}
 
 	keysFromEnvFile := make(map[string]bool)
-
+	// If there is an env_file, use ConfigMaps and add them using EnvFrom
-	// If there is an env_file, use ConfigMaps and ignore the environment variables
-	// already specified
 
 	if len(service.EnvFile) > 0 {
 		// Load each env_file
 		for _, file := range service.EnvFile {
 			envName := FormatEnvName(file, service.Name)
 
+			envsFrom = append(envsFrom, api.EnvFromSource{
+				ConfigMapRef: &api.ConfigMapEnvSource{
+					LocalObjectReference: api.LocalObjectReference{
+						Name: envName,
+					},
+				},
+			})
+
 			// Load environment variables from file
 			workDir, err := transformer.GetComposeFileDir(opt.InputFiles)
 			if err != nil {
@@ -1173,21 +1180,11 @@ func ConfigEnvs(service kobject.ServiceConfig, opt kobject.ConvertOptions) ([]ap
 			}
 			envLoad, err := GetEnvsFromFile(filepath.Join(workDir, file))
 			if err != nil {
-				return envs, errors.Wrap(err, "Unable to read env_file")
+				return envs, envsFrom, errors.Wrap(err, "Unable to read env_file")
 			}
 
-			// Add configMapKeyRef to each environment variable
+			// Mark environment variable source to env file
 			for k := range envLoad {
-				envs = append(envs, api.EnvVar{
-					Name: k,
-					ValueFrom: &api.EnvVarSource{
-						ConfigMapKeyRef: &api.ConfigMapKeySelector{
-							LocalObjectReference: api.LocalObjectReference{
-								Name: envName,
-							},
-							Key: k,
-						}},
-				})
 				keysFromEnvFile[k] = true
 			}
 		}
@@ -1210,7 +1207,7 @@ func ConfigEnvs(service kobject.ServiceConfig, opt kobject.ConvertOptions) ([]ap
 	// we need this because envs are not populated in any random order
 	// this sorting ensures they are populated in a particular order
 	sort.Stable(envs)
-	return envs, nil
+	return envs, envsFrom, nil
 }
 
 // ConfigAffinity configures the Affinity.

pkg/transformer/kubernetes/podspec.go

@@ -32,7 +32,7 @@ func AddContainer(service kobject.ServiceConfig, opt kobject.ConvertOptions) Pod
 			image = name
 		}
 
-		envs, err := ConfigEnvs(service, opt)
+		envs, envsFrom, err := ConfigEnvs(service, opt)
 		if err != nil {
 			panic("Unable to load env variables")
 		}
@@ -41,6 +41,7 @@ func AddContainer(service kobject.ServiceConfig, opt kobject.ConvertOptions) Pod
 			Name:           name,
 			Image:          image,
 			Env:            envs,
+			EnvFrom:        envsFrom,
 			Command:        service.Command,
 			Args:           service.Args,
 			WorkingDir:     service.WorkingDir,

script/test/fixtures/configmap-pod/output-k8s.yaml

@@ -23,21 +23,10 @@ metadata:
   name: redis
 spec:
   containers:
-    - env:
+    - envFrom:
-        - name: ALLOW_EMPTY_PASSWORD
+        - configMapRef:
-          valueFrom:
-            configMapKeyRef:
-              key: ALLOW_EMPTY_PASSWORD
             name: foo-env
-        - name: BAR
+        - configMapRef:
-          valueFrom:
-            configMapKeyRef:
-              key: BAR
-              name: bar-env
-        - name: FOO
-          valueFrom:
-            configMapKeyRef:
-              key: FOO
             name: bar-env
       image: bitnami/redis:latest
       name: redis

script/test/fixtures/configmap-pod/output-os.yaml

@@ -23,21 +23,10 @@ metadata:
   name: redis
 spec:
   containers:
-    - env:
+    - envFrom:
-        - name: ALLOW_EMPTY_PASSWORD
+        - configMapRef:
-          valueFrom:
-            configMapKeyRef:
-              key: ALLOW_EMPTY_PASSWORD
             name: foo-env
-        - name: BAR
+        - configMapRef:
-          valueFrom:
-            configMapKeyRef:
-              key: BAR
-              name: bar-env
-        - name: FOO
-          valueFrom:
-            configMapKeyRef:
-              key: FOO
             name: bar-env
       image: bitnami/redis:latest
       name: redis

script/test/fixtures/env/output-k8s.yaml

@@ -52,16 +52,8 @@ spec:
         io.kompose.service: another-namenode
     spec:
       containers:
-        - env:
+        - envFrom:
-            - name: BAR
+            - configMapRef:
-              valueFrom:
-                configMapKeyRef:
-                  key: BAR
-                  name: hadoop-hive-namenode-env
-            - name: FOO
-              valueFrom:
-                configMapKeyRef:
-                  key: FOO
                 name: hadoop-hive-namenode-env
           image: bde2020/hadoop-namenode:2.0.0-hadoop2.7.4-java8
           name: another-namenode
@@ -72,7 +64,6 @@ spec:
               protocol: TCP
       restartPolicy: Always
 
-
 ---
 apiVersion: v1
 data:
@@ -103,17 +94,10 @@ spec:
     spec:
       containers:
         - env:
-            - name: BAR
-              valueFrom:
-                configMapKeyRef:
-                  key: BAR
-                  name: hadoop-hive-namenode-env
             - name: CLUSTER_NAME
               value: test
-            - name: FOO
+          envFrom:
-              valueFrom:
+            - configMapRef:
-                configMapKeyRef:
-                  key: FOO
                 name: hadoop-hive-namenode-env
           image: bde2020/hadoop-namenode:2.0.0-hadoop2.7.4-java8
           name: namenode
@@ -124,4 +108,3 @@ spec:
               protocol: TCP
       restartPolicy: Always
 
-

script/test/fixtures/env/output-os.yaml

@@ -62,16 +62,8 @@ spec:
         io.kompose.service: another-namenode
     spec:
       containers:
-        - env:
+        - envFrom:
-            - name: BAR
+            - configMapRef:
-              valueFrom:
-                configMapKeyRef:
-                  key: BAR
-                  name: hadoop-hive-namenode-env
-            - name: FOO
-              valueFrom:
-                configMapKeyRef:
-                  key: FOO
                 name: hadoop-hive-namenode-env
           image: ' '
           name: another-namenode
@@ -129,17 +121,10 @@ spec:
     spec:
       containers:
         - env:
-            - name: BAR
-              valueFrom:
-                configMapKeyRef:
-                  key: BAR
-                  name: hadoop-hive-namenode-env
             - name: CLUSTER_NAME
               value: test
-            - name: FOO
+          envFrom:
-              valueFrom:
+            - configMapRef:
-                configMapKeyRef:
-                  key: FOO
                 name: hadoop-hive-namenode-env
           image: ' '
           name: namenode