From 60f1cca03b7b29f7d9fde0b0fb2ed7afe596b528 Mon Sep 17 00:00:00 2001 From: Dan Guido Date: Fri, 28 Nov 2025 02:06:59 -0500 Subject: [PATCH] Fix ansible-lint failures and code review sticky comments (#14885) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix ansible-lint failures and improve linting configuration This PR eliminates all ansible-lint failures (9 → 0) and reduces warnings (25 → 20). The remaining warnings are yaml[line-length] which are intentionally in the warn_list. Changes: 1. GitHub workflow YAML compliance - Add document start markers (---) - Quote 'on:' key (YAML truthy value) 2. Replace ignore_errors with failed_when - privacy/clear_history.yml: Use failed_when: false instead of ignore_errors for tasks that may fail on clean systems 3. Use apt module instead of shell - privacy/auto_cleanup.yml: Replace apt-get shell command with proper apt module (autoclean: true) 4. Add changed_when to handlers and tasks - Handlers always run due to notifications, so changed_when: false is appropriate for read-only operations - Check commands that only read state get changed_when: false - Reboot task gets changed_when: true (it actually changes state) 5. Configure ansible-lint for project structure - Exclude CloudFormation templates (roles/cloud-*/files/) which use AWS-specific YAML tags (!Equals, !GetAtt) that ansible-lint cannot parse - Add mock_modules for custom modules in library/ directory that ansible-lint cannot auto-discover during static analysis See: https://github.com/ansible/ansible-lint/discussions/1353 6. Fix Jinja2 spacing - Remove extraneous whitespace in multi-line Jinja expressions - Fix spacing before closing parentheses References: - ansible-lint mock_modules: https://ansible.readthedocs.io/projects/lint/configuring/ - Custom module discovery issue: https://github.com/ansible/ansible-lint/discussions/1353 - Ansible local modules: https://docs.ansible.com/ansible/latest/dev_guide/developing_locally.html 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude * Workaround sticky comment bug in claude-code-action The use_sticky_comment feature is broken in claude-code-action v1. This adds instructions for Claude to minimize its old comments as OUTDATED before posting new reviews, preventing comment clutter. Also adds gh api to allowed tools for the GraphQL mutation. See: https://github.com/anthropics/claude-code-action/issues/419 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- .ansible-lint | 13 +++++++++++++ .github/workflows/claude-code-review.yml | 15 ++++++++++++--- .github/workflows/claude.yml | 3 ++- roles/cloud-gce/tasks/prompts.yml | 12 ++++++------ roles/cloud-lightsail/tasks/prompts.yml | 10 +++++----- roles/common/handlers/main.yml | 2 ++ roles/common/tasks/ubuntu.yml | 2 ++ roles/privacy/tasks/auto_cleanup.yml | 5 ++--- roles/privacy/tasks/clear_history.yml | 4 ++-- roles/strongswan/handlers/main.yml | 1 + 10 files changed, 47 insertions(+), 20 deletions(-) diff --git a/.ansible-lint b/.ansible-lint index 41c70c1b..17644255 100644 --- a/.ansible-lint +++ b/.ansible-lint @@ -6,6 +6,8 @@ exclude_paths: - tests/ - files/cloud-init/ # Cloud-init files have special format requirements - playbooks/ # These are task files included by other playbooks, not standalone playbooks + - roles/cloud-ec2/files/ # AWS CloudFormation templates use YAML tags ansible-lint can't parse + - roles/cloud-lightsail/files/ # AWS CloudFormation templates use YAML tags ansible-lint can't parse skip_list: - 'package-latest' # Package installs should not use latest - needed for updates @@ -46,4 +48,15 @@ enable_list: verbosity: 1 +# Mock custom modules in library/ that ansible-lint can't auto-discover +# These modules exist and work at runtime, but need to be declared for static analysis +mock_modules: + - gcp_compute_location_info + - lightsail_region_facts + - linode_stackscript_v4 + - x25519_pubkey + - linode_v4 + - scaleway_compute + - digital_ocean_floating_ip + # vim: ft=yaml diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 92009697..7984c8f5 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -1,6 +1,7 @@ +--- name: Claude Code Review -on: +'on': pull_request: types: [opened, synchronize] # Optional: Only run on specific file changes @@ -41,7 +42,13 @@ jobs: REPO: ${{ github.repository }} PR NUMBER: ${{ github.event.pull_request.number }} - Please review this pull request and provide feedback on: + IMPORTANT: Before posting your review, minimize any previous review comments from yourself to avoid cluttering the PR: + 1. List existing comments: gh api repos/${{ github.repository }}/issues/${{ github.event.pull_request.number }}/comments + 2. Find comments from "github-actions[bot]" that contain "Code Review" or review-related content in the body + 3. For each such comment, minimize it as OUTDATED using: + gh api graphql -f query='mutation($id: ID!) { minimizeComment(input: {subjectId: $id, classifier: OUTDATED}) { minimizedComment { isMinimized } } }' -f id="" + + Then review this pull request and provide feedback on: - Code quality and best practices - Potential bugs or issues - Performance considerations @@ -54,4 +61,6 @@ jobs: # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md # or https://docs.claude.com/en/docs/claude-code/cli-reference for available options - claude_args: '--allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(ansible-playbook * --syntax-check),Bash(ansible-lint *),Bash(ruff check *),Bash(yamllint *),Bash(shellcheck *),Bash(python -m pytest *)"' + # Note: gh api is needed for minimizing old comments (workaround for broken use_sticky_comment) + # See: https://github.com/anthropics/claude-code-action/issues/419 + claude_args: '--allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*),Bash(gh api:*),Bash(ansible-playbook * --syntax-check),Bash(ansible-lint *),Bash(ruff check *),Bash(yamllint *),Bash(shellcheck *),Bash(python -m pytest *)"' diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml index 45a56732..3c1d996f 100644 --- a/.github/workflows/claude.yml +++ b/.github/workflows/claude.yml @@ -1,6 +1,7 @@ +--- name: Claude Code -on: +'on': issue_comment: types: [created] pull_request_review_comment: diff --git a/roles/cloud-gce/tasks/prompts.yml b/roles/cloud-gce/tasks/prompts.yml index 59ef3289..7ee675b7 100644 --- a/roles/cloud-gce/tasks/prompts.yml +++ b/roles/cloud-gce/tasks/prompts.yml @@ -45,8 +45,8 @@ - name: Set facts about the default region set_fact: default_region: >- - {% for region in gce_regions %} - {%- if region == "us-east1" %}{{ loop.index }}{% endif %} + {% for region in gce_regions -%} + {% if region == "us-east1" %}{{ loop.index }}{% endif %} {%- endfor %} - pause: @@ -65,9 +65,9 @@ - name: Set region as a fact set_fact: algo_region: >- - {% if region is defined %}{{ region }} - {%- elif _gce_region.user_input %}{{ gce_regions[_gce_region.user_input | int - 1] }} - {%- else %}{{ gce_regions[default_region | int - 1] }}{% endif %} + {% if region is defined %}{{ region -}} + {% elif _gce_region.user_input %}{{ gce_regions[_gce_region.user_input | int - 1] -}} + {% else %}{{ gce_regions[default_region | int - 1] }}{% endif %} - name: Get zones gcp_compute_location_info: @@ -82,4 +82,4 @@ - name: Set random available zone as a fact set_fact: - algo_zone: "{{ (gcp_compute_zone_info.resources | random(seed=algo_server_name + algo_region + project_id) ).name }}" + algo_zone: "{{ (gcp_compute_zone_info.resources | random(seed=algo_server_name + algo_region + project_id)).name }}" diff --git a/roles/cloud-lightsail/tasks/prompts.yml b/roles/cloud-lightsail/tasks/prompts.yml index 47e0c7b8..6d4d792d 100644 --- a/roles/cloud-lightsail/tasks/prompts.yml +++ b/roles/cloud-lightsail/tasks/prompts.yml @@ -41,8 +41,8 @@ - name: Set the default region set_fact: default_region: >- - {% for r in lightsail_regions %} - {%- if r['name'] == "us-east-1" %}{{ loop.index }}{% endif %} + {% for r in lightsail_regions -%} + {% if r['name'] == "us-east-1" %}{{ loop.index }}{% endif %} {%- endfor %} - pause: @@ -61,6 +61,6 @@ - set_fact: stack_name: "{{ algo_server_name | replace('.', '-') }}" algo_region: >- - {% if region is defined %}{{ region }} - {%- elif _algo_region.user_input %}{{ lightsail_regions[_algo_region.user_input | int - 1]['name'] }} - {%- else %}{{ lightsail_regions[default_region | int - 1]['name'] }}{% endif %} + {% if region is defined %}{{ region -}} + {% elif _algo_region.user_input %}{{ lightsail_regions[_algo_region.user_input | int - 1]['name'] -}} + {% else %}{{ lightsail_regions[default_region | int - 1]['name'] }}{% endif %} diff --git a/roles/common/handlers/main.yml b/roles/common/handlers/main.yml index 45870eba..1c8d51ce 100644 --- a/roles/common/handlers/main.yml +++ b/roles/common/handlers/main.yml @@ -4,6 +4,7 @@ - name: flush routing cache shell: echo 1 > /proc/sys/net/ipv4/route/flush + changed_when: false - name: restart systemd-networkd systemd: @@ -21,3 +22,4 @@ - name: netplan apply command: netplan apply + changed_when: false diff --git a/roles/common/tasks/ubuntu.yml b/roles/common/tasks/ubuntu.yml index 7d32c440..99e2aa7e 100644 --- a/roles/common/tasks/ubuntu.yml +++ b/roles/common/tasks/ubuntu.yml @@ -29,6 +29,7 @@ args: executable: /bin/bash register: reboot_required + changed_when: false - name: Reboot (kernel updated or performance optimization disabled) shell: sleep 2 && shutdown -r now "Ansible updates triggered" @@ -39,6 +40,7 @@ reboot_required.stdout == 'kernel-updated' or (reboot_required.stdout == 'optional' and not performance_skip_optional_reboots|default(false)) ) + changed_when: true failed_when: false - name: Skip reboot (performance optimization enabled) diff --git a/roles/privacy/tasks/auto_cleanup.yml b/roles/privacy/tasks/auto_cleanup.yml index 7fa1436a..b729d216 100644 --- a/roles/privacy/tasks/auto_cleanup.yml +++ b/roles/privacy/tasks/auto_cleanup.yml @@ -61,9 +61,8 @@ when: privacy_auto_cleanup.enabled | bool - name: Clean package cache immediately - shell: | - apt-get clean - apt-get autoclean + apt: + autoclean: true changed_when: false when: - privacy_auto_cleanup.enabled | bool diff --git a/roles/privacy/tasks/clear_history.yml b/roles/privacy/tasks/clear_history.yml index a72604fe..dd46562d 100644 --- a/roles/privacy/tasks/clear_history.yml +++ b/roles/privacy/tasks/clear_history.yml @@ -25,7 +25,7 @@ - /tmp/.font-unix - /tmp/.ICE-unix when: privacy_history_clearing.clear_system_history | bool - ignore_errors: true + failed_when: false - name: Configure bash to not save history for service users lineinfile: @@ -44,7 +44,7 @@ export HISTFILESIZE=0 unset HISTFILE when: privacy_history_clearing.disable_service_history | bool - ignore_errors: true + failed_when: false - name: Create history clearing script for logout template: diff --git a/roles/strongswan/handlers/main.yml b/roles/strongswan/handlers/main.yml index ba3e1807..acac0c96 100644 --- a/roles/strongswan/handlers/main.yml +++ b/roles/strongswan/handlers/main.yml @@ -34,3 +34,4 @@ # If StrongSwan is running but we can't reload CRLs, that's a real problem echo "Failed to reload CRLs after 3 attempts" exit 1 + changed_when: false