fix: Better support for check mode - #93
Merged
Merged
Conversation
If packages are not installed, commands will fail in check mode.
📝 WalkthroughWalkthroughThe PR modifies ChangesCheck-mode gating
Sequence Diagram(s)flowchart TD
PackageInstall --> RegisterResult
RegisterResult --> CheckModeGate
CheckModeGate -->|"not check_mode or unchanged"| PackagesInstalledBlock
PackagesInstalledBlock --> VersionDetection
PackagesInstalledBlock --> ServiceEnable
PackagesInstalledBlock --> ConfigRender
PackagesInstalledBlock --> DatabaseInit
PackagesInstalledBlock --> DatabaseFetch
PackagesInstalledBlock --> DatabaseCheck
PackagesInstalledBlock --> DatabaseUpdate
PackagesInstalledBlock --> CronConfig
PackagesInstalledBlock --> SuccessReport
Related issues: None specified. Related PRs: None specified. Suggested labels: ansible, check-mode, tasks Suggested reviewers: None specified. Poem: 🐰 A rabbit checked the mode with care, / Before it touched a database square. / "Not in check mode?" it did ask, / Then gently tackled every task. / Cron and fingerprint still stand true, / Gated safely, through and through. 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tasks/main.yml`:
- Around line 13-160: Add check-mode test coverage for the new gating in
tasks/main.yml, focusing on the top-level “Packages are installed” block and the
nested init/update/fetch/check paths. Create tests that run the role with
ansible_check_mode enabled in both package-present and package-absent states so
the new not ansible_check_mode guards are exercised and stay protected. Use the
existing task names like “Packages are installed”, “Initialize AIDE database”,
and “Update AIDE database and fetch it” to target the relevant behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 47275f3e-e5e7-4a28-8dc2-9a0eff1d525c
📒 Files selected for processing (1)
tasks/main.yml
Comment on lines
+13
to
+160
| register: __aide_install_packages | ||
|
|
||
| - name: Get AIDE version | ||
| ansible.builtin.command: | ||
| cmd: aide --version | ||
| register: __aide_version_register | ||
| changed_when: false | ||
|
|
||
| # assumes the version starts with a digit and goes to the end of the line | ||
| - name: Set AIDE version | ||
| set_fact: | ||
| aide_version: "{{ __output | regex_search('(?m)^A[iI][dD][eE] (\\d.*)$', '\\1') | first }}" | ||
| vars: | ||
| __output: "{{ __aide_version_register.stdout if __aide_version_register.stdout | length > 0 | ||
| else __aide_version_register.stderr }}" | ||
|
|
||
| - name: Ensure required services are enabled and started | ||
| ansible.builtin.service: | ||
| name: "{{ item }}" | ||
| state: started | ||
| enabled: true | ||
| loop: "{{ __aide_services }}" | ||
|
|
||
| - name: Generate "/etc/{{ __aide_config }}" | ||
| ansible.builtin.template: | ||
| src: "{{ aide_config_template }}" | ||
| dest: "/etc/{{ __aide_config }}" | ||
| mode: "0400" | ||
| when: aide_config_template is not none | ||
|
|
||
| # - name: Print Header | ||
| # ansible.builtin.command: head /etc/aide.conf || true | ||
|
|
||
| - name: Initialize AIDE database | ||
| when: aide_init | bool | ||
| - name: Packages are installed | ||
| # either run mode or check mode and no changes to packages | ||
| when: not ansible_check_mode or (ansible_check_mode and not __aide_install_packages.changed) | ||
| block: | ||
| - name: Initialize AIDE database | ||
| ansible.builtin.command: | ||
| cmd: aide --init | ||
| changed_when: true | ||
|
|
||
| - name: Copy AIDE reference database | ||
| ansible.builtin.copy: | ||
| remote_src: true | ||
| src: "{{ __aide_db_new_name }}" | ||
| dest: "{{ __aide_db_name }}" | ||
| owner: root | ||
| group: root | ||
| mode: "0440" | ||
| force: true | ||
| when: not aide_fetch_db | bool | ||
|
|
||
| - name: Remove remote AIDE database file | ||
| ansible.builtin.file: | ||
| path: "{{ __aide_db_new_name }}" | ||
| state: absent | ||
| when: not aide_fetch_db | bool | ||
|
|
||
| - name: Fetch AIDE database | ||
| when: aide_fetch_db | bool | ||
| block: | ||
| - name: Fetch AIDE database | ||
| ansible.builtin.fetch: | ||
| src: "{{ __aide_db_new_name }}" | ||
| dest: "{{ aide_db_fetch_dir }}" | ||
|
|
||
| - name: Remove remote AIDE database file | ||
| ansible.builtin.file: | ||
| path: "{{ __aide_db_new_name }}" | ||
| state: absent | ||
|
|
||
| - name: Check AIDE integrity | ||
| when: aide_check | bool | ||
| block: | ||
| - name: Copy AIDE reference database | ||
| ansible.builtin.copy: | ||
| src: >- | ||
| {{ aide_db_fetch_dir }}/{{ inventory_hostname }}{{ __aide_db_new_name }} | ||
| dest: "{{ __aide_db_name }}" | ||
| owner: root | ||
| group: root | ||
| mode: "0440" | ||
| when: aide_fetch_db | bool | ||
|
|
||
| - name: Check against AIDE reference database | ||
| - name: Get AIDE version | ||
| ansible.builtin.command: | ||
| cmd: aide --check | ||
| cmd: aide --version | ||
| check_mode: false | ||
| register: __aide_version_register | ||
| changed_when: false | ||
|
|
||
| - name: Update AIDE database and fetch it | ||
| when: aide_update | bool | ||
| block: | ||
| - name: Update AIDE database | ||
| ansible.builtin.command: | ||
| cmd: aide --update | ||
| register: __aide_update_result | ||
| failed_when: __msg not in __aide_update_result.stdout | ||
| changed_when: true | ||
| # assumes the version starts with a digit and goes to the end of the line | ||
| - name: Set AIDE version | ||
| set_fact: | ||
| aide_version: "{{ __output | regex_search('(?m)^A[iI][dD][eE] (\\d.*)$', '\\1') | first }}" | ||
| vars: | ||
| __msg: >- | ||
| AIDE found NO differences between database and filesystem. Looks okay!! | ||
| __output: "{{ __aide_version_register.stdout if __aide_version_register.stdout | length > 0 | ||
| else __aide_version_register.stderr }}" | ||
|
|
||
| - name: Ensure required services are enabled and started | ||
| ansible.builtin.service: | ||
| name: "{{ item }}" | ||
| state: started | ||
| enabled: true | ||
| loop: "{{ __aide_services }}" | ||
|
|
||
| - name: Generate "/etc/{{ __aide_config }}" | ||
| ansible.builtin.template: | ||
| src: "{{ aide_config_template }}" | ||
| dest: "/etc/{{ __aide_config }}" | ||
| mode: "0400" | ||
| when: aide_config_template is not none | ||
|
|
||
| # - name: Print Header | ||
| # ansible.builtin.command: head /etc/aide.conf || true | ||
|
|
||
| - name: Fetch AIDE database | ||
| ansible.builtin.fetch: | ||
| src: "{{ __aide_db_new_name }}" | ||
| dest: "{{ aide_db_fetch_dir }}" | ||
| - name: Initialize AIDE database | ||
| when: | ||
| - not ansible_check_mode | ||
| - aide_init | bool | ||
| block: | ||
| - name: Initialize AIDE database | ||
| ansible.builtin.command: | ||
| cmd: aide --init | ||
| changed_when: true | ||
|
|
||
| - name: Copy AIDE reference database | ||
| ansible.builtin.copy: | ||
| remote_src: true | ||
| src: "{{ __aide_db_new_name }}" | ||
| dest: "{{ __aide_db_name }}" | ||
| owner: root | ||
| group: root | ||
| mode: "0440" | ||
| force: true | ||
| when: not aide_fetch_db | bool | ||
|
|
||
| - name: Remove remote AIDE database file | ||
| ansible.builtin.file: | ||
| path: "{{ __aide_db_new_name }}" | ||
| state: absent | ||
| when: not aide_fetch_db | bool | ||
|
|
||
| - name: Remove remote AIDE database file | ||
| ansible.builtin.file: | ||
| path: "{{ __aide_db_new_name }}" | ||
| - name: Fetch AIDE database | ||
| when: | ||
| - not ansible_check_mode | ||
| - aide_fetch_db | bool | ||
| block: | ||
| - name: Fetch AIDE database | ||
| ansible.builtin.fetch: | ||
| src: "{{ __aide_db_new_name }}" | ||
| dest: "{{ aide_db_fetch_dir }}" | ||
|
|
||
| - name: Remove remote AIDE database file | ||
| ansible.builtin.file: | ||
| path: "{{ __aide_db_new_name }}" | ||
| state: absent | ||
|
|
||
| - name: Check AIDE integrity | ||
| when: | ||
| - not ansible_check_mode | ||
| - aide_check | bool | ||
| block: | ||
| - name: Copy AIDE reference database | ||
| ansible.builtin.copy: | ||
| src: >- | ||
| {{ aide_db_fetch_dir }}/{{ inventory_hostname }}{{ __aide_db_new_name }} | ||
| dest: "{{ __aide_db_name }}" | ||
| owner: root | ||
| group: root | ||
| mode: "0440" | ||
| when: aide_fetch_db | bool | ||
|
|
||
| - name: Check against AIDE reference database | ||
| ansible.builtin.command: | ||
| cmd: aide --check | ||
| changed_when: false | ||
|
|
||
| - name: Update AIDE database and fetch it | ||
| when: | ||
| - not ansible_check_mode | ||
| - aide_update | bool | ||
| block: | ||
| - name: Update AIDE database | ||
| ansible.builtin.command: | ||
| cmd: aide --update | ||
| register: __aide_update_result | ||
| failed_when: __msg not in __aide_update_result.stdout | ||
| changed_when: true | ||
| vars: | ||
| __msg: >- | ||
| AIDE found NO differences between database and filesystem. Looks okay!! | ||
|
|
||
| - name: Fetch AIDE database | ||
| ansible.builtin.fetch: | ||
| src: "{{ __aide_db_new_name }}" | ||
| dest: "{{ aide_db_fetch_dir }}" | ||
|
|
||
| - name: Remove remote AIDE database file | ||
| ansible.builtin.file: | ||
| path: "{{ __aide_db_new_name }}" | ||
| state: absent | ||
|
|
||
| - name: Update aide check cron configuration if necessary | ||
| ansible.builtin.lineinfile: | ||
| path: /etc/crontab | ||
| regexp: "^.* root {{ __aide_bin_path }} --check" | ||
| line: "{{ aide_cron_interval }} root {{ __aide_bin_path }} --check" | ||
| when: | ||
| - aide_cron_check is not none | ||
| - aide_cron_check | bool | ||
|
|
||
| - name: Remove aide check cron configuration if necessary | ||
| ansible.builtin.lineinfile: | ||
| path: /etc/crontab | ||
| state: absent | ||
|
|
||
| - name: Update aide check cron configuration if necessary | ||
| ansible.builtin.lineinfile: | ||
| path: /etc/crontab | ||
| regexp: "^.* root {{ __aide_bin_path }} --check" | ||
| line: "{{ aide_cron_interval }} root {{ __aide_bin_path }} --check" | ||
| when: | ||
| - aide_cron_check is not none | ||
| - aide_cron_check | bool | ||
|
|
||
| - name: Remove aide check cron configuration if necessary | ||
| ansible.builtin.lineinfile: | ||
| path: /etc/crontab | ||
| state: absent | ||
| regexp: "^.* root {{ __aide_bin_path }} --check" | ||
| when: | ||
| - aide_cron_check is not none | ||
| - not aide_cron_check | bool | ||
|
|
||
| - name: Record role success fingerprint | ||
| sr_fingerprint: | ||
| sr_message: >- | ||
| success system_role:aide ansible_version={{ ansible_version.full }} | ||
| {{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_version'] }} | ||
| regexp: "^.* root {{ __aide_bin_path }} --check" | ||
| when: | ||
| - aide_cron_check is not none | ||
| - not aide_cron_check | bool | ||
|
|
||
| - name: Record role success fingerprint | ||
| sr_fingerprint: | ||
| sr_message: >- | ||
| success system_role:aide ansible_version={{ ansible_version.full }} | ||
| {{ ansible_facts['distribution'] }}-{{ ansible_facts['distribution_version'] }} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for test files that exercise check-mode behavior for the AIDE role.
# Look for test files in the tests/ directory
fd -t f 'tests_' tests/ 2>/dev/null || echo "No tests/ directory or test files found"
# Search for check_mode references in test files
rg -n 'check_mode|ansible_check_mode|--check' tests/ 2>/dev/null || echo "No check_mode references found in tests/"Repository: linux-system-roles/aide
Length of output: 827
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tests files ==\n'
git ls-files 'tests/*.yml' | sed 's#^`#-` #'
printf '\n== check-mode related references ==\n'
rg -n 'check_mode|ansible_check_mode|--check|aide_check|aide_update|aide_init|aide_fetch_db' tests/*.yml
printf '\n== task file context ==\n'
sed -n '1,220p' tasks/main.ymlRepository: linux-system-roles/aide
Length of output: 6544
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== candidate test playbooks ==\n'
sed -n '1,220p' tests/tests_default.yml
printf '\n---\n'
sed -n '1,220p' tests/tests_deploy.yml
printf '\n---\n'
sed -n '1,220p' tests/tests_custom_template.yml
printf '\n---\n'
sed -n '1,220p' tests/tests_check_cron.yml
printf '\n== role runner helpers ==\n'
sed -n '1,220p' tests/roles/caller/tasks/main.yml
printf '\n---\n'
sed -n '1,220p' tests/tasks/run_role_with_clear_facts.yml
printf '\n---\n'
sed -n '1,220p' tests/tasks/check_header.yml
printf '\n---\n'
sed -n '1,220p' tests/tasks/check_not_present_header.ymlRepository: linux-system-roles/aide
Length of output: 8530
Add check-mode test coverage for the new gating
Existing playbooks cover install/init/cron paths, but none exercise the role under --check with packages already present and absent. Add coverage for both states so the new not ansible_check_mode guards stay protected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tasks/main.yml` around lines 13 - 160, Add check-mode test coverage for the
new gating in tasks/main.yml, focusing on the top-level “Packages are installed”
block and the nested init/update/fetch/check paths. Create tests that run the
role with ansible_check_mode enabled in both package-present and package-absent
states so the new not ansible_check_mode guards are exercised and stay
protected. Use the existing task names like “Packages are installed”,
“Initialize AIDE database”, and “Update AIDE database and fetch it” to target
the relevant behavior.
Source: Path instructions
Contributor
|
[citest] |
spetrosi
changed the title
spetrosi
approved these changes
Jul 8, 2026
richm
approved these changes
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enhancement:
Don't fail if check mode.
Reason:
Check mode failed because aide was not yet installed but the failure was about formatting in the "Set AIDE version" task (see below).
Result:
Check mode will not fail because the packages are not yet installed. If you launch with check mode and "__aide_install_packages" indicates a change (i.e. packages are not installed), nothing else will run.
Issue Tracker Tickets (Jira or BZ if any):
N/A
Summary by CodeRabbit