Support docker build --secret flag (#4731)

* Support docker build --secret flag

* split out buildkit test and run only on GCP

* Update hack/schemas/main.go

Co-authored-by: Brian de Alwis <[email protected]>

* Update hack/schemas/main.go

Co-authored-by: Brian de Alwis <[email protected]>

* add back Destination field and add more tests

* buildkit not currently supported in gcb

* generate schemas

* move secret check into helper method

Co-authored-by: Brian de Alwis <[email protected]>

Author: nkubala

docs/content/en/schemas/v2beta8.json

@@ -923,6 +923,11 @@
           "x-intellij-html-description": "used to pass in --no-cache to docker build to prevent caching.",
           "default": "false"
         },
+        "secret": {
+          "$ref": "#/definitions/DockerSecret",
+          "description": "contains information about a local secret passed to `docker build`, along with optional destination information.",
+          "x-intellij-html-description": "contains information about a local secret passed to <code>docker build</code>, along with optional destination information."
+        },
         "target": {
           "type": "string",
           "description": "Dockerfile target name to build.",
@@ -935,7 +940,8 @@
         "buildArgs",
         "network",
         "cacheFrom",
-        "noCache"
+        "noCache",
+        "secret"
       ],
       "additionalProperties": false,
       "description": "describes an artifact built from a Dockerfile, usually using `docker build`.",
@@ -962,6 +968,36 @@
       "description": "contains information about the docker `config.json` to mount.",
       "x-intellij-html-description": "contains information about the docker <code>config.json</code> to mount."
     },
