Skip to content

Add ssh to build#234

Merged
ndeloof merged 8 commits intocompose-spec:masterfrom
glours:add-ssh-to-build
Mar 9, 2022
Merged

Add ssh to build#234
ndeloof merged 8 commits intocompose-spec:masterfrom
glours:add-ssh-to-build

Conversation

@glours
Copy link
Copy Markdown
Contributor

@glours glours commented Mar 2, 2022

What this PR does / why we need it:
Introduce ssh attribute to the build section.
This allows the usage of SSH authentification when building an image (i.e., cloning a private repo...)

Which issue(s) this PR fixes:
This PR is part of the #233 proposal

@glours glours self-assigned this Mar 2, 2022
@glours glours requested review from EricHripko, ndeloof and ulyssessouza and removed request for EricHripko, ndeloof and ulyssessouza March 2, 2022 14:50
ndeloof
ndeloof approved these changes Mar 2, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ndeloof ndeloof left a comment

Choose a reason for hiding this comment

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

LGTM, with some minor notes

Comment thread build.md Outdated
Comment thread build.md Outdated
Comment thread build.md Outdated
ndeloof
ndeloof reviewed Mar 2, 2022
View reviewed changes
Comment thread build.md Outdated
@ndeloof ndeloof mentioned this pull request Mar 3, 2022
Closed
@zaucker
Copy link
Copy Markdown

zaucker commented Mar 4, 2022

Do I understand this correctly? This PR just adds the possibility to specify --ssh in the build section of the docker-compose file, but does not yet pass that to the build command upon running docker-compose build?
Just wondering if for my current project I can expect this to become functional soon or if I better don't count on it for the time being.

@glours
Copy link
Copy Markdown
Contributor Author

glours commented Mar 4, 2022

@zaucker adding the flag to the docker compose build command will be done when we'll implement this specification change in Compose

@ndeloof
Copy link
Copy Markdown
Collaborator

ndeloof commented Mar 4, 2022

implementation in docker/commose should not be a big deal, as we can rely on previous implementation by buildx bake as extension fields.

ndeloof
ndeloof reviewed Mar 4, 2022
View reviewed changes
Comment thread build.md
ndeloof
ndeloof reviewed Mar 4, 2022
View reviewed changes
Comment thread build.md Outdated
@glours glours force-pushed the add-ssh-to-build branch from 13d5547 to 4224b25 Compare March 4, 2022 15:55
@glours glours mentioned this pull request Mar 4, 2022
Merged
@glours glours force-pushed the add-ssh-to-build branch from 4224b25 to 1d009e7 Compare March 4, 2022 17:12
@glours
change json spec of ssh type
52c501e 
Signed-off-by: Guillaume Lours <[email protected]>
ndeloof
ndeloof reviewed Mar 5, 2022
View reviewed changes
Comment thread schema/compose-spec.json Outdated
ndeloof
ndeloof reviewed Mar 8, 2022
View reviewed changes
Comment thread build.md Outdated
ndeloof
ndeloof reviewed Mar 8, 2022
View reviewed changes
Comment thread build.md Outdated
build:
context: .
ssh:
default # mount the default ssh agent
Copy link
Copy Markdown
Collaborator

@ndeloof ndeloof Mar 8, 2022

Choose a reason for hiding this comment

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

Suggested change
default # mount the default ssh agent
- default # mount the default ssh agent

or

   [default]

@glours glours force-pushed the add-ssh-to-build branch from 743ccca to 8b21d37 Compare March 8, 2022 14:00
EricHripko
EricHripko approved these changes Mar 8, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@EricHripko EricHripko left a comment

Choose a reason for hiding this comment

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

Looks awesome ✅ Overjoyed to see SSH support coming to Compose 🙌 I've left a typo fix and some non-blocking comments inline. Feel free to land once the former is addressed.

Comment thread build.md Outdated
Comment thread build.md Outdated
Comment thread build.md Outdated
@glours
Copy link
Copy Markdown
Contributor Author

glours commented Mar 8, 2022

Thank you so much @EricHripko for your review, will address you comments before merging

@ndeloof ndeloof merged commit e13d6e4 into compose-spec:master Mar 9, 2022
@glours glours mentioned this pull request Mar 10, 2022
Closed
@jl-unitedlocating jl-unitedlocating mentioned this pull request Aug 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ndeloof ndeloof ndeloof approved these changes

@ulyssessouza ulyssessouza ulyssessouza approved these changes

@EricHripko EricHripko EricHripko approved these changes

+1 more reviewer

@levrik levrik levrik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@glours glours

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@glours @zaucker @ndeloof @ulyssessouza @EricHripko @levrik