Repository: appvia/kev
Pull request #608 (2021-07-29 18:30:24)

Fixes environment variable symbolic value handling
Addresses #607 

This Bugfix PR changes the way environment variable symbolic values are handled, specifically:
* variables values that contain special case keyword such as `secret`, `config`, `pod`, `container` is treated as literal string unless it matches specific format that would indicate Kubernetes object should be referenced instead.  
* Adds validation to the expected format for symbolic values
* Returns explicit errors on malformed / unsupported symbolic values


Patch:
diff --git a/pkg/kev/converter/kubernetes/transform.go b/pkg/kev/converter/kubernetes/transform.go
index 37b5694..f48601c 100644
--- a/pkg/kev/converter/kubernetes/transform.go
+++ b/pkg/kev/converter/kubernetes/transform.go
@@ -1333,6 +1333,8 @@ func (k *Kubernetes) configEnvs(projectService ProjectService) ([]v1.EnvVar, err
 	envs := EnvSort{}
 	envsWithDeps := []v1.EnvVar{}
 
+	refK8s := regexp.MustCompile(`^(config|pod|secret|container)\.[^\.]*\.[^\.]*`)
+
 	// @step load up the environment variables
 	for k, v := range projectService.environment() {
 		// @step for nil value we replace it with empty string
@@ -1341,15 +1343,29 @@ func (k *Kubernetes) configEnvs(projectService ProjectService) ([]v1.EnvVar, err
 			v = &temp
 		}
 
-		// @step generate EnvVar spec and handle special value reference cases for `secret`, `configmap`, `pod` field or `container` resource
+		// @step generate EnvVar spec and handle special value reference cases for kubernetes `secret`, `configmap`, `pod` field or `container` resource
 		// e.g. `secret.my-secret-name.my-key`,
-		// 		`config.my-config-name.config-key`,
-		// 		`pod.metadata.namespace`,
-		// 		`container.my-container-name.limits.cpu`,
+		//      `config.my-config-name.config-key`,
+		//      `pod.metadata.namespace`,
+		//      `container.my-container-name.limits.cpu`,
 		// if none of the special cases has been referenced by the env var value then it's going to be treated as literal value
+
+		specialCase := ""
+
+		// @step determine whether env var value matches special case
+		obj := refK8s.FindStringSubmatch(*v)
+		if len(obj) > 1 {
+			specialCase = obj[1]
+		}
+
 		parts := strings.Split(*v, ".")
-		switch parts[0] {
+
+		switch specialCase {
 		case "secret":
+			if len(parts) != 3 {
+				return nil, fmt.Errorf("environment variable %s referencing kubernetes secret is invalid: %s", k, *v)
+			}
+
 			envs = append(envs, v1.EnvVar{
 				Name: k,
 				ValueFrom: &v1.EnvVarSource{
@@ -1362,6 +1378,10 @@ func (k *Kubernetes) configEnvs(projectService ProjectService) ([]v1.EnvVar, err
 				},
 			})
 		case "config":
+			if len(parts) != 3 {
+				return nil, fmt.Errorf("environment variable %s referencing kubernetes config map is invalid: %s", k, *v)
+			}
+
 			envs = append(envs, v1.EnvVar{
 				Name: k,
 				ValueFrom: &v1.EnvVarSource{
@@ -1374,14 +1394,16 @@ func (k *Kubernetes) configEnvs(projectService ProjectService) ([]v1.EnvVar, err
 				},
 			})
 		case "pod":
-			// Selects a field of the pod
-			// supported paths: metadata.name, metadata.namespace, metadata.labels, metadata.annotations,
-			// 					spec.nodeName, spec.serviceAccountName, status.hostIP, status.podIP, status.podIPs.
+			// Selects a field of the pod; Supported paths:
 			paths := []string{
 				"metadata.name", "metadata.namespace", "metadata.labels", "metadata.annotations",
 				"spec.nodeName", "spec.serviceAccountName", "status.hostIP", "status.podIP", "status.podIPs",
 			}
 
+			if len(parts) != 3 {
+				return nil, fmt.Errorf("environment variable %s referencing kubernetes pod field is invalid: %s", k, *v)
+			}
+
 			thePath := strings.Join(parts[1:], ".")
 
 			if contains(paths, thePath) {
@@ -1394,20 +1416,25 @@ func (k *Kubernetes) configEnvs(projectService ProjectService) ([]v1.EnvVar, err
 					},
 				})
 			} else {
-				log.WarnfWithFields(log.Fields{
+				log.DebugfWithFields(log.Fields{
 					"project-service": projectService.Name,
 					"env-var":         k,
 					"path":            thePath,
 				}, "Unsupported Pod field reference: %s", thePath)
+
+				return nil, fmt.Errorf("environment variable %s references unsupported kubernetes pod field: %s", k, *v)
 			}
 		case "container":
 			// Selects a resource of the container. Only resources limits and requests are currently supported:
-			// 		limits.cpu, limits.memory, limits.ephemeral-storage,
-			//  	requests.cpu, requests.memory and requests.ephemeral-storage
 			resources := []string{
 				"limits.cpu", "limits.memory", "limits.ephemeral-storage",
 				"requests.cpu", "requests.memory", "requests.ephemeral-storage",
 			}
+
+			if len(parts) != 4 {
+				return nil, fmt.Errorf("environment variable %s referencing kubernetes container resource is invalid: %s", k, *v)
+			}
+
 			theResource := strings.Join(parts[2:], ".")
 
 			if contains(resources, theResource) {
@@ -1421,12 +1448,14 @@ func (k *Kubernetes) configEnvs(projectService ProjectService) ([]v1.EnvVar, err
 					},
 				})
 			} else {
-				log.WarnfWithFields(log.Fields{
+				log.DebugfWithFields(log.Fields{
 					"project-service": projectService.Name,
 					"env-var":         k,
 					"container":       parts[1],
 					"resource":        theResource,
 				}, "Unsupported Container resource reference: %s", theResource)
+
+				return nil, fmt.Errorf("environment variable %s references unsupported kubernetes container resource: %s", k, *v)
 			}
 		default:
 			if strings.Contains(*v, "{{") && strings.Contains(*v, "}}") {
@@ -1690,15 +1719,13 @@ func (k *Kubernetes) updateKubernetesObjects(projectService ProjectService, obje
 	// @step configure the environment variables
 	envs, err := k.configEnvs(projectService)
 	if err != nil {
-		log.Error("Unable to load env variables")
-		return err
+		return errors.Wrap(err, "Unable to load env variables")
 	}
 
 	// @step configure the container volumes
 	volumesMounts, volumes, pvcs, cms, err := k.configVolumes(projectService)
 	if err != nil {
-		log.Error("Unable to configure container volumes")
-		return err
+		return errors.Wrap(err, "Unable to configure container volumes")
 	}
 
 	// @step configure Tmpfs


Test patch:
diff --git a/pkg/kev/converter/kubernetes/transform_test.go b/pkg/kev/converter/kubernetes/transform_test.go
index 03d76a4..0d57c75 100644
--- a/pkg/kev/converter/kubernetes/transform_test.go
+++ b/pkg/kev/converter/kubernetes/transform_test.go
@@ -1752,6 +1752,72 @@ var _ = Describe("Transform", func() {
 
 		})
 
+		Context("for environment variables values that start with a special case keywords", func() {
+
+			When("env var value starts with a special keyword but doesn't have an expected format", func() {
+				secret := "secret.foo"
+				config := "config.bar"
+				pod := "pod.baz"
+				container := "container"
+
+				BeforeEach(func() {
+					projectService.Environment = composego.MappingWithEquals{
+						"MY_SECRET": &secret,
+						"MY_CONFIG": &config,
+						"MY_POD":    &pod,
+						"MY_CONT":   &container,
+					}
+				})
+
+				It("treats the value as literal", func() {
+					vars, err := k.configEnvs(projectService)
+
+					Expect(vars).To(HaveLen(4))
+					Expect(vars).To(ContainElements([]v1.EnvVar{
+						{
+							Name:  "MY_SECRET",
+							Value: secret,
+						},
+						{
+							Name:  "MY_CONFIG",
+							Value: config,
+						},
+						{
+							Name:  "MY_POD",
+							Value: pod,
+						},
+						{
+							Name:  "MY_CONT",
+							Value: container,
+						},
+					}))
+
+					Expect(err).ToNot(HaveOccurred())
+				})
+			})
+
+			Context("when special case value matches the format ", func() {
+
+				Context("and the symbolic value has insufficient number of elements", func() {
+					val := "secret.foo.bar.baz"
+
+					BeforeEach(func() {
+						projectService.Environment = composego.MappingWithEquals{
+							"MY_SECRET": &val,
+						}
+					})
+
+					It("returns an error", func() {
+						vars, err := k.configEnvs(projectService)
+
+						Expect(vars).To(HaveLen(0))
+						Expect(err).To(HaveOccurred())
+						Expect(err.Error()).To(Equal("environment variable MY_SECRET referencing kubernetes secret is invalid: secret.foo.bar.baz"))
+					})
+				})
+			})
+		})
+
 		Context("for env vars with symbolic values", func() {
 
 			Context("as secret.secret-name.secret-key", func() {
@@ -1838,16 +1904,8 @@ var _ = Describe("Transform", func() {
 						vars, err := k.configEnvs(projectService)
 
 						Expect(vars).To(HaveLen(0))
-
-						assertLog(logrus.WarnLevel,
-							"Unsupported Pod field reference: unsupported.path",
-							map[string]string{
-								"project-service": projectService.Name,
-								"env-var":         "MY_CONFIG",
-								"path":            "unsupported.path",
-							})
-
-						Expect(err).ToNot(HaveOccurred())
+						Expect(err).To(HaveOccurred())
+						Expect(err.Error()).To(Equal("environment variable MY_CONFIG references unsupported kubernetes pod field: pod.unsupported.path"))
 					})
 				})
 			})
@@ -1889,17 +1947,8 @@ var _ = Describe("Transform", func() {
 						vars, err := k.configEnvs(projectService)
 
 						Expect(vars).To(HaveLen(0))
-
-						assertLog(logrus.WarnLevel,
-							"Unsupported Container resource reference: unsupported.resource",
-							map[string]string{
-								"project-service": projectService.Name,
-								"env-var":         "MY_CONFIG",
-								"container":       "my-container",
-								"resource":        "unsupported.resource",
-							})
-
-						Expect(err).ToNot(HaveOccurred())
+						Expect(err).To(HaveOccurred())
+						Expect(err.Error()).To(Equal("environment variable MY_CONFIG references unsupported kubernetes container resource: container.my-container.unsupported.resource"))
 					})
 				})
 			})