Fixing project.toml build env to match specification changes in v0.2,…

… also adding backwards compatibility with previous version

Signed-off-by: Juan Bustamante <jbustamante@vmware.com>

pkg/cnb/env_vars.go

@@ -1,6 +1,7 @@
 package cnb
 
 import (
+	"github.com/pkg/errors"
 	"io/ioutil"
 	"os"
 	"path"
@@ -27,3 +28,55 @@ type envVariable struct {
 	Name  string `json:"name" toml:"name"`
 	Value string `json:"value" toml:"value"`
 }
+
+type buildEnvVariable struct {
+	Env []envVariable `toml:"env"`
+}
+
+type envBuildVariable struct {
+	Env []envVariable
+}
+
+func (a *envBuildVariable) UnmarshalTOML(f interface{}) error {
+	var (
+		env []envVariable
+		err error
+	)
+	switch v := f.(type) {
+	case map[string]interface{}:
+		if envs, ok := v["build"].([]map[string]interface{}); ok {
+			env, err = buildEnv(envs)
+			if err != nil {
+				return err
+			}
+		}
+	case []map[string]interface{}:
+		env, err = buildEnv(v)
+		if err != nil {
+			return err
+		}
+	default:
+		return errors.New("environment variables in project descriptor could not be parsed")
+	}
+	a.Env = env
+	return nil
+}
+
+func buildEnv(v []map[string]interface{}) ([]envVariable, error) {
+	var e []envVariable
+	for _, env := range v {
+		if name, nameOk := env["name"].(string); nameOk {
+			if value, valueOk := env["value"].(string); valueOk {
+				e = append(e, envVariable{
+					Name:  name,
+					Value: value,
+				})
+			} else {
+				return nil, errors.Errorf("environment variable '%s' is not a string value", name)
+			}
+		} else {
+			return nil, errors.New("environment variable 'name' is not a string")
+		}
+	}
+	return e, nil
+}

pkg/cnb/project_descriptor.go

@@ -41,7 +41,7 @@ func ProcessProjectDescriptor(appDir, descriptorPath, platformDir string, logger
 	if err := processFiles(appDir, d.IO.Buildpacks.build); err != nil {
 		return err
 	}
-	return serializeEnvVars(d.IO.Buildpacks.Env, platformDir)
+	return serializeEnvVars(d.env(), platformDir)
 }
 
 func parseProjectDescriptor(file string, logger *log.Logger) (descriptorV2, error) {
@@ -71,6 +71,11 @@ func v1ToV2(v1 descriptorV1) descriptorV2 {
 			Buildpacks: cnbTableV2{
 				build: v1.Build.build,
 				Group: v1.Build.Buildpacks,
+				buildEnvVariableV2: buildEnvVariableV2{
+					BuildEnv: buildEnvVariable{
+						Env: v1.Build.Env,
+					},
+				},
 			},
 		},
 	}
@@ -129,6 +134,17 @@ type descriptorV2 struct {
 	IO      ioTable `toml:"io"`
 }
 
+// Because CNB Project Descriptor v0.2 has two ways for defining environment variables.
+// see https://github.com/buildpacks/spec/blob/main/extensions/project-descriptor.md#iobuildpacksbuildenv-optional
+// This function calculates the final environment variables
+func (d *descriptorV2) env() []envVariable {
+	env := d.IO.Buildpacks.BuildEnv.Env
+	if env == nil {
+		env = d.IO.Buildpacks.EnvBuild.Env
+	}
+	return env
+}
+
 type project struct {
 	SchemaVersion string `toml:"schema-version"`
 }
@@ -138,15 +154,21 @@ type ioTable struct {
 }
 
 type cnbTableV2 struct {
-	build `toml:",inline"`
-	Group []buildpack `toml:"group"`
+	build              `toml:",inline"`
+	buildEnvVariableV2 `toml:",inline"`
+	Group              []buildpack `toml:"group"`
 }
 
 type build struct {
-	Include []string      `toml:"include"`
-	Exclude []string      `toml:"exclude"`
-	Builder string        `toml:"builder"`
-	Env     []envVariable `toml:"env"`
+	Include []string `toml:"include"`
+	Exclude []string `toml:"exclude"`
+	Builder string   `toml:"builder"`
+}
+
+type buildEnvVariableV2 struct {
+	BuildEnv buildEnvVariable `toml:"build"`
+	// Deprecated: use `[[io.buildpacks.build.env]]` instead. see https://github.com/buildpacks/pack/pull/1479
+	EnvBuild envBuildVariable `toml:"env"`
 }
 
 type buildpack struct {
@@ -161,5 +183,6 @@ type descriptorV1 struct {
 
 type cnbTableV1 struct {
 	build      `toml:",inline"`
-	Buildpacks []buildpack `toml:"buildpacks"`
+	Buildpacks []buildpack   `toml:"buildpacks"`
+	Env        []envVariable `toml:"env"`
 }

pkg/cnb/project_descriptor_test.go

@@ -225,8 +225,73 @@ uri = "check-this-out.com"
 
 			when("using descriptor v2", func() {
 				when("the descriptor has build env vars", func() {
-					it.Before(func() {
-						ioutil.WriteFile(projectToml, []byte(`
+					when("vars where set with older v0.2 project.toml", func() {
+						when("io.buildpacks.env.build format is used", func() {
+							when("format is valid", func() {
+								it.Before(func() {
+									ioutil.WriteFile(projectToml, []byte(`
+[_]
+schema-version = "0.2"
+[[io.buildpacks.env.build]]
+name = "keyA"
+value = "valueA"
+[[io.buildpacks.env.build]]
+name = "keyB"
+value = "valueB"
+[[io.buildpacks.env.build]]
+name = "keyC"
+value = "valueC"
+# check that later keys override previous ones
+[[io.buildpacks.env.build]]
+name = "keyC"
+value = "valueAnotherC"
+				`), 0644)
+								})
+								it("writes all env var files to the platform dir", func() {
+									assert.Nil(t, cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger))
+									checkEnvVar(t, platformDir, "keyA", "valueA")
+									checkEnvVar(t, platformDir, "keyB", "valueB")
+									checkEnvVar(t, platformDir, "keyC", "valueAnotherC")
+								})
+							})
+							when("format is invalid", func() {
+								when("'value' is invalid", func() {
+									it.Before(func() {
+										ioutil.WriteFile(projectToml, []byte(`
+[_]
+schema-version = "0.2"
+[[io.buildpacks.env.build]]
+name = "KeyA"
+value = 1
+				`), 0644)
+									})
+									it("writes all env var files to the platform dir", func() {
+										err := cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger)
+										assert.EqualError(t, err, "environment variable 'KeyA' is not a string value")
+									})
+								})
+								when("'name' is invalid", func() {
+									it.Before(func() {
+										ioutil.WriteFile(projectToml, []byte(`
+[_]
+schema-version = "0.2"
+[[io.buildpacks.env.build]]
+name = 1
+value = "ValueA"
+				`), 0644)
+									})
+									it("writes all env var files to the platform dir", func() {
+										err := cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger)
+										assert.EqualError(t, err, "environment variable 'name' is not a string")
+									})
+								})
+							})
+						})
+
+						when("io.buildpacks.env format is used", func() {
+							when("format is valid", func() {
+								it.Before(func() {
+									ioutil.WriteFile(projectToml, []byte(`
 [_]
 schema-version = "0.2"
 [[io.buildpacks.env]]
@@ -241,14 +306,96 @@ value = "valueC"
 # check that later keys override previous ones
 [[io.buildpacks.env]]
 name = "keyC"
+value = "valueAnotherC"`), 0644)
+								})
+								it("writes all env var files to the platform dir", func() {
+									assert.Nil(t, cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger))
+									checkEnvVar(t, platformDir, "keyA", "valueA")
+									checkEnvVar(t, platformDir, "keyB", "valueB")
+									checkEnvVar(t, platformDir, "keyC", "valueAnotherC")
+								})
+							})
+							when("format is invalid", func() {
+								when("'value' is invalid", func() {
+									it.Before(func() {
+										ioutil.WriteFile(projectToml, []byte(`
+[_]
+schema-version = "0.2"
+[[io.buildpacks.env]]
+name = "KeyA"
+value = 1
+				`), 0644)
+									})
+									it("writes all env var files to the platform dir", func() {
+										err := cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger)
+										assert.EqualError(t, err, "environment variable 'KeyA' is not a string value")
+									})
+								})
+								when("'name' is invalid", func() {
+									it.Before(func() {
+										ioutil.WriteFile(projectToml, []byte(`
+[_]
+schema-version = "0.2"
+[[io.buildpacks.env]]
+name = 1
+value = "ValueA"
+				`), 0644)
+									})
+									it("writes all env var files to the platform dir", func() {
+										err := cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger)
+										assert.EqualError(t, err, "environment variable 'name' is not a string")
+									})
+								})
+							})
+						})
+					})
+
+					when("vars where set with new v0.2 project.toml", func() {
+						it.Before(func() {
+							ioutil.WriteFile(projectToml, []byte(`
+[_]
+schema-version = "0.2"
+[[io.buildpacks.build.env]]
+name = "keyA"
+value = "valueA"
+[[io.buildpacks.build.env]]
+name = "keyB"
+value = "valueB"
+[[io.buildpacks.build.env]]
+name = "keyC"
+value = "valueC"
+# check that later keys override previous ones
+[[io.buildpacks.build.env]]
+name = "keyC"
 value = "valueAnotherC"
 				`), 0644)
+						})
+						it("writes all env var files to the platform dir", func() {
+							assert.Nil(t, cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger))
+							checkEnvVar(t, platformDir, "keyA", "valueA")
+							checkEnvVar(t, platformDir, "keyB", "valueB")
+							checkEnvVar(t, platformDir, "keyC", "valueAnotherC")
+						})
 					})
-					it("writes all env var files to the platform dir", func() {
-						assert.Nil(t, cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger))
-						checkEnvVar(t, platformDir, "keyA", "valueA")
-						checkEnvVar(t, platformDir, "keyB", "valueB")
-						checkEnvVar(t, platformDir, "keyC", "valueAnotherC")
+
+					when("vars where set with both versions of v0.2 project.toml", func() {
+						it.Before(func() {
+							ioutil.WriteFile(projectToml, []byte(`
+[_]
+schema-version = "0.2"
+[[io.buildpacks.env.build]]
+name = "keyA"
+value = "valueA"
+# check that new declaration must have higher precedence
+[[io.buildpacks.build.env]]
+name = "keyA"
+value = "newValueA"
+				`), 0644)
+						})
+						it("new env var declaration must have higher precedence writes, all new env var files to the platform dir", func() {
+							assert.Nil(t, cnb.ProcessProjectDescriptor(appDir, descriptorPath, platformDir, logger))
+							checkEnvVar(t, platformDir, "keyA", "newValueA")
+						})
 					})
 				})