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

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

Merged
merged 2 commits into from Oct 26, 2020

Conversation

jesserockz
Copy link
Contributor

@jesserockz jesserockz commented Sep 13, 2020

Closes #39

@codecov-commenter
Copy link

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
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
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
Member

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

Copy link
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
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
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
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

@tonistiigi tonistiigi merged commit 2e36e43 into docker:master Oct 26, 2020
@jesserockz
Copy link
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
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