ansible-meraki icon indicating copy to clipboard operation
ansible-meraki copied to clipboard

Added function for clearing the policy

Open y0rune opened this issue 2 years ago • 8 comments

Hello! I resolved the issue with #342. I added the function for clearing the policy when that policy is null. I have also added the tests but I was using a creating and deleting a access policy from my module PR #346 for that specific scenario which I described in #342 .

Another solution for testing: to hide or remove the code:

  • https://github.com/CiscoDevNet/ansible-meraki/pull/350/files#diff-c7551441a000d95142233dd4377aace76e328d696ab05d6941590e3d3c19a03bR727-R745

  • https://github.com/CiscoDevNet/ansible-meraki/pull/350/files#diff-c7551441a000d95142233dd4377aace76e328d696ab05d6941590e3d3c19a03bR800-R807

after that you will disable the module from #346, but the another requirement is to create a access policy for switch.

Thank you!

y0rune avatar Sep 02 '22 21:09 y0rune

I think it is ready to merge @kbreit ;)

y0rune avatar Sep 11 '22 11:09 y0rune

I am trying to understand the logic and need to sit down with the documentation and compare it to the code. Once I see how it works I'll test and merge.

kbreit avatar Sep 11 '22 11:09 kbreit

All right! Thank you!

y0rune avatar Sep 11 '22 11:09 y0rune

@y0rune I'd like to pick this up again. Do you have a sandbox that would work?

kbreit avatar Jan 16 '23 03:01 kbreit

Yup yup, I will today check it ;)

y0rune avatar Jan 16 '23 07:01 y0rune

I only hide the test of the flexible due to I do not have switch with stacking:

diff --git a/tests/integration/targets/meraki_ms_switchport/tasks/main.yml b/tests/integration/targets/meraki_ms_switchport/tasks/main.yml
index dd7b624..990bde2 100644
--- a/tests/integration/targets/meraki_ms_switchport/tasks/main.yml
+++ b/tests/integration/targets/meraki_ms_switchport/tasks/main.yml
@@ -147,23 +147,23 @@
     that:
       - update_access_port.data.vlan == 10

-- name: Configure flexible stacking
-  meraki_switchport:
-    auth_key: '{{auth_key}}'
-    state: present
-    serial: '{{ serial_switch_l3 }}'
-    number: 7
-    enabled: true
-    flexible_stacking_enabled: true
-  delegate_to: localhost
-  register: flex_stacking_enabled
+# - name: Configure flexible stacking
+#   meraki_switchport:
+#     auth_key: '{{auth_key}}'
+#     state: present
+#     serial: '{{ serial_switch_l3 }}'
+#     number: 7
+#     enabled: true
+#     flexible_stacking_enabled: true
+#   delegate_to: localhost
+#   register: flex_stacking_enabled

-- debug:
-    msg: '{{flex_stacking_enabled}}'
+# - debug:
+#     msg: '{{flex_stacking_enabled}}'

-- assert:
- 
   that:
-      - flex_stacking_enabled.data.flexible_stacking_enabled == true
+# - assert:
+#     that:
+#       - flex_stacking_enabled.data.flexible_stacking_enabled == true

I run the test and it worked. I committed the changes after that ;)
Output: output_of_the_tests.txt

y0rune avatar Jan 16 '23 20:01 y0rune

I have that test playbook with tasks:

- name: Create access policy with auth_method is "my Radius Server"
  cisco.meraki.meraki_ms_access_policies:
    auth_key: "{{ auth_key }}"
    access_policy_type: "802.1x"
    host_mode: "Single-Host"
    state: present
    name: "Meraki authentication policy 3"
    auth_method: "my RADIUS server"
    radius_servers:
      - host: 192.0.1.18
        port: 7890
        secret: secret123
    org_name: "{{ test_org_name }}"
    net_name: "{{ test_net_name }}"
    radius_coa_enabled: false
    radius_accounting_enabled: false
    guest_vlan: 10
    voice_vlan_clients: false
  delegate_to: localhost

- name: Configure port
  cisco.meraki.meraki_switchport:
    auth_key: "{{ auth_key }}"
    serial: "{{ serial_switch }}"
    name: "Port_with_access_policy"
    state: present
    number: 7
    type: access
    poe_enabled: true
    access_policy_type: "Custom access policy"
    access_policy_number: 1
  delegate_to: localhost
  register: port_update

- name: Query switchports
  meraki_switchport:
    auth_key: "{{ auth_key }}"
    state: query
    serial: "{{ serial_switch }}"
    number: 7
  delegate_to: localhost
  register: query_port_seven

