ansible_style_guide.md

Ansible style guide

A list of best practises and style choices that I think are good for writing clean and consistent Ansible.

Prefix global variables with the organisation / company name

This makes it easy to differentiate between a "global" variable and a playbook/role variable.

Do:

In inventory/group_vars/production.yml:
---
company_is_production: true

In inventory/group_vars/testing.yml:
---
company_is_production: false

In roles/bind9/tasks/main.yml:
---
- name: some task
  when: "{{ company_is_production }}" <-- "Ok, this task will run if we're in the production environment"

Don't:

In inventory/group_vars/production.yml:
---
is_production: true

In inventory/group_vars/testing.yml:
---
is_production: false

In roles/bind9/tasks/main.yml:
---
- name: some task
  when: "{{ is_production }}" <-- "Hmm, is this a variable defined in this role / playbook?"

Prefix temporary variables with an underscore

This makes it easier to understand where a variable is defined. You can't differentiate a variable is defined in this playbook from one defined in hostvars. Use an underscore to let the reader know that this variable is temporary and locally scoped.

Do:

- ansible.builtin.debug:
    var: _app_version  <-- "Aha! This is a locally scoped variable. It must be defined in the surrounding block"

Don't:

- ansible.builtin.debug:
    var: app_version   <-- "Hmm, where is this variable defined?"

Prefix role variables with the role name

This avoids conflicts with other variables and makes it easy to understand where variables are being used.

Do:

In roles/bind9/defaults/main.yml:
---
bind9_recursion: true

In inventory/group_vars/resolvers.yml:
---
bind9_recursion: false <-- "The bind9 role will probably be affected if I remove this"

Don't:

In roles/bind9/defaults/main.yml:
---
recursion: true

In inventory/group_vars/resolvers.yml:
---
recursion: false  <-- "Hmm, will any roles be affected if I remove this?"

Create separate variables for vaulted secrets

Very useful if you need to grep to find out which hosts/groups has a variable defined. If the variable is in an encrypted file you cannot do this unless you decrypt all your vaulted files. Add 'vault' as a suffix to these variables

Do:

In inventory/hostvars/host1/main.yml:
---
bind9_tsig_key: "{{ bind9_tsig_key_vault }}" <-- Can be found even if the secret itself is encrypted

In inventory/hostvars/host1/vault.yml:
---
bind9_tsig_key_vault: password

Don't:

In inventory/hostvars/host1/vault.yml:
---
bind9_tsig_key: password <-- Can't be found unless this file is decrypted first

Construct complex variables outside of module arguments

This allows you to create temporary variables with names that clearly show the intent. Also allows you to add comments without cluttering the task up.

Do:

- name: Create database server config
  ansible.builtin.copy:
    content: |
      admin_users: {{ _global_and_local_admins }}
      allowed_hosts: {{ _webservers }}
    dest: "{{ _filename }}"
  vars:
    _filename: "{{ db_config_directory }}/{{ is_production | ternary('prod', 'testing') }}.config" # Depends on env
    _webservers: "{{ ','.join(groups.web | map('extract', hostvars, 'inventory_hostname')) }}" # Allow webservers
    _global_and_local_admins: "{{ global_admins | union(local_admins) }}" # Add both global and local administrators

Don't:

# Filename depends on env
# Add both global and local administrators
# Allow webservers
- name: Create database server config
  ansible.builtin.copy:
    content: |
      admin_users: "{{ global_admins | union(local_admins) }}"
      allowed_hosts: "{{ ','.join(groups.web | map('extract', hostvars, 'inventory_hostname')) }}" 
    dest: "{{ db_config_directory }}/{{ is_production | ternary('prod', 'testing') }}.config"

Annotate constructed variables

This makes it easier to understand what the constructed variable actually looks like.

Do:

- name: Create release notes
  ansible.builtin.template:
    src: release_note.j2
    dest: /var/www/release_notes/{{ _app_major_version }}/RELEASE.md
  vars:
    _app_major_version: "{{ app_version.split('.')[0] }}" # 10.12.3 -> 10

Use fully-qualified collection names

Do:

- name: Install packages
  ansible.builtin.apt: 

Don't:

- name: Install packages
  apt:

Avoid looping if the module accepts an interable

Looping in ansible is slow and should be avoided when possible.

Do:

- name: Install packages
  ansible.builtin.apt:
    name:
      - packet 1
      - packet 2

Don't:

- name: Install packages
  ansible.builtin.apt:
    name: "{{ item }}"
  loop: "{{ packages }}"

Avoid unnecessary quoting

Avoid cluttering by only using quotes when necessary.

Do:

- ansible.builtin.debug:
    msg: Print this

