Support go Templates in Custom Builder commands (#3754)

* Remove duplication Signed-off-by: David Gageot <[email protected]> * Support Go Templates in command Fixes #3753 Signed-off-by: David Gageot <[email protected]> * Update documentation Signed-off-by: David Gageot <[email protected]> * Update docs/content/en/docs/pipeline-stages/builders/custom.md Co-Authored-By: Tejal Desai <[email protected]> Co-authored-by: Tejal Desai <[email protected]>
GoogleContainerTools · Feb 28, 2020 · c2877a6 · c2877a6
1 parent 64d8dcb
commit c2877a6
Show file tree

Hide file tree

Showing 8 changed files with 65 additions and 47 deletions.
diff --git 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
+
+This configuration is currently not supported.
 
 ### Dependencies for a Custom Artifact
 
@@ -146,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" %}}
+{{% readfile file="samples/builders/build.sh" %}}
diff --git 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
@@ -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)

diff --git 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
@@ -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,15 +533,15 @@ 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`)
 		}
 		args = append(args, "--version", v)
 	}
 
 	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
@@ -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
@@ -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
@@ -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)
 		})
 	}
 }