Skip to content

Commit 96ae8b8

Browse files
spetrosispetrosi
committed
ci: Add config file for CodeRabbit with custom rules
Sourcery that we currently use cannot read documentation files and best practices, it's rather a refactoring tool. So I want to introduce CodeRabbit that allows creating .coderabbit.yaml with custom rules and conventions. Signed-off-by: Sergei Petrosian <spetrosi@redhat.com>
1 parent 45ce454 commit 96ae8b8

1 file changed

Lines changed: 232 additions & 0 deletions

File tree

‎.coderabbit.yaml‎

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
2+
# CodeRabbit configuration for linux-system-roles
3+
# Based on conventions from https://linux-system-roles.github.io/contribute.html
4+
# This file is managed from https://github.com/linux-system-roles/.github,
5+
# any manual edits will be overwritten.
6+
7+
chat:
8+
art: false
9+
10+
reviews:
11+
# Disable fun features
12+
poem: false
13+
in_progress_fortune: false
14+
15+
# Disable auto-features
16+
auto_apply_labels: false
17+
auto_assign_reviewers: false
18+
request_changes_workflow: false
19+
20+
# Disable additional review features
21+
sequence_diagrams: false
22+
estimate_code_review_effort: false
23+
suggested_labels: false
24+
high_level_summary_in_walkthrough: false
25+
26+
# Disable finishing touches
27+
finishing_touches:
28+
unit_tests:
29+
enabled: false
30+
31+
# Enforce PR title and description requirements
32+
pre_merge_checks:
33+
title:
34+
mode: "warning"
35+
requirements: |
36+
PR title MUST follow Conventional Commits format:
37+
- Format: <type>: <description> or <type>!: <description> for breaking changes
38+
- Valid types: Refer to the 'type-enum' rule in .commitlintrc.js file for the complete list of allowed types
39+
- Examples:
40+
- "feat: Add backup functionality"
41+
- "fix: Correct OSTree package installation"
42+
- "fix!: Remove deprecated variable (breaking change)"
43+
44+
custom_checks:
45+
- mode: "warning"
46+
name: "Description Format"
47+
instructions: |
48+
PR description MUST follow the template structure from .github/pull_request_template.md:
49+
- Must contain "Enhancement:" or "Feature:" section describing what changed
50+
- Must contain "Reason:" section explaining why the change was needed
51+
- Must contain "Result:" section describing the outcome or impact
52+
- Can contain optional "Issue Tracker Tickets (Jira or BZ if any):" section
53+
54+
Example:
55+
```
56+
Feature: Introduce the kdump_secure_logging variable
57+
58+
Reason: Currently, all sensitive tasks use hard-coded no_log: true, which makes debugging difficult.
59+
60+
Result: Users can now set kdump_secure_logging: false for debugging while maintaining secure defaults.
61+
62+
Issue Tracker Tickets (Jira or BZ if any): RHEL-12345
63+
```
64+
65+
path_instructions:
66+
# ========================================
67+
# Ansible Tasks - Core role logic
68+
# ========================================
69+
- path: "tasks/**/*.yml"
70+
instructions: |
71+
**no_log patterns:**
72+
- For sensitive data (credentials, secrets, passwords, keys):
73+
```yaml
74+
- name: Task with sensitive data
75+
ansible.builtin.command: sensitive command
76+
no_log: "{{ kdump_secure_logging }}"
77+
```
78+
Ensure `kdump_secure_logging: true` is defined in defaults/main.yml
79+
80+
- For facts modules (package_facts, service_facts):
81+
```yaml
82+
- name: Gather package facts
83+
ansible.builtin.package_facts:
84+
no_log: "{{ ansible_verbosity < 3 }}"
85+
```
86+
This hides verbose facts output unless running with -vvv or higher verbosity
87+
88+
- NEVER use hardcoded `no_log: true` - always parametrize
89+
90+
**Package installation (OSTree compatibility):**
91+
- ALWAYS use `ansible.builtin.package` module (NEVER yum, dnf, or apt)
92+
- For Red Hat family systems: MUST include `use:` parameter for OSTree compatibility
93+
- For other systems (apt, etc.): OSTree `use:` parameter NOT needed
94+
95+
**Red Hat family pattern (rpm/dnf/yum based):**
96+
```yaml
97+
- name: Install packages
98+
ansible.builtin.package:
99+
name: "{{ __kdump_packages }}"
100+
state: present
101+
use: "{{ (__kdump_is_ostree | d(false)) | ternary('ansible.posix.rhel_rpm_ostree', omit) }}"
102+
```
103+
104+
**Non-Red Hat systems (apt based) - no OSTree handling needed:**
105+
```yaml
106+
- name: Install packages
107+
ansible.builtin.package:
108+
name: "{{ __kdump_packages }}"
109+
state: present
110+
when: ansible_facts["pkg_mgr"] == 'apt'
111+
```
112+
113+
**Pattern explanation:**
114+
- `__kdump_is_ostree` detects OSTree/rpm-ostree systems (RHEL/CentOS/Fedora variants)
115+
- This variable should already exist in vars/main.yml - do NOT suggest adding it
116+
- `| d(false)` provides safe default if variable undefined
117+
- `ternary()` selects `ansible.posix.rhel_rpm_ostree` for OSTree, `omit` otherwise
118+
- `omit` removes the parameter entirely on non-OSTree Red Hat systems
119+
120+
**Third-party collections:**
121+
- Avoid using third-party collections (community.general, community.crypto, etc.)
122+
- Use ansible.builtin modules or command module instead
123+
124+
**Referencing other system roles:**
125+
- Use FQCN: `fedora.linux_system_roles.kdump`
126+
127+
**Idempotency:**
128+
- Tasks must be idempotent - safe to run multiple times without unintended changes
129+
- Use proper state parameters (present/absent, started/stopped, etc.)
130+
- Command/shell tasks should use creates/removes or changed_when to avoid false changes
131+
- Example:
132+
```yaml
133+
- name: Initialize database
134+
ansible.builtin.command: /usr/bin/initialize-db
135+
args:
136+
creates: /var/lib/db/initialized.flag
137+
```
138+
139+
**Check mode support:**
140+
- Critical tasks should support check mode (--check flag)
141+
- Use check_mode: false only when absolutely necessary (e.g., fact gathering)
142+
- Test that role works with --check --diff
143+
144+
**Test coverage requirement:**
145+
- When adding new tasks to tasks/main.yml or creating new task files, verify that corresponding test coverage exists in tests/
146+
- New functionality MUST include test files (tests/tests_*.yml) that exercise the new code paths
147+
- Tests should verify both success scenarios and failure/edge cases
148+
- If this PR adds new tasks but does not include new or updated tests, flag it and request test coverage
149+
150+
# ========================================
151+
# Handlers
152+
# ========================================
153+
- path: "handlers/**/*.yml"
154+
instructions: |
155+
- Handlers with sensitive data must use `no_log: "{{ kdump_secure_logging }}"`
156+
157+
# ========================================
158+
# Test Playbooks
159+
# ========================================
160+
- path: "tests/tests_*.yml"
161+
instructions: |
162+
**CRITICAL: Role invocation pattern:**
163+
- NEVER use `ansible.builtin.include_role` directly
164+
- NEVER use `ansible.builtin.import_role` directly
165+
- NEVER use `roles:` keyword
166+
- ALWAYS use the centrally managed wrapper:
167+
```yaml
168+
- name: Run role
169+
ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml
170+
vars:
171+
kdump_<parameter>: <value>
172+
```
173+
174+
**Test quality requirements:**
175+
- Tests should verify both success and failure scenarios
176+
- Use assert module to verify expected state after role execution
177+
- Include cleanup tasks to ensure tests are rerunnable
178+
- Tests should be idempotent - running twice should not cause failures
179+
- Example verification:
180+
```yaml
181+
- name: Verify service is running
182+
ansible.builtin.assert:
183+
that:
184+
- ansible_facts.services['mssql-server.service'].state == 'running'
185+
fail_msg: "SQL Server service is not running"
186+
```
187+
188+
# ========================================
189+
# Templates
190+
# ========================================
191+
- path: "templates/**/*.j2"
192+
instructions: |
193+
**Required headers (in this order):**
194+
1. ansible_managed header: `{{ ansible_managed | comment }}`
195+
2. Role fingerprint: `{{ "system_role:kdump" | comment(prefix="", postfix="") }}`
196+
197+
# ========================================
198+
# Variable Definitions - defaults/
199+
# ========================================
200+
- path: "defaults/**/*.yml"
201+
instructions: |
202+
- All variables MUST be prefixed with `kdump_`
203+
- All variables MUST be stored in the file defaults/main.yml, Ansible
204+
doesn't include variables from other files.
205+
- These are user-facing API variables
206+
- For every new variable introduced in this file, verify that it is
207+
documented in README.md with a description and a usage example.
208+
If it is missing from README.md, flag it and request that it be added.
209+
210+
# ========================================
211+
# Variable Definitions - vars/
212+
# ========================================
213+
- path: "vars/**/*.yml"
214+
instructions: |
215+
- Internal variables MUST be prefixed with `__kdump_`
216+
- User-facing variables belong in defaults/main.yml, not here
217+
218+
# ========================================
219+
# Python Code
220+
# ========================================
221+
- path: "**/*.py"
222+
instructions: |
223+
- Must follow PEP 8 and be formatted with Python Black
224+
- Run `tox -e black,flake8` before committing
225+
226+
# ========================================
227+
# Documentation
228+
# ========================================
229+
- path: "README.md"
230+
instructions: |
231+
- Document all new user-facing variables
232+
- Include usage examples for new functionality

0 commit comments

Comments
 (0)