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

Fixes/cleanups for SSH key/systemd unit local file embedding #441

Merged
merged 7 commits into from
Mar 14, 2023
Merged

Fixes/cleanups for SSH key/systemd unit local file embedding #441

merged 7 commits into from
Mar 14, 2023

Conversation

bgilbert
Copy link
Contributor

Address various small items. Follows up on #289.

Record the specific ssh_authorized_keys_local entry that produced a key,
rather than blaming the ssh_authorized_keys_local array as a whole.

Add a test for multiple local files.
The existing checks wouldn't notice unexpected report entries, since
"" is always a suffix of any string.  Just hardcode the full report
string.
Be consistent with the ordering in translateUnit().
Deduplicate code to validate FilesDir, check for path traversal, and read
the file.
There's no benefit to including them in the output sshAuthorizedKeys
array, and if we had more than one, the config would fail validation.
@bgilbert bgilbert added the skip-notes This PR does not need release notes label Mar 14, 2023
@bgilbert bgilbert requested a review from prestist March 14, 2023 10:52
@travier
Copy link
Member

travier commented Mar 14, 2023

Otherwise LGTM.

Copy link
Collaborator

@prestist prestist left a comment

Choose a reason for hiding this comment

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

Wow, nice fixup!
LGTM

@bgilbert bgilbert merged commit 37974a2 into coreos:main Mar 14, 2023
@bgilbert bgilbert deleted the local-embed branch March 14, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-notes This PR does not need release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants