gh-phaezer-claude-mkt-plugins-ansible/commands/review-ansible-code.md

description, argument-hint

description	argument-hint
Review Ansible code for quality and best practices	Optional code to review

Ansible Code Review

You are reviewing Ansible code for errors, best practices, idempotency, and maintainability using the ansible-code-reviewer agent.

Workflow

1. Identify Review Scope

Determine what needs review:

• Single role: Specific role directory
• Multiple roles: All roles in project
• Playbook: Playbook file(s)
• Tasks file: Specific tasks/main.yml
• Entire project: All Ansible code

2. Gather Code and Context

If not specified, ask for:

• Code location:
  • File paths or directories
  • Or paste code directly
• Code type:
  • Role, playbook, tasks, handlers
• Target environment:
  • Development, staging, production
  • Criticality level
• Specific concerns:
  • Idempotency issues
  • Performance problems
  • Module selection
  • Code organization
• Constraints:
  • Required Ansible version
  • Target OS/distributions
  • Must-use modules

3. Pre-Review Analysis

Before launching agent, check:

• Code is accessible (file paths exist)
• Syntax is valid (won't fail parsing)
• Complete context provided

Run basic checks:

# Syntax validation
ansible-playbook playbook.yml --syntax-check

# Basic lint
ansible-lint roles/ --parseable

4. Launch Code Reviewer

Launch ansible-code-reviewer with:

"Review Ansible code at [location].
Type: [role/playbook/tasks]
Environment: [dev/staging/production]
Focus areas: [concerns if any]

Check for:
- Idempotency violations
- Module selection issues
- Task naming quality
- Variable management
- Handler usage
- Error handling
- Check mode support
- Performance issues
- Code organization"

5. Analyze Review Findings

The agent will categorize issues by severity:

Critical (Block deployment):

• Syntax errors preventing execution
• Idempotency violations causing data loss
• Destructive operations without safeguards
• Tasks that always fail
• Hardcoded credentials (security)

High (Fix before deployment):

• Non-idempotent operations
• Shell/command when module available
• Missing validation on critical changes
• No backup before destructive changes
• Improper handler usage
• Missing error handling

Medium (Address soon):

• Suboptimal code organization
• Poor task naming
• Generic variable names
• Missing documentation
• Performance inefficiencies
• Missing tags

Low (Best practice improvements):

• Style inconsistencies
• Could use better Ansible features
• Documentation could be more detailed
• Minor optimizations

6. Prioritize Remediation

Create action plan:

Immediate (Before any deployment):

1. Fix all Critical issues
2. Validate fixes with testing

Short-term (This sprint):

1. Fix High priority issues
2. Re-review after changes

Medium-term (Next sprint):

1. Address Medium issues
2. Improve documentation

Long-term (Backlog):

1. Low priority improvements
2. Refactoring for maintainability

Review Categories

Idempotency Check

Agent checks that tasks:

• Use proper state management modules
• Don't use shell/command without creates/removes
• Have changed_when set appropriately
• Can run multiple times safely
• Don't have uncontrolled side effects

Example Issues:

# BAD - Not idempotent
- name: Install nginx
  ansible.builtin.shell: apt-get install -y nginx

# GOOD - Idempotent
- name: Ensure nginx is installed
  ansible.builtin.package:
    name: nginx
    state: present

Module Selection

Agent verifies:

• Dedicated modules used over shell/command
• Correct module for each task
• Module parameters used properly
• No deprecated modules

Example Issues:

# BAD - Shell for directory creation
- ansible.builtin.shell: mkdir -p /var/app

# GOOD - File module
- ansible.builtin.file:
    path: /var/app
    state: directory

Task Naming

Agent checks:

• Names start with verbs
• Names are descriptive
• Consistent sentence case
• No generic names

Example Issues:

# BAD
- name: nginx
- name: task1
- name: copy file

# GOOD
- name: Ensure Nginx is installed
- name: Configure Nginx virtual host
- name: Copy application configuration to server

Variable Usage

Agent reviews:

• Variable namespacing (no conflicts)
• Defined defaults
• No hardcoded values
• Proper precedence understanding

Example Issues:

# BAD - Generic names, hardcoded
port: 80
user: webapp

# GOOD - Namespaced, from defaults
nginx_port: "{{ nginx_default_port | default(80) }}"
nginx_user: "{{ nginx_service_user | default('www-data') }}"

Handler Review

Agent checks:

• Handlers for service restarts
• Not direct task restarts
• Handler names clear
• Proper notify usage

Example Issues:

# BAD - Direct restart
- name: Update config
  ansible.builtin.template:
    src: nginx.conf.j2
    dest: /etc/nginx/nginx.conf

- name: Restart nginx
  ansible.builtin.service:
    name: nginx
    state: restarted

# GOOD - Using handler
- name: Update configuration
  ansible.builtin.template:
    src: nginx.conf.j2
    dest: /etc/nginx/nginx.conf
  notify: Reload nginx

Error Handling

Agent verifies:

• Block/rescue for critical operations
• Validation before destructive changes
• Backup before modifications
• Rollback mechanisms

Example Issues:

# BAD - No error handling
- name: Deploy new version
  ansible.builtin.copy:
    src: app.jar
    dest: /opt/app/app.jar

# GOOD - With rollback
- name: Deploy with rollback
  block:
    - name: Backup current version
      ansible.builtin.copy:
        src: /opt/app/app.jar
        dest: /opt/app/app.jar.backup
        remote_src: yes

    - name: Deploy new version
      ansible.builtin.copy:
        src: app.jar
        dest: /opt/app/app.jar

    - name: Restart service
      ansible.builtin.service:
        name: myapp
        state: restarted

  rescue:
    - name: Restore backup
      ansible.builtin.copy:
        src: /opt/app/app.jar.backup
        dest: /opt/app/app.jar
        remote_src: yes

    - name: Restart with old version
      ansible.builtin.service:
        name: myapp
        state: restarted

Output Format

Code Review Report

Summary:

• Overall Quality: [Excellent/Good/Needs Improvement/Poor]
• Critical Issues: [count]
• High Priority: [count]
• Medium Priority: [count]
• Low Priority: [count]
• Idempotency Status: [✓ Pass / ✗ Fail]

Critical Findings:

[CRITICAL] Idempotency: Non-idempotent shell command
Location: roles/nginx/tasks/main.yml:15
Issue: Using shell to install package
Impact: Task reports changed every run
Fix: Use ansible.builtin.package module

Recommendations:

1. Fix critical idempotency issues immediately
2. Replace shell/command with proper modules
3. Add error handling to deployment tasks
4. Improve task naming consistency
5. Add validation before config changes

Validation Commands

After fixing issues:

# Syntax check
ansible-playbook playbook.yml --syntax-check

# Lint with fixes
ansible-lint roles/ --fix

# Check mode to verify idempotency
ansible-playbook playbook.yml --check
ansible-playbook playbook.yml --check  # Run twice

# Test in development
ansible-playbook playbook.yml -i inventory/dev -vv

Best Practices

Pre-Review:

• Run syntax check first
• Run ansible-lint locally
• Fix obvious issues before agent review

During Review:

• Provide complete context
• Specify environment criticality
• Note any constraints or requirements
• Ask about specific concerns

Post-Review:

• Fix Critical and High issues first
• Test fixes in development
• Re-review after significant changes
• Document rationale for any exceptions

Continuous Improvement:

• Integrate ansible-lint in CI/CD
• Regular code reviews before production
• Keep roles modular and focused
• Maintain consistent coding standards
• Document patterns and decisions