From effc5e500867aa8b283470151ee56198d167d011 Mon Sep 17 00:00:00 2001 From: WYGIN Date: Wed, 4 Oct 2023 16:15:54 +0530 Subject: [PATCH 1/9] WIP added code to generate env files in buildConfigEnv dir Signed-off-by: WYGIN --- internal/commands/build.go | 62 ++++++++++++++++++++++++++++++++++++++ pkg/project/types/types.go | 18 +++++++++-- 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/internal/commands/build.go b/internal/commands/build.go index 3984da8503..4f4bc9a66a 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -147,6 +147,9 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob if err != nil { return errors.Wrapf(err, "parsing creation time %s", flags.DateTime) } + if err := generateBuildConfigEnvFiles(descriptor.Build.Env); err != nil { + return err + } if err := packClient.Build(cmd.Context(), client.BuildOptions{ AppPath: flags.AppPath, Builder: builder, @@ -367,3 +370,62 @@ func parseProjectToml(appPath, descriptorPath string) (projectTypes.Descriptor, descriptor, err := project.ReadProjectDescriptor(actualPath) return descriptor, actualPath, err } + +func generateBuildConfigEnvFiles(envList []projectTypes.EnvVar) error { + dir, err := createBuildConfigEnvDir() + if err != nil { + return err + } + for _, env := range envList { + f, err := os.Create(filepath.Join(dir, env.Name+getActionType(env.Action))) + if err != nil { + return err + } + f.WriteString(env.Value) + if e := f.Close(); e != nil { + return e + } + } + return nil +} + +func cnbBuildConfigDir() string { + if v := os.Getenv("CNB_BUILD_CONFIG_DIR"); v == "" { + return "/cnb/build-config" + } else { + return v + } +} + +func createBuildConfigEnvDir() (dir string, err error) { + dir = filepath.Join(cnbBuildConfigDir(), "env") + _, err = os.Stat(dir) + if os.IsNotExist(err) { + err := os.MkdirAll(dir, os.ModePerm) + if err != nil { + return dir, err + } + return dir, nil + } + return dir, err +} + +func getActionType(action projectTypes.ActionType) string { + const delim = "." + switch action { + case projectTypes.NONE: + return "" + case projectTypes.DEFAULT: + return delim + string(projectTypes.DEFAULT) + case projectTypes.OVERRIDE: + return delim + string(projectTypes.OVERRIDE) + case projectTypes.APPEND: + return delim + string(projectTypes.APPEND) + case projectTypes.PREPEND: + return delim + string(projectTypes.PREPEND) + case projectTypes.DELIMIT: + return delim + string(projectTypes.DELIMIT) + default: + return delim + string(projectTypes.DEFAULT) + } +} diff --git a/pkg/project/types/types.go b/pkg/project/types/types.go index 3371d8d388..fdbb3bd3a5 100644 --- a/pkg/project/types/types.go +++ b/pkg/project/types/types.go @@ -17,9 +17,23 @@ type Buildpack struct { Script Script `toml:"script"` } +type ActionType string + +var ActionTypes []ActionType = []ActionType{NONE, DEFAULT, OVERRIDE, APPEND, PREPEND, DELIMIT} + +const ( + NONE ActionType = "" + DEFAULT ActionType = "default" + OVERRIDE ActionType = "override" + APPEND ActionType = "append" + PREPEND ActionType = "prepend" + DELIMIT ActionType = "delim" +) + type EnvVar struct { - Name string `toml:"name"` - Value string `toml:"value"` + Name string `toml:"name"` + Value string `toml:"value"` + Action ActionType `toml:"action"` } type Build struct { From fea99119ab8801a9b3a0fa88748132feea6d61de Mon Sep 17 00:00:00 2001 From: WYGIN Date: Wed, 4 Oct 2023 15:45:11 +0000 Subject: [PATCH 2/9] WIP added required abstract test cases Signed-off-by: WYGIN --- builder/config_reader.go | 22 +++++- internal/commands/build.go | 62 ----------------- internal/commands/builder_create.go | 70 +++++++++++++++++++ internal/commands/builder_create_test.go | 87 +++++++++++++++++++++++- pkg/project/types/types.go | 18 +---- 5 files changed, 178 insertions(+), 81 deletions(-) diff --git a/builder/config_reader.go b/builder/config_reader.go index d719c5df8c..35ccd7ac92 100644 --- a/builder/config_reader.go +++ b/builder/config_reader.go @@ -70,7 +70,27 @@ type RunImageConfig struct { // BuildConfig build image configuration type BuildConfig struct { - Image string `toml:"image"` + Image string `toml:"image"` + Env []BuildConfigEnv `toml:"env"` +} + +type ActionType string + +var ActionTypes []ActionType = []ActionType{NONE, DEFAULT, OVERRIDE, APPEND, PREPEND, DELIMIT} + +const ( + NONE ActionType = "" + DEFAULT ActionType = "default" + OVERRIDE ActionType = "override" + APPEND ActionType = "append" + PREPEND ActionType = "prepend" + DELIMIT ActionType = "delim" +) + +type BuildConfigEnv struct { + Name string `toml:"name"` + Value string `toml:"value"` + Action ActionType `toml:"action"` } // ReadConfig reads a builder configuration from the file path provided and returns the diff --git a/internal/commands/build.go b/internal/commands/build.go index 4f4bc9a66a..3984da8503 100644 --- a/internal/commands/build.go +++ b/internal/commands/build.go @@ -147,9 +147,6 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob if err != nil { return errors.Wrapf(err, "parsing creation time %s", flags.DateTime) } - if err := generateBuildConfigEnvFiles(descriptor.Build.Env); err != nil { - return err - } if err := packClient.Build(cmd.Context(), client.BuildOptions{ AppPath: flags.AppPath, Builder: builder, @@ -370,62 +367,3 @@ func parseProjectToml(appPath, descriptorPath string) (projectTypes.Descriptor, descriptor, err := project.ReadProjectDescriptor(actualPath) return descriptor, actualPath, err } - -func generateBuildConfigEnvFiles(envList []projectTypes.EnvVar) error { - dir, err := createBuildConfigEnvDir() - if err != nil { - return err - } - for _, env := range envList { - f, err := os.Create(filepath.Join(dir, env.Name+getActionType(env.Action))) - if err != nil { - return err - } - f.WriteString(env.Value) - if e := f.Close(); e != nil { - return e - } - } - return nil -} - -func cnbBuildConfigDir() string { - if v := os.Getenv("CNB_BUILD_CONFIG_DIR"); v == "" { - return "/cnb/build-config" - } else { - return v - } -} - -func createBuildConfigEnvDir() (dir string, err error) { - dir = filepath.Join(cnbBuildConfigDir(), "env") - _, err = os.Stat(dir) - if os.IsNotExist(err) { - err := os.MkdirAll(dir, os.ModePerm) - if err != nil { - return dir, err - } - return dir, nil - } - return dir, err -} - -func getActionType(action projectTypes.ActionType) string { - const delim = "." - switch action { - case projectTypes.NONE: - return "" - case projectTypes.DEFAULT: - return delim + string(projectTypes.DEFAULT) - case projectTypes.OVERRIDE: - return delim + string(projectTypes.OVERRIDE) - case projectTypes.APPEND: - return delim + string(projectTypes.APPEND) - case projectTypes.PREPEND: - return delim + string(projectTypes.PREPEND) - case projectTypes.DELIMIT: - return delim + string(projectTypes.DELIMIT) - default: - return delim + string(projectTypes.DEFAULT) - } -} diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index 1b0d709895..cd8cb098a9 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -2,6 +2,7 @@ package commands import ( "fmt" + "os" "path/filepath" "strings" @@ -75,6 +76,10 @@ Creating a custom builder allows you to control what buildpacks are used and wha return errors.Wrap(err, "getting absolute path for config") } + if err := generateBuildConfigEnvFiles(builderConfig.Build.Env); err != nil { + return err + } + imageName := args[0] if err := pack.CreateBuilder(cmd.Context(), client.CreateBuilderOptions{ RelativeBaseDir: relativeBaseDir, @@ -137,3 +142,68 @@ func validateCreateFlags(flags *BuilderCreateFlags, cfg config.Config) error { return nil } + +func generateBuildConfigEnvFiles(envList []builder.BuildConfigEnv) error { + dir, err := createBuildConfigEnvDir() + if err != nil { + return err + } + for _, env := range envList { + var path string + if a := getActionType(env.Action); a == "" || len(a) == 0 { + path = env.Name + } else { + path = env.Name + getActionType(env.Action) + } + f, err := os.Create(filepath.Join(dir, path)) + if err != nil { + return err + } + f.WriteString(env.Value) + if e := f.Close(); e != nil { + return e + } + } + return nil +} + +func cnbBuildConfigDir() string { + if v := os.Getenv("CNB_BUILD_CONFIG_DIR"); v == "" || len(v) == 0 { + return "/cnb/build-config" + } else { + return v + } +} + +func createBuildConfigEnvDir() (dir string, err error) { + dir = filepath.Join(cnbBuildConfigDir(), "env") + _, err = os.Stat(dir) + if os.IsNotExist(err) { + err := os.MkdirAll(dir, os.ModePerm) + if err != nil { + return dir, err + } + return dir, nil + } + return dir, err +} + +func getActionType(action builder.ActionType) string { + const delim = "." + switch action { + case builder.NONE: + return "" + case builder.DEFAULT: + return delim + string(builder.DEFAULT) + case builder.OVERRIDE: + return delim + string(builder.OVERRIDE) + case builder.APPEND: + return delim + string(builder.APPEND) + case builder.PREPEND: + return delim + string(builder.PREPEND) + case builder.DELIMIT: + return delim + string(builder.DELIMIT) + default: + return delim + string(builder.DEFAULT) + } +} diff --git a/internal/commands/builder_create_test.go b/internal/commands/builder_create_test.go index 12c89465f4..a70b5cd59b 100644 --- a/internal/commands/builder_create_test.go +++ b/internal/commands/builder_create_test.go @@ -157,15 +157,98 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { }) }) - when("uses --builder-config", func() { + when("uses --config", func() { it.Before(func() { h.AssertNil(t, os.WriteFile(builderConfigPath, []byte(validConfig), 0666)) }) + when("buildConfigEnv files generated", func() { + const ActionNONE = validConfig + ` + [[build.env]] + name = "actionNone" + value = "actionNoneValue" + action = "" + ` + const ActionDEFAULT = validConfig + ` + [[build.env]] + name = "actionDefault" + value = "actionDefaultValue" + action = "default" + ` + const ActionOVERRIDE = validConfig + ` + [[build.env]] + name = "actionOverride" + value = "actionOverrideValue" + action = "override" + ` + const ActionAPPEND = validConfig + ` + [[build.env]] + name = "actionAppend" + value = "actionAppendValue" + action = "append" + ` + const ActionPREPEND = validConfig + ` + [[build.env]] + name = "actionPrepend" + value = "actionPrependValue" + action = "prepend" + ` + const ActionDELIMIT = validConfig + ` + [[build.env]] + name = "actionDelimit" + value = "actionDelimitValue" + action = "delim" + ` + const ActionUNKNOWN = validConfig + ` + [[build.env]] + name = "actionUnknown" + value = "actionUnknownValue" + action = "unknown" + ` + const ActionMULTIPLE = validConfig + ` + [[build.env]] + name = "actionAppend" + value = "actionAppendValue" + action = "append" + [[build.env]] + name = "actionPrepend" + value = "actionPrependValue" + action = "prepend" + [[build.env]] + name = "actionDelimit" + value = "actionDelimitValue" + action = "delim" + ` + it("should create content as expected when ActionType `NONE`", func() { + + }) + it("should create content as expected when ActionType `DEFAULT`", func() { + + }) + it("should create content as expected when ActionType `OVERRIDE`", func() { + + }) + it("should create content as expected when ActionType `APPEND`", func() { + + }) + it("should create content as expected when ActionType `PREPEND`", func() { + + }) + it("should create content as expected when ActionType `DELIMIT`", func() { + + }) + it("should create content as expected when unknown ActionType passed", func() { + + }) + it("should create content as expected when multiple ActionTypes passed", func() { + + }) + }) + it("errors with a descriptive message", func() { command.SetArgs([]string{ "some/builder", - "--builder-config", builderConfigPath, + "--config", builderConfigPath, }) h.AssertError(t, command.Execute(), "unknown flag: --builder-config") }) diff --git a/pkg/project/types/types.go b/pkg/project/types/types.go index fdbb3bd3a5..3371d8d388 100644 --- a/pkg/project/types/types.go +++ b/pkg/project/types/types.go @@ -17,23 +17,9 @@ type Buildpack struct { Script Script `toml:"script"` } -type ActionType string - -var ActionTypes []ActionType = []ActionType{NONE, DEFAULT, OVERRIDE, APPEND, PREPEND, DELIMIT} - -const ( - NONE ActionType = "" - DEFAULT ActionType = "default" - OVERRIDE ActionType = "override" - APPEND ActionType = "append" - PREPEND ActionType = "prepend" - DELIMIT ActionType = "delim" -) - type EnvVar struct { - Name string `toml:"name"` - Value string `toml:"value"` - Action ActionType `toml:"action"` + Name string `toml:"name"` + Value string `toml:"value"` } type Build struct { From 0035427e957383d2a675cde1ad5939f1acaf798b Mon Sep 17 00:00:00 2001 From: WYGIN Date: Thu, 5 Oct 2023 10:00:34 +0000 Subject: [PATCH 3/9] WIP added tests that still nedd to be fixed Signed-off-by: WYGIN --- builder/config_reader.go | 2 +- internal/commands/builder_create.go | 74 +++-- internal/commands/builder_create_test.go | 366 ++++++++++++++++++----- 3 files changed, 344 insertions(+), 98 deletions(-) diff --git a/builder/config_reader.go b/builder/config_reader.go index 35ccd7ac92..7c17ac331a 100644 --- a/builder/config_reader.go +++ b/builder/config_reader.go @@ -90,7 +90,7 @@ const ( type BuildConfigEnv struct { Name string `toml:"name"` Value string `toml:"value"` - Action ActionType `toml:"action"` + Action ActionType `toml:"action,omitempty"` } // ReadConfig reads a builder configuration from the file path provided and returns the diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index cd8cb098a9..7cdf1faa75 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -76,7 +76,14 @@ Creating a custom builder allows you to control what buildpacks are used and wha return errors.Wrap(err, "getting absolute path for config") } - if err := generateBuildConfigEnvFiles(builderConfig.Build.Env); err != nil { + envMap, warnings, err := parseBuildConfigEnv(builderConfig.Build.Env, flags.BuilderTomlPath) + for _, v := range warnings { + logger.Warn(v) + } + if err != nil { + return err + } + if err := generateBuildConfigEnvFiles(envMap); err != nil { return err } @@ -143,23 +150,17 @@ func validateCreateFlags(flags *BuilderCreateFlags, cfg config.Config) error { return nil } -func generateBuildConfigEnvFiles(envList []builder.BuildConfigEnv) error { +func generateBuildConfigEnvFiles(envMap map[string]string) error { dir, err := createBuildConfigEnvDir() if err != nil { return err } - for _, env := range envList { - var path string - if a := getActionType(env.Action); a == "" || len(a) == 0 { - path = env.Name - } else { - path = env.Name + getActionType(env.Action) - } - f, err := os.Create(filepath.Join(dir, path)) + for k, v := range envMap { + f, err := os.Create(filepath.Join(dir, k)) if err != nil { return err } - f.WriteString(env.Value) + f.WriteString(v) if e := f.Close(); e != nil { return e } @@ -167,7 +168,7 @@ func generateBuildConfigEnvFiles(envList []builder.BuildConfigEnv) error { return nil } -func cnbBuildConfigDir() string { +func CnbBuildConfigDir() string { if v := os.Getenv("CNB_BUILD_CONFIG_DIR"); v == "" || len(v) == 0 { return "/cnb/build-config" } else { @@ -176,7 +177,7 @@ func cnbBuildConfigDir() string { } func createBuildConfigEnvDir() (dir string, err error) { - dir = filepath.Join(cnbBuildConfigDir(), "env") + dir = filepath.Join(CnbBuildConfigDir(), "env") _, err = os.Stat(dir) if os.IsNotExist(err) { err := os.MkdirAll(dir, os.ModePerm) @@ -188,22 +189,53 @@ func createBuildConfigEnvDir() (dir string, err error) { return dir, err } -func getActionType(action builder.ActionType) string { +func getActionType(action builder.ActionType) (actionString string, err error) { const delim = "." switch action { case builder.NONE: - return "" + return "", nil case builder.DEFAULT: - return delim + string(builder.DEFAULT) + return delim + string(builder.DEFAULT), nil case builder.OVERRIDE: - return delim + string(builder.OVERRIDE) + return delim + string(builder.OVERRIDE), nil case builder.APPEND: - return delim + string(builder.APPEND) + return delim + string(builder.APPEND), nil case builder.PREPEND: - return delim + string(builder.PREPEND) + return delim + string(builder.PREPEND), nil case builder.DELIMIT: - return delim + string(builder.DELIMIT) + return delim + string(builder.DELIMIT), nil default: - return delim + string(builder.DEFAULT) + return actionString, errors.Errorf("unknown action type %s", style.Symbol(string(action))) + } +} +func GetBuildConfigEnvFileName(env builder.BuildConfigEnv) (path string, err error) { + if a, err := getActionType(env.Action); err != nil { + return path, err + } else if a == "" || len(a) == 0 { + path = strings.ToUpper(env.Name) + } else { + path = strings.ToUpper(env.Name) + a + } + return path, err +} + +func parseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[string]string, warnings []string, err error) { + envMap = map[string]string{} + for _, v := range env { + if name := v.Name; name == "" || len(name) == 0 { + return nil, nil, errors.Wrapf(errors.Errorf("env name should not be empty"), "parse contents of '%s'", path) + } + if val := v.Value; val == "" || len(val) == 0 { + warnings = append(warnings, fmt.Sprintf("empty value for key/name %s", style.Symbol(v.Name))) + } + val, err := GetBuildConfigEnvFileName(v) + if err != nil { + return envMap, warnings, err + } + if _, e := envMap[val]; e { + return nil, nil, errors.Wrapf(errors.Errorf("env with name: %s and action: %s is already defined", style.Symbol(v.Name), style.Symbol(string(v.Action))), "parse contents of '%s'", path) + } + envMap[val] = v.Value } + return envMap, warnings, err } diff --git a/internal/commands/builder_create_test.go b/internal/commands/builder_create_test.go index a70b5cd59b..fcf3cc59cd 100644 --- a/internal/commands/builder_create_test.go +++ b/internal/commands/builder_create_test.go @@ -47,6 +47,80 @@ const validConfigWithExtensions = ` ` +const ActionNONE = validConfig + ` +[[build.env]] +name = "actionNone" +value = "actionNoneValue" +action = "" +` +const ActionDEFAULT = validConfig + ` +[[build.env]] +name = "actionDefault" +value = "actionDefaultValue" +action = "default" +` +const ActionOVERRIDE = validConfig + ` +[[build.env]] +name = "actionOverride" +value = "actionOverrideValue" +action = "override" +` +const ActionAPPEND = validConfig + ` +[[build.env]] +name = "actionAppend" +value = "actionAppendValue" +action = "append" +` +const ActionPREPEND = validConfig + ` +[[build.env]] +name = "actionPrepend" +value = "actionPrependValue" +action = "prepend" +` +const ActionDELIMIT = validConfig + ` +[[build.env]] +name = "actionDelimit" +value = ":" +action = "delim" +` +const ActionUNKNOWN = validConfig + ` +[[build.env]] +name = "actionUnknown" +value = "actionUnknownValue" +action = "unknown" +` +const ActionMULTIPLE = validConfig + ` +[[build.env]] +name = "MY_VAR" +value = "actionAppendValue" +action = "append" +[[build.env]] +name = "MY_VAR" +value = "actionDefaultValue" +action = "default" +[[build.env]] +name = "MY_VAR" +value = "actionPrependValue" +action = "prepend" +[[build.env]] +name = "MY_VAR" +value = ":" +action = "delim" +` + +const ActionWarning = validConfig + ` +[[build.env]] +name = "actionWarning" +value = "" +` + +const ActionError = validConfig + ` +[[build.env]] +name = "" +value = "some-value" +action = "default" +` + func TestCreateCommand(t *testing.T) { color.Disable(true) defer color.Disable(false) @@ -157,100 +231,195 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { }) }) - when("uses --config", func() { + when("uses --builder-config", func() { it.Before(func() { h.AssertNil(t, os.WriteFile(builderConfigPath, []byte(validConfig), 0666)) }) - when("buildConfigEnv files generated", func() { - const ActionNONE = validConfig + ` - [[build.env]] - name = "actionNone" - value = "actionNoneValue" - action = "" - ` - const ActionDEFAULT = validConfig + ` - [[build.env]] - name = "actionDefault" - value = "actionDefaultValue" - action = "default" - ` - const ActionOVERRIDE = validConfig + ` - [[build.env]] - name = "actionOverride" - value = "actionOverrideValue" - action = "override" - ` - const ActionAPPEND = validConfig + ` - [[build.env]] - name = "actionAppend" - value = "actionAppendValue" - action = "append" - ` - const ActionPREPEND = validConfig + ` - [[build.env]] - name = "actionPrepend" - value = "actionPrependValue" - action = "prepend" - ` - const ActionDELIMIT = validConfig + ` - [[build.env]] - name = "actionDelimit" - value = "actionDelimitValue" - action = "delim" - ` - const ActionUNKNOWN = validConfig + ` - [[build.env]] - name = "actionUnknown" - value = "actionUnknownValue" - action = "unknown" - ` - const ActionMULTIPLE = validConfig + ` - [[build.env]] - name = "actionAppend" - value = "actionAppendValue" - action = "append" - [[build.env]] - name = "actionPrepend" - value = "actionPrependValue" - action = "prepend" - [[build.env]] - name = "actionDelimit" - value = "actionDelimitValue" - action = "delim" - ` - it("should create content as expected when ActionType `NONE`", func() { - + it("errors with a descriptive message", func() { + command.SetArgs([]string{ + "some/builder", + "--builder-config", builderConfigPath, }) - it("should create content as expected when ActionType `DEFAULT`", func() { + h.AssertError(t, command.Execute(), "unknown flag: --builder-config") + }) + }) + when("buildConfigEnv files generated", func() { + var fileIndex = 0 + buildConfigEnvDir := commands.CnbBuildConfigDir() + it.Before(func() { + h.AssertNil(t, os.WriteFile(builderConfigPath, []byte(getBuildConfigEnvFileContent(fileIndex)), 0666)) + }) + it.After(func() { + err := os.RemoveAll(buildConfigEnvDir) + h.AssertNil(t, err) + fileIndex++ + }) + it("should create content as expected when ActionType `NONE`", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, }) - it("should create content as expected when ActionType `OVERRIDE`", func() { - + h.AssertNil(t, command.Execute()) + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + }) + it("should create content as expected when ActionType `DEFAULT`", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, }) - it("should create content as expected when ActionType `APPEND`", func() { - + h.AssertNil(t, command.Execute()) + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + }) + it("should create content as expected when ActionType `OVERRIDE`", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, }) - it("should create content as expected when ActionType `PREPEND`", func() { - + h.AssertNil(t, command.Execute()) + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + }) + it("should create content as expected when ActionType `APPEND`", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, }) - it("should create content as expected when ActionType `DELIMIT`", func() { - + h.AssertNil(t, command.Execute()) + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + }) + it("should create content as expected when ActionType `PREPEND`", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, }) - it("should create content as expected when unknown ActionType passed", func() { - + h.AssertNil(t, command.Execute()) + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + }) + it("should create content as expected when ActionType `DELIMIT`", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, }) - it("should create content as expected when multiple ActionTypes passed", func() { - + h.AssertNil(t, command.Execute()) + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + }) + it("should return an error when unknown ActionType passed", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, + }) + var bufErr bytes.Buffer + command.SetErr(&bufErr) + h.AssertNil(t, command.Execute()) + h.AssertNotEq(t, bufErr.String(), "") + name := actionTypesMap[fileIndex][0][0] + _, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNotNil(t, err) + }) + it("should create content as expected when multiple ActionTypes passed", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, }) + h.AssertNil(t, command.Execute()) + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + + name = actionTypesMap[fileIndex][1][0] + file, err = os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err = os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][1][1], string(content)) + + name = actionTypesMap[fileIndex][2][0] + file, err = os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err = os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][2][1], string(content)) + + name = actionTypesMap[fileIndex][3][0] + file, err = os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err = os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][3][1], string(content)) }) - - it("errors with a descriptive message", func() { + it("should show warnings when env value is empty", func() { command.SetArgs([]string{ "some/builder", "--config", builderConfigPath, }) - h.AssertError(t, command.Execute(), "unknown flag: --builder-config") + var bufOut bytes.Buffer + command.SetOut(&bufOut) + h.AssertNil(t, command.Execute()) + h.AssertNotEq(t, bufOut.String(), "") + name := actionTypesMap[fileIndex][0][0] + file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNil(t, err) + h.AssertEq(t, name, file.Name()) + content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertNil(t, err) + h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) + }) + it("should return an error when env.Name is empty", func() { + command.SetArgs([]string{ + "some/builder", + "--config", builderConfigPath, + }) + var bufErr bytes.Buffer + command.SetErr(&bufErr) + h.AssertNil(t, command.Execute()) + h.AssertNotEq(t, bufErr.String(), "") + name := actionTypesMap[fileIndex][0][0] + _, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNotNil(t, err) }) }) @@ -296,3 +465,48 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { }) }) } + +func getBuildConfigEnvFileContent(index int) string { + switch index { + case 0: + return ActionNONE + case 1: + return ActionDEFAULT + case 2: + return ActionOVERRIDE + case 3: + return ActionAPPEND + case 4: + return ActionPREPEND + case 5: + return ActionDELIMIT + case 6: + return ActionUNKNOWN + case 7: + return ActionMULTIPLE + case 8: + return ActionWarning + case 9: + return ActionError + default: + return "" + } +} + +var actionTypesMap = map[int]map[int]map[int]string{ + 0: {0: {0: "ACTIONNONE", 1: "actionNoneValue"}}, + 1: {0: {0: "ACTIONDEFAULT.default", 1: "actionDefaultValue"}}, + 2: {0: {0: "ACTIONOVERRIDE.override", 1: "actionOverrideValue"}}, + 3: {0: {0: "ACTIONAPPEND.append", 1: "actionAppendValue"}}, + 4: {0: {0: "ACTIONPREPEND.prepend", 1: "actionPrependValue"}}, + 5: {0: {0: "ACTIONDELIMIT.delim", 1: ":"}}, + 6: {0: {0: "ACTIONUNKNOWN.unknown", 1: "actionUnknownValue"}}, + 7: { + 0: {0: "MY_VAR.append", 1: "actionAppendValue"}, + 1: {0: "MY_VAR.default", 1: "actionDefaultValue"}, + 2: {0: "MY_VAR.prepend", 1: "actionPrependValue"}, + 3: {0: "MY_VAR.delim", 1: ":"}, + }, + 8: {0: {0: "actionWarning", 1: ""}}, + 9: {0: {0: ".default", 1: "some-value"}}, +} From 7fa8e4dae7a16dad05f2bdb45631e5a5efa0a77a Mon Sep 17 00:00:00 2001 From: WYGIN Date: Thu, 5 Oct 2023 15:47:15 +0000 Subject: [PATCH 4/9] WIP change from action to suffix and delim in builder.toml Signed-off-by: WYGIN --- builder/config_reader.go | 22 ++++++------ internal/builder/builder.go | 5 +-- internal/commands/builder_create.go | 43 +++++++++++++--------- internal/commands/builder_create_test.go | 45 ++++++++++++------------ 4 files changed, 62 insertions(+), 53 deletions(-) diff --git a/builder/config_reader.go b/builder/config_reader.go index 7c17ac331a..9cd6117a2c 100644 --- a/builder/config_reader.go +++ b/builder/config_reader.go @@ -74,23 +74,23 @@ type BuildConfig struct { Env []BuildConfigEnv `toml:"env"` } -type ActionType string +type Suffix string -var ActionTypes []ActionType = []ActionType{NONE, DEFAULT, OVERRIDE, APPEND, PREPEND, DELIMIT} +var SuffixSlice []Suffix = []Suffix{NONE, DEFAULT, OVERRIDE, APPEND, PREPEND} const ( - NONE ActionType = "" - DEFAULT ActionType = "default" - OVERRIDE ActionType = "override" - APPEND ActionType = "append" - PREPEND ActionType = "prepend" - DELIMIT ActionType = "delim" + NONE Suffix = "" + DEFAULT Suffix = "default" + OVERRIDE Suffix = "override" + APPEND Suffix = "append" + PREPEND Suffix = "prepend" ) type BuildConfigEnv struct { - Name string `toml:"name"` - Value string `toml:"value"` - Action ActionType `toml:"action,omitempty"` + Name string `toml:"name"` + Value string `toml:"value"` + Suffix Suffix `toml:"suffix,omitempty"` + Delim string `toml:"delim,omitempty"` } // ReadConfig reads a builder configuration from the file path provided and returns the diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 51b5a4a00e..8228ab779b 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -35,8 +35,9 @@ import ( const ( packName = "Pack CLI" - cnbDir = "/cnb" - buildpacksDir = "/cnb/buildpacks" + cnbDir = "/cnb" + buildConfigDir = "/cnb/build-config" + buildpacksDir = "/cnb/buildpacks" orderPath = "/cnb/order.toml" stackPath = "/cnb/stack.toml" diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index 7cdf1faa75..b606b08738 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -186,12 +186,12 @@ func createBuildConfigEnvDir() (dir string, err error) { } return dir, nil } - return dir, err + return dir, nil } -func getActionType(action builder.ActionType) (actionString string, err error) { +func getActionType(suffix builder.Suffix) (suffixString string, err error) { const delim = "." - switch action { + switch suffix { case builder.NONE: return "", nil case builder.DEFAULT: @@ -202,21 +202,24 @@ func getActionType(action builder.ActionType) (actionString string, err error) { return delim + string(builder.APPEND), nil case builder.PREPEND: return delim + string(builder.PREPEND), nil - case builder.DELIMIT: - return delim + string(builder.DELIMIT), nil default: - return actionString, errors.Errorf("unknown action type %s", style.Symbol(string(action))) + return suffixString, errors.Errorf("unknown action type %s", style.Symbol(string(suffix))) } } -func GetBuildConfigEnvFileName(env builder.BuildConfigEnv) (path string, err error) { - if a, err := getActionType(env.Action); err != nil { - return path, err - } else if a == "" || len(a) == 0 { - path = strings.ToUpper(env.Name) +func GetBuildConfigEnvFileName(env builder.BuildConfigEnv) (suffixName, delimName string, err error) { + suffix, err := getActionType(env.Suffix) + if err != nil { + return suffixName, delimName, err + } + if suffix == "" || len(suffix) == 0 { + suffixName = env.Name } else { - path = strings.ToUpper(env.Name) + a + suffixName = env.Name + suffix + } + if delim := env.Delim; delim != "" || len(delim) != 0 { + delimName = env.Name + ".delim" } - return path, err + return suffixName, delimName, err } func parseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[string]string, warnings []string, err error) { @@ -228,14 +231,20 @@ func parseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[ if val := v.Value; val == "" || len(val) == 0 { warnings = append(warnings, fmt.Sprintf("empty value for key/name %s", style.Symbol(v.Name))) } - val, err := GetBuildConfigEnvFileName(v) + suffixName, delimName, err := GetBuildConfigEnvFileName(v) if err != nil { return envMap, warnings, err } - if _, e := envMap[val]; e { - return nil, nil, errors.Wrapf(errors.Errorf("env with name: %s and action: %s is already defined", style.Symbol(v.Name), style.Symbol(string(v.Action))), "parse contents of '%s'", path) + if val, e := envMap[suffixName]; e { + warnings = append(warnings, fmt.Sprintf(errors.Errorf("overriding env with name: %s and suffix: %s from %s to %s", style.Symbol(v.Name), style.Symbol(string(v.Suffix)), style.Symbol(val), style.Symbol(v.Value)).Error(), "parse contents of '%s'", path)) + } + if val, e := envMap[delimName]; e { + warnings = append(warnings, fmt.Sprintf(errors.Errorf("overriding env with name: %s and delim: %s from %s to %s", style.Symbol(v.Name), style.Symbol(v.Delim), style.Symbol(val), style.Symbol(v.Value)).Error(), "parse contents of '%s'", path)) + } + if delim := v.Delim; (delim != "" || len(delim) != 0) && (delimName != "" || len(delimName) != 0) { + envMap[delimName] = delim } - envMap[val] = v.Value + envMap[suffixName] = v.Value } return envMap, warnings, err } diff --git a/internal/commands/builder_create_test.go b/internal/commands/builder_create_test.go index fcf3cc59cd..b36b67611c 100644 --- a/internal/commands/builder_create_test.go +++ b/internal/commands/builder_create_test.go @@ -51,61 +51,58 @@ const ActionNONE = validConfig + ` [[build.env]] name = "actionNone" value = "actionNoneValue" -action = "" ` const ActionDEFAULT = validConfig + ` [[build.env]] name = "actionDefault" value = "actionDefaultValue" -action = "default" +suffix = "default" ` const ActionOVERRIDE = validConfig + ` [[build.env]] name = "actionOverride" value = "actionOverrideValue" -action = "override" +suffix = "override" ` const ActionAPPEND = validConfig + ` [[build.env]] name = "actionAppend" value = "actionAppendValue" -action = "append" +suffix = "append" ` const ActionPREPEND = validConfig + ` [[build.env]] name = "actionPrepend" value = "actionPrependValue" -action = "prepend" +suffix = "prepend" ` const ActionDELIMIT = validConfig + ` [[build.env]] name = "actionDelimit" -value = ":" -action = "delim" +delim = ":" ` const ActionUNKNOWN = validConfig + ` [[build.env]] name = "actionUnknown" value = "actionUnknownValue" -action = "unknown" +suffix = "unknown" ` const ActionMULTIPLE = validConfig + ` [[build.env]] name = "MY_VAR" value = "actionAppendValue" -action = "append" +suffix = "append" +delim = ":" [[build.env]] name = "MY_VAR" value = "actionDefaultValue" -action = "default" +suffix = "default" +delim = ":" [[build.env]] name = "MY_VAR" value = "actionPrependValue" -action = "prepend" -[[build.env]] -name = "MY_VAR" -value = ":" -action = "delim" +suffix = "prepend" +delim = ":" ` const ActionWarning = validConfig + ` @@ -118,7 +115,7 @@ const ActionError = validConfig + ` [[build.env]] name = "" value = "some-value" -action = "default" +suffix = "default" ` func TestCreateCommand(t *testing.T) { @@ -249,6 +246,8 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { var fileIndex = 0 buildConfigEnvDir := commands.CnbBuildConfigDir() it.Before(func() { + err := os.MkdirAll(buildConfigEnvDir, os.ModePerm) + h.AssertNil(t, err) h.AssertNil(t, os.WriteFile(builderConfigPath, []byte(getBuildConfigEnvFileContent(fileIndex)), 0666)) }) it.After(func() { @@ -494,13 +493,13 @@ func getBuildConfigEnvFileContent(index int) string { } var actionTypesMap = map[int]map[int]map[int]string{ - 0: {0: {0: "ACTIONNONE", 1: "actionNoneValue"}}, - 1: {0: {0: "ACTIONDEFAULT.default", 1: "actionDefaultValue"}}, - 2: {0: {0: "ACTIONOVERRIDE.override", 1: "actionOverrideValue"}}, - 3: {0: {0: "ACTIONAPPEND.append", 1: "actionAppendValue"}}, - 4: {0: {0: "ACTIONPREPEND.prepend", 1: "actionPrependValue"}}, - 5: {0: {0: "ACTIONDELIMIT.delim", 1: ":"}}, - 6: {0: {0: "ACTIONUNKNOWN.unknown", 1: "actionUnknownValue"}}, + 0: {0: {0: "actionNone", 1: "actionNoneValue"}}, + 1: {0: {0: "actionDefault.default", 1: "actionDefaultValue"}}, + 2: {0: {0: "actionOverride.override", 1: "actionOverrideValue"}}, + 3: {0: {0: "actionAppend.append", 1: "actionAppendValue"}}, + 4: {0: {0: "actionPrepend.prepend", 1: "actionPrependValue"}}, + 5: {0: {0: "actionDelim.delim", 1: ":"}}, + 6: {0: {0: "actionUnknown.unknown", 1: "actionUnknownValue"}}, 7: { 0: {0: "MY_VAR.append", 1: "actionAppendValue"}, 1: {0: "MY_VAR.default", 1: "actionDefaultValue"}, From 6e085aef7a3391d9b7cf2dd8bf23c6a18f227937 Mon Sep 17 00:00:00 2001 From: WYGIN Date: Sun, 8 Oct 2023 08:31:18 +0000 Subject: [PATCH 5/9] WIP added logic to add buildConfigEnvs to layer of image Signed-off-by: WYGIN --- internal/builder/builder.go | 64 +++- internal/builder/builder_test.go | 43 +++ internal/commands/builder_create.go | 81 ++--- internal/commands/builder_create_test.go | 438 +++++++++-------------- pkg/client/create_builder.go | 13 + 5 files changed, 328 insertions(+), 311 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 8228ab779b..47d392c82d 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -32,12 +32,13 @@ import ( lifecycleplatform "github.com/buildpacks/lifecycle/platform" ) +var buildConfigDir = cnbBuildConfigDir() + const ( packName = "Pack CLI" - cnbDir = "/cnb" - buildConfigDir = "/cnb/build-config" - buildpacksDir = "/cnb/buildpacks" + cnbDir = "/cnb" + buildpacksDir = "/cnb/buildpacks" orderPath = "/cnb/order.toml" stackPath = "/cnb/stack.toml" @@ -68,6 +69,7 @@ const ( // Builder represents a pack builder, used to build images type Builder struct { baseImageName string + buildConfigEnv map[string]string image imgutil.Image layerWriterFactory archive.TarWriterFactory lifecycle Lifecycle @@ -147,6 +149,7 @@ func constructBuilder(img imgutil.Image, newName string, errOnMissingLabel bool, metadata: metadata, lifecycleDescriptor: constructLifecycleDescriptor(metadata), env: map[string]string{}, + buildConfigEnv: map[string]string{}, validateMixins: true, additionalBuildpacks: *buildpack.NewModuleManager(opts.flatten, opts.depth), additionalExtensions: *buildpack.NewModuleManager(opts.flatten, opts.depth), @@ -350,6 +353,11 @@ func (b *Builder) SetEnv(env map[string]string) { b.env = env } +// SetBuildConfigEnv sets an environment variable to a value that will take action on platform environment variables basedon filename suffix +func (b *Builder) SetBuildConfigEnv(env map[string]string) { + b.buildConfigEnv = env +} + // SetOrder sets the order of the builder func (b *Builder) SetOrder(order dist.Order) { b.order = order @@ -526,6 +534,19 @@ func (b *Builder) Save(logger logging.Logger, creatorMetadata CreatorMetadata) e return errors.Wrap(err, "adding run.tar layer") } + if len(b.buildConfigEnv) > 0 { + logger.Debugf("Provided Build Config Environment Variables\n %s", style.Map(b.env, " ", "\n")) + } + + buildConfigEnvTar, err := b.buildConfigEnvLayer(tmpDir, b.buildConfigEnv) + if err != nil { + return errors.Wrap(err, "retrieving build-config-env layer") + } + + if err := b.image.AddLayer(buildConfigEnvTar); err != nil { + return errors.Wrap(err, "adding build-config-env layer") + } + if len(b.env) > 0 { logger.Debugf("Provided Environment Variables\n %s", style.Map(b.env, " ", "\n")) } @@ -904,7 +925,7 @@ func (b *Builder) defaultDirsLayer(dest string) (string, error) { } // can't use filepath.Join(), to ensure Windows doesn't transform it to Windows join - for _, path := range []string{cnbDir, dist.BuildpacksDir, dist.ExtensionsDir, platformDir, platformDir + "/env"} { + for _, path := range []string{cnbDir, dist.BuildpacksDir, dist.ExtensionsDir, platformDir, platformDir + "/env", buildConfigDir, buildConfigDir + "/env"} { if err := lw.WriteHeader(b.rootOwnedDir(path, ts)); err != nil { return "", errors.Wrapf(err, "creating %s dir in layer", style.Symbol(path)) } @@ -1103,6 +1124,33 @@ func (b *Builder) envLayer(dest string, env map[string]string) (string, error) { return fh.Name(), nil } +func (b *Builder) buildConfigEnvLayer(dest string, env map[string]string) (string, error) { + fh, err := os.Create(filepath.Join(dest, "build-config-env.tar")) + if err != nil { + return "", err + } + defer fh.Close() + + lw := b.layerWriterFactory.NewWriter(fh) + defer lw.Close() + + for k, v := range env { + if err := lw.WriteHeader(&tar.Header{ + Name: path.Join(buildConfigDir, "env", k), + Size: int64(len(v)), + Mode: 0644, + ModTime: archive.NormalizedDateTime, + }); err != nil { + return "", err + } + if _, err := lw.Write([]byte(v)); err != nil { + return "", err + } + } + + return fh.Name(), nil +} + func (b *Builder) whiteoutLayer(tmpDir string, i int, bpInfo dist.ModuleInfo) (string, error) { bpWhiteoutsTmpDir := filepath.Join(tmpDir, strconv.Itoa(i)+"_whiteouts") if err := os.MkdirAll(bpWhiteoutsTmpDir, os.ModePerm); err != nil { @@ -1258,3 +1306,11 @@ func (e errModuleTar) Info() dist.ModuleInfo { func (e errModuleTar) Path() string { return "" } + +func cnbBuildConfigDir() string { + if env, ok := os.LookupEnv("CNB_BUILD_CONFIG_DIR"); !ok { + return "/cnb/build-config" + } else { + return env + } +} diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 13a958c72e..2745e9e97b 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -384,6 +384,20 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { ) }) + it("creates the build-config dir", func() { + h.AssertNil(t, subject.Save(logger, builder.CreatorMetadata{})) + h.AssertEq(t, baseImage.IsSaved(), true) + + layerTar, err := baseImage.FindLayerWithPath("/cnb/build-config") + h.AssertNil(t, err) + h.AssertOnTarEntry(t, layerTar, "/cnb/build-config", + h.IsDirectory(), + h.HasOwnerAndGroup(0, 0), + h.HasFileMode(0755), + h.HasModTime(archive.NormalizedDateTime), + ) + }) + it("creates the buildpacks dir", func() { h.AssertNil(t, subject.Save(logger, builder.CreatorMetadata{})) h.AssertEq(t, baseImage.IsSaved(), true) @@ -1607,6 +1621,35 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) }) + when("#SetBuildConfigEnv", func() { + it.Before(func() { + subject.SetBuildConfigEnv(map[string]string{ + "SOME_KEY": "some-val", + "OTHER_KEY.append": "other-val", + "OTHER_KEY.delim": ":", + }) + h.AssertNil(t, subject.Save(logger, builder.CreatorMetadata{})) + h.AssertEq(t, baseImage.IsSaved(), true) + }) + + it("adds the env vars as files to the image", func() { + layerTar, err := baseImage.FindLayerWithPath("/cnb/build-config/env/SOME_KEY") + h.AssertNil(t, err) + h.AssertOnTarEntry(t, layerTar, "/cnb/build-config/env/SOME_KEY", + h.ContentEquals(`some-val`), + h.HasModTime(archive.NormalizedDateTime), + ) + h.AssertOnTarEntry(t, layerTar, "/cnb/build-config/env/OTHER_KEY.append", + h.ContentEquals(`other-val`), + h.HasModTime(archive.NormalizedDateTime), + ) + h.AssertOnTarEntry(t, layerTar, "/cnb/build-config/env/OTHER_KEY.delim", + h.ContentEquals(`:`), + h.HasModTime(archive.NormalizedDateTime), + ) + }) + }) + when("#SetEnv", func() { it.Before(func() { subject.SetEnv(map[string]string{ diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index b606b08738..a98cb43521 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -2,7 +2,6 @@ package commands import ( "fmt" - "os" "path/filepath" "strings" @@ -76,20 +75,18 @@ Creating a custom builder allows you to control what buildpacks are used and wha return errors.Wrap(err, "getting absolute path for config") } - envMap, warnings, err := parseBuildConfigEnv(builderConfig.Build.Env, flags.BuilderTomlPath) + envMap, warnings, err := ParseBuildConfigEnv(builderConfig.Build.Env, flags.BuilderTomlPath) for _, v := range warnings { logger.Warn(v) } if err != nil { return err } - if err := generateBuildConfigEnvFiles(envMap); err != nil { - return err - } imageName := args[0] if err := pack.CreateBuilder(cmd.Context(), client.CreateBuilderOptions{ RelativeBaseDir: relativeBaseDir, + BuildConfigEnv: envMap, BuilderName: imageName, Config: builderConfig, Publish: flags.Publish, @@ -150,45 +147,6 @@ func validateCreateFlags(flags *BuilderCreateFlags, cfg config.Config) error { return nil } -func generateBuildConfigEnvFiles(envMap map[string]string) error { - dir, err := createBuildConfigEnvDir() - if err != nil { - return err - } - for k, v := range envMap { - f, err := os.Create(filepath.Join(dir, k)) - if err != nil { - return err - } - f.WriteString(v) - if e := f.Close(); e != nil { - return e - } - } - return nil -} - -func CnbBuildConfigDir() string { - if v := os.Getenv("CNB_BUILD_CONFIG_DIR"); v == "" || len(v) == 0 { - return "/cnb/build-config" - } else { - return v - } -} - -func createBuildConfigEnvDir() (dir string, err error) { - dir = filepath.Join(CnbBuildConfigDir(), "env") - _, err = os.Stat(dir) - if os.IsNotExist(err) { - err := os.MkdirAll(dir, os.ModePerm) - if err != nil { - return dir, err - } - return dir, nil - } - return dir, nil -} - func getActionType(suffix builder.Suffix) (suffixString string, err error) { const delim = "." switch suffix { @@ -206,7 +164,21 @@ func getActionType(suffix builder.Suffix) (suffixString string, err error) { return suffixString, errors.Errorf("unknown action type %s", style.Symbol(string(suffix))) } } -func GetBuildConfigEnvFileName(env builder.BuildConfigEnv) (suffixName, delimName string, err error) { + +func getFilePrefixSuffix(filename string) (prefix, suffix string, err error) { + val := strings.Split(filename, ".") + if len(val) <= 1 { + return val[0], suffix, errors.Errorf("Suffix might be null") + } + if len(val) == 2 { + suffix = val[1] + } else { + strings.Join(val[1:], ".") + } + return val[0], suffix, err +} + +func getBuildConfigEnvFileName(env builder.BuildConfigEnv) (suffixName, delimName string, err error) { suffix, err := getActionType(env.Suffix) if err != nil { return suffixName, delimName, err @@ -222,8 +194,9 @@ func GetBuildConfigEnvFileName(env builder.BuildConfigEnv) (suffixName, delimNam return suffixName, delimName, err } -func parseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[string]string, warnings []string, err error) { +func ParseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[string]string, warnings []string, err error) { envMap = map[string]string{} + var appendOrPrependWithoutDelim = 0 for _, v := range env { if name := v.Name; name == "" || len(name) == 0 { return nil, nil, errors.Wrapf(errors.Errorf("env name should not be empty"), "parse contents of '%s'", path) @@ -231,7 +204,7 @@ func parseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[ if val := v.Value; val == "" || len(val) == 0 { warnings = append(warnings, fmt.Sprintf("empty value for key/name %s", style.Symbol(v.Name))) } - suffixName, delimName, err := GetBuildConfigEnvFileName(v) + suffixName, delimName, err := getBuildConfigEnvFileName(v) if err != nil { return envMap, warnings, err } @@ -246,5 +219,19 @@ func parseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[ } envMap[suffixName] = v.Value } + + for k := range envMap { + name, suffix, err := getFilePrefixSuffix(k) + if err != nil { + continue + } + if _, ok := envMap[name+".delim"]; (suffix == "append" || suffix == "prepend") && !ok { + warnings = append(warnings, fmt.Sprintf(errors.Errorf("env with name/key %s with suffix %s must to have a %s value", style.Symbol(name), style.Symbol(suffix), style.Symbol("delim")).Error(), "parse contents of '%s'", path)) + appendOrPrependWithoutDelim++ + } + } + if appendOrPrependWithoutDelim > 0 { + return envMap, warnings, errors.Errorf("error parsing [[build.env]] in file '%s'", path) + } return envMap, warnings, err } diff --git a/internal/commands/builder_create_test.go b/internal/commands/builder_create_test.go index b36b67611c..f5f9684390 100644 --- a/internal/commands/builder_create_test.go +++ b/internal/commands/builder_create_test.go @@ -13,6 +13,7 @@ import ( "github.com/sclevine/spec/report" "github.com/spf13/cobra" + "github.com/buildpacks/pack/builder" "github.com/buildpacks/pack/internal/commands" "github.com/buildpacks/pack/internal/commands/testmocks" "github.com/buildpacks/pack/internal/config" @@ -47,76 +48,109 @@ const validConfigWithExtensions = ` ` -const ActionNONE = validConfig + ` -[[build.env]] -name = "actionNone" -value = "actionNoneValue" -` -const ActionDEFAULT = validConfig + ` -[[build.env]] -name = "actionDefault" -value = "actionDefaultValue" -suffix = "default" -` -const ActionOVERRIDE = validConfig + ` -[[build.env]] -name = "actionOverride" -value = "actionOverrideValue" -suffix = "override" -` -const ActionAPPEND = validConfig + ` -[[build.env]] -name = "actionAppend" -value = "actionAppendValue" -suffix = "append" -` -const ActionPREPEND = validConfig + ` -[[build.env]] -name = "actionPrepend" -value = "actionPrependValue" -suffix = "prepend" -` -const ActionDELIMIT = validConfig + ` -[[build.env]] -name = "actionDelimit" -delim = ":" -` -const ActionUNKNOWN = validConfig + ` -[[build.env]] -name = "actionUnknown" -value = "actionUnknownValue" -suffix = "unknown" -` -const ActionMULTIPLE = validConfig + ` -[[build.env]] -name = "MY_VAR" -value = "actionAppendValue" -suffix = "append" -delim = ":" -[[build.env]] -name = "MY_VAR" -value = "actionDefaultValue" -suffix = "default" -delim = ":" -[[build.env]] -name = "MY_VAR" -value = "actionPrependValue" -suffix = "prepend" -delim = ":" -` +var BuildConfigEnvSuffixNone = builder.BuildConfigEnv{ + Name: "suffixNone", + Value: "suffixNoneValue", +} -const ActionWarning = validConfig + ` -[[build.env]] -name = "actionWarning" -value = "" -` +var BuildConfigEnvSuffixNoneWithEmptySuffix = builder.BuildConfigEnv{ + Name: "suffixNoneWithEmptySuffix", + Value: "suffixNoneWithEmptySuffixValue", + Suffix: "", +} -const ActionError = validConfig + ` -[[build.env]] -name = "" -value = "some-value" -suffix = "default" -` +var BuildConfigEnvSuffixDefault = builder.BuildConfigEnv{ + Name: "suffixDefault", + Value: "suffixDefaultValue", + Suffix: "default", +} + +var BuildConfigEnvSuffixOverride = builder.BuildConfigEnv{ + Name: "suffixOverride", + Value: "suffixOverrideValue", + Suffix: "override", +} + +var BuildConfigEnvSuffixAppend = builder.BuildConfigEnv{ + Name: "suffixAppend", + Value: "suffixAppendValue", + Suffix: "append", + Delim: ":", +} + +var BuildConfigEnvSuffixPrepend = builder.BuildConfigEnv{ + Name: "suffixPrepend", + Value: "suffixPrependValue", + Suffix: "prepend", + Delim: ":", +} + +var BuildConfigEnvDelimWithoutSuffix = builder.BuildConfigEnv{ + Name: "delimWithoutSuffix", + Delim: ":", +} + +var BuildConfigEnvSuffixUnknown = builder.BuildConfigEnv{ + Name: "suffixUnknown", + Value: "suffixUnknownValue", + Suffix: "unknown", +} + +var BuildConfigEnvSuffixMultiple = []builder.BuildConfigEnv{ + { + Name: "MY_VAR", + Value: "suffixAppendValueValue", + Suffix: "append", + Delim: ";", + }, + { + Name: "MY_VAR", + Value: "suffixDefaultValue", + Suffix: "default", + Delim: "%", + }, + { + Name: "MY_VAR", + Value: "suffixPrependValue", + Suffix: "prepend", + Delim: ":", + }, +} + +var BuildConfigEnvEmptyValue = builder.BuildConfigEnv{ + Name: "warning", + Value: "", +} + +var BuildConfigEnvEmptyName = builder.BuildConfigEnv{ + Name: "", + Value: "suffixUnknownValue", + Suffix: "default", +} + +var BuildConfigEnvSuffixPrependWithoutDelim = builder.BuildConfigEnv{ + Name: "suffixPrepend", + Value: "suffixPrependValue", + Suffix: "prepend", +} + +var BuildConfigEnvDelimWithoutSuffixAppendOrPrepend = builder.BuildConfigEnv{ + Name: "delimWithoutActionAppendOrPrepend", + Value: "some-value", + Delim: ":", +} + +var BuildConfigEnvDelimWithSameSuffixAndName = []builder.BuildConfigEnv{ + { + Name: "MY_VAR", + Value: "some-value", + Suffix: "", + }, + { + Name: "MY_VAR", + Value: "some-value", + }, +} func TestCreateCommand(t *testing.T) { color.Disable(true) @@ -242,183 +276,112 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { }) }) - when("buildConfigEnv files generated", func() { - var fileIndex = 0 - buildConfigEnvDir := commands.CnbBuildConfigDir() - it.Before(func() { - err := os.MkdirAll(buildConfigEnvDir, os.ModePerm) + when("#ParseBuildpackConfigEnv", func() { + it("should create envMap as expected when suffix is omitted", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixNone}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixNone.Name: BuildConfigEnvSuffixNone.Value, + }) + h.AssertEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertNil(t, os.WriteFile(builderConfigPath, []byte(getBuildConfigEnvFileContent(fileIndex)), 0666)) }) - it.After(func() { - err := os.RemoveAll(buildConfigEnvDir) + it("should create envMap as expected when suffix is empty string", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixNoneWithEmptySuffix}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixNoneWithEmptySuffix.Name: BuildConfigEnvSuffixNoneWithEmptySuffix.Value, + }) + h.AssertEq(t, len(warnings), 0) h.AssertNil(t, err) - fileIndex++ }) - it("should create content as expected when ActionType `NONE`", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should create envMap as expected when suffix is `default`", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixDefault}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixDefault.Name + "." + string(BuildConfigEnvSuffixDefault.Suffix): BuildConfigEnvSuffixDefault.Value, }) - h.AssertNil(t, command.Execute()) - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) }) - it("should create content as expected when ActionType `DEFAULT`", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should create envMap as expected when suffix is `override`", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixOverride}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixOverride.Name + "." + string(BuildConfigEnvSuffixOverride.Suffix): BuildConfigEnvSuffixOverride.Value, }) - h.AssertNil(t, command.Execute()) - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) }) - it("should create content as expected when ActionType `OVERRIDE`", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should create envMap as expected when suffix is `append`", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixAppend}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixAppend.Name + "." + string(BuildConfigEnvSuffixAppend.Suffix): BuildConfigEnvSuffixAppend.Value, + BuildConfigEnvSuffixAppend.Name + ".delim": BuildConfigEnvSuffixAppend.Delim, }) - h.AssertNil(t, command.Execute()) - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) }) - it("should create content as expected when ActionType `APPEND`", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should create envMap as expected when suffix is `prepend`", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixPrepend}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixPrepend.Name + "." + string(BuildConfigEnvSuffixPrepend.Suffix): BuildConfigEnvSuffixPrepend.Value, + BuildConfigEnvSuffixPrepend.Name + ".delim": BuildConfigEnvSuffixPrepend.Delim, }) - h.AssertNil(t, command.Execute()) - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) + h.AssertEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) }) - it("should create content as expected when ActionType `PREPEND`", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should create envMap as expected when delim is specified", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvDelimWithoutSuffix}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvDelimWithoutSuffix.Name: BuildConfigEnvDelimWithoutSuffix.Value, + BuildConfigEnvDelimWithoutSuffix.Name + ".delim": BuildConfigEnvDelimWithoutSuffix.Delim, }) - h.AssertNil(t, command.Execute()) - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNotEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) - h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) }) - it("should create content as expected when ActionType `DELIMIT`", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should create envMap with a warning when `value` is empty", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvEmptyValue}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvEmptyValue.Name: BuildConfigEnvEmptyValue.Value, }) - h.AssertNil(t, command.Execute()) - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNotEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) - h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) }) - it("should return an error when unknown ActionType passed", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, - }) - var bufErr bytes.Buffer - command.SetErr(&bufErr) - h.AssertNil(t, command.Execute()) - h.AssertNotEq(t, bufErr.String(), "") - name := actionTypesMap[fileIndex][0][0] - _, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + it("should return an error when `name` is empty", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvEmptyName}, "") + h.AssertEq(t, envMap, map[string]string(nil)) + h.AssertEq(t, len(warnings), 0) h.AssertNotNil(t, err) }) - it("should create content as expected when multiple ActionTypes passed", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should return warnings when `apprend` or `prepend` is used without `delim`", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixPrependWithoutDelim}, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixPrependWithoutDelim.Name + "." + string(BuildConfigEnvSuffixPrependWithoutDelim.Suffix): BuildConfigEnvSuffixPrependWithoutDelim.Value, }) - h.AssertNil(t, command.Execute()) - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) - h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) - - name = actionTypesMap[fileIndex][1][0] - file, err = os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err = os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) - h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][1][1], string(content)) - - name = actionTypesMap[fileIndex][2][0] - file, err = os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err = os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) - h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][2][1], string(content)) - - name = actionTypesMap[fileIndex][3][0] - file, err = os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err = os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) - h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][3][1], string(content)) + h.AssertNotEq(t, len(warnings), 0) + h.AssertNotNil(t, err) }) - it("should show warnings when env value is empty", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should return an error when unknown `suffix` is used", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixUnknown}, "") + h.AssertEq(t, envMap, map[string]string{}) + h.AssertEq(t, len(warnings), 0) + h.AssertNotNil(t, err) + }) + it("should override with the last specified delim when `[[build.env]]` has multiple delims with same `name` with a `append` or `prepend` suffix", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv(BuildConfigEnvSuffixMultiple, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvSuffixMultiple[0].Name + "." + string(BuildConfigEnvSuffixMultiple[0].Suffix): BuildConfigEnvSuffixMultiple[0].Value, + BuildConfigEnvSuffixMultiple[1].Name + "." + string(BuildConfigEnvSuffixMultiple[1].Suffix): BuildConfigEnvSuffixMultiple[1].Value, + BuildConfigEnvSuffixMultiple[2].Name + "." + string(BuildConfigEnvSuffixMultiple[2].Suffix): BuildConfigEnvSuffixMultiple[2].Value, + BuildConfigEnvSuffixMultiple[2].Name + ".delim": BuildConfigEnvSuffixMultiple[2].Delim, }) - var bufOut bytes.Buffer - command.SetOut(&bufOut) - h.AssertNil(t, command.Execute()) - h.AssertNotEq(t, bufOut.String(), "") - name := actionTypesMap[fileIndex][0][0] - file, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) + h.AssertNotEq(t, len(warnings), 0) h.AssertNil(t, err) - h.AssertEq(t, name, file.Name()) - content, err := os.ReadFile(filepath.Join(buildConfigEnvDir, file.Name())) - h.AssertNil(t, err) - h.AssertEq(t, actionTypesMap[fileIndex][0][1], string(content)) }) - it("should return an error when env.Name is empty", func() { - command.SetArgs([]string{ - "some/builder", - "--config", builderConfigPath, + it("should override `value` with the last read value when a `[[build.env]]` has same `name` with same `suffix`", func() { + envMap, warnings, err := commands.ParseBuildConfigEnv(BuildConfigEnvDelimWithSameSuffixAndName, "") + h.AssertEq(t, envMap, map[string]string{ + BuildConfigEnvDelimWithSameSuffixAndName[1].Name: BuildConfigEnvDelimWithSameSuffixAndName[1].Value, }) - var bufErr bytes.Buffer - command.SetErr(&bufErr) - h.AssertNil(t, command.Execute()) - h.AssertNotEq(t, bufErr.String(), "") - name := actionTypesMap[fileIndex][0][0] - _, err := os.Stat(filepath.Clean(filepath.Join(buildConfigEnvDir, name))) - h.AssertNotNil(t, err) + h.AssertNotEq(t, len(warnings), 0) + h.AssertNil(t, err) }) }) @@ -464,48 +427,3 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { }) }) } - -func getBuildConfigEnvFileContent(index int) string { - switch index { - case 0: - return ActionNONE - case 1: - return ActionDEFAULT - case 2: - return ActionOVERRIDE - case 3: - return ActionAPPEND - case 4: - return ActionPREPEND - case 5: - return ActionDELIMIT - case 6: - return ActionUNKNOWN - case 7: - return ActionMULTIPLE - case 8: - return ActionWarning - case 9: - return ActionError - default: - return "" - } -} - -var actionTypesMap = map[int]map[int]map[int]string{ - 0: {0: {0: "actionNone", 1: "actionNoneValue"}}, - 1: {0: {0: "actionDefault.default", 1: "actionDefaultValue"}}, - 2: {0: {0: "actionOverride.override", 1: "actionOverrideValue"}}, - 3: {0: {0: "actionAppend.append", 1: "actionAppendValue"}}, - 4: {0: {0: "actionPrepend.prepend", 1: "actionPrependValue"}}, - 5: {0: {0: "actionDelim.delim", 1: ":"}}, - 6: {0: {0: "actionUnknown.unknown", 1: "actionUnknownValue"}}, - 7: { - 0: {0: "MY_VAR.append", 1: "actionAppendValue"}, - 1: {0: "MY_VAR.default", 1: "actionDefaultValue"}, - 2: {0: "MY_VAR.prepend", 1: "actionPrependValue"}, - 3: {0: "MY_VAR.delim", 1: ":"}, - }, - 8: {0: {0: "actionWarning", 1: ""}}, - 9: {0: {0: ".default", 1: "some-value"}}, -} diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index 28cd2fa7c5..12a6527c6c 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -29,6 +29,9 @@ type CreateBuilderOptions struct { // Name of the builder. BuilderName string + // BuildConfigEnv for Builder + BuildConfigEnv map[string]string + // Configuration that defines the functionality a builder provides. Config pubbldr.Config @@ -79,6 +82,11 @@ func (c *Client) CreateBuilder(ctx context.Context, opts CreateBuilderOptions) e bldr.SetStack(opts.Config.Stack) } bldr.SetRunImage(opts.Config.Run) + if opts.BuildConfigEnv == nil || len(opts.BuildConfigEnv) == 0 { + bldr.SetBuildConfigEnv(make(map[string]string)) + } else { + bldr.SetBuildConfigEnv(opts.BuildConfigEnv) + } return bldr.Save(c.logger, builder.CreatorMetadata{Version: c.version}) } @@ -191,6 +199,11 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption } bldr.SetLifecycle(lifecycle) + if opts.BuildConfigEnv == nil || len(opts.BuildConfigEnv) == 0 { + bldr.SetBuildConfigEnv(make(map[string]string)) + } else { + bldr.SetBuildConfigEnv(opts.BuildConfigEnv) + } return bldr, nil } From 5e9ce4c3d93b5bc6a7f6f46f4757f8e9b21984ff Mon Sep 17 00:00:00 2001 From: WYGIN Date: Thu, 12 Oct 2023 06:11:46 +0000 Subject: [PATCH 6/9] build-config-env layer oly added when it is not empty Signed-off-by: WYGIN --- internal/builder/builder.go | 15 +++++++-------- pkg/client/create_builder.go | 6 +----- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 47d392c82d..7c4e8d9a9a 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -536,15 +536,14 @@ func (b *Builder) Save(logger logging.Logger, creatorMetadata CreatorMetadata) e if len(b.buildConfigEnv) > 0 { logger.Debugf("Provided Build Config Environment Variables\n %s", style.Map(b.env, " ", "\n")) - } - - buildConfigEnvTar, err := b.buildConfigEnvLayer(tmpDir, b.buildConfigEnv) - if err != nil { - return errors.Wrap(err, "retrieving build-config-env layer") - } + buildConfigEnvTar, err := b.buildConfigEnvLayer(tmpDir, b.buildConfigEnv) + if err != nil { + return errors.Wrap(err, "retrieving build-config-env layer") + } - if err := b.image.AddLayer(buildConfigEnvTar); err != nil { - return errors.Wrap(err, "adding build-config-env layer") + if err := b.image.AddLayer(buildConfigEnvTar); err != nil { + return errors.Wrap(err, "adding build-config-env layer") + } } if len(b.env) > 0 { diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index 12a6527c6c..2d1a67164c 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -82,11 +82,7 @@ func (c *Client) CreateBuilder(ctx context.Context, opts CreateBuilderOptions) e bldr.SetStack(opts.Config.Stack) } bldr.SetRunImage(opts.Config.Run) - if opts.BuildConfigEnv == nil || len(opts.BuildConfigEnv) == 0 { - bldr.SetBuildConfigEnv(make(map[string]string)) - } else { - bldr.SetBuildConfigEnv(opts.BuildConfigEnv) - } + bldr.SetBuildConfigEnv(opts.BuildConfigEnv) return bldr.Save(c.logger, builder.CreatorMetadata{Version: c.version}) } From 43f6ec88f6d3c971ae544f1c3a9a92f60c1fafdf Mon Sep 17 00:00:00 2001 From: WYGIN Date: Wed, 25 Oct 2023 15:35:28 +0000 Subject: [PATCH 7/9] added requested changes and fix bugs Signed-off-by: WYGIN --- builder/config_reader.go | 93 +++++++++++++++++++++++- internal/builder/builder.go | 10 +-- internal/builder/builder_test.go | 37 +++++++++- internal/commands/builder_create.go | 91 +---------------------- internal/commands/builder_create_test.go | 26 +++---- pkg/client/create_builder.go | 6 +- 6 files changed, 146 insertions(+), 117 deletions(-) diff --git a/builder/config_reader.go b/builder/config_reader.go index 9cd6117a2c..f0195f9e4c 100644 --- a/builder/config_reader.go +++ b/builder/config_reader.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "github.com/BurntSushi/toml" "github.com/pkg/errors" @@ -76,8 +77,6 @@ type BuildConfig struct { type Suffix string -var SuffixSlice []Suffix = []Suffix{NONE, DEFAULT, OVERRIDE, APPEND, PREPEND} - const ( NONE Suffix = "" DEFAULT Suffix = "default" @@ -182,3 +181,93 @@ func parseConfig(file *os.File) (Config, error) { return builderConfig, nil } + +func ParseBuildConfigEnv(env []BuildConfigEnv, path string) (envMap map[string]string, warnings []string, err error) { + envMap = map[string]string{} + var appendOrPrependWithoutDelim = 0 + for _, v := range env { + if name := v.Name; name == "" || len(name) == 0 { + return nil, nil, errors.Wrapf(errors.Errorf("env name should not be empty"), "parse contents of '%s'", path) + } + if val := v.Value; val == "" || len(val) == 0 { + warnings = append(warnings, fmt.Sprintf("empty value for key/name %s", style.Symbol(v.Name))) + } + suffixName, delimName, err := getBuildConfigEnvFileName(v) + if err != nil { + return envMap, warnings, err + } + if val, e := envMap[suffixName]; e { + warnings = append(warnings, fmt.Sprintf(errors.Errorf("overriding env with name: %s and suffix: %s from %s to %s", style.Symbol(v.Name), style.Symbol(string(v.Suffix)), style.Symbol(val), style.Symbol(v.Value)).Error(), "parse contents of '%s'", path)) + } + if val, e := envMap[delimName]; e { + warnings = append(warnings, fmt.Sprintf(errors.Errorf("overriding env with name: %s and delim: %s from %s to %s", style.Symbol(v.Name), style.Symbol(v.Delim), style.Symbol(val), style.Symbol(v.Value)).Error(), "parse contents of '%s'", path)) + } + if delim := v.Delim; (delim != "" || len(delim) != 0) && (delimName != "" || len(delimName) != 0) { + envMap[delimName] = delim + } + envMap[suffixName] = v.Value + } + + for k := range envMap { + name, suffix, err := getFilePrefixSuffix(k) + if err != nil { + continue + } + if _, ok := envMap[name+".delim"]; (suffix == "append" || suffix == "prepend") && !ok { + warnings = append(warnings, fmt.Sprintf(errors.Errorf("env with name/key %s with suffix %s must to have a %s value", style.Symbol(name), style.Symbol(suffix), style.Symbol("delim")).Error(), "parse contents of '%s'", path)) + appendOrPrependWithoutDelim++ + } + } + if appendOrPrependWithoutDelim > 0 { + return envMap, warnings, errors.Errorf("error parsing [[build.env]] in file '%s'", path) + } + return envMap, warnings, err +} + +func getBuildConfigEnvFileName(env BuildConfigEnv) (suffixName, delimName string, err error) { + suffix, err := getActionType(env.Suffix) + if err != nil { + return suffixName, delimName, err + } + if suffix == "" || len(suffix) == 0 { + suffixName = env.Name + } else { + suffixName = env.Name + suffix + } + if delim := env.Delim; delim != "" || len(delim) != 0 { + delimName = env.Name + ".delim" + } + return suffixName, delimName, err +} + +func getActionType(suffix Suffix) (suffixString string, err error) { + const delim = "." + switch suffix { + case NONE: + return "", nil + case DEFAULT: + return delim + string(DEFAULT), nil + case OVERRIDE: + return delim + string(OVERRIDE), nil + case APPEND: + return delim + string(APPEND), nil + case PREPEND: + return delim + string(PREPEND), nil + default: + return suffixString, errors.Errorf("unknown action type %s", style.Symbol(string(suffix))) + } +} + +func getFilePrefixSuffix(filename string) (prefix, suffix string, err error) { + val := strings.Split(filename, ".") + if len(val) <= 1 { + return val[0], suffix, errors.Errorf("Suffix might be null") + } + if len(val) == 2 { + suffix = val[1] + } else { + suffix = strings.Join(val[1:], ".") + } + return val[0], suffix, err +} + diff --git a/internal/builder/builder.go b/internal/builder/builder.go index 7c4e8d9a9a..fa7dd6e9d3 100644 --- a/internal/builder/builder.go +++ b/internal/builder/builder.go @@ -1129,13 +1129,11 @@ func (b *Builder) buildConfigEnvLayer(dest string, env map[string]string) (strin return "", err } defer fh.Close() - lw := b.layerWriterFactory.NewWriter(fh) defer lw.Close() - for k, v := range env { if err := lw.WriteHeader(&tar.Header{ - Name: path.Join(buildConfigDir, "env", k), + Name: path.Join(cnbBuildConfigDir(), "env", k), Size: int64(len(v)), Mode: 0644, ModTime: archive.NormalizedDateTime, @@ -1307,9 +1305,9 @@ func (e errModuleTar) Path() string { } func cnbBuildConfigDir() string { - if env, ok := os.LookupEnv("CNB_BUILD_CONFIG_DIR"); !ok { - return "/cnb/build-config" - } else { + if env, ok := os.LookupEnv("CNB_BUILD_CONFIG_DIR"); ok { return env } + + return "/cnb/build-config" } diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 2745e9e97b..8f0e18350c 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -397,7 +397,6 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { h.HasModTime(archive.NormalizedDateTime), ) }) - it("creates the buildpacks dir", func() { h.AssertNil(t, subject.Save(logger, builder.CreatorMetadata{})) h.AssertEq(t, baseImage.IsSaved(), true) @@ -1621,8 +1620,44 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) }) + when("when CNB_BUILD_CONFIG_DIR is defined", func () { + var buildConfigEnvName = "CNB_BUILD_CONFIG_DIR" + var buildConfigEnvValue = "/cnb/dup-build-config-dir" + it.Before(func() { + os.Setenv(buildConfigEnvName, buildConfigEnvValue) + subject.SetBuildConfigEnv(map[string]string{ + "SOME_KEY": "some-val", + "OTHER_KEY.append": "other-val", + "OTHER_KEY.delim": ":", + }) + h.AssertNil(t, subject.Save(logger, builder.CreatorMetadata{})) + h.AssertEq(t, baseImage.IsSaved(), true) + }) + it.After(func() { + os.Unsetenv(buildConfigEnvName) + }) + + it("adds the env vars as files to the image", func() { + layerTar, err := baseImage.FindLayerWithPath(buildConfigEnvValue + "/env/SOME_KEY") + h.AssertNil(t, err) + h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue + "/env/SOME_KEY", + h.ContentEquals(`some-val`), + h.HasModTime(archive.NormalizedDateTime), + ) + h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue + "/env/OTHER_KEY.append", + h.ContentEquals(`other-val`), + h.HasModTime(archive.NormalizedDateTime), + ) + h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue + "/env/OTHER_KEY.delim", + h.ContentEquals(`:`), + h.HasModTime(archive.NormalizedDateTime), + ) + }) + }) + when("#SetBuildConfigEnv", func() { it.Before(func() { + os.Unsetenv("CNB_BUILD_CONFIG_DIR") subject.SetBuildConfigEnv(map[string]string{ "SOME_KEY": "some-val", "OTHER_KEY.append": "other-val", diff --git a/internal/commands/builder_create.go b/internal/commands/builder_create.go index a98cb43521..c3c1ccb90b 100644 --- a/internal/commands/builder_create.go +++ b/internal/commands/builder_create.go @@ -75,7 +75,7 @@ Creating a custom builder allows you to control what buildpacks are used and wha return errors.Wrap(err, "getting absolute path for config") } - envMap, warnings, err := ParseBuildConfigEnv(builderConfig.Build.Env, flags.BuilderTomlPath) + envMap, warnings, err := builder.ParseBuildConfigEnv(builderConfig.Build.Env, flags.BuilderTomlPath) for _, v := range warnings { logger.Warn(v) } @@ -146,92 +146,3 @@ func validateCreateFlags(flags *BuilderCreateFlags, cfg config.Config) error { return nil } - -func getActionType(suffix builder.Suffix) (suffixString string, err error) { - const delim = "." - switch suffix { - case builder.NONE: - return "", nil - case builder.DEFAULT: - return delim + string(builder.DEFAULT), nil - case builder.OVERRIDE: - return delim + string(builder.OVERRIDE), nil - case builder.APPEND: - return delim + string(builder.APPEND), nil - case builder.PREPEND: - return delim + string(builder.PREPEND), nil - default: - return suffixString, errors.Errorf("unknown action type %s", style.Symbol(string(suffix))) - } -} - -func getFilePrefixSuffix(filename string) (prefix, suffix string, err error) { - val := strings.Split(filename, ".") - if len(val) <= 1 { - return val[0], suffix, errors.Errorf("Suffix might be null") - } - if len(val) == 2 { - suffix = val[1] - } else { - strings.Join(val[1:], ".") - } - return val[0], suffix, err -} - -func getBuildConfigEnvFileName(env builder.BuildConfigEnv) (suffixName, delimName string, err error) { - suffix, err := getActionType(env.Suffix) - if err != nil { - return suffixName, delimName, err - } - if suffix == "" || len(suffix) == 0 { - suffixName = env.Name - } else { - suffixName = env.Name + suffix - } - if delim := env.Delim; delim != "" || len(delim) != 0 { - delimName = env.Name + ".delim" - } - return suffixName, delimName, err -} - -func ParseBuildConfigEnv(env []builder.BuildConfigEnv, path string) (envMap map[string]string, warnings []string, err error) { - envMap = map[string]string{} - var appendOrPrependWithoutDelim = 0 - for _, v := range env { - if name := v.Name; name == "" || len(name) == 0 { - return nil, nil, errors.Wrapf(errors.Errorf("env name should not be empty"), "parse contents of '%s'", path) - } - if val := v.Value; val == "" || len(val) == 0 { - warnings = append(warnings, fmt.Sprintf("empty value for key/name %s", style.Symbol(v.Name))) - } - suffixName, delimName, err := getBuildConfigEnvFileName(v) - if err != nil { - return envMap, warnings, err - } - if val, e := envMap[suffixName]; e { - warnings = append(warnings, fmt.Sprintf(errors.Errorf("overriding env with name: %s and suffix: %s from %s to %s", style.Symbol(v.Name), style.Symbol(string(v.Suffix)), style.Symbol(val), style.Symbol(v.Value)).Error(), "parse contents of '%s'", path)) - } - if val, e := envMap[delimName]; e { - warnings = append(warnings, fmt.Sprintf(errors.Errorf("overriding env with name: %s and delim: %s from %s to %s", style.Symbol(v.Name), style.Symbol(v.Delim), style.Symbol(val), style.Symbol(v.Value)).Error(), "parse contents of '%s'", path)) - } - if delim := v.Delim; (delim != "" || len(delim) != 0) && (delimName != "" || len(delimName) != 0) { - envMap[delimName] = delim - } - envMap[suffixName] = v.Value - } - - for k := range envMap { - name, suffix, err := getFilePrefixSuffix(k) - if err != nil { - continue - } - if _, ok := envMap[name+".delim"]; (suffix == "append" || suffix == "prepend") && !ok { - warnings = append(warnings, fmt.Sprintf(errors.Errorf("env with name/key %s with suffix %s must to have a %s value", style.Symbol(name), style.Symbol(suffix), style.Symbol("delim")).Error(), "parse contents of '%s'", path)) - appendOrPrependWithoutDelim++ - } - } - if appendOrPrependWithoutDelim > 0 { - return envMap, warnings, errors.Errorf("error parsing [[build.env]] in file '%s'", path) - } - return envMap, warnings, err -} diff --git a/internal/commands/builder_create_test.go b/internal/commands/builder_create_test.go index f5f9684390..4ddf115b32 100644 --- a/internal/commands/builder_create_test.go +++ b/internal/commands/builder_create_test.go @@ -278,7 +278,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { when("#ParseBuildpackConfigEnv", func() { it("should create envMap as expected when suffix is omitted", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixNone}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixNone}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixNone.Name: BuildConfigEnvSuffixNone.Value, }) @@ -286,7 +286,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should create envMap as expected when suffix is empty string", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixNoneWithEmptySuffix}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixNoneWithEmptySuffix}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixNoneWithEmptySuffix.Name: BuildConfigEnvSuffixNoneWithEmptySuffix.Value, }) @@ -294,7 +294,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should create envMap as expected when suffix is `default`", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixDefault}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixDefault}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixDefault.Name + "." + string(BuildConfigEnvSuffixDefault.Suffix): BuildConfigEnvSuffixDefault.Value, }) @@ -302,7 +302,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should create envMap as expected when suffix is `override`", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixOverride}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixOverride}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixOverride.Name + "." + string(BuildConfigEnvSuffixOverride.Suffix): BuildConfigEnvSuffixOverride.Value, }) @@ -310,7 +310,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should create envMap as expected when suffix is `append`", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixAppend}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixAppend}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixAppend.Name + "." + string(BuildConfigEnvSuffixAppend.Suffix): BuildConfigEnvSuffixAppend.Value, BuildConfigEnvSuffixAppend.Name + ".delim": BuildConfigEnvSuffixAppend.Delim, @@ -319,7 +319,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should create envMap as expected when suffix is `prepend`", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixPrepend}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixPrepend}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixPrepend.Name + "." + string(BuildConfigEnvSuffixPrepend.Suffix): BuildConfigEnvSuffixPrepend.Value, BuildConfigEnvSuffixPrepend.Name + ".delim": BuildConfigEnvSuffixPrepend.Delim, @@ -328,7 +328,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should create envMap as expected when delim is specified", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvDelimWithoutSuffix}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvDelimWithoutSuffix}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvDelimWithoutSuffix.Name: BuildConfigEnvDelimWithoutSuffix.Value, BuildConfigEnvDelimWithoutSuffix.Name + ".delim": BuildConfigEnvDelimWithoutSuffix.Delim, @@ -337,7 +337,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should create envMap with a warning when `value` is empty", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvEmptyValue}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvEmptyValue}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvEmptyValue.Name: BuildConfigEnvEmptyValue.Value, }) @@ -345,13 +345,13 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should return an error when `name` is empty", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvEmptyName}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvEmptyName}, "") h.AssertEq(t, envMap, map[string]string(nil)) h.AssertEq(t, len(warnings), 0) h.AssertNotNil(t, err) }) it("should return warnings when `apprend` or `prepend` is used without `delim`", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixPrependWithoutDelim}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixPrependWithoutDelim}, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixPrependWithoutDelim.Name + "." + string(BuildConfigEnvSuffixPrependWithoutDelim.Suffix): BuildConfigEnvSuffixPrependWithoutDelim.Value, }) @@ -359,13 +359,13 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNotNil(t, err) }) it("should return an error when unknown `suffix` is used", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixUnknown}, "") + envMap, warnings, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{BuildConfigEnvSuffixUnknown}, "") h.AssertEq(t, envMap, map[string]string{}) h.AssertEq(t, len(warnings), 0) h.AssertNotNil(t, err) }) it("should override with the last specified delim when `[[build.env]]` has multiple delims with same `name` with a `append` or `prepend` suffix", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv(BuildConfigEnvSuffixMultiple, "") + envMap, warnings, err := builder.ParseBuildConfigEnv(BuildConfigEnvSuffixMultiple, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvSuffixMultiple[0].Name + "." + string(BuildConfigEnvSuffixMultiple[0].Suffix): BuildConfigEnvSuffixMultiple[0].Value, BuildConfigEnvSuffixMultiple[1].Name + "." + string(BuildConfigEnvSuffixMultiple[1].Suffix): BuildConfigEnvSuffixMultiple[1].Value, @@ -376,7 +376,7 @@ func testCreateCommand(t *testing.T, when spec.G, it spec.S) { h.AssertNil(t, err) }) it("should override `value` with the last read value when a `[[build.env]]` has same `name` with same `suffix`", func() { - envMap, warnings, err := commands.ParseBuildConfigEnv(BuildConfigEnvDelimWithSameSuffixAndName, "") + envMap, warnings, err := builder.ParseBuildConfigEnv(BuildConfigEnvDelimWithSameSuffixAndName, "") h.AssertEq(t, envMap, map[string]string{ BuildConfigEnvDelimWithSameSuffixAndName[1].Name: BuildConfigEnvDelimWithSameSuffixAndName[1].Value, }) diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index 2d1a67164c..2d9610c3df 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -195,11 +195,7 @@ func (c *Client) createBaseBuilder(ctx context.Context, opts CreateBuilderOption } bldr.SetLifecycle(lifecycle) - if opts.BuildConfigEnv == nil || len(opts.BuildConfigEnv) == 0 { - bldr.SetBuildConfigEnv(make(map[string]string)) - } else { - bldr.SetBuildConfigEnv(opts.BuildConfigEnv) - } + bldr.SetBuildConfigEnv(opts.BuildConfigEnv) return bldr, nil } From 8714d1006a8442bf63927add58db273957f8876b Mon Sep 17 00:00:00 2001 From: WYGIN Date: Wed, 25 Oct 2023 15:56:16 +0000 Subject: [PATCH 8/9] fix: code format Signed-off-by: WYGIN --- builder/config_reader.go | 1 - internal/builder/builder_test.go | 8 ++++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/builder/config_reader.go b/builder/config_reader.go index f0195f9e4c..008984bff3 100644 --- a/builder/config_reader.go +++ b/builder/config_reader.go @@ -270,4 +270,3 @@ func getFilePrefixSuffix(filename string) (prefix, suffix string, err error) { } return val[0], suffix, err } - diff --git a/internal/builder/builder_test.go b/internal/builder/builder_test.go index 8f0e18350c..37124533da 100644 --- a/internal/builder/builder_test.go +++ b/internal/builder/builder_test.go @@ -1620,7 +1620,7 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { }) }) - when("when CNB_BUILD_CONFIG_DIR is defined", func () { + when("when CNB_BUILD_CONFIG_DIR is defined", func() { var buildConfigEnvName = "CNB_BUILD_CONFIG_DIR" var buildConfigEnvValue = "/cnb/dup-build-config-dir" it.Before(func() { @@ -1640,15 +1640,15 @@ func testBuilder(t *testing.T, when spec.G, it spec.S) { it("adds the env vars as files to the image", func() { layerTar, err := baseImage.FindLayerWithPath(buildConfigEnvValue + "/env/SOME_KEY") h.AssertNil(t, err) - h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue + "/env/SOME_KEY", + h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue+"/env/SOME_KEY", h.ContentEquals(`some-val`), h.HasModTime(archive.NormalizedDateTime), ) - h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue + "/env/OTHER_KEY.append", + h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue+"/env/OTHER_KEY.append", h.ContentEquals(`other-val`), h.HasModTime(archive.NormalizedDateTime), ) - h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue + "/env/OTHER_KEY.delim", + h.AssertOnTarEntry(t, layerTar, buildConfigEnvValue+"/env/OTHER_KEY.delim", h.ContentEquals(`:`), h.HasModTime(archive.NormalizedDateTime), ) From 064f25e6979e281efdcccb9f0016733f83d03d28 Mon Sep 17 00:00:00 2001 From: WYGIN Date: Thu, 26 Oct 2023 10:34:16 +0000 Subject: [PATCH 9/9] improved code coverage Signed-off-by: WYGIN --- builder/config_reader.go | 2 +- builder/config_reader_test.go | 107 ++++++++++++++++++++++++++++++++++ testhelpers/testhelpers.go | 27 +++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) diff --git a/builder/config_reader.go b/builder/config_reader.go index 008984bff3..6ebd688d55 100644 --- a/builder/config_reader.go +++ b/builder/config_reader.go @@ -229,7 +229,7 @@ func getBuildConfigEnvFileName(env BuildConfigEnv) (suffixName, delimName string if err != nil { return suffixName, delimName, err } - if suffix == "" || len(suffix) == 0 { + if suffix == "" { suffixName = env.Name } else { suffixName = env.Name + suffix diff --git a/builder/config_reader_test.go b/builder/config_reader_test.go index 93a1bf851d..6c5512f897 100644 --- a/builder/config_reader_test.go +++ b/builder/config_reader_test.go @@ -229,4 +229,111 @@ uri = "noop-buildpack.tgz" h.AssertError(t, builder.ValidateConfig(config), "build.image is required") }) }) + when("#ParseBuildConfigEnv()", func() { + it("should return an error when name is not defined", func() { + _, _, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{ + { + Name: "", + Value: "vaiue", + }, + }, "") + h.AssertNotNil(t, err) + }) + it("should warn when the value is nil or empty string", func() { + env, warn, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{ + { + Name: "key", + Value: "", + Suffix: "override", + }, + }, "") + + h.AssertNotNil(t, warn) + h.AssertNil(t, err) + h.AssertMapContains[string, string](t, env, h.NewKeyValue[string, string]("key.override", "")) + }) + it("should return an error when unknown suffix is specified", func() { + _, _, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{ + { + Name: "key", + Value: "", + Suffix: "invalid", + }, + }, "") + + h.AssertNotNil(t, err) + }) + it("should override and show a warning when suffix or delim is defined multiple times", func() { + env, warn, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{ + { + Name: "key1", + Value: "value1", + Suffix: "append", + Delim: "%", + }, + { + Name: "key1", + Value: "value2", + Suffix: "append", + Delim: ",", + }, + { + Name: "key1", + Value: "value3", + Suffix: "default", + Delim: ";", + }, + { + Name: "key1", + Value: "value4", + Suffix: "prepend", + Delim: ":", + }, + }, "") + + h.AssertNotNil(t, warn) + h.AssertNil(t, err) + h.AssertMapContains[string, string]( + t, + env, + h.NewKeyValue[string, string]("key1.append", "value2"), + h.NewKeyValue[string, string]("key1.default", "value3"), + h.NewKeyValue[string, string]("key1.prepend", "value4"), + h.NewKeyValue[string, string]("key1.delim", ":"), + ) + h.AssertMapNotContains[string, string]( + t, + env, + h.NewKeyValue[string, string]("key1.append", "value1"), + h.NewKeyValue[string, string]("key1.delim", "%"), + h.NewKeyValue[string, string]("key1.delim", ","), + h.NewKeyValue[string, string]("key1.delim", ";"), + ) + }) + it("should return an error when `suffix` is defined as `append` or `prepend` without a `delim`", func() { + _, warn, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{ + { + Name: "key", + Value: "value", + Suffix: "append", + }, + }, "") + + h.AssertNotNil(t, warn) + h.AssertNotNil(t, err) + }) + it("when suffix is NONE or omitted should default to `override`", func() { + env, warn, err := builder.ParseBuildConfigEnv([]builder.BuildConfigEnv{ + { + Name: "key", + Value: "value", + Suffix: "", + }, + }, "") + + h.AssertNotNil(t, warn) + h.AssertNil(t, err) + h.AssertMapContains[string, string](t, env, h.NewKeyValue[string, string]("key", "value")) + }) + }) } diff --git a/testhelpers/testhelpers.go b/testhelpers/testhelpers.go index 7868691a3c..3eeb4b6431 100644 --- a/testhelpers/testhelpers.go +++ b/testhelpers/testhelpers.go @@ -179,6 +179,33 @@ func AssertNotContains(t *testing.T, actual, expected string) { } } +type KeyValue[k comparable, v any] struct { + key k + value v +} + +func NewKeyValue[k comparable, v any](key k, value v) KeyValue[k, v] { + return KeyValue[k, v]{key: key, value: value} +} + +func AssertMapContains[key comparable, value any](t *testing.T, actual map[key]value, expected ...KeyValue[key, value]) { + t.Helper() + for _, i := range expected { + if v, ok := actual[i.key]; !ok || !reflect.DeepEqual(v, i.value) { + t.Fatalf("Expected %s to contain elements %s", reflect.ValueOf(actual), reflect.ValueOf(expected)) + } + } +} + +func AssertMapNotContains[key comparable, value any](t *testing.T, actual map[key]value, expected ...KeyValue[key, value]) { + t.Helper() + for _, i := range expected { + if v, ok := actual[i.key]; ok && reflect.DeepEqual(v, i.value) { + t.Fatalf("Expected %s to not contain elements %s", reflect.ValueOf(actual), reflect.ValueOf(expected)) + } + } +} + func AssertSliceContains(t *testing.T, slice []string, expected ...string) { t.Helper() _, missing, _ := stringset.Compare(slice, expected)