aristanetworks / ansible-cvp

Ansible modules for Arista CloudVision
http://cvp.avd.sh
Apache License 2.0
66 stars 61 forks source link

The use of lookup('file', '...) for configlets fails input validation #313

Closed kjellrg closed 3 years ago

kjellrg commented 3 years ago

Issue Type

Summary

The use of lookup('file', 'path') for configlets fails after input validation with JSON schema was added.

e.g.: LEAF-DEFAULT: "{{ lookup('file', '../arista/configs/LEAF-DEFAULT.txt') }}"

cv_configlet tasks fails with: fatal: [server01]: FAILED! => {"changed": false, "msg": "Configlet input data are not compliant with module."}

Role or Module Name

Module name: module name

arista.cvp collection and Python libraries version

# Ansible version
ansible 2.9.19

# ansible-cvp version
v2.1.1

OS / Environment

Cloudvision version

2020.3.1

EOS Version

4.25.2F

OS running Ansible

Ubuntu

Steps to reproduce

Have a configlet in configlet_list use the lookup() function

  LEAF-DEFAULT: "{{ lookup('file', '../arista/configs/LEAF-DEFAULT.txt') }}"
---

Expected results

Have the cv_configlet task complete and update the configlets on CVP if there are any changes to the file specified.

$

Actual results

cv_configlet tasks fails with: fatal: [server01]: FAILED! => {"changed": false, "msg": "Configlet input data are not compliant with module."}

Edit the file: ansible-cvp/ansible_collections/arista/cvp/plugins/module_utils/schema.py and changing patternProperties under SCHEMA_CV_CONFIGLET to ".*" makes the configlets pass validation and work as before. Not sure what proper regex to use to allow for the lookup() function.

$
titom73 commented 3 years ago

Hello @kjellrg

As per this error message, it seems var structure for your configlet is not valid.

I have run the following playbook successfully. Can you test with your own setup ?

---
- name: Upload configlet
  hosts: cv_server
  connection: local
  gather_facts: false
  collections:
    - arista.avd
    - arista.cvp
  vars:
    CVP_CONFIGLETS:
      01DEMO-alias: "alias a{{ 999 | random }} show version"
      01DEMO-01: "alias a{{ 999 | random }} show version"
      01DEMO-02: "alias sib show interface brief"
      01DEMO-03: "{{lookup('file', '../configlets/DEVICE-ALIASES.conf')}}"

  tasks:
    - name: "Gather CVP facts {{inventory_hostname}}"
      arista.cvp.cv_facts:
        facts:
          configlets
      register: CVP_FACTS

    - name: "Configure configlet on {{inventory_hostname}}"
      arista.cvp.cv_configlet:
        cvp_facts: "{{CVP_FACTS.ansible_facts}}"
        configlets: "{{CVP_CONFIGLETS}}"
        configlet_filter: ["DEMO"]
        state: present
      register: CVP_CONFIGLET_RESULT

Note that lookup is done based on playbook_dir

kjellrg commented 3 years ago

Hi @titom73

Tested your example and that worked. Did some more testing, and it seems the error message is triggered when we use configlet names with spacing in the name, e.g.

configlet_list:
  NL00007-08 MLAG INTERFACES: "{{ lookup('file', '../arista-netbox-interface-config/configs/NL00007-08 MLAG_INTERFACES.txt') }}"

Tried replacing the spaces with "-" in the configlet name and it passed. Ended up with adding "\ " to the regex, which passes validation when using spaces in the configlet name

ansible-cvp/ansible_collections/arista/cvp/plugins/module_utils/schema.py

"patternProperties": {
    "^[A-Za-z0-9\ \\\._%\\+-]+$": {
        "type": "string"
    }
titom73 commented 3 years ago

Hi @kjellrg

Thanks for the feedback and good catch for space missing in regexp field. I will do a PR to add support for space in this string.

Thanks