Skip to content

Conversation

@piterpunk
Copy link

@piterpunk piterpunk commented Oct 16, 2025

What does this PR do?

It changes the how ssh_auth.present and ssh_auth.absent handles when the keys to be added or removed comes from a source file. This fixes:

  1. The state reporting no changes even when keys are added or removed;
  2. The wrongly options handling when using source file;

Added in this patch is a fix to handle keys with @ and . in their encryption types and many tests to execution module ssh.py and state module ssh_auth.py:

  1. Tests all occurrences of the encryption types validation regexp in ssh_auth.py against all supported OpenSSH 8.7 key types;
  2. Tests if ssh.set_auth_key_from_file is correctly called from ssh_auth.present sending the options if specified;
  3. Tests if ssh.rm_auth_key_from_file is correctly called from ssh_auth.absent
  4. Tests if ssh.set_auth_key_from_file and ssh.rm_auth_key_from_file correctly handles multiple keys
  5. Tests if ssh.set_auth_key_from_file and ssh.rm_auth_key_from_file accept keys with @ and .
  6. Tests if ssh.set_auth_key_from_file and ssh.rm_auth_key_from_file parses multiple words comments
  7. Tests ssh._validate_keys against all encryption keys supported by OpenSSH 8.7, with options, without options, with simple comments, with multiple word comments and even without any comment.

I hope this set of tests are enough to prevent any regression and warranty that reading keys from source works flawlessly.

What issues does this PR fix or reference?

Fixes #68403
Fixes #61299
Fixes #60769
Fixes #57447
Fixes #40078
Tests #63434

Previous Behavior

  • Changes are not reported when reading keys from file
  • State defined options weren't respected when reading keys from file
  • Key types with @ and . weren't accepted

New Behavior

  • If the state changes something by adding or removing one or more keys, it reports as "changed". Individual changes are not reported.
  • State defined options overwrites those in the key when reading keys from file
  • Key types with @ and . are accepted

Merge requirements satisfied?

Commits signed with GPG?

No

@piterpunk
Copy link
Author

piterpunk commented Oct 16, 2025

While testing, I check #63434 and it seems to already being fixed somewhere in time.

@bdrx312
Copy link
Contributor

bdrx312 commented Oct 20, 2025

Thanks. Nice job.

@piterpunk piterpunk requested a review from bdrx312 October 20, 2025 14:06
Copy link
Contributor

@bdrx312 bdrx312 left a comment

Choose a reason for hiding this comment

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

Looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants