Support '--ssh' option in 'docker build' #4660

h-michael · 2020-08-14T00:13:17Z

This is a feature addition related to issue #2273.

Related: #2273
Merge after: #4659

Description
This is PR for the addition of a feature that allows us to use the docker build --ssh option.

Docker document link

Should I make sure to check the useBuildkit option?

h-michael · 2020-09-21T08:12:06Z

I'll update this PR after merging #4731.

tejal29 · 2020-09-21T22:03:30Z

@h-michael Can you please update this PR? #4731 is now merged.

h-michael · 2020-09-28T01:58:15Z

@tejal29 Sorry for late replying. I'll work this week.

gemscoder · 2020-10-27T21:24:07Z

@h-michael Thank you for working on this. Do you happen to have approximate ETA? We would very much like this feature. We are happy to contribute if you need help.

MarlonGamez · 2020-11-09T21:20:34Z

Hey @h-michael! Sorry to bother, but I just want to check and see if you still have the free time to work on this. If not, I think someone on the skaffold team could take it over and push it through :)

gemscoder · 2020-11-10T00:43:50Z

@MarlonGamez do you foresee 'ssh' feature to be implemented similarly to 'secret'? I am wondering if it is possible to add 'args' sections (similar to 'jib') so that all docker build options can be passed in.

h-michael · 2020-11-10T15:55:51Z

@MarlonGamez I will make time this weekend. I'll let you know if I can't.

MarlonGamez · 2020-11-11T18:43:25Z

@gemscoder yes, I think it would be implemented similarly, with it's own section in the config. If you'd like to pass additional args to docker build, we have a buildArgs section that you can specify in your config for docker artifacts. https://skaffold.dev/docs/references/yaml/#build-artifacts-docker-buildArgs

Is that what you're looking for? :)

gemscoder · 2020-11-11T23:28:26Z

@marlon-gamez Thank you for your reply. Yes I know about buildargs. I actually meant to ask about other docker build options. I see you already have a lot of them addressed, i.e. secret, build-arg,no-cache and others but not all of them. For example, I would like to use '--squash' so I thought it might a good idea to have 'other-options' section for any additional options users might want to pass to docker. Not a big deal. Just asking...

codecov · 2020-11-15T11:11:34Z

Codecov Report

Merging #4660 (165e612) into master (d54c930) will increase coverage by 0.00%.
The diff coverage is 66.66%.

@@           Coverage Diff           @@
##           master    #4660   +/-   ##
=======================================
  Coverage   72.18%   72.18%           
=======================================
  Files         365      365           
  Lines       12859    12862    +3     
=======================================
+ Hits         9282     9285    +3     
  Misses       2886     2886           
  Partials      691      691

