From 2db6a8c26ac39f91e1c8e08abea53581fae2b957 Mon Sep 17 00:00:00 2001 From: Deepak Agrawal Date: Tue, 5 Jun 2018 07:56:53 +0530 Subject: [PATCH] iosxr_config crash if config has route-policy with multiple levels of 'ifelseif' and other caveats (#41091) * diff in as-path-set or prefix-set * fix caveat diff can not have last line with comma in prefix-set/as-path/community-set * Simplify fix to include indentation before parse * remove debugger * route-policy diffs * fix iosxr_config crash issue * new changes in iosxr_config after git add * end-policy-map and end-class-map are properly indented so match misplaced children only when end-* is at the beigining also fix pep8 * Remaining config blocks of route-policy which needs exclusion from diff. added new tests * pylint/pep8 warnings * Review comments , sanity test fix * shbang warning * remove unused import --- .../modules/network/iosxr/iosxr_config.py | 131 ++++++++++++++---- .../templates/basic/route_policy.j2 | 121 ++++++++++++++++ .../templates/basic/route_policy_change.j2 | 65 +++++++++ .../templates/basic/route_policy_clean.j2 | 32 +++++ .../iosxr_config/tests/cli/route_policy.yaml | 53 +++++++ 5 files changed, 374 insertions(+), 28 deletions(-) create mode 100644 test/integration/targets/iosxr_config/templates/basic/route_policy.j2 create mode 100644 test/integration/targets/iosxr_config/templates/basic/route_policy_change.j2 create mode 100644 test/integration/targets/iosxr_config/templates/basic/route_policy_clean.j2 create mode 100644 test/integration/targets/iosxr_config/tests/cli/route_policy.yaml diff --git a/lib/ansible/modules/network/iosxr/iosxr_config.py b/lib/ansible/modules/network/iosxr/iosxr_config.py index 9cdea3083c7..50d9b31b139 100644 --- a/lib/ansible/modules/network/iosxr/iosxr_config.py +++ b/lib/ansible/modules/network/iosxr/iosxr_config.py @@ -186,7 +186,38 @@ from ansible.module_utils.network.common.config import NetworkConfig, dumps DEFAULT_COMMIT_COMMENT = 'configured by iosxr_config' CONFIG_MISPLACED_CHILDREN = [ - re.compile(r'end-\s*(.+)$') + re.compile(r'^end-\s*(.+)$') +] + +# Objects defined in Route-policy Language guide of IOS_XR. +# Reconfiguring these objects replace existing configurations. +# Hence these objects should be played direcly from candidate +# configurations +CONFIG_BLOCKS_FORCED_IN_DIFF = [ + { + 'start': re.compile(r'route-policy'), + 'end': re.compile(r'end-policy') + }, + { + 'start': re.compile(r'prefix-set'), + 'end': re.compile(r'end-set') + }, + { + 'start': re.compile(r'as-path-set'), + 'end': re.compile(r'end-set') + }, + { + 'start': re.compile(r'community-set'), + 'end': re.compile(r'end-set') + }, + { + 'start': re.compile(r'rd-set'), + 'end': re.compile(r'end-set') + }, + { + 'start': re.compile(r'extcommunity-set'), + 'end': re.compile(r'end-set') + } ] @@ -214,46 +245,93 @@ def check_args(module, warnings): 'removed in the future') +# A list of commands like {end-set, end-policy, ...} are part of configuration +# block like { prefix-set, as-path-set , ... } but they are not indented properly +# to be included with their parent. sanitize_config will add indentation to +# end-* commands so they are included with their parents +def sanitize_config(config, force_diff_prefix=None): + conf_lines = config.split('\n') + for regex in CONFIG_MISPLACED_CHILDREN: + for index, line in enumerate(conf_lines): + m = regex.search(line) + if m and m.group(0): + if force_diff_prefix: + conf_lines[index] = ' ' + m.group(0) + force_diff_prefix + else: + conf_lines[index] = ' ' + m.group(0) + conf = ('\n').join(conf_lines) + return conf + + +def mask_config_blocks_from_diff(config, candidate, force_diff_prefix): + conf_lines = config.split('\n') + candidate_lines = candidate.split('\n') + + for regex in CONFIG_BLOCKS_FORCED_IN_DIFF: + block_index_start_end = [] + for index, line in enumerate(candidate_lines): + startre = regex['start'].search(line) + if startre and startre.group(0): + start_index = index + else: + endre = regex['end'].search(line) + if endre and endre.group(0): + end_index = index + new_block = True + for prev_start, prev_end in block_index_start_end: + if start_index == prev_start: + # This might be end-set of another regex + # otherwise we would be having new start + new_block = False + break + if new_block: + block_index_start_end.append((start_index, end_index)) + + for start, end in block_index_start_end: + diff = False + if candidate_lines[start] in conf_lines: + run_conf_start_index = conf_lines.index(candidate_lines[start]) + else: + diff = False + continue + for i in range(start, end + 1): + if conf_lines[run_conf_start_index] == candidate_lines[i]: + run_conf_start_index = run_conf_start_index + 1 + else: + diff = True + break + if diff: + run_conf_start_index = conf_lines.index(candidate_lines[start]) + for i in range(start, end + 1): + conf_lines[run_conf_start_index] = conf_lines[run_conf_start_index] + force_diff_prefix + run_conf_start_index = run_conf_start_index + 1 + + conf = ('\n').join(conf_lines) + return conf + + def get_running_config(module): contents = module.params['config'] if not contents: contents = get_config(module) + if module.params['src']: + contents = mask_config_blocks_from_diff(contents, module.params['src'], "ansible") + contents = sanitize_config(contents) return NetworkConfig(indent=1, contents=contents) def get_candidate(module): candidate = NetworkConfig(indent=1) if module.params['src']: - candidate.load(module.params['src']) + config = module.params['src'] + config = sanitize_config(config) + candidate.load(config) elif module.params['lines']: parents = module.params['parents'] or list() candidate.add(module.params['lines'], parents=parents) return candidate -def sanitize_candidate_config(config): - last_parents = None - for regex in CONFIG_MISPLACED_CHILDREN: - for index, line in enumerate(config): - if line._parents: - last_parents = line._parents - m = regex.search(line.text) - if m and m.group(0): - config[index]._parents = last_parents - - -def sanitize_running_config(config): - last_parents = None - for regex in CONFIG_MISPLACED_CHILDREN: - for index, line in enumerate(config): - if line._parents: - last_parents = line._parents - m = regex.search(line.text) - if m and m.group(0): - config[index].text = ' ' + m.group(0) - config[index]._parents = last_parents - - def run(module, result): match = module.params['match'] replace = module.params['replace'] @@ -266,9 +344,6 @@ def run(module, result): candidate_config = get_candidate(module) running_config = get_running_config(module) - sanitize_candidate_config(candidate_config.items) - sanitize_running_config(running_config.items) - commands = None if match != 'none' and replace != 'config': commands = candidate_config.difference(running_config, path=path, match=match, replace=replace) diff --git a/test/integration/targets/iosxr_config/templates/basic/route_policy.j2 b/test/integration/targets/iosxr_config/templates/basic/route_policy.j2 new file mode 100644 index 00000000000..ecc68e81470 --- /dev/null +++ b/test/integration/targets/iosxr_config/templates/basic/route_policy.j2 @@ -0,0 +1,121 @@ +router ospf 1 + area 0 +! +prefix-set EBGP-PEER-BOGONS + 0.0.0.0/0, + 0.0.0.0/8 le 32, + 10.0.0.0/8 le 32, + 127.0.0.0/8 le 32, + 169.254.0.0/16 le 32, + 172.16.0.0/12 le 32, + 192.0.0.0/24 le 32, + 192.0.2.0/24 le 32, + 192.168.0.0/16 le 32, + 198.18.0.0/15 le 32, + 224.0.0.0/4 le 32, + 240.0.0.0/4 le 32 +end-set +! +prefix-set cust-ddos-DDOS +end-set +! +prefix-set cust-no-export +end-set +! +prefix-set acme_DC_Internal + 137.1.0.0/16, + 137.1.16.0/24, + 137.1.18.0/24, + 137.1.20.0/24, + 137.1.22.0/24, + 137.1.23.0/24, + 137.1.24.0/24, + 137.1.29.0/24, + 137.1.30.0/24, + 137.1.31.0/24, + 137.1.32.0/21, + 137.1.40.0/22, + 209.1.0.0/16 +end-set +! +as-path-set EBGP-PEER-AS16509-403-PERMIT-PATHS + ios-regex '^11164_8075_', + ios-regex '^11164_16509$', + ios-regex '^1116_16509_[0-9]+$', + ios-regex '^8075_', + ios-regex '^16509$', + ios-regex '^16509_[0-9]+$' +end-set +! +community-set cust-announce + 1525:65298, + 1525:65436, + 1525:65438, + 1525:65439, + 1525:65498, + 1525:65511, + 1523:65418, + 1523:65436, + 1523:65438 +end-set +! +community-set cust-no-export + 1525:65439, + 1525:65511, + 1525:65535 +end-set +! + +route-policy POLICY2 +end-policy +! +route-policy cust2bgp + set origin igp + set next-hop 137.1.16.12 +end-policy +! +rd-set ebpg-1 +end-set +! +rd-set EBGP_INCOMING_RD_SET + 172.16.0.0/16:*, + 172.17.0.0/16:100, + 192:*, + 192:100 +end-set +! +extcommunity-set rt EBGP_INCOMIG_RT_SET + 10:615, + 10:6150, + 15.15.15.15:15 +end-set +! +extcommunity-set rt ebpg-1 +end-set +! +route-policy static-to-bgp + if destination in cust-no-export then + apply cust2bgp + set community cust-no-export additive + elseif destination in cust-announce then + apply cust2bgp + set community cust-announce additive + elseif destination in cust-announce-backup then + apply cust2bgp + set local-preference 100 + set weight 0 + set community cust-announce additive + elseif destination in cust-no-export-backup then + apply cust2bgp + set local-preference 98 + set weight 0 + set community cust-no-export additive + else + drop + endif +end-policy +! +class-map match-any data + match precedence ipv4 0 1 + end-class-map +! diff --git a/test/integration/targets/iosxr_config/templates/basic/route_policy_change.j2 b/test/integration/targets/iosxr_config/templates/basic/route_policy_change.j2 new file mode 100644 index 00000000000..45448bae535 --- /dev/null +++ b/test/integration/targets/iosxr_config/templates/basic/route_policy_change.j2 @@ -0,0 +1,65 @@ +prefix-set EBGP-PEER-BOGONS + 192.0.2.0/24 le 32, + 192.168.0.0/16 le 32, + 198.18.0.0/16 le 32, + 224.0.0.0/4 le 32, + 240.0.0.0/4 le 32 +end-set +! +as-path-set EBGP-PEER-AS16509-403-PERMIT-PATHS + ios-regex '^11164_8075_', + ios-regex '^1164_16509$', + ios-regex '^1116_16409_[0-9]+$', + ios-regex '^8075_' +end-set +! +community-set cust-announce + 1525:65298, + 1525:6546, + 1525:6438, + 1525:65439, + 1525:65498 +end-set +! +rd-set EBGP_INCOMING_RD_SET + 172.16.0.0/16:*, + 172.14.0.0/16:100, + 192:*, + 192:100 +end-set +! +extcommunity-set rt EBGP_INCOMIG_RT_SET + 10:615, + 10:6120, + 15.15.15.15:15 +end-set +! +route-policy POLICY2 +end-policy +! +route-policy static-to-bgp + if destination in cust-no-export then + apply cust2bgp + set community cust-no-export additive + elseif destination in cust-announce then + apply cust2bgp + set community cust-announce additive + elseif destination in cust-announce-backup then + apply cust2bgp + set local-preference 100 + set weight 23 + set community cust-announce additive + elseif destination in cust-no-export-backup then + apply cust2bgp + set local-preference 98 + set weight 0 + set community cust-no-export additive + else + drop + endif +end-policy +! +class-map match-any data + match precedence ipv4 0 1 2 + end-class-map +! diff --git a/test/integration/targets/iosxr_config/templates/basic/route_policy_clean.j2 b/test/integration/targets/iosxr_config/templates/basic/route_policy_clean.j2 new file mode 100644 index 00000000000..08ba7686712 --- /dev/null +++ b/test/integration/targets/iosxr_config/templates/basic/route_policy_clean.j2 @@ -0,0 +1,32 @@ +no router ospf 1 +! +no prefix-set EBGP-PEER-BOGONS +! +no prefix-set cust-ddos-DDOS +! +no prefix-set cust-no-export +! +no prefix-set acme_DC_Internal +! +no as-path-set EBGP-PEER-AS16509-403-PERMIT-PATHS +! +no community-set cust-announce +! +no community-set cust-no-export +! +no rd-set ebpg-1 +! +no rd-set EBGP_INCOMING_RD_SET +! +no extcommunity-set rt EBGP_INCOMIG_RT_SET +! +no extcommunity-set rt ebpg-1 +! +no route-policy POLICY2 +! +no route-policy cust2bgp +! +no route-policy static-to-bgp +! +no class-map match-any data +! diff --git a/test/integration/targets/iosxr_config/tests/cli/route_policy.yaml b/test/integration/targets/iosxr_config/tests/cli/route_policy.yaml new file mode 100644 index 00000000000..ab6c40d00bc --- /dev/null +++ b/test/integration/targets/iosxr_config/tests/cli/route_policy.yaml @@ -0,0 +1,53 @@ +--- +- debug: msg="START cli/route_policy.yaml on connection={{ ansible_connection }}" + +- name: Cleanup + iosxr_config: + src: basic/route_policy_clean.j2 + +- name: config setup route-policy/prefix-set/as-path-set/community-set + iosxr_config: + src: basic/route_policy.j2 + register: result + +- assert: + that: + - "result.changed == true" + +- name: Configure same route-policy/prefix-set ... verify change=0 + iosxr_config: + src: basic/route_policy.j2 + register: result + +- assert: + that: + - "result.changed == false" + +- name: Do a change in multi-sublevel route-policy/prefix-set/community-set + iosxr_config: + src: basic/route_policy_change.j2 + register: result + +- assert: + that: + - "result.changed == true" + +- name: Configure same route-policy/prefix-set ... verify change=0 + iosxr_config: + src: basic/route_policy_change.j2 + register: result + +- assert: + that: + - "result.changed == false" + +- name: Cleanup + iosxr_config: + src: basic/route_policy_clean.j2 + register: result + +- assert: + that: + - "result.changed == true" + +- debug: msg="END cli/route_policy.yaml on connection={{ ansible_connection }}"