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

ndeloof
ndeloof reviewed Mar 2, 2022
View reviewed changes
@ndeloof ndeloof mentioned this pull request Mar 3, 2022
Closed
@zaucker
Copy link

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
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
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
ndeloof
ndeloof reviewed Mar 4, 2022
View reviewed changes
glours added 4 commits March 4, 2022 16:53
@glours
Add ssh definition to the service build section
1b8314b 
Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@glours
simplify the syntaxt section of ssh property
2b73dad 
Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@glours
change ssh type to string and update ssh section with contributors fe…
fa563cd 
…ebacks

Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@glours
be more specific on the ssh format
b6e5db2 
Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@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
remove the empty alias to the default ssh agent in the build section
1d009e7 
Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@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 <guillaume.lours@docker.com>
ndeloof
ndeloof reviewed Mar 5, 2022
View reviewed changes
ndeloof
ndeloof reviewed Mar 8, 2022
View reviewed changes
ndeloof
ndeloof reviewed Mar 8, 2022
View reviewed changes
build.md Outdated
build:
context: .
ssh:
default # mount the default ssh agent
Copy link
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
change ssh property to a simple collection (list or dict)
8b21d37 
Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@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
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.

@glours
Copy link
Contributor Author

glours commented Mar 8, 2022

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

@glours
fix typos and improvement of the ssh section readability
006c0de 
Signed-off-by: Guillaume Lours <guillaume.lours@docker.com>
@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