-
Notifications
You must be signed in to change notification settings - Fork 259
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
support GitHub Enterprise servers #163
base: master
Are you sure you want to change the base?
Conversation
Added support for GitHub Enterprise servers by loosening the restrictions on the hostname portion of the deploy key comment being "github.com" to any hostname and then using that hostname in the rest of the config. Adjusted the regex to match the end of the line since the comment portion is at the end. fixes #934
I ended up writing my own version of this action because in addition to this feature I've found this action doesn't clean up after itself. There's also no tests in this action. My action is here: https://github.com/marketplace/actions/ssh-agent-deploy-key |
@cardoe The action does (or should) cleanup after it's self? There's |
@jackbentley The cleanup only kills the ssh-agent. It doesn't do anything about the changes it made to the worker's home folder. My action also doesn't write the SSH private keys to the disk. It writes the SSH public keys to the disk. Which is identical to what this action does as well, and fails to clean up. Because to be able to have two different SSH deploy keys for two different GitHub repos, you must specify the public key via an on disk file that corresponds to the SSH private key from the ssh-agent to be used. Here's the link to how this action writes out that file: Line 69 in fd34b8d
While here's a copy of the cleanup: https://github.com/webfactory/ssh-agent/blob/fd34b8dee206fe74b288a5e61bc95fba2f1911eb/cleanup.js You can see those written keys are never cleaned up. |
To further follow up, I wrote my action after not getting traction in this repo around these issues. There's others getting burned by the broken cleanup as well. See #183 and the comments there. I'm sure mine has some deficiencies but I'm more than happy to address them and release an update. |
@cardoe Thanks for taking the time to write back. I see what you mean about only writing public keys - my mistake. And I agree, this action seems to be a bit dead. We will see what happens when node 16 support is removed 🤷🏻 |
Added support for GitHub Enterprise servers by loosening the restrictions on the hostname portion of the deploy key comment being "github.com" to any hostname and then using that hostname in the rest of the config. Adjusted the regex to match the end of the line since the comment portion is at the end. ref #128 fixes actions/checkout#934