-
-
Notifications
You must be signed in to change notification settings - Fork 265
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
Using SSH_AUTH_SOCK (ssh agent forwarding) to pull upm private repos #256
Using SSH_AUTH_SOCK (ssh agent forwarding) to pull upm private repos #256
Conversation
89441fd
to
49a6983
Compare
49a6983
to
cac9b2d
Compare
d5ec3c9
to
f8ebc05
Compare
This looks like a great approach to me. The hardest part about merging this is actually figuring out whether this would be the default approach that we'll support from here on forward (or risk bc break in future versions) @davidmfinol @GabLeRoux @frostebite please let me know your thoughts on this. |
dist/Dockerfile
Outdated
@@ -17,4 +17,6 @@ ADD entrypoint.sh /entrypoint.sh | |||
RUN chmod +x /entrypoint.sh | |||
RUN ls | |||
|
|||
RUN apt-get update && apt-get install -y openssh-client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should add openssh-client
to the base image. We've been trying to work around it but use cases keep popping up that require this.
Does your use-case work without recommends (how we usually install packages in the base image)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@webbertakken is there any alternative? My understanding is this demonstrates a situation we simply need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I think we need this, but not as part of builder. This is why I'm suggesting to move it to the base image.
The base image installs everything with the flag --no-install-recommends
, to keep the image as small as possible.
The reason I'm asking is that it would be too much for me to check these kind of things for every PR by myself (as it requires reproducing exact use cases).
What made you chose this approach over mounting |
Codecov Report
@@ Coverage Diff @@
## main #256 +/- ##
==========================================
- Coverage 56.32% 56.21% -0.11%
==========================================
Files 23 23
Lines 767 772 +5
Branches 143 147 +4
==========================================
+ Hits 432 434 +2
- Misses 334 337 +3
Partials 1 1
|
db16958
to
fe464b5
Compare
… change needs to be done upstream. See game-ci/docker#117
Great work. Thank you very much! Note that this will start working only after 0.14 of the docker images has been released. |
@ivan-hernandez-scopely Does your solution works with private GitHub repositories? |
Note that this will start working only after 0.14 of the docker images has been released @uchar yes. Notice that you need to provide the ssh private key to the webfactory/ssh-agent as a github secret: And setup its ssh public key in: |
@webbertakken When does the new version going to release? |
@uchar I should have time in the weekend. In the meantime you can use the specific commit hash until the new version is there. |
But this commit hash will require the docker images 0.14 released. right? |
Oh yes, you're right. I was waiting for game-ci/docker#116 to be merged for that. It's currently pending a last change. |
@webbertakken Can you please release the new version, I really need this 🙏 |
Thanks for the heads-up. Indeed, the other PR doesn't seem to be moving as fast as we'd hoped. I just released a new version for both docker and builder. Please follow the progress of the docker images on our image versions page. Please allow up to 24 hours for all of them to be published. |
Changes
Usage
Checklist