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

Clarify usage for Docker build processes, especially with deployment keys #145

Merged
merged 4 commits into from
Jan 27, 2023

Conversation

j-riebe
Copy link
Contributor

@j-riebe j-riebe commented Oct 27, 2022

The current docs mention only docker/build-push-action in conjunction with deploy keys.

This might mislead users to believe, that this only applies to said Action. But the concept applies to all workflows that somehow use docker build with deploy keys.

This PR clarifies the relevant section.

The current docs mention only `docker/build-push-action` in conjunction with deploy keys.

This might mislead users to believe, that this only applies to said Action. But the concept applies to all workflows that somehow use `docker build` with deploy keys.

This PR clarifies the relevant section.
@mpdude
Copy link
Member

mpdude commented Nov 25, 2022

@j-riebe I did some minor changes and additions, would like to get your 👍.

Also, @danseeley, would you say this resolves #152?

@mpdude mpdude changed the title Clarify usage of docker build and deploy keys Clarify usage for Docker build processes, especially with deployment keys Nov 25, 2022
ssh: |
default=${{ env.SSH_AUTH_SOCK }}
- name: Docker build
# build-push-action | docker [compose] build | etc.
Copy link
Member

Choose a reason for hiding this comment

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

@j-riebe should we mention here that additional flags/parameters need to be given to docker to forward the SSH_AUTH_SOCK?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think a hint would be good (although it should be "clear" from the previous section).

Would you only mention it (and expect the user to be aware of the --ssh parameter) or provide examples?
Like:

  • build-push-action (copied from section above)
      - name: Build and push
        id: docker_build
        uses: docker/build-push-action@v2
        with:
          ssh: |
            default=${{ env.SSH_AUTH_SOCK }}
  • docker build --ssh default=SSH_AUTH_SOCK (docker docs)
  • docker compose build --ssh default=SSH_AUTH_SOCK (docker docs)

Maybe we just append the --ssh default=${{ env.SSH_AUTH_SOCK}} to the lines 132 & 133.
That should make it pretty obvious.

@danseeley
Copy link

@j-riebe I did some minor changes and additions, would like to get your 👍.

Also, @danseeley, would you say this resolves #152?

Looking for @danseely

Copy link
Contributor Author

@j-riebe j-riebe left a comment

Choose a reason for hiding this comment

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

@mpdude 👍

@danseely
Copy link

danseely commented Nov 28, 2022

I do see that this adds some additional helpful context around how to use this action with a raw docker build command. This definitely would have helped me get up & running a bit more quickly.

However, it does not cover the issue I found and outlined in #152. Namely, I found that in my particular situation, the ssh config wasn't being written into ~/.ssh, rather into /root/.ssh, which meant I had to alter the cp command. Which seems to be internal to the functionality of the action. You can get more details about my particular context in that issue.

Full disclosure, I'm currently migrating our CI stack from Gitlab over to Github, and don't have a ton of experience with Github Actions & Workflows, so I'm learning as I go here. Thus, I can't say if the issue I ran across is particular to my specific situation or not. I can't say if my issue should be addressed in this change, that's up to @mpdude et al.

@lightningspirit
Copy link

lightningspirit commented Dec 23, 2022

Very good, but I think the Docker documentation is also worth mentioning:
https://docs.docker.com/engine/reference/commandline/buildx_build/#ssh

Edit: I've lost a couple of hours just debugging multi deploy keys when I found that this change was what I needed

@mpdude mpdude merged commit 9fbc246 into webfactory:master Jan 27, 2023
@mpdude
Copy link
Member

mpdude commented Jan 27, 2023

Finally got a chance to finish this. Thanks to everyone involved!

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.

5 participants