feat(groq): An Ansible playbook that safely generates new SSH host keys on target machines and restarts the SSH service.

[polsala]

Implementation Summary

• Utility: nightly-ansible-ssh-key-rotator
• Provider: groq
• Location: ansible-playbooks/nightly-nightly-ansible-ssh-key-rota-5
• Files Created: 3
• Description: An Ansible playbook that safely generates new SSH host keys on target machines and restarts the SSH service.

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-ansible-ssh-key-rota-5.
• 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-ansible-ssh-key-rota-5/README.md
• Run tests located in ansible-playbooks/nightly-nightly-ansible-ssh-key-rota-5/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 playbook lives in its own directory (nightly‑...‑ssh-key-rota-5) and does not touch any existing code‑base.
• Idempotent key generation – Using creates: with the command module guarantees that existing host keys are left untouched unless the user explicitly removes them first.
• Configurable variables – key_dir and ssh_service_name are exposed as extra‑vars, making the utility reusable across environments.
• Service restart guard – The when: ssh_service_name != "" clause lets callers skip the restart (useful for CI or systems with non‑standard service names).
• Self‑contained test suite – The test playbook creates a temporary directory, runs the rotation playbook, and asserts the presence of both key files.
• Documentation – The README provides a concise overview, feature list, variable table, usage examples, and a testing command.

🧪 Tests

• Test coverage is good for the happy path – It verifies that both RSA and Ed25519 keys are created in a fresh directory.

• Add cleanup step – The temporary test directory is removed at the start but left behind after the run. Adding a final file: path={{ test_key_dir }} state=absent (or using ansible.builtin.tempfile with state=absent) keeps the CI workspace tidy.

  - name: Clean up test directory
    file:
      path: "{{ test_key_dir }}"
      state: absent
    when: test_key_dir is defined

• Make the random directory deterministic for CI caching – Using {{ 999999 | random }} can produce a different path on each run, which may interfere with caching or parallel jobs. Consider using {{ lookup('pipe','uuidgen') }} or the built‑in tempfile module:

  - name: Create temporary test directory
    tempfile:
      state: directory
      suffix: _ssh_key_rotator_test
    register: tmpdir
  - set_fact:
      test_key_dir: "{{ tmpdir.path }}"

• Validate service‑restart behavior – Add a test case where ssh_service_name is set to a non‑existent service and assert that the playbook fails gracefully (or skips). This guards against accidental restarts on mis‑typed service names.

• Add a negative test – Run the playbook a second time without deleting the keys and assert that the command tasks are skipped (e.g., check changed: false via register and assert). This confirms true idempotency.

🔒 Security

• File permissions – ssh-keygen creates private keys with mode 0600, but the playbook does not explicitly enforce this. Adding a file task after key generation to set the correct permissions can prevent accidental world‑readable keys on mis‑configured systems.

  - name: Secure private RSA key permissions
    file:
      path: "{{ key_dir }}/ssh_host_rsa_key"
      mode: "0600"
    when: rsa_key_created is changed

• Avoid running as root on the control node – The playbook uses become: true on the target hosts, which is appropriate. Ensure that any CI runner invoking the tests also runs with limited privileges (e.g., ansible-playbook -i localhost, ... --become only when needed).

• Passphrase handling – Host keys are intentionally generated without a passphrase (-N ""). This matches typical SSH daemon expectations, but consider documenting the security implication in the README (e.g., “host keys are stored unencrypted; protect the host filesystem accordingly”).

• Validate key_dir input – If a user supplies a non‑standard directory (e.g., /tmp), the playbook will create keys there. Adding a simple sanity check can prevent accidental key placement in insecure locations:

  - name: Ensure key_dir is an absolute path under /etc or /usr/local
    assert:
      that:
        - key_dir.startswith('/etc/') or key_dir.startswith('/usr/local/')
      fail_msg: "key_dir must be a secure system directory"

🧩 Docs / DX

• README formatting – The diff shows escaped \n characters; ensure the committed file contains proper line breaks (it likely does, but double‑check).