+    "DockerSecret": {
+      "required": [
+        "id"
+      ],
+      "properties": {
+        "dst": {
+          "type": "string",
+          "description": "path in the container to mount the secret.",
+          "x-intellij-html-description": "path in the container to mount the secret."
+        },
+        "id": {
+          "type": "string",
+          "description": "id of the secret.",
+          "x-intellij-html-description": "id of the secret."
+        },
+        "src": {
+          "type": "string",
+          "description": "path to the secret on the host machine.",
+          "x-intellij-html-description": "path to the secret on the host machine."
+        }
+      },
+      "preferredOrder": [
+        "id",
+        "src",
+        "dst"
+      ],
+      "additionalProperties": false,
+      "description": "contains information about a local secret passed to `docker build`, along with optional destination information.",
+      "x-intellij-html-description": "contains information about a local secret passed to <code>docker build</code>, along with optional destination information."
+    },
     "DockerfileDependency": {
       "properties": {
         "buildArgs": {

hack/schemas/main.go

@@ -264,8 +264,11 @@ func (g *schemaGenerator) newDefinition(name string, t ast.Expr, comment string,
 	}
 
 	if g.strict && name != "" {
+		if comment == "" {
+			panic(fmt.Sprintf("field %q needs comment (all public fields require comments)", name))
+		}
 		if !strings.HasPrefix(comment, name+" ") {
-			panic(fmt.Sprintf("comment should start with field name on field %s", name))
+			panic(fmt.Sprintf("comment %q should start with field name on field %s", comment, name))
 		}
 	}
 

integration/testdata/build/secret/Dockerfile

@@ -0,0 +1,5 @@
+# syntax=docker/dockerfile:1.0.0-experimental
+
+FROM alpine:3.10
+
+RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret

integration/testdata/build/secret/mysecret.txt

@@ -0,0 +1 @@
+secret

integration/testdata/build/secret/skaffold.yaml

@@ -0,0 +1,12 @@
+apiVersion: skaffold/v2beta8
+kind: Config
+build:
+  local:
+    useBuildkit: true
+    push: false
+  artifacts:
+  - image: secret
+    docker:
+      secret:
+        id: mysecret
+        src: mysecret.txt

pkg/skaffold/build/gcb/docker.go

@@ -17,6 +17,7 @@ limitations under the License.
 package gcb
 
 import (
+	"errors"
 	"fmt"
 
 	cloudbuild "google.golang.org/api/cloudbuild/v1"
@@ -62,6 +63,10 @@ func (b *Builder) cacheFromSteps(artifact *latest.DockerArtifact) []*cloudbuild.
 
 // dockerBuildArgs lists the arguments passed to `docker` to build a given image.
 func (b *Builder) dockerBuildArgs(artifact *latest.DockerArtifact, tag string) ([]string, error) {
+	// TODO(nkubala): remove when buildkit is supported in GCB (#4773)
+	if artifact.Secret != nil {
+		return nil, errors.New("docker build secrets not currently supported in GCB builds")
+	}
 	buildArgs, err := util.EvaluateEnvTemplateMap(artifact.BuildArgs)
 	if err != nil {
 		return nil, fmt.Errorf("unable to evaluate build args: %w", err)

pkg/skaffold/build/gcb/docker_test.go

@@ -27,49 +27,75 @@ import (
 )
 
 func TestDockerBuildSpec(t *testing.T) {
-	artifact := &latest.Artifact{
-		ArtifactType: latest.ArtifactType{
-			DockerArtifact: &latest.DockerArtifact{
-				DockerfilePath: "Dockerfile",
-				BuildArgs: map[string]*string{
-					"arg1": util.StringPtr("value1"),
-					"arg2": nil,
+	tests := []struct {
+		description string
+		artifact    *latest.Artifact
+		expected    cloudbuild.Build
+		shouldErr   bool
+	}{
+		{
+			description: "normal docker build",
+			artifact: &latest.Artifact{
+				ArtifactType: latest.ArtifactType{
+					DockerArtifact: &latest.DockerArtifact{
+						DockerfilePath: "Dockerfile",
+						BuildArgs: map[string]*string{
+							"arg1": util.StringPtr("value1"),
+							"arg2": nil,
+						},
+					},
 				},
 			},
-		},
-	}
-
-	builder := NewBuilder(&mockConfig{
-		gcb: latest.GoogleCloudBuild{
-			DockerImage: "docker/docker",
-			DiskSizeGb:  100,
-			MachineType: "n1-standard-1",
-			Timeout:     "10m",
-		},
-	})
-	desc, err := builder.buildSpec(artifact, "nginx", "bucket", "object")
-
-	expected := cloudbuild.Build{
-		LogsBucket: "bucket",
-		Source: &cloudbuild.Source{
-			StorageSource: &cloudbuild.StorageSource{
-				Bucket: "bucket",
-				Object: "object",
+			expected: cloudbuild.Build{
+				LogsBucket: "bucket",
+				Source: &cloudbuild.Source{
+					StorageSource: &cloudbuild.StorageSource{
+						Bucket: "bucket",
+						Object: "object",
+					},
+				},
+				Steps: []*cloudbuild.BuildStep{{
+					Name: "docker/docker",
+					Args: []string{"build", "--tag", "nginx", "-f", "Dockerfile", "--build-arg", "arg1=value1", "--build-arg", "arg2", "."},
+				}},
+				Images: []string{"nginx"},
+				Options: &cloudbuild.BuildOptions{
+					DiskSizeGb:  100,
+					MachineType: "n1-standard-1",
+				},
+				Timeout: "10m",
 			},
 		},
-		Steps: []*cloudbuild.BuildStep{{
-			Name: "docker/docker",
-			Args: []string{"build", "--tag", "nginx", "-f", "Dockerfile", "--build-arg", "arg1=value1", "--build-arg", "arg2", "."},
-		}},
-		Images: []string{"nginx"},
-		Options: &cloudbuild.BuildOptions{
-			DiskSizeGb:  100,
-			MachineType: "n1-standard-1",
+		{
+			description: "buildkit features not supported in GCB",
+			artifact: &latest.Artifact{
+				ArtifactType: latest.ArtifactType{
+					DockerArtifact: &latest.DockerArtifact{
+						DockerfilePath: "Dockerfile",
+						Secret: &latest.DockerSecret{
+							ID: "secret",
+						},
+					},
+				},
+			},
+			shouldErr: true,
 		},
-		Timeout: "10m",
 	}
 
-	testutil.CheckErrorAndDeepEqual(t, false, err, expected, desc)
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			builder := NewBuilder(&mockConfig{
+				gcb: latest.GoogleCloudBuild{
+					DockerImage: "docker/docker",
+					DiskSizeGb:  100,
+					MachineType: "n1-standard-1",
+					Timeout:     "10m",
+				},
+			})
+			desc, err := builder.buildSpec(test.artifact, "nginx", "bucket", "object")
+			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, desc)
+		})
+	}
 }
 
 func TestPullCacheFrom(t *testing.T) {

pkg/skaffold/docker/image.go

@@ -156,10 +156,21 @@ func (l *localDaemon) ConfigFile(ctx context.Context, image string) (*v1.ConfigF
 	return cfg, nil
 }
 
+func (l *localDaemon) CheckCompatible(a *latest.DockerArtifact) error {
+	if a.Secret != nil {
+		return fmt.Errorf("docker build secrets require BuildKit - set `useBuildkit: true` in your config, or run with `DOCKER_BUILDKIT=1`")
+	}
+	return nil
+}
+
 // Build performs a docker build and returns the imageID.
 func (l *localDaemon) Build(ctx context.Context, out io.Writer, workspace string, a *latest.DockerArtifact, ref string, mode config.RunMode) (string, error) {
 	logrus.Debugf("Running docker build: context: %s, dockerfile: %s", workspace, a.DockerfilePath)
 
+	if err := l.CheckCompatible(a); err != nil {
+		return "", err
+	}
+
 	buildArgs, err := EvalBuildArgs(mode, workspace, a)
 	if err != nil {
 		return "", fmt.Errorf("unable to evaluate build args: %w", err)
@@ -463,6 +474,17 @@ func ToCLIBuildArgs(a *latest.DockerArtifact, evaluatedArgs map[string]*string)
 		args = append(args, "--no-cache")
 	}
 
+	if a.Secret != nil {
+		secretString := fmt.Sprintf("id=%s", a.Secret.ID)
+		if a.Secret.Source != "" {
+			secretString += ",src=" + a.Secret.Source
+		}
+		if a.Secret.Destination != "" {
+			secretString += ",dst=" + a.Secret.Destination
+		}
+		args = append(args, "--secret", secretString)
+	}
+
 	return args, nil
 }
 

pkg/skaffold/docker/image_test.go

@@ -312,6 +312,47 @@ func TestGetBuildArgs(t *testing.T) {
 			},
 			want: []string{"--no-cache"},
 		},
+
+		{
+			description: "secret with no source",
+			artifact: &latest.DockerArtifact{
+				Secret: &latest.DockerSecret{
+					ID: "mysecret",
+				},
+			},
+			want: []string{"--secret", "id=mysecret"},
+		},
+		{
+			description: "secret with source",
+			artifact: &latest.DockerArtifact{
+				Secret: &latest.DockerSecret{
+					ID:     "mysecret",
+					Source: "foo.src",
+				},
+			},
+			want: []string{"--secret", "id=mysecret,src=foo.src"},
+		},
+		{
+			description: "secret with destination",
+			artifact: &latest.DockerArtifact{
+				Secret: &latest.DockerSecret{
+					ID:          "mysecret",
+					Destination: "foo.dst",
+				},
+			},
+			want: []string{"--secret", "id=mysecret,dst=foo.dst"},
+		},
+		{
+			description: "secret with source and destination",
+			artifact: &latest.DockerArtifact{
+				Secret: &latest.DockerSecret{
+					ID:          "mysecret",
+					Source:      "foo.src",
+					Destination: "foo.dst",
+				},
+			},
+			want: []string{"--secret", "id=mysecret,src=foo.src,dst=foo.dst"},
+		},
 		{
 			description: "all",
 			artifact: &latest.DockerArtifact{

pkg/skaffold/schema/latest/config.go

@@ -1030,6 +1030,23 @@ type DockerArtifact struct {
 
 	// NoCache used to pass in --no-cache to docker build to prevent caching.
 	NoCache bool `yaml:"noCache,omitempty"`
+
+	// Secret contains information about a local secret passed to `docker build`,
+	// along with optional destination information.
+	Secret *DockerSecret `yaml:"secret,omitempty"`
+}
+
+// DockerSecret contains information about a local secret passed to `docker build`,
+// along with optional destination information.
+type DockerSecret struct {
+	// ID is the id of the secret.
+	ID string `yaml:"id,omitempty" yamltags:"required"`
+
+	// Source is the path to the secret on the host machine.
+	Source string `yaml:"src,omitempty"`
+
+	// Destination is the path in the container to mount the secret.
+	Destination string `yaml:"dst,omitempty"`
 }
 
 // BazelArtifact describes an artifact built with [Bazel](https://bazel.build/).