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

chore: update ssh-key format in tests to match real-world usage #522

Merged
merged 2 commits into from
Jun 28, 2023

Conversation

7flying
Copy link
Contributor

@7flying 7flying commented Jun 28, 2023

This change allows us to reproduce RHBZ#2218231 (and to catch more related bugs in the future)

and it also fixes that same BZ.

Our FSIM for creating users with an associated ssh key did
not take into account that ssh keys are strings that may
have spaces, e.g. 'ssh-ed25519 long-string user@example.com',
and used ' ' as the split character.

This sets ';' as the split character in the FSIM avoiding
installing unusable ssh keys.

@7flying 7flying changed the title chore: update ssh-key format in tests to match real-world use chore: update ssh-key format in tests to match real-world usage Jun 28, 2023
Copy link
Contributor

@nullr0ute nullr0ute left a comment

Choose a reason for hiding this comment

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

One minor nit is the general accepted for non real domains is example.com, I think email.com is a real domain so we may not want to use that.

@7flying
Copy link
Contributor Author

7flying commented Jun 28, 2023

One minor nit is the general accepted for non real domains is example.com, I think email.com is a real domain so we may not want to use that.

oh, I did not know that, will update this accordingly, thanks 👍

nullr0ute
nullr0ute previously approved these changes Jun 28, 2023
Copy link
Contributor

@nullr0ute nullr0ute left a comment

Choose a reason for hiding this comment

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

LGTM

7flying added 2 commits June 28, 2023 17:39
Signed-off-by: Irene Diez <idiez@redhat.com>
Our FSIM for creating users with an associated ssh key did
not take into account that ssh keys are strings that may
have spaces, e.g. 'ssh-ed25519 long-string user@example.com',
and used ' ' as the split character.

This sets ';' as the split character in the FSIM avoiding
installing unusable ssh keys.

Fixes rhbz#2218231.

Signed-off-by: Irene Diez <idiez@redhat.com>
@7flying
Copy link
Contributor Author

7flying commented Jun 28, 2023

@nullr0ute sorry about the dismissed review, I ended up fixing the BZ also in this PR

@7flying
Copy link
Contributor Author

7flying commented Jun 28, 2023

Linking #507 for awareness

Copy link
Contributor

@nullr0ute nullr0ute left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify mergify bot merged commit 0735bba into fdo-rs:main Jun 28, 2023
@7flying 7flying deleted the ssh-key-test branch June 28, 2023 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants