-
Notifications
You must be signed in to change notification settings - Fork 101
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
Smarter ssh authorized key search for system-reinstall-bootc #1146
Conversation
9f77b79
to
39e0527
Compare
This is prep for running authorizedkeyscommand to collect the user's authorized keys. Signed-off-by: ckyrouac <ckyrouac@redhat.com>
39e0527
to
9b036a8
Compare
|
||
// Safety: The UID should be valid because we got it from uzers | ||
#[allow(unsafe_code)] | ||
let user_uid = unsafe { Uid::from_raw(user.uid()) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This is OK for now but I think per previous discussion we actually don't need to change uid for this, we can just use cap-std)
9b036a8
to
ae33afb
Compare
Prior to this, the prompt to select users other that root would result in an error. Now, all ssh keys will be gathered into a single file and passed to bootc install to-existing-root --root-ssh-authorized-keys. Signed-off-by: ckyrouac <ckyrouac@redhat.com>
ae33afb
to
870da95
Compare
This is ready for a final review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's ship it - I had two totally minor things that can just be rolled into a subsequent PR.
I'll try to use the prefix "nit-for-followup:" to mean "next time you (or someone else) does a PR let's consider fixing this then"
@@ -24,6 +24,7 @@ rustix = { workspace = true } | |||
serde = { workspace = true, features = ["derive"] } | |||
serde_json = { workspace = true } | |||
serde_yaml = "0.9.22" | |||
tempfile = "3.10.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit-for-followup: Let's use { workspace = true }
@@ -10,7 +7,9 @@ const NO_SSH_PROMPT: &str = "None of the users on this system found have authori | |||
|
|||
fn prompt_single_user(user: &crate::users::UserKeys) -> Result<Vec<&crate::users::UserKeys>> { | |||
let prompt = format!( | |||
"Found only one user ({}) with {} SSH authorized keys. Would you like to import it and its keys to the system?", | |||
"Found only one user ({}) with {} SSH authorized keys.\n\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion-for-followup: This would be more readable with indoc!
Pushing this branch early for visibility. The first commit just parses the
sshd -T
output into a struct.