From 1b29e031e3318c8e13a9bdee50c52ccf1ac93ee7 Mon Sep 17 00:00:00 2001 From: robertaistleitner Date: Tue, 15 Apr 2025 17:39:45 +0200 Subject: [PATCH] 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 | 10 +++--- pkg/transformer/kubernetes/k8sutils_test.go | 6 ++-- pkg/transformer/kubernetes/kubernetes.go | 31 +++++++++---------- pkg/transformer/kubernetes/podspec.go | 3 +- .../fixtures/configmap-pod/output-k8s.yaml | 21 +++---------- .../fixtures/configmap-pod/output-os.yaml | 21 +++---------- script/test/fixtures/env/output-k8s.yaml | 29 ++++------------- script/test/fixtures/env/output-os.yaml | 27 ++++------------ 8 files changed, 45 insertions(+), 103 deletions(-) diff --git a/pkg/transformer/kubernetes/k8sutils.go b/pkg/transformer/kubernetes/k8sutils.go index 61b940d2..5e7da57e 100644 --- a/pkg/transformer/kubernetes/k8sutils.go +++ b/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 } diff --git a/pkg/transformer/kubernetes/k8sutils_test.go b/pkg/transformer/kubernetes/k8sutils_test.go index 71c7cfdb..c8b137a1 100644 --- a/pkg/transformer/kubernetes/k8sutils_test.go +++ b/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 { diff --git a/pkg/transformer/kubernetes/kubernetes.go b/pkg/transformer/kubernetes/kubernetes.go index a1fecbc5..360cfcc7 100644 --- a/pkg/transformer/kubernetes/kubernetes.go +++ b/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 ignore the environment variables - // already specified + // If there is an env_file, use ConfigMaps and add them using EnvFrom 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. diff --git a/pkg/transformer/kubernetes/podspec.go b/pkg/transformer/kubernetes/podspec.go index 420b078b..c09c6d1c 100644 --- a/pkg/transformer/kubernetes/podspec.go +++ b/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, diff --git a/script/test/fixtures/configmap-pod/output-k8s.yaml b/script/test/fixtures/configmap-pod/output-k8s.yaml index 04bf4198..fffc7cf0 100644 --- a/script/test/fixtures/configmap-pod/output-k8s.yaml +++ b/script/test/fixtures/configmap-pod/output-k8s.yaml @@ -23,22 +23,11 @@ metadata: name: redis spec: containers: - - env: - - name: ALLOW_EMPTY_PASSWORD - valueFrom: - configMapKeyRef: - key: ALLOW_EMPTY_PASSWORD - name: foo-env - - name: BAR - valueFrom: - configMapKeyRef: - key: BAR - name: bar-env - - name: FOO - valueFrom: - configMapKeyRef: - key: FOO - name: bar-env + - envFrom: + - configMapRef: + name: foo-env + - configMapRef: + name: bar-env image: bitnami/redis:latest name: redis ports: diff --git a/script/test/fixtures/configmap-pod/output-os.yaml b/script/test/fixtures/configmap-pod/output-os.yaml index 04bf4198..fffc7cf0 100644 --- a/script/test/fixtures/configmap-pod/output-os.yaml +++ b/script/test/fixtures/configmap-pod/output-os.yaml @@ -23,22 +23,11 @@ metadata: name: redis spec: containers: - - env: - - name: ALLOW_EMPTY_PASSWORD - valueFrom: - configMapKeyRef: - key: ALLOW_EMPTY_PASSWORD - name: foo-env - - name: BAR - valueFrom: - configMapKeyRef: - key: BAR - name: bar-env - - name: FOO - valueFrom: - configMapKeyRef: - key: FOO - name: bar-env + - envFrom: + - configMapRef: + name: foo-env + - configMapRef: + name: bar-env image: bitnami/redis:latest name: redis ports: diff --git a/script/test/fixtures/env/output-k8s.yaml b/script/test/fixtures/env/output-k8s.yaml index 974a042c..cd9031d7 100644 --- a/script/test/fixtures/env/output-k8s.yaml +++ b/script/test/fixtures/env/output-k8s.yaml @@ -52,17 +52,9 @@ spec: io.kompose.service: another-namenode spec: containers: - - env: - - name: BAR - valueFrom: - configMapKeyRef: - key: BAR - name: hadoop-hive-namenode-env - - name: FOO - valueFrom: - configMapKeyRef: - key: FOO - name: hadoop-hive-namenode-env + - envFrom: + - configMapRef: + name: hadoop-hive-namenode-env image: bde2020/hadoop-namenode:2.0.0-hadoop2.7.4-java8 name: another-namenode ports: @@ -72,7 +64,6 @@ spec: protocol: TCP restartPolicy: Always - --- apiVersion: v1 data: @@ -103,18 +94,11 @@ spec: spec: containers: - env: - - name: BAR - valueFrom: - configMapKeyRef: - key: BAR - name: hadoop-hive-namenode-env - name: CLUSTER_NAME value: test - - name: FOO - valueFrom: - configMapKeyRef: - key: FOO - name: hadoop-hive-namenode-env + envFrom: + - configMapRef: + name: hadoop-hive-namenode-env image: bde2020/hadoop-namenode:2.0.0-hadoop2.7.4-java8 name: namenode ports: @@ -124,4 +108,3 @@ spec: protocol: TCP restartPolicy: Always - diff --git a/script/test/fixtures/env/output-os.yaml b/script/test/fixtures/env/output-os.yaml index 1f4ef079..a51c0a88 100644 --- a/script/test/fixtures/env/output-os.yaml +++ b/script/test/fixtures/env/output-os.yaml @@ -62,17 +62,9 @@ spec: io.kompose.service: another-namenode spec: containers: - - env: - - name: BAR - valueFrom: - configMapKeyRef: - key: BAR - name: hadoop-hive-namenode-env - - name: FOO - valueFrom: - configMapKeyRef: - key: FOO - name: hadoop-hive-namenode-env + - envFrom: + - configMapRef: + name: hadoop-hive-namenode-env image: ' ' name: another-namenode ports: @@ -129,18 +121,11 @@ spec: spec: containers: - env: - - name: BAR - valueFrom: - configMapKeyRef: - key: BAR - name: hadoop-hive-namenode-env - name: CLUSTER_NAME value: test - - name: FOO - valueFrom: - configMapKeyRef: - key: FOO - name: hadoop-hive-namenode-env + envFrom: + - configMapRef: + name: hadoop-hive-namenode-env image: ' ' name: namenode ports: