Ansible Best Practices Suggestions for MikroTik Playbooks

[JanWelker]

After reviewing your MikroTik Ansible playbooks, here are my recommendations for improvements based on Ansible best practices:

Structure and Organization

Convert to Roles-Based Structure

roles/
  mikrotik-common/
    defaults/
    tasks/
    handlers/
    templates/
  mikrotik-backup/
  mikrotik-security/
  mikrotik-network/

Recommendation: Convert your playbooks into roles for better organization and reusability. Group related functionality (backup, configuration, security) into separate roles.

Use Include/Import for Common Tasks
Current Issue: Several playbooks repeat the same tasks (e.g., creating backup directories).
Recommendation: Create task files for common operations and use include_tasks or import_tasks.

# common-tasks/create-backup-dir.yml
- name: Create backups folder in the ansible control host
  ansible.builtin.file:
    path: "{{ local_backups_top_folder }}/{{ inventory_hostname }}"
    state: directory
    mode: '0700'
  delegate_to: localhost
  become: false

Clean Up Commented Code
Current Issue: Many playbooks contain commented-out code blocks.
Recommendation: Remove commented code or document why it's kept for reference.

Variable Management

Use Default Variables
Recommendation: Define default variables in role defaults and override them in inventory when needed.

# roles/mikrotik-backup/defaults/main.yml
local_backups_top_folder: "{{ playbook_dir }}/../backups"
backup_retention_days: 30

Use Ansible Vault for Sensitive Data
Current Issue: Credentials appear to be stored in inventory variables.
Recommendation: Use Ansible Vault for sensitive data like API credentials.

# Create encrypted file
ansible-vault create group_vars/mikrotik/vault.yml

# Reference in your playbook
- name: Configure RouterOS switches
  hosts: mikrotik
  vars_files:
    - group_vars/mikrotik/vault.yml

Standardize Variable Naming
Current Issue: Variable naming is inconsistent (mix of routeros_ prefix and others).
Recommendation: Adopt a consistent naming convention for all variables.

Error Handling and Idempotency

Improve Error Handling
Current Issue: Basic error handling with ignore_errors: True in some playbooks.
Recommendation: Use block/rescue/always for more sophisticated error handling.

- name: Backup configuration with proper error handling
  block:
    - name: Perform backup
      community.routeros.command:
        commands: "/export show-sensitive terse file={{ inventory_hostname }}.cfg.backup"
  rescue:
    - name: Log backup failure
      ansible.builtin.debug:
        msg: "Backup failed for {{ inventory_hostname }}"
    - name: Send notification
      ansible.builtin.mail:
        subject: "Backup failed for {{ inventory_hostname }}"
  always:
    - name: Ensure cleanup happens
      community.routeros.command:
        commands: "/file/remove {{ inventory_hostname }}.cfg.backup"
      ignore_errors: yes

Add Conditional Checks
Recommendation: Add checks before operations to ensure idempotency.

- name: Check if backup file exists
  ansible.builtin.stat:
    path: "{{ local_backups_top_folder }}/{{ inventory_hostname }}/{{ inventory_hostname }}.cfg.backup.rsc"
  register: backup_stat
  delegate_to: localhost

- name: Perform backup only if needed
  block:
    # Backup tasks
  when: not backup_stat.stat.exists or backup_stat.stat.mtime < (ansible_date_time.epoch|int - 86400)

Task Naming and Documentation

Improve Task Names
Current Issue: Some task names are generic or unclear.
Recommendation: Make task names more descriptive and action-oriented.

# Before
- name: "Import the certificates in the switch"

# After
- name: "Import CA certificate into RouterOS certificate store"

Add Playbook Documentation
Recommendation: Add comprehensive documentation at the beginning of each playbook.

---
# Playbook: mikrotik-backup-config.yml
# Description: Creates configuration backups of MikroTik devices
# 
# Prerequisites:
#   - Network connectivity to MikroTik devices
#   - Valid API credentials in inventory
#
# Variables:
#   - local_backups_top_folder: Directory to store backups
#
# Tags:
#   - backup
#   - config

Security Enhancements

Improve Certificate Management
Current Issue: Certificate generation is good but could be more secure.
Recommendation: Add more security options and validation.

- name: Create SSL private key with stronger parameters
  community.crypto.openssl_privatekey:
    path: "{{ local_certs_destination_folder }}/{{ local_ca_privkey }}"
    type: 'RSA'
    size: 4096  # Increased from 2048
    cipher: auto
    passphrase: "{{ ca_key_passphrase }}"

Add Backup Encryption
Recommendation: Encrypt sensitive backup files.

- name: Encrypt backup file
  ansible.builtin.command:
    cmd: "openssl enc -aes-256-cbc -salt -in {{ local_backups_top_folder }}/{{ inventory_hostname }}/{{ inventory_hostname }}.cfg.backup.rsc -out {{ local_backups_top_folder }}/{{ inventory_hostname }}/{{ inventory_hostname }}.cfg.backup.rsc.enc -k {{ backup_encryption_key }}"
  delegate_to: localhost
  when: encrypt_backups | default(true)

Performance and Efficiency

Use API Instead of Commands Where Possible
Current Issue: Some playbooks use community.routeros.command where API calls would be more efficient.
Recommendation: Replace command modules with API modules where possible.

# Before
- name: "Starting ROS Configuration Backup to memory"
  community.routeros.command:
    commands: "/export show-sensitive terse file={{ inventory_hostname }}.cfg.backup"

# After (if possible with API)
- name: "Export configuration using API"
  community.routeros.api_command:
    path: "system backup"
    command: "save"
    args:
      name: "{{ inventory_hostname }}.cfg.backup"

Add Tags for Selective Execution
Current Issue: Some playbooks have tags, but they could be more comprehensive.
Recommendation: Add tags to all tasks for more granular execution.

- name: Write the config to switch's memory and copy it to ansible control host
  block:
    # Tasks
  tags: 
    - backup
    - config
    - export

Testing and Validation

Add Pre-flight Checks
Recommendation: Add validation tasks before making changes.

- name: Verify connectivity before making changes
  community.routeros.command:
    commands: "/system identity print"
  register: identity_check
  failed_when: identity_check.failed

- name: Verify API access
  community.routeros.api_info:
    path: "system identity"
  register: api_check

Add Post-change Validation
Recommendation: Validate changes after they're applied.

- name: Verify configuration after changes
  community.routeros.command:
    commands: "/system resource print"
  register: post_change_check
  failed_when: post_change_check.failed

Specific Playbook Improvements

mikrotik-backup-config.yml and mikrotik-backup-system.yml
Recommendation: Combine these into a single role with different tasks.

mikrotik-configure.yml
Recommendation: Split this large playbook into smaller, focused roles.
Issue: The commented rollback mechanism should be implemented properly.

mikrotik-enable-api-ssl.yml
Issue: Duplicate task names for certificate imports.
Recommendation: Use unique, descriptive names for each certificate import.

mikrotik-dump-cfg-vars-to-disk.yml
Issue: Very large list of API endpoints hardcoded in the playbook.
Recommendation: Move this to a separate variable file or use dynamic discovery.

By implementing these suggestions, your Ansible playbooks will be more maintainable, secure, and aligned with best practices. The role-based structure will make it easier to reuse code and maintain consistency across your infrastructure.

[pescobar]

Hi @JanWelker ! ;) thanks for reviewing the playbooks

I helped @mischadiehm to develop these playbooks so I will add some comments.

Convert to Roles-Based Structure

Indeed using roles is a good practice and I try to apply it as much as possible but in this specific case where most of the playbooks only interact with the api IMHO adding so many roles will add extra complexity for little advantage

• Every role will have empty folders files and templates because we will never use files or templates to configure mikrotik network devices
• file defaults/main.yml would be a set of empty vars because there are not sensible defaults that apply to every device
• instead of using defaults we use a sanity check like this one in most tasks in the playbook to easily identify which var applies to each task. IMO this is simpler an easier to read and we provide examples in the example inventory which can be adapted for each use case

In any case, even if I think the current approach without roles is simpler and easier to read and maintain (for this specific use case) this is something subjective and using roles is always a good practice in ansible so do it as it works better for you.

Use Default Variables

I think the most sensible value for defaults would be an empty var e.g. routeros_interface_bridge_port: {} so I prefer providing an example inventory as the one included in this repo so the users can see real world examples and adapt them to their needs.

We also do jinja templating in the inventory to create the final json that is sent to the api (see this example) so providing useful defaults is not as simple as in other use cases

Use Ansible Vault for Sensitive Data

Indeed this should be done.

Standardize Variable Naming

We are doing it but probably some fixes are missing. See https://github.com/narrowin/ansible-mikrotik?tab=readme-ov-file#naming-convention-for-inventory-files-and-variables

Improve Error Handling and Add Conditional Checks

A nice improvement to add. We first focused in getting it working and we could not find the time for this yet but we should.

Improve Task Names

This is something that can also be improved. Also a little bit subjective.

Add Playbook Documentation

For sure this is always something that can be improved

Improve Certificate Management

A nice improvement. I remember these devices had some limitations about supported formats .e.g only rsa ssh keys are supported if I remember correctly? What ssl certificates are supported should also be tested.

Add Backup Encryption

For this I prefer git-crypt but your solution is also good. A matter of taste I guess.

Use API Instead of Commands Where Possible

We use API whenever possible but some tasks cannot be done using the api.

• IIRC api doesn't support triggering backups
• the playbook enabling the API and configuring the SSL cert for the api must use command because API is still not available until we execute this playbook

Add Tags for Selective Execution

I think most tasks have tags. This can be improved for sure.

Add Pre-flight Checks and Add Post-change Validation

A nice addition. By now we execute the playbook with --check --diff to preview changes before applying them

Specific Playbook Improvements mikrotik-configure.yml

The order of this playbook matters. IMO having a single file makes easier to read/debug/extend it

Thanks again for your feedback! :)

[mischadiehm]

Hi @JanWelker and @pescobar,

thanks for your inputs. @th-nw has made a prototype with roles so lets discuss based on that. He will commit it one of these days.

We have documented that the use of ansible-vault and ssl-enabled api are a must in prod environments but please understand that this repo should have the lowest possible entry barrier for new people to test and use what we provide and both of these things are not needed in test labs and not needed from day 0 so lets close the discussion on those please.

Thanks and lets keep this movin'

[tomashoryl]

Hi @JanWelker and @pescobar, thanks for your valuable comments!

Because using the repo for a side-project which is heavily focused on L3 features, I already realized that indeed splitting the playbook into roles would make sense. So I started pulling certain tasks into separate roles and changing the playbook to use these roles. Which is fine for common tasks (like those without any dependencies). But as soon as the role itself is dependent on another one, things start to be tricky, let's assume this scenario:

• I don't want to create role for each specific feature in order to not end up with tons of empty files. So I'm grouping various related features into a single role (like setting up clock, logging, DNS,...)
• Let's assume I have a role which also contains creation of interface lists.
• Now, I have another role which is dependent on interface lists, but I don't want to run all the other tasks from the role that contains the interface list creation.
• If I specify the dependency in meta/tasks.yml it obviously invokes the whole "parent" role execution which I might want to avoid for various reasons. Even worse, what if the dependent role gets invoked because it's hit by a tag, but the interface lists role doesn't have the same tag?

I might be making up problems here, but so far I got stuck on this dependency cross-referencing. Obviously creating role for each specific task would solve the problem, but add to the overall complexity. Also, carefully assigning tags would also help, but that is too fragile and error prone. Do you have any suggestions?

Thanks in advance.