From 79e769ab5d15b53d8e927f3052ef0e2eec9ede5e Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 22 Sep 2023 11:48:49 -0400 Subject: [PATCH 01/17] Fix misleading log message when publishing a buildpack package When using --publish it should say "saved to registry" not "saved to docker daemon" Signed-off-by: Natalie Arellano --- internal/commands/buildpack_package.go | 4 ++-- internal/commands/extension_package.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/commands/buildpack_package.go b/internal/commands/buildpack_package.go index 6cac0ad74..dff2bf398 100644 --- a/internal/commands/buildpack_package.go +++ b/internal/commands/buildpack_package.go @@ -118,14 +118,14 @@ func BuildpackPackage(logger logging.Logger, cfg config.Config, packager Buildpa } action := "created" + location := "docker daemon" if flags.Publish { action = "published" + location = "registry" } - location := "docker daemon" if flags.Format == client.FormatFile { location = "file" } - logger.Infof("Successfully %s package %s and saved to %s", action, style.Symbol(name), location) return nil }), diff --git a/internal/commands/extension_package.go b/internal/commands/extension_package.go index 69370c460..f8d729b2c 100644 --- a/internal/commands/extension_package.go +++ b/internal/commands/extension_package.go @@ -84,15 +84,16 @@ func ExtensionPackage(logger logging.Logger, cfg config.Config, packager Extensi }); err != nil { return err } + action := "created" + location := "docker daemon" if flags.Publish { action = "published" + location = "registry" } - location := "docker daemon" if flags.Format == client.FormatFile { location = "file" } - logger.Infof("Successfully %s package %s and saved to %s", action, style.Symbol(name), location) return nil }), From effc5e500867aa8b283470151ee56198d167d011 Mon Sep 17 00:00:00 2001 From: WYGIN Date: Wed, 4 Oct 2023 16:15:54 +0530 Subject: [PATCH 02/17] 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 3984da850..4f4bc9a66 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 3371d8d38..fdbb3bd3a 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 03/17] 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 d719c5df8..35ccd7ac9 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 4f4bc9a66..3984da850 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 1b0d70989..cd8cb098a 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 12c89465f..a70b5cd59 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 fdbb3bd3a..3371d8d38 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 04/17] 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 35ccd7ac9..7c17ac331 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 cd8cb098a..7cdf1faa7 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 a70b5cd59..fcf3cc59c 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 05/17] 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 7c17ac331..9cd6117a2 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 51b5a4a00..8228ab779 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 7cdf1faa7..b606b0873 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 fcf3cc59c..b36b67611 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 06/17] 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 8228ab779..47d392c82 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 13a958c72..2745e9e97 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 b606b0873..a98cb4352 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 b36b67611..f5f968439 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 28cd2fa7c..12a6527c6 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 07/17] 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 47d392c82..7c4e8d9a9 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 12a6527c6..2d1a67164 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 5728158e3d2915d9bcedde53f11e5f0178cfa556 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 12 Oct 2023 10:01:06 -0400 Subject: [PATCH 08/17] Ensure the run image os/arch always matches: - the builder for `pack build` - the previous image for `pack rebase` Signed-off-by: Natalie Arellano --- pkg/client/build.go | 36 ++++++++++++++++++++++-------------- pkg/client/build_test.go | 9 +++++---- pkg/client/rebase.go | 17 ++++++++++++++++- pkg/client/rebase_test.go | 2 ++ 4 files changed, 45 insertions(+), 19 deletions(-) diff --git a/pkg/client/build.go b/pkg/client/build.go index c7c35465e..c94e862df 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -318,6 +318,16 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return errors.Wrapf(err, "failed to fetch builder image '%s'", builderRef.Name()) } + builderOS, err := rawBuilderImage.OS() + if err != nil { + return errors.Wrapf(err, "getting builder OS") + } + + builderArch, err := rawBuilderImage.Architecture() + if err != nil { + return errors.Wrapf(err, "getting builder architecture") + } + bldr, err := c.getBuilder(rawBuilderImage) if err != nil { return errors.Wrapf(err, "invalid builder %s", style.Symbol(opts.Builder)) @@ -325,7 +335,11 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { runImageName := c.resolveRunImage(opts.RunImage, imgRegistry, builderRef.Context().RegistryStr(), bldr.DefaultRunImage(), opts.AdditionalMirrors, opts.Publish) - fetchOptions := image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy} + fetchOptions := image.FetchOptions{ + Daemon: !opts.Publish, + PullPolicy: opts.PullPolicy, + Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), + } if opts.Layout() { targetRunImagePath, err := layout.ParseRefToPath(runImageName) if err != nil { @@ -361,11 +375,6 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { return err } - imgOS, err := rawBuilderImage.OS() - if err != nil { - return errors.Wrapf(err, "getting builder OS") - } - // Default mode: if the TrustBuilder option is not set, trust the suggested builders. if opts.TrustBuilder == nil { opts.TrustBuilder = IsSuggestedBuilderFunc @@ -396,15 +405,14 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { lifecycleImageName = fmt.Sprintf("%s:%s", internalConfig.DefaultLifecycleImageRepo, lifecycleVersion.String()) } - imgArch, err := rawBuilderImage.Architecture() - if err != nil { - return errors.Wrapf(err, "getting builder architecture") - } - lifecycleImage, err := c.imageFetcher.Fetch( ctx, lifecycleImageName, - image.FetchOptions{Daemon: true, PullPolicy: opts.PullPolicy, Platform: fmt.Sprintf("%s/%s", imgOS, imgArch)}, + image.FetchOptions{ + Daemon: true, + PullPolicy: opts.PullPolicy, + Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), + }, ) if err != nil { return fmt.Errorf("fetching lifecycle image: %w", err) @@ -455,7 +463,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { if !c.experimental { return fmt.Errorf("experimental features must be enabled when builder contains image extensions") } - if imgOS == "windows" { + if builderOS == "windows" { return fmt.Errorf("builder contains image extensions which are not supported for Windows builds") } if !(opts.PullPolicy == image.PullAlways) { @@ -467,7 +475,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error { opts.ContainerConfig.Volumes = appendLayoutVolumes(opts.ContainerConfig.Volumes, pathsConfig) } - processedVolumes, warnings, err := processVolumes(imgOS, opts.ContainerConfig.Volumes) + processedVolumes, warnings, err := processVolumes(builderOS, opts.ContainerConfig.Volumes) if err != nil { return err } diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 06b215104..2f43619c5 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -2085,11 +2085,12 @@ api = "0.2" })) h.AssertEq(t, fakeLifecycle.Opts.Publish, true) - args := fakeImageFetcher.FetchCalls["default/run"] - h.AssertEq(t, args.Daemon, false) - - args = fakeImageFetcher.FetchCalls[defaultBuilderName] + args := fakeImageFetcher.FetchCalls[defaultBuilderName] h.AssertEq(t, args.Daemon, true) + + args = fakeImageFetcher.FetchCalls["default/run"] + h.AssertEq(t, args.Daemon, false) + h.AssertEq(t, args.Platform, "linux/amd64") }) when("builder is untrusted", func() { diff --git a/pkg/client/rebase.go b/pkg/client/rebase.go index 168c72757..276c56026 100644 --- a/pkg/client/rebase.go +++ b/pkg/client/rebase.go @@ -2,6 +2,7 @@ package client import ( "context" + "fmt" "os" "path/filepath" @@ -60,6 +61,16 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { return err } + appOS, err := appImage.OS() + if err != nil { + return errors.Wrapf(err, "getting app OS") + } + + appArch, err := appImage.Architecture() + if err != nil { + return errors.Wrapf(err, "getting app architecture") + } + var md files.LayersMetadataCompat if ok, err := dist.GetLabel(appImage, platform.LifecycleMetadataLabel, &md); err != nil { return err @@ -90,7 +101,11 @@ func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error { return errors.New("run image must be specified") } - baseImage, err := c.imageFetcher.Fetch(ctx, runImageName, image.FetchOptions{Daemon: !opts.Publish, PullPolicy: opts.PullPolicy}) + baseImage, err := c.imageFetcher.Fetch(ctx, runImageName, image.FetchOptions{ + Daemon: !opts.Publish, + PullPolicy: opts.PullPolicy, + Platform: fmt.Sprintf("%s/%s", appOS, appArch), + }) if err != nil { return err } diff --git a/pkg/client/rebase_test.go b/pkg/client/rebase_test.go index 86eff76de..101fb683a 100644 --- a/pkg/client/rebase_test.go +++ b/pkg/client/rebase_test.go @@ -258,6 +258,8 @@ func testRebase(t *testing.T, when spec.G, it spec.S) { h.AssertEq(t, fakeAppImage.Base(), "some/run") lbl, _ := fakeAppImage.Label("io.buildpacks.lifecycle.metadata") h.AssertContains(t, lbl, `"runImage":{"topLayer":"remote-top-layer-sha","reference":"remote-digest"`) + args := fakeImageFetcher.FetchCalls["some/run"] + h.AssertEq(t, args.Platform, "linux/amd64") }) }) }) From 83e94572b2a3676a02747135776f1439217b839d Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Thu, 12 Oct 2023 11:30:07 -0400 Subject: [PATCH 09/17] When downloading buildpacks or extensions for `pack build` or `pack builder create`, ensure the os/arch matches the builder Signed-off-by: Natalie Arellano --- pkg/buildpack/downloader.go | 15 +++++++++++++-- pkg/buildpack/downloader_test.go | 24 +++++++++++++++--------- pkg/client/build.go | 13 ++++++++++--- pkg/client/build_test.go | 2 ++ pkg/client/create_builder.go | 12 +++++++++--- pkg/client/create_builder_test.go | 14 ++++++++++++-- 6 files changed, 61 insertions(+), 19 deletions(-) diff --git a/pkg/buildpack/downloader.go b/pkg/buildpack/downloader.go index 41a5a61cd..454afda0c 100644 --- a/pkg/buildpack/downloader.go +++ b/pkg/buildpack/downloader.go @@ -67,6 +67,9 @@ type DownloadOptions struct { // The OS of the builder image ImageOS string + // The OS/Architecture to download + Platform string + // Deprecated: the older alternative to buildpack URI ImageName string @@ -102,7 +105,11 @@ func (c *buildpackDownloader) Download(ctx context.Context, moduleURI string, op case PackageLocator: imageName := ParsePackageLocator(moduleURI) c.logger.Debugf("Downloading %s from image: %s", kind, style.Symbol(imageName)) - mainBP, depBPs, err = extractPackaged(ctx, kind, imageName, c.imageFetcher, image.FetchOptions{Daemon: opts.Daemon, PullPolicy: opts.PullPolicy}) + mainBP, depBPs, err = extractPackaged(ctx, kind, imageName, c.imageFetcher, image.FetchOptions{ + Daemon: opts.Daemon, + PullPolicy: opts.PullPolicy, + Platform: opts.Platform, + }) if err != nil { return nil, nil, errors.Wrapf(err, "extracting from registry %s", style.Symbol(moduleURI)) } @@ -113,7 +120,11 @@ func (c *buildpackDownloader) Download(ctx context.Context, moduleURI string, op return nil, nil, errors.Wrapf(err, "locating in registry: %s", style.Symbol(moduleURI)) } - mainBP, depBPs, err = extractPackaged(ctx, kind, address, c.imageFetcher, image.FetchOptions{Daemon: opts.Daemon, PullPolicy: opts.PullPolicy}) + mainBP, depBPs, err = extractPackaged(ctx, kind, address, c.imageFetcher, image.FetchOptions{ + Daemon: opts.Daemon, + PullPolicy: opts.PullPolicy, + Platform: opts.Platform, + }) if err != nil { return nil, nil, errors.Wrapf(err, "extracting from registry %s", style.Symbol(moduleURI)) } diff --git a/pkg/buildpack/downloader_test.go b/pkg/buildpack/downloader_test.go index 16c30b6e1..2aefdb571 100644 --- a/pkg/buildpack/downloader_test.go +++ b/pkg/buildpack/downloader_test.go @@ -127,8 +127,12 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { downloadOptions = buildpack.DownloadOptions{ImageOS: "linux"} ) - shouldFetchPackageImageWith := func(demon bool, pull image.PullPolicy) { - mockImageFetcher.EXPECT().Fetch(gomock.Any(), packageImage.Name(), image.FetchOptions{Daemon: demon, PullPolicy: pull}).Return(packageImage, nil) + shouldFetchPackageImageWith := func(demon bool, pull image.PullPolicy, platform string) { + mockImageFetcher.EXPECT().Fetch(gomock.Any(), packageImage.Name(), image.FetchOptions{ + Daemon: demon, + PullPolicy: pull, + Platform: platform, + }).Return(packageImage, nil) } when("package image lives in cnb registry", func() { @@ -141,11 +145,12 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { downloadOptions = buildpack.DownloadOptions{ RegistryName: "some-registry", ImageOS: "linux", + Platform: "linux/amd64", Daemon: true, PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "linux/amd64") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "urn:cnb:registry:example/foo@1.1.0", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -161,7 +166,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "example/foo@1.1.0", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -185,10 +190,11 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { Daemon: true, PullPolicy: image.PullAlways, ImageOS: "linux", + Platform: "linux/amd64", ImageName: "some/package:tag", } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "linux/amd64") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -204,7 +210,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(true, image.PullAlways) + shouldFetchPackageImageWith(true, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -220,7 +226,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(false, image.PullAlways) + shouldFetchPackageImageWith(false, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -234,7 +240,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { Daemon: false, PullPolicy: image.PullAlways, } - shouldFetchPackageImageWith(false, image.PullAlways) + shouldFetchPackageImageWith(false, image.PullAlways, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), packageImage.Name(), downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") @@ -250,7 +256,7 @@ func testBuildpackDownloader(t *testing.T, when spec.G, it spec.S) { PullPolicy: image.PullNever, } - shouldFetchPackageImageWith(false, image.PullNever) + shouldFetchPackageImageWith(false, image.PullNever, "") mainBP, _, err := buildpackDownloader.Download(context.TODO(), "", downloadOptions) h.AssertNil(t, err) h.AssertEq(t, mainBP.Descriptor().Info().ID, "example/foo") diff --git a/pkg/client/build.go b/pkg/client/build.go index c94e862df..a46cab5e8 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -1032,13 +1032,19 @@ func (c *Client) fetchBuildpack(ctx context.Context, bp string, relativeBaseDir Version: version, } default: - imageOS, err := builderImage.OS() + builderOS, err := builderImage.OS() if err != nil { - return nil, nil, errors.Wrapf(err, "getting OS from %s", style.Symbol(builderImage.Name())) + return nil, nil, errors.Wrapf(err, "getting builder OS") + } + + builderArch, err := builderImage.Architecture() + if err != nil { + return nil, nil, errors.Wrapf(err, "getting builder architecture") } downloadOptions := buildpack.DownloadOptions{ RegistryName: registry, - ImageOS: imageOS, + ImageOS: builderOS, + Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), RelativeBaseDir: relativeBaseDir, Daemon: !publish, PullPolicy: pullPolicy, @@ -1076,6 +1082,7 @@ func (c *Client) fetchBuildpackDependencies(ctx context.Context, bp string, pack mainBP, deps, err := c.buildpackDownloader.Download(ctx, dep.URI, buildpack.DownloadOptions{ RegistryName: downloadOptions.RegistryName, ImageOS: downloadOptions.ImageOS, + Platform: downloadOptions.Platform, Daemon: downloadOptions.Daemon, PullPolicy: downloadOptions.PullPolicy, RelativeBaseDir: filepath.Join(bp, packageCfg.Buildpack.URI), diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index 2f43619c5..ed5693f7c 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -1240,6 +1240,8 @@ api = "0.2" Version: "child.buildpack.version", }, }) + args := fakeImageFetcher.FetchCalls[fakePackage.Name()] + h.AssertEq(t, args.Platform, "linux/amd64") }) it("fails when no metadata label on package", func() { diff --git a/pkg/client/create_builder.go b/pkg/client/create_builder.go index 28cd2fa7c..dc03d73d6 100644 --- a/pkg/client/create_builder.go +++ b/pkg/client/create_builder.go @@ -256,14 +256,20 @@ func (c *Client) addExtensionsToBuilder(ctx context.Context, opts CreateBuilderO func (c *Client) addConfig(ctx context.Context, kind string, config pubbldr.ModuleConfig, opts CreateBuilderOptions, bldr *builder.Builder) error { c.logger.Debugf("Looking up %s %s", kind, style.Symbol(config.DisplayString())) - imageOS, err := bldr.Image().OS() + builderOS, err := bldr.Image().OS() if err != nil { - return errors.Wrapf(err, "getting OS from %s", style.Symbol(bldr.Image().Name())) + return errors.Wrapf(err, "getting builder OS") } + builderArch, err := bldr.Image().Architecture() + if err != nil { + return errors.Wrapf(err, "getting builder architecture") + } + mainBP, depBPs, err := c.buildpackDownloader.Download(ctx, config.URI, buildpack.DownloadOptions{ Daemon: !opts.Publish, ImageName: config.ImageName, - ImageOS: imageOS, + ImageOS: builderOS, + Platform: fmt.Sprintf("%s/%s", builderOS, builderArch), ModuleKind: kind, PullPolicy: opts.PullPolicy, RegistryName: opts.Registry, diff --git a/pkg/client/create_builder_test.go b/pkg/client/create_builder_test.go index 3b4cfc41e..ea0c95a93 100644 --- a/pkg/client/create_builder_test.go +++ b/pkg/client/create_builder_test.go @@ -843,12 +843,22 @@ func testCreateBuilder(t *testing.T, when spec.G, it spec.S) { buildpackBlob := blob.NewBlob(filepath.Join("testdata", "buildpack-api-0.4")) bp, err := buildpack.FromBuildpackRootBlob(buildpackBlob, archive.DefaultTarWriterFactory()) h.AssertNil(t, err) - mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/bp-one-with-api-4.tgz", gomock.Any()).Return(bp, bpDependencies, nil) + mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/bp-one-with-api-4.tgz", gomock.Any()).DoAndReturn( + func(ctx context.Context, buildpackURI string, opts buildpack.DownloadOptions) (buildpack.BuildModule, []buildpack.BuildModule, error) { + // test options + h.AssertEq(t, opts.Platform, "linux/amd64") + return bp, bpDependencies, nil + }) extensionBlob := blob.NewBlob(filepath.Join("testdata", "extension-api-0.9")) extension, err := buildpack.FromExtensionRootBlob(extensionBlob, archive.DefaultTarWriterFactory()) h.AssertNil(t, err) - mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/ext-one-with-api-9.tgz", gomock.Any()).Return(extension, nil, nil) + mockBuildpackDownloader.EXPECT().Download(gomock.Any(), "https://example.fake/ext-one-with-api-9.tgz", gomock.Any()).DoAndReturn( + func(ctx context.Context, buildpackURI string, opts buildpack.DownloadOptions) (buildpack.BuildModule, []buildpack.BuildModule, error) { + // test options + h.AssertEq(t, opts.Platform, "linux/amd64") + return extension, nil, nil + }) successfullyCreateDeterministicBuilder() From 43f6ec88f6d3c971ae544f1c3a9a92f60c1fafdf Mon Sep 17 00:00:00 2001 From: WYGIN Date: Wed, 25 Oct 2023 15:35:28 +0000 Subject: [PATCH 10/17] 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 9cd6117a2..f0195f9e4 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 7c4e8d9a9..fa7dd6e9d 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 2745e9e97..8f0e18350 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 a98cb4352..c3c1ccb90 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 f5f968439..4ddf115b3 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 2d1a67164..2d9610c3d 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 11/17] 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 f0195f9e4..008984bff 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 8f0e18350..37124533d 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 12/17] 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 008984bff..6ebd688d5 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 93a1bf851..6c5512f89 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 7868691a3..3eeb4b643 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) From 97ed7b2460086eab95e92c487f9bfba3d85d9441 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 18 Oct 2023 22:50:55 +0000 Subject: [PATCH 13/17] build(deps): bump github.com/buildpacks/lifecycle from 0.17.1 to 0.17.2 Bumps [github.com/buildpacks/lifecycle](https://github.com/buildpacks/lifecycle) from 0.17.1 to 0.17.2. - [Release notes](https://github.com/buildpacks/lifecycle/releases) - [Changelog](https://github.com/buildpacks/lifecycle/blob/main/RELEASE.md) - [Commits](https://github.com/buildpacks/lifecycle/compare/v0.17.1...v0.17.2) --- updated-dependencies: - dependency-name: github.com/buildpacks/lifecycle dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Signed-off-by: Juan Bustamante --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index d4a8ecda6..833398d0e 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/Microsoft/go-winio v0.6.1 github.com/apex/log v1.9.0 github.com/buildpacks/imgutil v0.0.0-20230626185301-726f02e4225c - github.com/buildpacks/lifecycle v0.17.1 + github.com/buildpacks/lifecycle v0.17.2 github.com/docker/cli v24.0.6+incompatible github.com/docker/docker v24.0.6+incompatible github.com/docker/go-connections v0.4.0 diff --git a/go.sum b/go.sum index 14aae3e62..9c312db20 100644 --- a/go.sum +++ b/go.sum @@ -97,8 +97,8 @@ github.com/awslabs/amazon-ecr-credential-helper/ecr-login v0.0.0-20230522190001- github.com/aybabtme/rgbterm v0.0.0-20170906152045-cc83f3b3ce59/go.mod h1:q/89r3U2H7sSsE2t6Kca0lfwTK8JdoNGS/yzM/4iH5I= github.com/buildpacks/imgutil v0.0.0-20230626185301-726f02e4225c h1:HlRuSz+JGAzudNtNCfHIzXe0AEuHX6Vx8uZgmjvX02o= github.com/buildpacks/imgutil v0.0.0-20230626185301-726f02e4225c/go.mod h1:mBG5M3GJW5nknCEOOqtmMHyPYnSpw/5GEiciuYU/COw= -github.com/buildpacks/lifecycle v0.17.1 h1:sCNj83TH1YE8Z3+CKHoFx/HK+llCVF1RlQUbj3xdNBQ= -github.com/buildpacks/lifecycle v0.17.1/go.mod h1:WFzcNp1WG4bwgHuXtKxMg4tdU3AguL44ZlP3knANeVs= +github.com/buildpacks/lifecycle v0.17.2 h1:CfJYWHIC5v996idgjDamYHBTk+G+c1Qt7Yk80MlbWpw= +github.com/buildpacks/lifecycle v0.17.2/go.mod h1:h8MrqltqMM+HQnn2F2JOQaKWmeybZ54qvlNV3pAiAqw= github.com/bwesterb/go-ristretto v1.2.3/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= github.com/chrismellard/docker-credential-acr-env v0.0.0-20230304212654-82a0ddb27589 h1:krfRl01rzPzxSxyLyrChD+U+MzsBXbm0OwYYB67uF+4= github.com/chrismellard/docker-credential-acr-env v0.0.0-20230304212654-82a0ddb27589/go.mod h1:OuDyvmLnMCwa2ep4Jkm6nyA0ocJuZlGyk2gGseVzERM= From 737d3e4c0bcc6a18ef811e7a2e0f07e66440bb2d Mon Sep 17 00:00:00 2001 From: Juan Bustamante Date: Thu, 26 Oct 2023 16:52:01 -0500 Subject: [PATCH 14/17] Updating default lifecycle version to 0.17.2 Signed-off-by: Juan Bustamante --- acceptance/testdata/pack_fixtures/report_output.txt | 2 +- internal/builder/lifecycle.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/testdata/pack_fixtures/report_output.txt b/acceptance/testdata/pack_fixtures/report_output.txt index 58e3baa5d..beaae617b 100644 --- a/acceptance/testdata/pack_fixtures/report_output.txt +++ b/acceptance/testdata/pack_fixtures/report_output.txt @@ -2,7 +2,7 @@ Pack: Version: {{ .Version }} OS/Arch: {{ .OS }}/{{ .Arch }} -Default Lifecycle Version: 0.17.1 +Default Lifecycle Version: 0.17.2 Supported Platform APIs: 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12 diff --git a/internal/builder/lifecycle.go b/internal/builder/lifecycle.go index 318427902..6c3193d47 100644 --- a/internal/builder/lifecycle.go +++ b/internal/builder/lifecycle.go @@ -14,7 +14,7 @@ import ( // A snapshot of the latest tested lifecycle version values const ( - DefaultLifecycleVersion = "0.17.1" + DefaultLifecycleVersion = "0.17.2" DefaultBuildpackAPIVersion = "0.2" ) From fb7319ea88411be3ab4932d0dcde6c8db07a5d94 Mon Sep 17 00:00:00 2001 From: Ed Morley <501702+edmorley@users.noreply.github.com> Date: Fri, 27 Oct 2023 09:20:05 +0100 Subject: [PATCH 15/17] Group minor/patch version Go Dependabot updates into one PR Go minor/patch dependencies will now be grouped, using the new Dependabot grouping feature: https://github.blog/changelog/2023-08-17-grouped-version-updates-by-semantic-version-level-for-dependabot/ Major updates, as well as security updates will still be opened as separate PRs. I've not grouped GitHub Actions update PRs, since the volume is typically much lower for those. In addition, the schedule has been changed from daily to weekly. This reduces project maintenance toil (no more having to manually create combined update PRs), plus makes it less painful for contributors to subscribe to repository notifications (currently there is a lot of noise from Dependabot PRs being opened/auto-rebased etc). Signed-off-by: Ed Morley <501702+edmorley@users.noreply.github.com> --- .github/dependabot.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 393b765af..f1b8489ba 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,8 +4,13 @@ updates: - package-ecosystem: "gomod" directory: "/" schedule: - # Check for updates to GitHub Actions every weekday - interval: "daily" + interval: "weekly" + groups: + # Group all minor/patch go dependencies into a single PR. + go-dependencies: + update-types: + - "minor" + - "patch" labels: - "dependencies" - "go" @@ -15,8 +20,7 @@ updates: - package-ecosystem: "github-actions" directory: "/" schedule: - # Check for updates to GitHub Actions every weekday - interval: "daily" + interval: "weekly" labels: - "dependencies" - "github_actions" From 074586f43710a7fd4aebfaed1dd19329858282a0 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Fri, 27 Oct 2023 12:41:30 -0400 Subject: [PATCH 16/17] Add buildpacksio/pack:-base images to delivery Signed-off-by: Natalie Arellano --- .github/workflows/delivery-docker.yml | 26 +++++++++---------- .../workflows/delivery/docker/project.toml | 7 ----- Dockerfile | 6 ++--- 3 files changed, 16 insertions(+), 23 deletions(-) delete mode 100644 .github/workflows/delivery/docker/project.toml diff --git a/.github/workflows/delivery-docker.yml b/.github/workflows/delivery-docker.yml index 5db2c7e4b..10675f99d 100644 --- a/.github/workflows/delivery-docker.yml +++ b/.github/workflows/delivery-docker.yml @@ -16,12 +16,21 @@ on: default: false env: - BUILDER: "paketobuildpacks/builder-jammy-tiny" IMG_NAME: 'pack' USERNAME: 'buildpacksio' jobs: deliver-docker: + strategy: + matrix: + config: [tiny, base] + include: + - config: tiny + base_image: gcr.io/distroless/static + suffix: + - config: base + base_image: ubuntu:jammy + suffix: -base runs-on: ubuntu-latest steps: - name: Determine version @@ -42,16 +51,6 @@ jobs: uses: actions/checkout@v4 with: ref: v${{ steps.version.outputs.result }} - # This has to come after the first checkout, so it isn't clobbered - - name: Checkout delivery configuration - uses: actions/checkout@v4 - with: - path: ./head - - name: Setup Working Dir - shell: bash - run: | - rm project.toml || true - cp head/.github/workflows/delivery/docker/project.toml project.toml - name: Determine App Name run: 'echo "IMG_NAME=${{ env.USERNAME }}/${{ env.IMG_NAME }}" >> $GITHUB_ENV' - name: Login to Dockerhub @@ -65,12 +64,13 @@ jobs: - name: Buildx Build/Publish run: | docker buildx build . \ - --tag ${{ env.IMG_NAME }}:${{ steps.version.outputs.result }} \ + --tag ${{ env.IMG_NAME }}:${{ steps.version.outputs.result }}${{ matrix.suffix }} \ --platform linux/amd64,linux/arm64 \ --build-arg pack_version=${{ steps.version.outputs.result }} \ + --build-arg base_image=${{ matrix.base_image }} \ --provenance=false \ --push - name: Tag Image as Latest - if: ${{ github.event.release != '' || github.event.inputs.tag_latest }} + if: ${{ (github.event.release != '' || github.event.inputs.tag_latest) && matrix.config != 'base' }} run: | crane copy ${{ env.IMG_NAME }}:${{ steps.version.outputs.result }} ${{ env.IMG_NAME }}:latest diff --git a/.github/workflows/delivery/docker/project.toml b/.github/workflows/delivery/docker/project.toml deleted file mode 100644 index da9c5c982..000000000 --- a/.github/workflows/delivery/docker/project.toml +++ /dev/null @@ -1,7 +0,0 @@ -[project] -version = "1.0.2" -source-url = "https://github.com/buildpacks/pack" - -[[build.env]] -name = "BP_GO_TARGETS" -value = "./cmd/pack" diff --git a/Dockerfile b/Dockerfile index 70c75ed45..c81b2f0c0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,3 +1,5 @@ +ARG base_image=gcr.io/distroless/static + FROM golang:1.20 as builder ARG pack_version ENV PACK_VERSION=$pack_version @@ -5,8 +7,6 @@ WORKDIR /app COPY . . RUN make build -FROM scratch +FROM ${base_image} COPY --from=builder /app/out/pack /usr/local/bin/pack -COPY --from=builder /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/ -COPY --from=builder /tmp /tmp ENTRYPOINT [ "/usr/local/bin/pack" ] From 01fbd91d76a734dd1a5af22340d334b5500aaa11 Mon Sep 17 00:00:00 2001 From: Natalie Arellano Date: Mon, 30 Oct 2023 09:19:54 -0400 Subject: [PATCH 17/17] Add floating :base tag Signed-off-by: Natalie Arellano --- .github/workflows/delivery-docker.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/delivery-docker.yml b/.github/workflows/delivery-docker.yml index 10675f99d..c95619517 100644 --- a/.github/workflows/delivery-docker.yml +++ b/.github/workflows/delivery-docker.yml @@ -70,6 +70,10 @@ jobs: --build-arg base_image=${{ matrix.base_image }} \ --provenance=false \ --push + - name: Tag Image as Base + if: ${{ (github.event.release != '' || github.event.inputs.tag_latest) && matrix.config == 'base' }} + run: | + crane copy ${{ env.IMG_NAME }}:${{ steps.version.outputs.result }} ${{ env.IMG_NAME }}:base - name: Tag Image as Latest if: ${{ (github.event.release != '' || github.event.inputs.tag_latest) && matrix.config != 'base' }} run: |