• Consistent naming – The directory name repeats “nightly” (nightly-nightly-...). Consider simplifying to nightly-ansible-ssh-key-rotator for clarity and to avoid confusion in CI paths.

• Variable defaults in README – The table lists defaults, which is great. Adding a note that ssh_service_name can be omitted on systems that use ssh instead of sshd would help newcomers.

• Usage examples – Show an example that runs the playbook against a group of hosts with a custom key_dir, e.g.:

  ansible-playbook -i inventory.ini rotate_ssh_keys.yml \
    -e "key_dir=/custom/ssh ssh_service_name=ssh"

• Testing instructions – Mention that the test playbook requires ansible version ≥2.9 (or whichever version you target) and that it runs against localhost only. This sets expectations for contributors.

🧱 Mocks / Fakes

• No external services – The utility does not depend on external APIs, so mocks are unnecessary.
• Future extensibility – If you later add a step that fetches a list of hosts from an inventory service, consider adding a mock inventory fixture in the test suite to keep tests deterministic.

---

Overall, the contribution delivers a useful, well‑documented Ansible utility with a solid test harness. The suggestions above focus on tightening CI hygiene, reinforcing security best practices, and polishing the developer experience.

[polsala]

🤖 Review by GEMINI Agent

✅ What's solid

• The playbook is well-structured, adhering to Ansible best practices for variable definition and task naming.
• The use of creates in the command module for ssh-keygen ensures idempotency, preventing accidental overwrites of existing host keys, which is critical for a "safe" key rotation utility.
• The ssh_service_name variable, combined with a conditional restart (when: ssh_service_name != ""), provides excellent flexibility, allowing for dry runs or use on systems with non-standard SSH service names.
• The README.md is comprehensive, clearly outlining the utility's purpose, features, variables, usage examples, and testing instructions, significantly enhancing developer experience.

🧪 Tests

• The provided test playbook (tests/test_rotate_ssh_keys.yml) establishes a good foundation by running the main playbook in an isolated, temporary directory and verifying the creation of key files.
• Actionable Feedback:
  • Extend the test suite to verify the permissions of the generated private host keys (e.g., ssh_host_rsa_key should be 0600). This is a critical security aspect that ssh-keygen usually handles, but explicit verification adds robustness.
  • Add a test case that explicitly verifies the idempotency of the playbook. This could involve running the playbook twice in the same test directory and asserting that the ssh-keygen tasks report changed: false on the second run, confirming keys are not regenerated unnecessarily.
  • Consider adding a test that explicitly asserts the Restart SSH service task is skipped when ssh_service_name is set to an empty string, ensuring the conditional logic works as expected.

🔒 Security

• The playbook correctly uses become: true for system-level operations, which is necessary.
• The file task sets appropriate permissions (0755) for the key_dir.
• Actionable Feedback:
  • The ssh-keygen commands use -N "", which generates host keys without a passphrase. While standard for host keys, it would be beneficial to explicitly mention this in the README.md or as a comment in the playbook to ensure users are aware.
  • While ssh-keygen typically sets correct permissions for private keys, consider adding an explicit file task after key generation to ensure the private host keys (e.g., ssh_host_rsa_key) have restrictive permissions (e.g., mode: '0600'). This provides an additional layer of assurance.

🧩 Docs/DX

• The README.md is a strong asset, offering clear explanations and practical usage examples.
• Actionable Feedback:
  • Under the "Features" section in README.md, clarify that if an existing key is deleted, a new key will be generated, not just the old one restored. This is implied by "unless you delete them first" but could be more explicit.
  • Add a "Prerequisites" section to the README.md to explicitly state that ssh-keygen must be installed and available on the target machines.
  • For the ssh_service_name variable in the README.md, consider mentioning common alternative service names (e.g., ssh on some distributions) or providing guidance on how users can determine the correct service name for their specific OS.

🧱 Mocks/Fakes

• The PR correctly states that traditional mocks/fakes are not applicable. The approach of using a temporary directory and localhost for testing effectively isolates the playbook's execution, serving a similar purpose to mocking the target environment.
• Actionable Feedback:
  • No specific actionable feedback for this section, as the current testing methodology is appropriate and effective for Ansible playbooks.