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

[BUGFIX] Not working properly on self-hosted runners #27

Merged
merged 6 commits into from
May 18, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,20 @@ The `ssh-agent` will load all of the keys and try each one in order when establi
There's one **caveat**, though: SSH servers may abort the connection attempt after a number of mismatching keys have been presented. So if, for example, you have
six different keys loaded into the `ssh-agent`, but the server aborts after five unknown keys, the last key (which might be the right one) will never even be tried.

## Exported variables
The action exports `SSH_AUTH_SOCK` and `SSH_AGENT_PID` through the Github Actions core module.
The `$SSH_AUTH_SOCK` is used by several applications like git or rsync to connect to the SSH authentication agent.
The `$SSH_AGENT_PID` can be used to kill the SSH agent, e.g. when using on a self-hosted runner:
Copy link
Member

Choose a reason for hiding this comment

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

I'd shorten this to only say "... contains the process id of the agent running for this job". I think it should be the action's responsibility to kill the agent.

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 added a post action to kill the agent. Seems to work as expected.

```yaml
jobs:
job_1:
steps:
# ...
- name: Stop SSH agent
run: kill $SSH_AGENT_PID
```
The variables are only available inside the current job.

## Known issues and limitations

### Currently OS X and Linux only
Expand Down
1 change: 0 additions & 1 deletion action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ inputs:
required: true
ssh-auth-sock:
description: 'Where to place the SSH Agent auth socket'
default: /tmp/ssh-auth.sock
runs:
using: 'node12'
main: 'dist/index.js'
Expand Down
21 changes: 17 additions & 4 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ try {
const home = process.env['HOME'];
const homeSsh = home + '/.ssh';

const privateKey = core.getInput('ssh-private-key').trim();
const privateKey = core.getInput('ssh-private-key');
Copy link
Member

Choose a reason for hiding this comment

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

Could you rebase this PR against master please? Then this change should go away.


if (!privateKey) {
core.setFailed("The ssh-private-key argument is empty. Maybe the secret has not been configured, or you are using a wrong secret name in your workflow file.");
Expand All @@ -71,14 +71,27 @@ try {
}

console.log(`Adding github.com keys to ${homeSsh}/known_hosts`);
fs.mkdirSync(homeSsh, { recursive: true});
fs.mkdirSync(homeSsh, { recursive: true });
Copy link
Member

Choose a reason for hiding this comment

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

Same as before, should go away with a rebase.

fs.appendFileSync(`${homeSsh}/known_hosts`, '\ngit.luolix.top ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==\n');
fs.appendFileSync(`${homeSsh}/known_hosts`, '\ngit.luolix.top ssh-dss AAAAB3NzaC1kc3MAAACBANGFW2P9xlGU3zWrymJgI/lKo//ZW2WfVtmbsUZJ5uyKArtlQOT2+WRhcg4979aFxgKdcsqAYW3/LS1T2km3jYW/vr4Uzn+dXWODVk5VlUiZ1HFOHf6s6ITcZvjvdbp6ZbpM+DuJT7Bw+h5Fx8Qt8I16oCZYmAPJRtu46o9C2zk1AAAAFQC4gdFGcSbp5Gr0Wd5Ay/jtcldMewAAAIATTgn4sY4Nem/FQE+XJlyUQptPWMem5fwOcWtSXiTKaaN0lkk2p2snz+EJvAGXGq9dTSWHyLJSM2W6ZdQDqWJ1k+cL8CARAqL+UMwF84CR0m3hj+wtVGD/J4G5kW2DBAf4/bqzP4469lT+dF2FRQ2L9JKXrCWcnhMtJUvua8dvnwAAAIB6C4nQfAA7x8oLta6tT+oCk2WQcydNsyugE8vLrHlogoWEicla6cWPk7oXSspbzUcfkjN3Qa6e74PhRkc7JdSdAlFzU3m7LMkXo1MHgkqNX8glxWNVqBSc0YRdbFdTkL0C6gtpklilhvuHQCdbgB3LBAikcRkDp+FCVkUgPC/7Rw==\n');

console.log("Starting ssh-agent");
const authSock = core.getInput('ssh-auth-sock');
child_process.execFileSync('ssh-agent', ['-a', authSock]);
core.exportVariable('SSH_AUTH_SOCK', authSock);
let sshAgentOutput = ''
if (authSock && authSock.length > 0) {
sshAgentOutput = child_process.execFileSync('ssh-agent', ['-a', authSock]);
} else {
sshAgentOutput = child_process.execFileSync('ssh-agent')
Copy link
Member

Choose a reason for hiding this comment

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

This is a slight BC break since now not providing a value for the ssh-auth-sock input will bind it to a random port, instead of the fixed value we used previously. (Yes, I completely understand that this is part of the goal to be able to run multiple instances in parallel; just sayin').

Copy link
Member

Choose a reason for hiding this comment

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

Since we're still in the 0.x version ranges, IMO nothing we should worry about too much.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It binds to a random socket file, just for being precise. I don't think the value matters as long as it written to the $SSH_AUTH_SOCK variable.

}

// Extract auth socket path and agent pid and set them as job variables
const lines = sshAgentOutput.toString().split("\n")
for (const lineNumber in lines) {
const matches = /^(SSH_AUTH_SOCK|SSH_AGENT_PID)=(.*); export \1/.exec(lines[lineNumber])
if (matches && matches.length > 0) {
core.exportVariable(matches[1], matches[2])
}
}

console.log("Adding private key to agent");
privateKey.split(/(?=-----BEGIN)/).forEach(function(key) {
Expand Down
17 changes: 15 additions & 2 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,21 @@ try {

console.log("Starting ssh-agent");
const authSock = core.getInput('ssh-auth-sock');
child_process.execFileSync('ssh-agent', ['-a', authSock]);
core.exportVariable('SSH_AUTH_SOCK', authSock);
let sshAgentOutput = ''
if (authSock && authSock.length > 0) {
sshAgentOutput = child_process.execFileSync('ssh-agent', ['-a', authSock]);
} else {
sshAgentOutput = child_process.execFileSync('ssh-agent')
}

// Extract auth socket path and agent pid and set them as job variables
const lines = sshAgentOutput.toString().split("\n")
for (const lineNumber in lines) {
const matches = /^(SSH_AUTH_SOCK|SSH_AGENT_PID)=(.*); export \1/.exec(lines[lineNumber])
if (matches && matches.length > 0) {
core.exportVariable(matches[1], matches[2])
}
}

console.log("Adding private key to agent");
privateKey.split(/(?=-----BEGIN)/).forEach(function(key) {
Expand Down