Skip to content

Comments

Refactor ssh related role#135

Merged
thaim merged 5 commits intomasterfrom
ssh-refactor
Jul 25, 2025
Merged

Refactor ssh related role#135
thaim merged 5 commits intomasterfrom
ssh-refactor

Conversation

@thaim
Copy link
Owner

@thaim thaim commented Jul 25, 2025

No description provided.

@thaim thaim requested a review from Copilot July 25, 2025 17:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates SSH-related functionality by removing redundant roles (ssh_hosts and secure-sshd) and enhancing the main ssh role with comprehensive SSH client and server configuration capabilities.

  • Consolidated multiple SSH-related roles into a single ssh role
  • Added SSH client configuration with version detection and keystroke timing settings
  • Updated SSH server configuration with modern Ansible modules and best practices

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
roles/ssh_hosts/tasks/main.yml Removed entire role that handled SSH host key scanning
roles/ssh/tasks/main.yml Enhanced with SSH client config, version detection, and modernized server config
roles/ssh/files/sshd_config Removed static SSH daemon configuration file
roles/secure-sshd/tasks/main.yml Removed entire role that handled authorized keys
roles/secure-sshd/files/sshd_config Removed static secure SSH daemon configuration file
playbook-desktop.yml Added ssh role to desktop playbook

- name: Get SSH client version
ansible.builtin.shell: |
set -o pipefail
ssh -V 2>&1 | head -n1 | sed 's/.*_\([0-9]\+\.[0-9]\+\).*/\1/'
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

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

The regex pattern may not reliably extract version numbers from all SSH implementations. Consider using a more robust approach like ssh -V 2>&1 | awk '{print $1}' | cut -d'_' -f2 or add error handling for cases where the pattern doesn't match.

Suggested change
ssh -V 2>&1 | head -n1 | sed 's/.*_\([0-9]\+\.[0-9]\+\).*/\1/'
ssh -V 2>&1 | awk '{print $1}' | cut -d'_' -f2 || echo "0.0"

Copilot uses AI. Check for mistakes.
marker: "# {mark} ANSIBLE MANAGED BLOCK - ObscureKeystrokeTiming"
create: yes
mode: '0600'
when: ssh_version < 10.0
Copy link

Copilot AI Jul 25, 2025

Choose a reason for hiding this comment

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

The condition assumes ssh_version will always be a valid float, but if the regex extraction fails, this could result in an empty string or invalid comparison. Add validation to ensure ssh_version is a valid number before the comparison.

Suggested change
when: ssh_version < 10.0
when: ssh_version is defined and ssh_version < 10.0

Copilot uses AI. Check for mistakes.
@thaim thaim merged commit 56090ff into master Jul 25, 2025
2 checks passed
@thaim thaim deleted the ssh-refactor branch July 25, 2025 18:22
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.

1 participant