Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pack builder create should accept builder env config #1926

Merged
merged 11 commits into from
Oct 31, 2023
110 changes: 109 additions & 1 deletion builder/config_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/BurntSushi/toml"
"github.com/pkg/errors"
Expand Down Expand Up @@ -70,7 +71,25 @@ type RunImageConfig struct {

// BuildConfig build image configuration
type BuildConfig struct {
Image string `toml:"image"`
Image string `toml:"image"`
Env []BuildConfigEnv `toml:"env"`
}

type Suffix string

const (
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"`
Suffix Suffix `toml:"suffix,omitempty"`
Delim string `toml:"delim,omitempty"`
}

// ReadConfig reads a builder configuration from the file path provided and returns the
Expand Down Expand Up @@ -162,3 +181,92 @@ 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 == "" {
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
}
107 changes: 107 additions & 0 deletions builder/config_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
})
})
}
56 changes: 55 additions & 1 deletion internal/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import (
lifecycleplatform "github.com/buildpacks/lifecycle/platform"
)

var buildConfigDir = cnbBuildConfigDir()

const (
packName = "Pack CLI"

Expand Down Expand Up @@ -67,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
Expand Down Expand Up @@ -146,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),
Expand Down Expand Up @@ -349,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
Expand Down Expand Up @@ -525,6 +534,18 @@ 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"))
WYGIN marked this conversation as resolved.
Show resolved Hide resolved
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"))
}
Expand Down Expand Up @@ -903,7 +924,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))
}
Expand Down Expand Up @@ -1102,6 +1123,31 @@ 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(cnbBuildConfigDir(), "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 {
Expand Down Expand Up @@ -1257,3 +1303,11 @@ func (e errModuleTar) Info() dist.ModuleInfo {
func (e errModuleTar) Path() string {
return ""
}

func cnbBuildConfigDir() string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this allows the pack user to specify the location of the build config directory within the builder by setting an environment variable in pack's environment. We might want to add some text to pack builder create --help so that this option is discoverable. Or maybe we could add it as a field in builder.toml ...or just leave it non-configurable. WDYT @WYGIN @jjbustamante?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am not sure about adding pack builder create --help, IMO the pack builder create --help is about giving better context to user about the list of commands available for the end-user to use, and i think CNB_BUILD_CONFIG_DIR in cli help might not required
so i added it to the docs where builder.toml fields and tables are specified

if env, ok := os.LookupEnv("CNB_BUILD_CONFIG_DIR"); ok {
return env
}

return "/cnb/build-config"
}
Loading
Loading