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

sshkeys: fix missing directory on empty set #318

Merged
merged 1 commit into from
Nov 29, 2019

Conversation

lucab
Copy link
Contributor

@lucab lucab commented Nov 28, 2019

This fixes a buggy interaction between the file writing logic and
the set of SSH keys being empty.
Afterburn tries to either write or remove a file, then sync the
parent directory. In both cases, the directory must exist in order
to be successfully sync'ed.

Closes: #317

@bgilbert
Copy link
Contributor

I'm okayish with this, but wouldn't it be cleaner to skip the directory sync if the directory doesn't exist? That would correctly sync in both cases where we do something, and skip the sync only in the case where it doesn't matter anyway.

@jlebon
Copy link
Member

jlebon commented Nov 28, 2019

I'm okayish with this, but wouldn't it be cleaner to skip the directory sync if the directory doesn't exist?

Or slight variation: always sync, but just handle ENOENT gracefully?

I like not creating the authorized_keys.d dir at all if we don't need to.

@lucab lucab force-pushed the ups/create_dir_empty_ssh_keys branch from 6a10fd9 to 79dd62a Compare November 29, 2019 09:47
This fixes a buggy interaction between the file writing logic and
the set of SSH keys being empty.
Afterburn tries to either write or remove a file, then sync the
parent directory. In both cases, the directory must exist in order
to be successfully sync'ed.
@lucab lucab force-pushed the ups/create_dir_empty_ssh_keys branch from 79dd62a to 093bcb7 Compare November 29, 2019 09:49
@lucab
Copy link
Contributor Author

lucab commented Nov 29, 2019

Thanks both for the feedback! I've switched this to sync_all only if it manages to open the directory, and to ignore the error on ENOENT.

@lucab lucab merged commit 60b566a into coreos:master Nov 29, 2019
@lucab lucab deleted the ups/create_dir_empty_ssh_keys branch November 29, 2019 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sshkeys: do not fail if no keys are deployed
3 participants