From 9d7d935d9c459ad56fae82f3e8a9e41c6e375f98 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 28 Feb 2020 08:54:18 +0100 Subject: [PATCH 1/4] Remove duplication Signed-off-by: David Gageot --- pkg/skaffold/build/misc/env.go | 7 +------ pkg/skaffold/deploy/helm.go | 24 +++++++----------------- pkg/skaffold/docker/image.go | 8 ++------ pkg/skaffold/util/env_template.go | 10 ++++++++++ pkg/skaffold/util/env_template_test.go | 3 +++ 5 files changed, 23 insertions(+), 29 deletions(-) diff --git a/pkg/skaffold/build/misc/env.go b/pkg/skaffold/build/misc/env.go index b0568d5e09a..2f9358cd160 100644 --- a/pkg/skaffold/build/misc/env.go +++ b/pkg/skaffold/build/misc/env.go @@ -38,12 +38,7 @@ func EvaluateEnv(env []string) ([]string, error) { k := kvp[0] v := kvp[1] - tmpl, err := util.ParseEnvTemplate(v) - if err != nil { - return nil, errors.Wrapf(err, "unable to parse template for env variable: %s=%s", k, v) - } - - value, err := util.ExecuteEnvTemplate(tmpl, nil) + value, err := util.ExpandEnvTemplate(v, nil) if err != nil { return nil, errors.Wrapf(err, "unable to get value for env variable: %s", k) } diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index ee1a7d2366d..0847f151892 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -108,7 +108,7 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build for _, r := range h.Releases { results, err := h.deployRelease(ctx, out, r, builds, valuesSet, hv) if err != nil { - releaseName, _ := expand(r.Name, nil) + releaseName, _ := util.ExpandEnvTemplate(r.Name, nil) event.DeployFailed(err) return NewDeployErrorResult(errors.Wrapf(err, "deploying %s", releaseName)) @@ -196,7 +196,7 @@ func (h *HelmDeployer) Cleanup(ctx context.Context, out io.Writer) error { } for _, r := range h.Releases { - releaseName, err := expand(r.Name, nil) + releaseName, err := util.ExpandEnvTemplate(r.Name, nil) if err != nil { return errors.Wrap(err, "cannot parse the release name template") } @@ -241,7 +241,7 @@ func (h *HelmDeployer) exec(ctx context.Context, out io.Writer, useSecrets bool, // deployRelease deploys a single release func (h *HelmDeployer) deployRelease(ctx context.Context, out io.Writer, r latest.HelmRelease, builds []build.Artifact, valuesSet map[string]bool, helmVersion semver.Version) ([]Artifact, error) { - releaseName, err := expand(r.Name, nil) + releaseName, err := util.ExpandEnvTemplate(r.Name, nil) if err != nil { return nil, errors.Wrap(err, "cannot parse the release name template") } @@ -456,7 +456,7 @@ func installArgs(r latest.HelmRelease, builds []build.Artifact, valuesSet map[st } sort.Strings(sortedKeys) for _, k := range sortedKeys { - v, err := expand(r.SetValueTemplates[k], envMap) + v, err := util.ExpandEnvTemplate(r.SetValueTemplates[k], envMap) if err != nil { return nil, err } @@ -471,7 +471,7 @@ func installArgs(r latest.HelmRelease, builds []build.Artifact, valuesSet map[st return nil, errors.Wrapf(err, "unable to expand %s", v) } - exp, err = expand(exp, envMap) + exp, err = util.ExpandEnvTemplate(exp, envMap) if err != nil { return nil, err } @@ -533,7 +533,7 @@ func (h *HelmDeployer) packageChart(ctx context.Context, r latest.HelmRelease) ( args := []string{"package", r.ChartPath, "--destination", tmpDir} if r.Packaged.Version != "" { - v, err := expand(r.Packaged.Version, nil) + v, err := util.ExpandEnvTemplate(r.Packaged.Version, nil) if err != nil { return "", errors.Wrap(err, `packaged.version template`) } @@ -541,7 +541,7 @@ func (h *HelmDeployer) packageChart(ctx context.Context, r latest.HelmRelease) ( } if r.Packaged.AppVersion != "" { - av, err := expand(r.Packaged.AppVersion, nil) + av, err := util.ExpandEnvTemplate(r.Packaged.AppVersion, nil) if err != nil { return "", errors.Wrap(err, `packaged.appVersion template`) } @@ -612,13 +612,3 @@ func pairParamsToArtifacts(builds []build.Artifact, params map[string]string) (m return paramToBuildResult, nil } - -// expand parses and executes template s with an optional environment map -func expand(s string, envMap map[string]string) (string, error) { - tmpl, err := util.ParseEnvTemplate(s) - if err != nil { - return "", errors.Wrap(err, "parsing template") - } - - return util.ExecuteEnvTemplate(tmpl, envMap) -} diff --git a/pkg/skaffold/docker/image.go b/pkg/skaffold/docker/image.go index ba4a2e89c70..0baa05be924 100644 --- a/pkg/skaffold/docker/image.go +++ b/pkg/skaffold/docker/image.go @@ -462,15 +462,11 @@ func EvaluateBuildArgs(args map[string]*string) (map[string]*string, error) { continue } - tmpl, err := util.ParseEnvTemplate(*v) - if err != nil { - return nil, errors.Wrapf(err, "unable to parse template for build arg: %s=%s", k, *v) - } - - value, err := util.ExecuteEnvTemplate(tmpl, nil) + value, err := util.ExpandEnvTemplate(*v, nil) if err != nil { return nil, errors.Wrapf(err, "unable to get value for build arg: %s", k) } + evaluated[k] = &value } diff --git a/pkg/skaffold/util/env_template.go b/pkg/skaffold/util/env_template.go index 0d8fd7e36ab..6297165dfec 100644 --- a/pkg/skaffold/util/env_template.go +++ b/pkg/skaffold/util/env_template.go @@ -31,6 +31,16 @@ var ( OSEnviron = os.Environ ) +// ExpandEnvTemplate parses and executes template s with an optional environment map +func ExpandEnvTemplate(s string, envMap map[string]string) (string, error) { + tmpl, err := ParseEnvTemplate(s) + if err != nil { + return "", errors.Wrapf(err, "unable to parse template: %q", s) + } + + return ExecuteEnvTemplate(tmpl, envMap) +} + // ParseEnvTemplate is a simple wrapper to parse an env template func ParseEnvTemplate(t string) (*template.Template, error) { return template.New("envTemplate").Parse(t) diff --git a/pkg/skaffold/util/env_template_test.go b/pkg/skaffold/util/env_template_test.go index 417ba9d7f82..84755f1155e 100644 --- a/pkg/skaffold/util/env_template_test.go +++ b/pkg/skaffold/util/env_template_test.go @@ -72,6 +72,9 @@ func TestEnvTemplate_ExecuteEnvTemplate(t *testing.T) { got, err := ExecuteEnvTemplate(testTemplate, test.customMap) t.CheckErrorAndDeepEqual(test.shouldErr, err, test.want, got) + + got, err = ExpandEnvTemplate(test.template, test.customMap) + t.CheckErrorAndDeepEqual(test.shouldErr, err, test.want, got) }) } } From 84254e34d5c85267e2e5b3a53f7a4230def5c3ca Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 28 Feb 2020 09:14:32 +0100 Subject: [PATCH 2/4] Support Go Templates in command Fixes #3753 Signed-off-by: David Gageot --- pkg/skaffold/build/custom/script.go | 10 ++++++++-- pkg/skaffold/build/custom/script_test.go | 20 ++++++++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/build/custom/script.go b/pkg/skaffold/build/custom/script.go index e9d114fd386..f57905ce476 100644 --- a/pkg/skaffold/build/custom/script.go +++ b/pkg/skaffold/build/custom/script.go @@ -53,13 +53,19 @@ func (b *Builder) runBuildScript(ctx context.Context, out io.Writer, a *latest.A func (b *Builder) retrieveCmd(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (*exec.Cmd, error) { artifact := a.CustomArtifact + // Expand command + command, err := util.ExpandEnvTemplate(artifact.BuildCommand, nil) + if err != nil { + return nil, errors.Wrap(err, "unable to parse build command %q") + } + var cmd *exec.Cmd // We evaluate the command with a shell so that it can contain // env variables. if runtime.GOOS == "windows" { - cmd = exec.CommandContext(ctx, "cmd.exe", "/C", artifact.BuildCommand) + cmd = exec.CommandContext(ctx, "cmd.exe", "/C", command) } else { - cmd = exec.CommandContext(ctx, "sh", "-c", artifact.BuildCommand) + cmd = exec.CommandContext(ctx, "sh", "-c", command) } cmd.Stdout = out cmd.Stderr = out diff --git a/pkg/skaffold/build/custom/script_test.go b/pkg/skaffold/build/custom/script_test.go index 5eef8f90bfd..9803dcaf40f 100644 --- a/pkg/skaffold/build/custom/script_test.go +++ b/pkg/skaffold/build/custom/script_test.go @@ -83,6 +83,7 @@ func TestRetrieveCmd(t *testing.T) { description string artifact *latest.Artifact tag string + env []string expected *exec.Cmd expectedOnWindows *exec.Cmd }{ @@ -99,7 +100,8 @@ func TestRetrieveCmd(t *testing.T) { tag: "image:tag", expected: expectedCmd("workspace", "sh", []string{"-c", "./build.sh"}, []string{"IMAGE=image:tag", "IMAGES=image:tag", "PUSH_IMAGE=false", "BUILD_CONTEXT=workspace"}), expectedOnWindows: expectedCmd("workspace", "cmd.exe", []string{"/C", "./build.sh"}, []string{"IMAGE=image:tag", "IMAGES=image:tag", "PUSH_IMAGE=false", "BUILD_CONTEXT=workspace"}), - }, { + }, + { description: "buildcommand with multiple args", artifact: &latest.Artifact{ ArtifactType: latest.ArtifactType{ @@ -112,10 +114,24 @@ func TestRetrieveCmd(t *testing.T) { expected: expectedCmd("", "sh", []string{"-c", "./build.sh --flag=$IMAGES --anotherflag"}, []string{"IMAGE=image:tag", "IMAGES=image:tag", "PUSH_IMAGE=false", "BUILD_CONTEXT="}), expectedOnWindows: expectedCmd("", "cmd.exe", []string{"/C", "./build.sh --flag=$IMAGES --anotherflag"}, []string{"IMAGE=image:tag", "IMAGES=image:tag", "PUSH_IMAGE=false", "BUILD_CONTEXT="}), }, + { + description: "buildcommand with go template", + artifact: &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + CustomArtifact: &latest.CustomArtifact{ + BuildCommand: "./build.sh --flag={{ .FLAG }}", + }, + }, + }, + tag: "image:tag", + env: []string{"FLAG=some-flag"}, + expected: expectedCmd("", "sh", []string{"-c", "./build.sh --flag=some-flag"}, []string{"IMAGE=image:tag", "IMAGES=image:tag", "PUSH_IMAGE=false", "BUILD_CONTEXT=", "FLAG=some-flag"}), + expectedOnWindows: expectedCmd("", "cmd.exe", []string{"/C", "./build.sh --flag=some-flag"}, []string{"IMAGE=image:tag", "IMAGES=image:tag", "PUSH_IMAGE=false", "BUILD_CONTEXT=", "FLAG=some-flag"}), + }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - t.Override(&util.OSEnviron, func() []string { return nil }) + t.Override(&util.OSEnviron, func() []string { return test.env }) t.Override(&buildContext, func(string) (string, error) { return test.artifact.Workspace, nil }) builder := NewArtifactBuilder(nil, nil, false, nil) From a0512bd52746f7a411cdc9f5ddc8d41b9c807b8c Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 28 Feb 2020 09:31:28 +0100 Subject: [PATCH 3/4] Update documentation Signed-off-by: David Gageot --- .../docs/pipeline-stages/builders/custom.md | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/docs/content/en/docs/pipeline-stages/builders/custom.md b/docs/content/en/docs/pipeline-stages/builders/custom.md index 1eaec4da1a5..25f08bddba8 100644 --- a/docs/content/en/docs/pipeline-stages/builders/custom.md +++ b/docs/content/en/docs/pipeline-stages/builders/custom.md @@ -5,14 +5,12 @@ weight: 40 featureId: build.custom --- -Custom build scripts allow skaffold users the flexibility to build artifacts with any builder they desire. -Users can write a custom build script which must abide by the following contract for skaffold to work as expected: +Custom build scripts allow Skaffold users the flexibility to build artifacts with any builder they desire. +Users can write a custom build script which must abide by the following contract for Skaffold to work as expected: -Currently, this only works with [local](#custom-build-script-locally) and -[cluster](#custom-build-script-in-cluster) build types. ### Contract between Skaffold and Custom Build Script -Skaffold will pass in the following environment variables to the custom build script: +Skaffold will pass in the following additional environment variables to the custom build script: | Environment Variable | Description | Expectation | | ------------- |-------------| -----| @@ -26,18 +24,20 @@ As described above, the custom build script is expected to: 1. Build and tag the `$IMAGE` image 2. Push the image if `$PUSH_IMAGE=true` -Once the build script has finished executing, skaffold will try to obtain the digest of the newly built image from a remote registry (if `$PUSH_IMAGE=true`) or the local daemon (if `$PUSH_IMAGE=false`). -If skaffold fails to obtain the digest, it will error out. +Once the build script has finished executing, Skaffold will try to obtain the digest of the newly built image from a remote registry (if `$PUSH_IMAGE=true`) or the local daemon (if `$PUSH_IMAGE=false`). +If Skaffold fails to obtain the digest, it will error out. ### Configuration -To use a custom build script, add a `custom` field to each corresponding artifact in the `build` section of the skaffold.yaml. +To use a custom build script, add a `custom` field to each corresponding artifact in the `build` section of the `skaffold.yaml`. Supported schema for `custom` includes: {{< schema root="CustomArtifact" >}} -`buildCommand` is *required* and points skaffold to the custom build script which will be executed to build the artifact. - +`buildCommand` is *required* and points Skaffold to the custom build script which will be executed to build the artifact. +The [Go templates](https://golang.org/pkg/text/template/) syntax can be used to inject environment variables into the build +command. For example: `buildCommand: ./build.sh --flag={{ .SOME_FLAG }}` will replace `{{ .SOME_FLAG }}` with the value of +the `SOME_FLAG` environment variable. #### Custom Build Script Locally @@ -66,11 +66,13 @@ Skaffold will pass in the following additional environment variables for cluster | $DOCKER_CONFIG_SECRET_NAME | The secret containing any required docker authentication for custom builds on cluster.| None. | | $TIMEOUT | The amount of time an on cluster build is allowed to run.| None. | - **Configuration** -To configure custom build script locally, in addition to adding a [`custom` field](#configuration) to each corresponding artifact in the `build` -add `cluster` to you `build` config. +To configure custom build script locally, in addition to adding a [`custom` field](#configuration) to each corresponding artifact in the `build`, add `cluster` to you `build` config. + +#### Custom Build Script on Google Cloud Build + +Such configuration is currently not supported. ### Dependencies for a Custom Artifact From 25a24240855dcbab8d61c21431b73c43d878c117 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 28 Feb 2020 20:56:21 +0100 Subject: [PATCH 4/4] Update docs/content/en/docs/pipeline-stages/builders/custom.md Co-Authored-By: Tejal Desai --- docs/content/en/docs/pipeline-stages/builders/custom.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/en/docs/pipeline-stages/builders/custom.md b/docs/content/en/docs/pipeline-stages/builders/custom.md index 25f08bddba8..a35235e6025 100644 --- a/docs/content/en/docs/pipeline-stages/builders/custom.md +++ b/docs/content/en/docs/pipeline-stages/builders/custom.md @@ -72,7 +72,7 @@ To configure custom build script locally, in addition to adding a [`custom` fiel #### Custom Build Script on Google Cloud Build -Such configuration is currently not supported. +This configuration is currently not supported. ### Dependencies for a Custom Artifact @@ -148,4 +148,4 @@ The following `build` section instructs Skaffold to build an image `gcr.io/k8s-s A sample `build.sh` file, which builds an image with bazel and docker: -{{% readfile file="samples/builders/build.sh" %}} \ No newline at end of file +{{% readfile file="samples/builders/build.sh" %}}