[polsala]

Implementation Summary

• Utility: nightly-ssh-key-rotator
• Provider: groq
• Location: ansible-playbooks/nightly-nightly-ssh-key-rotator
• Files Created: 3
• Description: An Ansible playbook that generates fresh SSH key pairs and distributes the new public key to target hosts, safely rotating access credentials.

Rationale

• Automated proposal from the Groq generator delivering a fresh community utility.
• This utility was generated using the groq AI provider.

Why safe to merge

• Utility is isolated to ansible-playbooks/nightly-nightly-ssh-key-rotator.
• README + tests ship together (see folder contents).
• No secrets or credentials touched.
• All changes are additive and self-contained.

Test Plan

• Follow the instructions in the generated README at ansible-playbooks/nightly-nightly-ssh-key-rotator/README.md
• Run tests located in ansible-playbooks/nightly-nightly-ssh-key-rotator/tests/

Links

• Generated docs and examples committed alongside this change.

Mock Justification

• Not applicable; generator did not introduce new mocks.

[polsala]

🤖 Review by GROQ Agent

✅ What's solid

• Clear purpose & isolation – The utility lives in its own nightly-nightly-ssh-key-rotator folder, keeping the rest of the repo untouched.
• Idempotent key generation – Using creates: "{{ key_path }}" prevents unnecessary re‑generation of the RSA pair.
• Backup strategy – The copy task creates a timestamped backup of the existing authorized_keys, which is a good safety net.
• Readme completeness – The README explains the feature set, requirements, usage, variables, and how to run the bundled test.
• Minimal external dependencies – Only openssh-client is required on the control node, and the playbook ensures it is present on the targets.

🧪 Tests

• Current test approach

  • Executes the main playbook in --check mode via a command task and asserts a zero exit code.
  • Performs a simple cleanup of any pre‑existing test key.

• Actionable improvements

  • Validate side‑effects – After the --check run, assert that the expected files would be created (e.g., {{ key_path }} and {{ key_path }}.pub). Use the stat module on the control node to confirm the paths exist in a dry‑run safe way:

    - name: Verify key files would be created
      ansible.builtin.stat:
        path: "{{ key_path }}"
      register: key_stat
    - name: Assert key file presence (dry‑run)
      ansible.builtin.assert:
        that:
          - key_stat.stat.exists

  • Test real execution on localhost – Run the playbook without --check against localhost (or a Docker‑based test host) and then verify:
    • The private key has mode 0600.
    • The public key is correctly added to ~/.ssh/authorized_keys.
    • The backup file exists and contains the previous key.
  • Separate test files – Consider moving the “run playbook” step to an ansible-test integration test rather than nesting a command inside an Ansible playbook. This keeps the test suite pure Ansible and avoids recursion.
  • Parameterize inventory – The test currently hard‑codes localhost,. Adding a variable (e.g., test_inventory) makes the test reusable for CI containers.

🔒 Security

• Private key storage location – Storing the private key under /etc/ssh/{{ key_name }} makes it readable by any user with sudo rights. If the control node is shared, consider:
  • Using a more restrictive directory such as /root/.ssh/rotate_keys/ (or a path supplied via an extra‑var).
  • Enforcing file permissions explicitly:

    - name: Ensure private key permissions are strict
      ansible.builtin.file:
        path: "{{ key_path }}"
        mode: '0600'
        owner: root
        group: root

• Passphrase handling – The key is generated with an empty passphrase (-N ""). For production environments, encourage a passphrase and store it in an Ansible Vault variable, then feed it to ssh-keygen:

  - name: Generate new SSH key pair with passphrase
    ansible.builtin.command: >
      ssh-keygen -t rsa -b 4096 -f {{ key_path }} -N "{{ ssh_key_passphrase }}"
    args:
      creates: "{{ key_path }}"

• Authorized key source – The authorized_key task pulls the public key via lookup('file', ...). Ensure the lookup runs on the control node (default) and that the file is not inadvertently exposed to remote hosts. Adding delegate_to: localhost makes the intent explicit:

  - name: Deploy new public key to authorized_keys
    ansible.builtin.authorized_key:
      user: "{{ ansible_user }}"
      state: present
      key: "{{ lookup('file', key_path + '.pub') }}"
    delegate_to: localhost

• Backup naming – The backup filename uses ansible_date_time.iso8601_basic_short, which contains colons on some platforms and may be illegal in filenames. Prefer ansible_date_time.iso8601_basic or a custom format:

  dest: "{{ ansible_user_dir }}/.ssh/authorized_keys.bak_{{ ansible_date_time.date }}{{ ansible_date_time.time | replace(':', '') }}"

🧩 Docs/DX

• Variable documentation – The README lists key_name and key_path, but the playbook also uses key_path internally. Clarify that key_path can be overridden via extra‑vars if a non‑default location is desired.
• Example inventory – Adding a minimal inventory.ini example (e.g., a [targets] group with a localhost entry) would help newcomers run the utility locally.
• Prerequisite checklist – Mention that the control node must have write access to the chosen key_path directory and that the user executing the playbook needs sudo rights on the target hosts.
• Cleanup guidance – The “Cleanup” section notes manual pruning of backups. Providing a sample task to rotate out old backups (e.g., keep the last 7) would make the utility more production‑ready.
• Testing instructions – The README’s test command runs the test playbook against localhost,. It would be clearer to show the full command, including the inventory flag:

  ansible-playbook -i localhost, tests/test_rotate_ssh_keys.yml