Impacted Files	Coverage Δ
pkg/skaffold/schema/latest/config.go	`100.00% <ø> (ø)`
pkg/skaffold/docker/image.go	`79.53% <50.00%> (+0.19%)`	⬆️
pkg/skaffold/build/gcb/docker.go	`87.87% <100.00%> (ø)`
pkg/skaffold/deploy/kpt/kpt.go	`75.26% <0.00%> (+0.08%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d54c930...165e612. Read the comment docs.

h-michael · 2020-11-15T11:23:48Z

pkg/skaffold/docker/image.go

+	if a.Secret != nil || a.SSH != "" {
+		return fmt.Errorf("docker build options, secrets and ssh, require BuildKit - set `useBuildkit: true` in your config, or run with `DOCKER_BUILDKIT=1`")


This code is ad hoc and not very good, but I think it's out of the scope of this PR to refactor these and the only options that BuildKit needs, for now, are "secret" and "ssh" so I'll leave that for this PR I'll leave it.

h-michael · 2020-11-15T11:34:03Z

pkg/skaffold/schema/latest/config.go

@@ -1125,6 +1125,9 @@ type DockerArtifact struct {
 	// Secret contains information about a local secret passed to `docker build`,
 	// along with optional destination information.
 	Secret *DockerSecret `yaml:"secret,omitempty"`
+
+	// SSH is passed as a argument to the --ssh option.
+	SSH string `yaml:"ssh,omitempty"`


I was wondering what to do here. "--ssh" option can have only one argument "default", "<id>[=<socket>]" or "<key>[,<key>]".
It would be nice if we could used a type representation like a union type, but the JSON schema(and Golang) doesn't support such a type, so I decided to just make it string type.

SGTM — I think we could use a yaml oneOf annotation, but this will do for now.

h-michael · 2020-11-15T12:38:14Z

Travis CI failed. It seems that a Travis CI issue.
https://travis-ci.com/github/GoogleContainerTools/skaffold/jobs/440128346

briandealwis

Thanks for the revised PR! I suggested two tiny tweaks.

briandealwis · 2020-11-16T16:49:32Z

pkg/skaffold/build/gcb/docker_test.go

@@ -104,7 +104,7 @@ func TestDockerBuildSpec(t *testing.T) {
 			},
 		},
 		{
-			description: "buildkit features not supported in GCB",
+			description: "buildkit feature of secret not supported in GCB",


Suggested change

description: "buildkit feature of secret not supported in GCB",

description: "buildkit `secret` option not supported in GCB",

briandealwis · 2020-11-16T16:49:42Z

pkg/skaffold/build/gcb/docker_test.go

@@ -117,6 +117,18 @@ func TestDockerBuildSpec(t *testing.T) {
 			},
 			shouldErr: true,
 		},
+		{
+			description: "buildkit feature of ssh not supported in GCB",


Suggested change

description: "buildkit feature of ssh not supported in GCB",

description: "buildkit `ssh` option not supported in GCB",

briandealwis · 2020-11-16T16:50:45Z

pkg/skaffold/schema/latest/config.go

@@ -1125,6 +1125,9 @@ type DockerArtifact struct {
 	// Secret contains information about a local secret passed to `docker build`,
 	// along with optional destination information.
 	Secret *DockerSecret `yaml:"secret,omitempty"`
+
+	// SSH is passed as a argument to the --ssh option.
+	SSH string `yaml:"ssh,omitempty"`


SGTM — I think we could use a yaml oneOf annotation, but this will do for now.

nkubala · 2020-11-16T23:06:46Z

For example, I would like to use '--squash' so I thought it might a good idea to have 'other-options' section for any additional options users might want to pass to docker. Not a big deal. Just asking...

@gemscoder the issue is that we pass args to docker generically using the --build-arg flag, so it wasn't possible to pass these through build args since they would end up in arg strings that look like:

docker build --build-arg --secret id=myscret

which fails (since that isn't a valid build arg).

we could maybe consider making these truly generic args that get passed verbatim to docker, but it seemed easier to just natively support these two since they're pretty special case. if you'd like to submit a PR or a quick design proposal for other-options contributions are of course welcome :)

h-michael · 2020-11-17T01:48:44Z

@briandealwis Thank you for your review.

SGTM — I think we could use a yaml oneOf annotation, but this will do for now.

Oh, I forgot specifications such as "oneof", "anyof and "allof".
But I thought it would be better to keep things the way they are.
For example, if we were to use oneof, would it be the following yaml?

- ssh: default

- ssh:
  - id: <id>
  - socket: <socket>

- ssh:
  - <key1>
  ...
  - <keyn>

The part where "default" is specified is a string type, but only "default" can be specified in the first place, so it seems a bit strange to me that it's a schema.
What do you think?

MarlonGamez · 2020-11-23T20:30:05Z

@h-michael i think that yaml you mocked up is pretty much what @briandealwis was thinking.

I agree with you that we should keep it how it is for now. Maybe we can revisit it in the future if users find that just having an open ended string field is a bad experience.

MarlonGamez

LGTM 👍🏼 Thanks so much for taking the time to work on this :)

AdemUstaReminiz · 2020-12-02T10:49:07Z

Hello guys, I would like to use this feature, but it seems to not be available in the current release (1.17).

Any ideas on when it will be available ?

h-michael · 2020-12-03T11:30:31Z

@ADBalici I don't know Skaffold release cycle.
But you can build yourself.
https://github.com/GoogleContainerTools/skaffold/blob/master/DEVELOPMENT.md#building-skaffold

ADBalici · 2020-12-03T12:37:10Z

wrong mention 👍

MarlonGamez · 2020-12-03T18:46:39Z

hey @AdemUstaReminiz, this feature should be present in v1.17.1, which just released recently :) To access it you will need to set both useDockerCLI: true and useBuildkit: true in build.local if your skaffold config

googlebot added the cla: yes label Aug 14, 2020

pull-request-size bot added the size/S label Aug 14, 2020

h-michael mentioned this pull request Sep 12, 2020

Enabling '--secret' option in 'docker build' #4659

Closed

nkubala self-assigned this Sep 17, 2020

h-michael force-pushed the docker-ssh branch 2 times, most recently from d52e6c2 to cbfbebd Compare November 15, 2020 11:02

pull-request-size bot added size/M and removed size/S labels Nov 15, 2020

h-michael force-pushed the docker-ssh branch 3 times, most recently from c900418 to a6876ba Compare November 15, 2020 11:17

h-michael commented Nov 15, 2020

View reviewed changes

h-michael marked this pull request as ready for review November 15, 2020 11:34

h-michael requested a review from a team as a code owner November 15, 2020 11:34

h-michael requested a review from MarlonGamez November 15, 2020 11:34

h-michael changed the title ~~Enabling '--ssh' option in 'docker build'~~ Support '--ssh' option in 'docker build' Nov 15, 2020

h-michael force-pushed the docker-ssh branch 2 times, most recently from fd4a878 to 55b9ce8 Compare November 15, 2020 12:13

briandealwis reviewed Nov 16, 2020

View reviewed changes

briandealwis added the kokoro:run runs the kokoro jobs on a PR label Nov 16, 2020

kokoro-team removed the kokoro:run runs the kokoro jobs on a PR label Nov 16, 2020

Support '--ssh' option in 'docker build'

165e612

h-michael force-pushed the docker-ssh branch from 55b9ce8 to 165e612 Compare November 17, 2020 01:31

MarlonGamez approved these changes Nov 23, 2020

View reviewed changes

MarlonGamez merged commit feb20b0 into GoogleContainerTools:master Nov 23, 2020

h-michael deleted the docker-ssh branch November 24, 2020 03:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support '--ssh' option in 'docker build' #4660

Support '--ssh' option in 'docker build' #4660

h-michael commented Aug 14, 2020 •

edited

Loading

h-michael commented Sep 21, 2020

tejal29 commented Sep 21, 2020

h-michael commented Sep 28, 2020

gemscoder commented Oct 27, 2020

MarlonGamez commented Nov 9, 2020

gemscoder commented Nov 10, 2020

h-michael commented Nov 10, 2020

MarlonGamez commented Nov 11, 2020

gemscoder commented Nov 11, 2020

codecov bot commented Nov 15, 2020 •

edited

Loading

h-michael Nov 15, 2020

h-michael Nov 15, 2020 •

edited

Loading

briandealwis Nov 16, 2020

h-michael commented Nov 15, 2020

briandealwis left a comment

briandealwis Nov 16, 2020

briandealwis Nov 16, 2020

briandealwis Nov 16, 2020

nkubala commented Nov 16, 2020

h-michael commented Nov 17, 2020

MarlonGamez commented Nov 23, 2020

MarlonGamez left a comment

AdemUstaReminiz commented Dec 2, 2020

h-michael commented Dec 3, 2020

ADBalici commented Dec 3, 2020

MarlonGamez commented Dec 3, 2020

		if a.Secret != nil \|\| a.SSH != "" {
		return fmt.Errorf("docker build options, secrets and ssh, require BuildKit - set `useBuildkit: true` in your config, or run with `DOCKER_BUILDKIT=1`")

	description: "buildkit feature of secret not supported in GCB",
	description: "buildkit `secret` option not supported in GCB",

	description: "buildkit feature of ssh not supported in GCB",
	description: "buildkit `ssh` option not supported in GCB",

Support '--ssh' option in 'docker build' #4660

Support '--ssh' option in 'docker build' #4660

Conversation

h-michael commented Aug 14, 2020 • edited Loading

h-michael commented Sep 21, 2020

tejal29 commented Sep 21, 2020

h-michael commented Sep 28, 2020

gemscoder commented Oct 27, 2020

MarlonGamez commented Nov 9, 2020

gemscoder commented Nov 10, 2020

h-michael commented Nov 10, 2020

MarlonGamez commented Nov 11, 2020

gemscoder commented Nov 11, 2020

codecov bot commented Nov 15, 2020 • edited Loading

Codecov Report

h-michael Nov 15, 2020

Choose a reason for hiding this comment

h-michael Nov 15, 2020 • edited Loading

Choose a reason for hiding this comment

briandealwis Nov 16, 2020

Choose a reason for hiding this comment

h-michael commented Nov 15, 2020

briandealwis left a comment

Choose a reason for hiding this comment

briandealwis Nov 16, 2020

Choose a reason for hiding this comment

briandealwis Nov 16, 2020

Choose a reason for hiding this comment

briandealwis Nov 16, 2020

Choose a reason for hiding this comment

nkubala commented Nov 16, 2020

h-michael commented Nov 17, 2020

MarlonGamez commented Nov 23, 2020

MarlonGamez left a comment

Choose a reason for hiding this comment

AdemUstaReminiz commented Dec 2, 2020

h-michael commented Dec 3, 2020

ADBalici commented Dec 3, 2020

MarlonGamez commented Dec 3, 2020

h-michael commented Aug 14, 2020 •

edited

Loading

codecov bot commented Nov 15, 2020 •

edited

Loading

h-michael Nov 15, 2020 •

edited

Loading