-
Notifications
You must be signed in to change notification settings - Fork 525
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 ssh agent option #624
Add ssh agent option #624
Conversation
f994e21
to
52cc98b
Compare
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.
Changes make sense. Are you planning on adding another test for the new ssh agent setting, since it looks like the E2E tested added here is for SSH through a private key file.
TMPDIR: /usr/src/app/.tmp | ||
volumes: | ||
- .:/usr/src/app | ||
version: '3' |
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.
Are you still using this file for anything? Should it be removed?
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.
It was used for builds when we just had Ubuntu OS in the CI. But, it maybe makes sense keeping it around just in case we need to test a build locally for other OS than our own host OS.
The intention was adding the test for basic private key only because of the fixing for private keys which comes with the db-core upgrade. But it should be simple to add a test for ssh agent as well. I will do it later today after working hours. |
Will close #471
Working in progress, still need to add tests