CiscoDevNet / ansible-dcnm

Apache License 2.0
47 stars 37 forks source link

dcnm_fabric: Fixes for input validation #334

Closed allenrobel closed 1 month ago

allenrobel commented 1 month ago

Community Note

Ansible Version and collection version

ansible [core 2.16.6]
  config file = /Users/arobel/repos/ansible_dev/dcnm_fabric/playbooks/ansible.cfg
  configured module search path = ['/Users/arobel/repos/ansible_dev/dcnm_fabric/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /Users/arobel/repos/ansible_dev/dcnm_fabric/ansible_collections/cisco/dcnm/.venv/lib/python3.12/site-packages/ansible
  ansible collection location = /Users/arobel/repos/ansible_dev/dcnm_fabric
  executable location = /Users/arobel/repos/ansible_dev/dcnm_fabric/ansible_collections/cisco/dcnm/.venv/bin/ansible
  python version = 3.12.0 (v3.12.0:0fb18b02c8, Oct  2 2023, 09:45:56) [Clang 13.0.0 (clang-1300.0.29.30)] (/Users/arobel/repos/ansible_dev/dcnm_fabric/ansible_collections/cisco/dcnm/.venv/bin/python)
  jinja version = 3.1.4
  libyaml = True

DCNM version

Affected module(s)

Ansible Playbook

Certain combinations of ENABLE_SGT, ENABLE_PVLAN, and UNDERLAY_IS_V6 result (inappropriately) in errors.

Very likely this impacts other parameters not mentioned above.

    -   name: merged fabrics (dcnm_send)
        cisco.dcnm.dcnm_fabric:
            state: merged
            config:
            -   FABRIC_NAME: FOO
                FABRIC_TYPE: VXLAN_EVPN
                BGP_AS: 65001
                ENABLE_SGT: false # various combinations
                ENABLE_PVLAN: false # various combinations
                UNDERLAY_IS_V6: false # various combinations

Debug Output

Root case analysis is complete. Debug output not needed.

Expected Behavior

While some errors are appropriate for the above parameters, others are not. In particular, the following behaviors are expected (based on GUI behavior):

GUI behavior

  1. If UNDERLAY_IS_V6 is enabled, ENABLE_SGT is disabled (greyed out)
  2. If ENABLE_PVLAN is enabled, ENABLE_SGT is disabled (greyed out)
  3. If ENABLE_SGT is enabled, ENABLE_PVLAN is disabled (greyed out)

PLAYBOOK implications

  1. Per GUI.1
    • UNDERLAY_IS_V6 must be True (or commented out) to configure ENABLE_SGT
  2. Per GUI.2
    • ENABLE_PVLAN must be False (or commented out) to configure ENABLE_SGT
  3. Per GUI.3
    • ENABLE_SGT must be False (or commented out) to configure ENABLE_PVLAN
  4. Per GUI.3 and GUI.4
    • ENABLE_PVLAN and ENABLE_SGT are mutually-exclusive so cannot both be in a playbook.

NOTES

  1. Depending on the current state on the controller (e.g. if the fabric exists and merged state is used in the playbook) the controller value may play a part. However, the playbook value always takes precedence over the controller value.
  2. Precedence during parameter verification
    • Playbook takes precedence over controller
    • Controller takes precedence over template default value
    • Template default value is considered only when the parameter is not in the playbook and the fabric does not exist.

Actual Behavior

Errors are experienced for some valid combinations of parameters.

Root cause analysis

There were a couple issues with the existing code.

  1. A conditional was evaluating a (potential None) value as if not <value> when it should have been if <value> is False

  2. If checks for all sources for the parameter value (playbook, controller, template default) return None (i.e. the parameter should not be considered when validating the dependent parameter), then we now set the decision_set to {True} (meaning "accept" the parameter as valid).

References

PR #335