ssh-agent icon indicating copy to clipboard operation
ssh-agent copied to clipboard

known_hosts bloats up on self-hosted runners

Open AllDmeat opened this issue 3 years ago • 4 comments

In index.js you are always executing fs.appendFileSync. We are using self-hosted runners and such behaviour lead us to known_hosts reaching almost 4k of same lines and breaking some git clone-related steps. Manually removing this file helped us, but I think it should be done automatically in your post-step action.

AllDmeat avatar Jan 17 '22 05:01 AllDmeat

Oh yes!

Removing in the post-step would be one option. But do self-hosted runners share state for multiple jobs? Might it happen that one job removes the line while another one is still running?

If so, maybe it would make more sense to add the line only if it does not already exist?

mpdude avatar Jan 17 '22 07:01 mpdude

maybe it would make more sense to add the line only if it does not already exist?

Maybe yes

do self-hosted runners share state for multiple jobs?

AFAIK — no. But better check out documentation.

AllDmeat avatar Jan 17 '22 11:01 AllDmeat

If so, maybe it would make more sense to add the line only if it does not already exist?

This would make sense.

Is adding the fingerprints to known_hosts really needed? Alternatively, how about adding a switch e.g., update-known-hosts: bool that would control if the known_hosts will be altered? I can imagine that one can pass parameters to git/ssh to ignore the known_hosts altogether or, especially for self-hosting, you can save the fingerprints beforehand.

cojocar avatar Feb 10 '22 12:02 cojocar

Adding the known_hosts entry was necessary a while ago before GitHub's worker images came with the keys pre-installed. Nowadays, probably we don't need it for GitHub environments at all. For self-hosted runners, one can pre-install the keys, also for other servers that might possbily be needed. I'd be glad not having to maintain and guarantee for the correct keys in this action.

So, do you see a nice way how we could deprecate and eventually remove the feature of adding the keys?

mpdude avatar May 23 '22 20:05 mpdude

how about adding a switch e.g., update-known-hosts: bool that would control if the known_hosts will be altered?

^ This would be my preferred action. So the default would be to add to known hosts for the current version (keep backwards compatibility) but allow an option to "skip" adding to the known hosts.

michaelgambold avatar Oct 21 '22 11:10 michaelgambold

Would something like this PR https://github.com/webfactory/ssh-agent/pull/142 work?

michaelgambold avatar Oct 21 '22 11:10 michaelgambold

That does not really point out to users that they might need to review/change something on their side, and opens up the new question of how to get rid of that switch in the future.

We might start printing a deprecation warning (if the GHA system permits us to do so?), stating that future versions will no longer add the keys. Then, on the next minor release (we're still 0.x!) drop the keys.

WDYT?

mpdude avatar Oct 24 '22 12:10 mpdude

We might start printing a deprecation warning (if the GHA system permits us to do so?), stating that future versions will no longer add the keys. Then, on the next minor release (we're still 0.x!) drop the keys.

Sounds fine to me.

michaelgambold avatar Nov 28 '22 00:11 michaelgambold