Don't:

- ansible.builtin.debug:
    msg: "Print this"

Don't use role meta dependencies

Using role dependencies introduces complexity and makes it harder to understand what will run when executing a role. Try to not import roles in other roles at all. In the below example I cannot run the nginx role without running letsencrypt.

Do:

In playbook.yml:
---
- ansible.builtin.import_role:  <-- "I'm running the nginx role"
  name: nginx

In roles/nginx/tasks/main.yml
---
- ansible.builtin.import_role:  <-- "I can clearly see that this role imports another role"
  name: letsencrypt

Don't:

In playbook.yml:
---
- ansible.builtin.import_role:  <-- "I'm running the nginx role"
  name: nginx

In roles/nginx/meta/main.yml:
---
dependencies:
  - role: letsencrypt           <-- "Apperently I'm running the letsencrypt role as well"

Roles should start with a block tagged with the role name

This allows you to at any time run this whole role by passing --tags <role_name> when executing a playbook. Role tasks can be tagged with more specific tags but should be inside the main role block. By using this method you don't need to tag every import_role in every playboook where you use the role.

Do:

In roles/nginx/tasks/main.yml:
---
- tags: nginx  <-- "Nice, I can run this role by just adding the role name to the playbook --tags flag"
  block:

    - name: Install nginx 
      ansible.builtin.apt:
        name: nginx
        state: present
      tags: nginx_install

    - name: Configure nginx
      ansible.builtin.template:
        src: nginx.conf.j2
        dest: /etc/nginx/nginx.conf
      tags: nginx_configure

Don't:

In roles/nginx/tasks/main.yml:
---
- name: Install nginx 
  ansible.builtin.apt:
    name: nginx
    state: present
  tags: nginx_install    <-- "Hmm, did I tag every import_role in every playbook with 'nginx'?"

- name: Configure nginx
  ansible.builtin.template:
    src: nginx.conf.j2
    dest: /etc/nginx/nginx.conf
  tags: nginx_configure  <-- "Or do I need to pass every nginx_* tag to the playbook?"

Role tasks should be split into task files when appropriate

When a part of a role becomes to lengthy it should be put in a separate file. I usually separate related tasks into a separate file if the tasks take up the entire height of the screen.

In roles/nginx/tasks/main.yml:
---
- tags: nginx  <-- "Nice, I can run this role by just adding the role name to the playbook --tags flag"
  block:

    - name: Install nginx 
      ansible.builtin.apt:
        name: nginx
        state: present
      tags: nginx_install

    - name: Configure nginx
      ansible.builtin.template:
        src: nginx.conf.j2
        dest: /etc/nginx/nginx.conf
      tags: nginx_configure

    - name: Perform a bunch of small optimization tasks
      ansible.builtin.import_tasks: optimizations.yml
      tags: nginx_optimize

Roles should elevate privileges when required, not playbooks

This avoids situtations where you're suddenly executing something that absolutely shouldn't run as root, as root.

Do:

In playbook.yml:
---
- hosts: resolvers
  
  tasks:
    - ansible.builtin.import_role:
      name: bind9

In roles/bind9/tasks/main.yml:
---
- name: Install bind9
  ansible.builtin.apt:
    name: bind9
    state: present
  become: true  <-- "Good, only tasks that require elevated privileges are run as root"

- name: Some task that doesn't require privileges
  ansible.collection.module:  <-- "Good, this runs as the user I'm connecting to the server with"
    parameter: value

Don't:

In playbook.yml:
---
- hosts: resolvers
  become: true  <-- "Oops! Now all tasks are run as root. Even those tasks that don't require it!"

  tasks:
    - ansible.builtin.import_role:
      name: bind9

In roles/bind9/tasks/main.yml:
---
- name: Install bind9
  ansible.builtin.apt:
    name: bind9
    state: present

- name: Some task that doesn't require privileges
  ansible.collection.module:  <-- "Oops! This task run as root"
    parameter: value

Interal role variables should be prefixed with double underscores

An internal variable is a variable that's used by the role but not expected be overrided using defaults. This is done to differentiate internal variables from default vars for the reader.

Do:

In roles/app/vars/main.yml
---
# We don't allow the user to override the filepath
__app_config_location: /etc/app/app.config

In roles/app/tasks/main.yml:
---
- name: Backup app configuration
  ansible.builtin.template:
    src: "{{ __app_config_location }}"       # <-- "Ah! I'm not allowed to override this"
    dest: "{{ app_config_location_backup }}" # <-- "Ah! I'm allowed to override this"

- name: Generate app configuration
  ansible.builtin.template:
    src: app_config.j2
    dest: "{{ __app_config_location }}"