- name: Checking if query_port_seven has changed the type to access
  ansible.builtin.assert:
    that:
      - query_port_seven.data.type == "access"
      - port_update is changed

- name: Configure port - Update
  cisco.meraki.meraki_switchport:
    auth_key: "{{ auth_key }}"
    serial: "{{ serial_switch }}"
    name: "Port_with_access_policy"
    state: present
    number: 7
    type: trunk
    allowed_vlans:
      - 1
      - 15
    poe_enabled: true
  delegate_to: localhost
  register: port_update

When I was on that PR (commits) It is working. When I revert the changes to the default ones I got the error:

TASK [meraki_ms_switchport : Configure port - Update] **************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "HTTP error 400 - https://api.meraki.com/api/v1/devices/Q2HP-DT5F-KMJE/switch/ports/7 - Trunk ports do not support user-defined Access Policies", "response": "OK (unknown bytes)", "status": 400}

So in the default code I could not change from access with access_policy to trunk. I can add additional task to change the port access with access_policy to access with Open and to trunk, but I think it is so much complicated to create another task to only change the type of the access policy. Of course if that action is right to you.

Example test playbook when it worked (with additional tasks):

- name: Create access policy with auth_method is "my Radius Server"
  cisco.meraki.meraki_ms_access_policies:
    auth_key: "{{ auth_key }}"
    access_policy_type: "802.1x"
    host_mode: "Single-Host"
    state: present
    name: "Meraki authentication policy 3"
    auth_method: "my RADIUS server"
    radius_servers:
      - host: 192.0.1.18
        port: 7890
        secret: secret123
    org_name: "{{ test_org_name }}"
    net_name: "{{ test_net_name }}"
    radius_coa_enabled: false
    radius_accounting_enabled: false
    guest_vlan: 10
    voice_vlan_clients: false
  delegate_to: localhost

- name: Configure port
  cisco.meraki.meraki_switchport:
    auth_key: "{{ auth_key }}"
    serial: "{{ serial_switch }}"
    name: "Port_with_access_policy"
    state: present
    number: 7
    type: access
    poe_enabled: true
    access_policy_type: "Custom access policy"
    access_policy_number: 1
  delegate_to: localhost
  register: port_update

- name: Configure port
  cisco.meraki.meraki_switchport:
    auth_key: "{{ auth_key }}"
    serial: "{{ serial_switch }}"
    name: "Port_with_access_policy"
    state: present
    number: 7
    type: access
    poe_enabled: true
    access_policy_type: "Open"
  delegate_to: localhost
  register: port_update

- name: Query switchports
  meraki_switchport:
    auth_key: "{{ auth_key }}"
    state: query
    serial: "{{ serial_switch }}"
    number: 7
  delegate_to: localhost
  register: query_port_seven

- name: Checking if query_port_seven has changed the type to access
  ansible.builtin.assert:
    that:
      - query_port_seven.data.type == "access"
      - port_update is changed

- name: Configure port - Update
  cisco.meraki.meraki_switchport:
    auth_key: "{{ auth_key }}"
    serial: "{{ serial_switch }}"
    name: "Port_with_access_policy"
    state: present
    number: 7
    type: trunk
    allowed_vlans:
      - 1
      - 15
    poe_enabled: true
  delegate_to: localhost
  register: port_update

- name: Query switchport
  meraki_switchport:
    auth_key: "{{ auth_key }}"
    state: query
    serial: "{{ serial_switch }}"
    number: 7
  delegate_to: localhost
  register: query_port_seven

- name: Checking if query_port_seven has changed the type to trunk
  ansible.builtin.assert:
    that:
      - query_port_seven.data.type == "trunk"
      - port_update is changed

- name: Delete access policy
  cisco.meraki.meraki_ms_access_policies:
    auth_key: "{{ auth_key }}"
    state: absent
    number: 1
    net_id: "{{ test_net_id }}"
  delegate_to: localhost
  register: delete_access_policy
PLAY RECAP *********************************************************************
localhost                  : ok=9    changed=5    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

y0rune avatar Jan 21 '23 13:01 y0rune

The situation makes sense. I'm going to have to experiment with the patch to see how it could negatively affect playbooks. As I said, my biggest concern is if someone doesn't include access_policy_type, if it sets it to Open the module is making a change they didn't ask for. And in this situation, it would be a security problem.

kbreit avatar Jan 21 '23 20:01 kbreit