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

Support docker build --secret flag #4731

Merged
merged 8 commits into from
Sep 21, 2020

Conversation

nkubala
Copy link
Contributor

@nkubala nkubala commented Aug 27, 2020

fixes #2273

this change adds support for the new(ish) docker build secret flag. this wasn't supported in skaffold through build args like we might have thought.

when configured in the skaffold.yaml, this looks something like

build:
  local:
    useBuildkit: true
  artifacts:
  - name: secret_artifact
    docker:
      secret:
        id: mysecret
        src: mysecret.txt
        dst: /secrets/my_secret # this is optional

@nkubala nkubala requested a review from a team as a code owner August 27, 2020 22:05
@nkubala nkubala requested a review from briandealwis August 27, 2020 22:05
@codecov
Copy link

codecov bot commented Aug 27, 2020

Codecov Report

Merging #4731 into master will decrease coverage by 0.08%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4731      +/-   ##
==========================================
- Coverage   71.85%   71.77%   -0.09%     
==========================================
  Files         347      347              
  Lines       11869    11945      +76     
==========================================
+ Hits         8528     8573      +45     
- Misses       2724     2741      +17     
- Partials      617      631      +14     
Impacted Files Coverage Δ
pkg/skaffold/schema/latest/config.go 100.00% <ø> (ø)
pkg/skaffold/docker/image.go 71.85% <66.66%> (-1.94%) ⬇️
pkg/skaffold/build/gcb/docker.go 87.09% <100.00%> (+7.78%) ⬆️
pkg/skaffold/build/tag/util.go 89.47% <0.00%> (-10.53%) ⬇️
pkg/skaffold/deploy/helm.go 75.07% <0.00%> (-2.08%) ⬇️
pkg/skaffold/runner/new.go 64.05% <0.00%> (-1.92%) ⬇️
pkg/skaffold/deploy/kustomize.go 76.19% <0.00%> (-0.24%) ⬇️
pkg/skaffold/kubectl/cli.go 100.00% <0.00%> (ø)
pkg/skaffold/deploy/kubectl.go 66.00% <0.00%> (+0.26%) ⬆️
... and 2 more

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 b938b5d...007c9fc. Read the comment docs.

@nkubala nkubala force-pushed the docker-secret branch 2 times, most recently from 6050b80 to 3b435ae Compare August 28, 2020 17:45
@nkubala
Copy link
Contributor Author

nkubala commented Aug 31, 2020

this PR adds an integration test for buildkit builds, but it fails in travis because the daemon doesn't support buildkit. i'm going to leave the tests in but comment them out - opened #4746 to track

Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

Is buildkit supported on GCB? If not, we should error if it's defined.

@pull-request-size pull-request-size bot added size/L and removed size/M labels Sep 11, 2020
@nkubala
Copy link
Contributor Author

nkubala commented Sep 11, 2020

Is buildkit supported on GCB? If not, we should error if it's defined.

in skaffold it isn't, but there's no reason why it couldn't be. I was able to hack the build spec to include DOCKER_BUILDKIT=1 in the env of the build step and things seem to work. I'll make things error out now but i'll open an issue to track adding support in GCB since it wouldn't be difficult.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@briandealwis
Copy link
Member

@googlebot I consent

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

Copy link
Member

@briandealwis briandealwis left a comment

Choose a reason for hiding this comment

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

One optional suggestion.

@tejal29 tejal29 merged commit 9ba0e85 into GoogleContainerTools:master Sep 21, 2020
@nkubala nkubala deleted the docker-secret branch September 21, 2020 22:27
h-michael added a commit to h-michael/skaffold that referenced this pull request Nov 15, 2020
h-michael added a commit to h-michael/skaffold that referenced this pull request Nov 15, 2020
h-michael added a commit to h-michael/skaffold that referenced this pull request Nov 15, 2020
h-michael added a commit to h-michael/skaffold that referenced this pull request Nov 15, 2020
h-michael added a commit to h-michael/skaffold that referenced this pull request Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove
4 participants