Skip to content

Add support to pass --ssh flag to the build#123

Merged
tonistiigi merged 2 commits intodocker:masterfrom
sharesight:ssh-arg
Oct 26, 2020
Merged

Add support to pass --ssh flag to the build#123
tonistiigi merged 2 commits intodocker:masterfrom
sharesight:ssh-arg

Conversation

@jesserockz
Copy link
Copy Markdown
Contributor

@jesserockz jesserockz commented Sep 13, 2020

Closes #39

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 13, 2020

Codecov Report

Merging #123 into master will decrease coverage by 0.82%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
- Coverage   44.85%   44.03%   -0.83%     
==========================================
  Files           3        3              
  Lines         107      109       +2     
  Branches       17       17              
==========================================
  Hits           48       48              
- Misses         46       48       +2     
  Partials       13       13              
Impacted Files Coverage Δ
src/context.ts 27.27% <0.00%> (-0.86%) ⬇️

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 4b42944...5e17fae. Read the comment docs.

@tonistiigi
Copy link
Copy Markdown
Member

Not sure if something like ssh agent socket makes sense in GH actions context. Maybe this should just take direct ssh key value from secrets and create a temp file to expose via --ssh.

@jesserockz
Copy link
Copy Markdown
Contributor Author

That seems like it might be overcomplicating it and moving the action ssh key away from what the actual --ssh flag does.
I start up an ssh agent with a key from a secret and forward that through with this method.
The user would also be able to write the key to a file and expose that file themselves using this, if they choose to do it that way.

@tonistiigi
Copy link
Copy Markdown
Member

Sorry, I forgot this issue. I'm fine with this approach but looks like PR needs a rebase.

Copy link
Copy Markdown
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

@jesserockz
Copy link
Copy Markdown
Contributor Author

Thanks guys. I will take a look at rebasing again on Monday as I merged master on Friday and it broke our deployments.

Signed-off-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
@jesserockz
Copy link
Copy Markdown
Contributor Author

I have rebased locally, but just waiting on a PR merge in our repo before I can push to the branch so it shows up here.

Signed-off-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>
Copy link
Copy Markdown
Member

@crazy-max crazy-max left a comment

Choose a reason for hiding this comment

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

LGTM

@crazy-max crazy-max requested a review from tonistiigi October 26, 2020 19:41
@tonistiigi tonistiigi merged commit 2e36e43 into docker:master Oct 26, 2020
@jesserockz
Copy link
Copy Markdown
Contributor Author

Thanks for merging. Any chance of a new tag/release so I can update our workflows? @crazy-max @tonistiigi

@crazy-max
Copy link
Copy Markdown
Member

@jesserockz A new release will be created tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: provide way to add --ssh argument to docker build command

4 participants