• Formatting – The README uses a mixture of backticks and code fences. Consistently using triple backticks for command blocks improves readability.

🧱 Mocks/Fakes

• Current state – No explicit mocks are used; the test relies on a real ansible-playbook execution in check mode.
• Potential mock usage
  • Mock ssh-keygen – In CI environments where generating real keys may be undesirable, you can mock the ssh-keygen command with a simple shell script placed earlier in $PATH that creates dummy files. This speeds up the test and avoids leaving real keys on the runner.
  • Mock authorized_key – Use the ansible.builtin.debug module with when: false to simulate the module’s effect when you only need to verify control‑flow logic.
  • Ansible test harness – Consider leveraging ansible-test’s network-integration or unit modes, which provide built‑in mocking for modules like package and command. This would let you assert that the ssh-keygen command is called with the expected arguments without actually invoking it.

---

Overall, the contribution delivers a functional, well‑documented utility for rotating SSH keys. Tightening the security posture, expanding the test coverage, and polishing the documentation will make the playbook safer and easier to adopt in production pipelines.

[polsala]

🤖 Review by GEMINI Agent

✅ What's solid

• The playbook clearly defines its purpose: generating and distributing SSH keys from the control node.
• Idempotency is considered for key generation using creates: "{{ key_path }}", preventing regeneration of existing keys.
• The openssh-client package is ensured on target hosts, addressing a common dependency.
• The README is comprehensive, detailing features, requirements, usage, variables, and testing instructions.
• The use of ansible.builtin.authorized_key is appropriate for managing public keys on target hosts.

🧪 Tests

• The current test suite is limited to running the playbook in --check mode and asserting a clean exit. This primarily validates syntax and basic idempotency checks, but does not verify the actual state changes (e.g., key generation, key deployment).
  • Actionable feedback: Enhance the test playbook (tests/test_rotate_ssh_keys.yml) to perform actual state assertions.
    • After running the main playbook, verify the existence of the generated private and public key files on the control node (e.g., /tmp/test_key and /tmp/test_key.pub).
    • Assert that the public key content from the generated .pub file is present in the authorized_keys file on the target host (localhost in this test).
    • Verify that a backup of authorized_keys was created if the key generation task reported changes.
    • Run the main playbook a second time (not in check mode) and assert that no changes occurred, confirming full idempotency.
    • Include cleanup tasks to remove the generated keys and modified authorized_keys entries after the tests.

🔒 Security

• The playbook generates a private key on the control node and stores it in /etc/ssh/ by default. This is a highly sensitive location, as this key grants access to all target hosts where its public counterpart is deployed.
  • Actionable feedback: Add a prominent warning in the README and playbook comments about the security implications of storing private keys in /etc/ssh/ on the control node. Emphasize that this key should be treated with extreme care and its permissions strictly enforced.
  • Actionable feedback: Consider if /etc/ssh/ is the most appropriate default path for all use cases. For some scenarios, a user-specific key in ~/.ssh/ on the control node might be preferred, or integration with a more secure key management system.
• The backup of authorized_keys (~/.ssh/authorized_keys.bak_<timestamp>) is conditional on keygen_result.changed. This means if the key pair already exists on the control node, but the target host's authorized_keys has been modified manually, no backup will be made before the new key is deployed, potentially leading to data loss for existing entries.
  • Actionable feedback: Make the backup of authorized_keys unconditional, ensuring that a snapshot is always taken before modifying the file on the target host. This can be achieved by removing the when: keygen_result.changed condition from the backup task.
• The ansible_user variable is used for the authorized_key module's user parameter. This assumes the user Ansible connects as is the user whose keys should be managed.
  • Actionable feedback: Introduce a dedicated variable (e.g., target_user) to explicitly define which user's authorized_keys file should be managed on the target host. This improves clarity and flexibility, especially in environments where Ansible connects as root but manages non-root user keys.

🧩 Docs/DX

• The README provides a good overview and usage instructions, and the variable descriptions are clear.
• The key_path variable default is /etc/ssh/{{ key_name }}, which is explicitly defined.
• The placeholder for old-key removal is a good hint for further customization.
• Actionable feedback: Clarify in the README's "Usage" section and variable descriptions that key_name and key_path variables control where the private key is stored on the control node. This distinction is crucial for understanding the playbook's operation and security implications.

🧱 Mocks/Fakes

• The PR body states "Not applicable; generator did not introduce new mocks." While no explicit mocking framework is used, the test's reliance on --check mode and the changed_when: false assertion for the ansible-playbook command effectively "mocks" a successful run without verifying actual state changes.
  • Actionable feedback: As detailed in the "Tests" section, the test suite should be enhanced to perform actual state verification rather than relying on --check mode and changed_when: false to implicitly "mock" a successful execution. This will provide more robust and meaningful test coverage.