From 69f19176a6cf825787c6cd8b3269e1e2a2cb2fe6 Mon Sep 17 00:00:00 2001 From: Sumit Jaiswal Date: Thu, 11 Jul 2024 09:49:15 +0530 Subject: [PATCH] PR to fix to correctly identify task based on Ansible block and when indentation (#250) * fix for ari issue 232 Signed-off-by: Sumit Jaiswal * fix test Signed-off-by: Sumit Jaiswal * fix test Signed-off-by: Sumit Jaiswal * fix new line Signed-off-by: Sumit Jaiswal --------- Signed-off-by: Sumit Jaiswal --- ansible_risk_insight/finder.py | 6 +- ansible_risk_insight/models.py | 9 ++- test/test_inline_replace.py | 63 ++++++++++++++++ .../block_and_when_play.yml | 71 +++++++++++++++++++ .../block_and_when_play_fixed.yml | 71 +++++++++++++++++++ 5 files changed, 215 insertions(+), 5 deletions(-) create mode 100644 test/test_inline_replace.py create mode 100644 test/testdata/inline_replace_data/block_and_when_play.yml create mode 100644 test/testdata/inline_replace_data/block_and_when_play_fixed.yml diff --git a/ansible_risk_insight/finder.py b/ansible_risk_insight/finder.py index 1d5eafceb..e21adcd4e 100644 --- a/ansible_risk_insight/finder.py +++ b/ansible_risk_insight/finder.py @@ -784,8 +784,8 @@ def check_diff_and_copy_olddata_to_newdata(line_number_list, lines, new_data): if line_number_list and isinstance(line_number_list, list): new_content_last_set = line_number_list[-1] new_content_last_line = int(new_content_last_set.lstrip("L").split("-")[1]) - if new_content_last_line < len(lines) - 1: - for i in range(new_content_last_line, len(lines) - 1): + if new_content_last_line < len(lines): + for i in range(new_content_last_line, len(lines)): new_data.append(lines[i]) return new_data @@ -830,9 +830,9 @@ def update_the_yaml_target(file_path, line_number_list, new_content_list): stop_line_number = int(input_line_number[1]) diff_in_lines = stop_line_number - start_line_number temp_content = [] - data_copy.append('\n') start = start_line_number - 1 end = stop_line_number - 1 + data_copy.append('\n') for i in range(start, end): line_number = i if len(lines) == i: diff --git a/ansible_risk_insight/models.py b/ansible_risk_insight/models.py index ef38679c8..a32b43dd9 100644 --- a/ansible_risk_insight/models.py +++ b/ansible_risk_insight/models.py @@ -1263,8 +1263,12 @@ def _find_task_block(self, yaml_lines: list, start_line_num: int): break _line = lines[index] is_top_of_block = _line.replace(" ", "").startswith("-") - if is_top_of_block: - _indent = len(_line.split("-")[0]) + is_when_at_same_indent = _line.replace(" ", "").startswith("when") + if is_top_of_block or is_when_at_same_indent: + if is_top_of_block: + _indent = len(_line.split("-")[0]) + elif is_when_at_same_indent: + _indent = len(_line.split("when")[0]) if _indent <= indent_of_block: end_found = True end_line_num = index - 1 @@ -1274,6 +1278,7 @@ def _find_task_block(self, yaml_lines: list, start_line_num: int): end_found = True end_line_num = index break + if not end_found: return None, None if begin_line_num < 0 or end_line_num > len(lines) or begin_line_num > end_line_num: diff --git a/test/test_inline_replace.py b/test/test_inline_replace.py new file mode 100644 index 000000000..e3d818f2e --- /dev/null +++ b/test/test_inline_replace.py @@ -0,0 +1,63 @@ +# -*- mode:python; coding:utf-8 -*- + +# Copyright (c) 2024 RedHat. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from ansible_risk_insight.finder import update_the_yaml_target + + +def test_inline_replace_for_block_and_when(): + file_path = "test/testdata/inline_replace_data/block_and_when_play.yml" + file_path_out = "test/testdata/inline_replace_data/block_and_when_play_fixed.yml" + line_number = [ + "L6-11", + "L12-20", + "L23-30", + "L31-34", + "L39-46", + "L47-50", + "L55-61", + "L62-65" + ] + new_content = [ + '''- name: Validate server authentication input provided by user\n when:\n + - (username is not defined or password is not defined) and (cert_file is not defined or key_file is not defined) + and (auth_token is not defined)\n ansible.builtin.fail:\n msg: "username/password or cert_file/key_file or auth_token + is mandatory"\n''', + '''- name: Fail when more than one valid authentication method is provided\n when:\n + - ((username is defined or password is defined) and (cert_file is defined or key_file is defined) and + auth_token is defined) or ((username is defined or password is defined) and (cert_file is defined or key_file is defined)) + or ((username is defined or password is defined) and auth_token is defined) or ((cert_file is defined or key_file is defined) and + auth_token is defined)\n ansible.builtin.fail:\n msg: "Only one authentication method is allowed. + Provide either username/password or cert_file/key_file or auth_token."\n''', + ''' - ilo_network:\n category: Systems\n command: GetNetworkAdapters\n + baseuri: "{{ baseuri }}"\n username: "{{ username }}"\n password: "{{ password }}"\n + register: network_adapter_details\n''', + '- name: Physical network adapter details in the server\n ansible.builtin.debug:\n msg: "{{ network_adapter_details }}"\n', + ''' - ilo_network:\n category: Systems\n command: GetNetworkAdapters\n + baseuri: "{{ baseuri }}"\n cert_file: "{{ cert_file }}"\n key_file: "{{ key_file }}"\n + register: network_adapter_details\n''', + '- name: Physical network adapter details present in the server\n ansible.builtin.debug:\n msg: "{{ network_adapter_details }}"\n', + ''' - ilo_network:\n category: Systems\n command: GetNetworkAdapters\n + baseuri: "{{ baseuri }}"\n auth_token: "{{ auth_token }}"\n register: network_adapter_details\n''', + '- name: Physical network adapter details in the server\n ansible.builtin.debug:\n msg: "{{ network_adapter_details }}"\n' + ] + + update_the_yaml_target(file_path, line_number, new_content) + with open(file_path, 'r') as file: + data = file.read() + with open(file_path_out, 'r') as file: + data_fixed = file.read() + + assert data == data_fixed diff --git a/test/testdata/inline_replace_data/block_and_when_play.yml b/test/testdata/inline_replace_data/block_and_when_play.yml new file mode 100644 index 000000000..b50c57984 --- /dev/null +++ b/test/testdata/inline_replace_data/block_and_when_play.yml @@ -0,0 +1,71 @@ +- hosts: myhosts + connection: local + gather_facts: false + tasks: + + - name: Validate server authentication input provided by user + when: + - (username is not defined or password is not defined) and (cert_file is not defined + or key_file is not defined) and (auth_token is not defined) + ansible.builtin.fail: + msg: username/password or cert_file/key_file or auth_token is mandatory + + - name: Fail when more than one valid authentication method is provided + when: + - ((username is defined or password is defined) and (cert_file is defined or key_file + is defined) and auth_token is defined) or ((username is defined or password + is defined) and (cert_file is defined or key_file is defined)) or ((username + is defined or password is defined) and auth_token is defined) or ((cert_file + is defined or key_file is defined) and auth_token is defined) + ansible.builtin.fail: + msg: Only one authentication method is allowed. Provide either username/password + or cert_file/key_file or auth_token. + + - name: Get physical network adapter details when username and password are defined + block: + + - ilo_network: + category: Systems + command: GetNetworkAdapters + baseuri: '{{ baseuri }}' + username: '{{ username }}' + password: '{{ password }}' + register: network_adapter_details + + - name: Physical network adapter details in the server + ansible.builtin.debug: + msg: '{{ network_adapter_details }}' + + when: username is defined and password is defined + + - name: Get physical network adapter details when cert_file and key_file are defined + block: + + - ilo_network: + category: Systems + command: GetNetworkAdapters + baseuri: '{{ baseuri }}' + cert_file: '{{ cert_file }}' + key_file: '{{ key_file }}' + register: network_adapter_details + + - name: Physical network adapter details present in the server + ansible.builtin.debug: + msg: '{{ network_adapter_details }}' + + when: cert_file is defined and key_file is defined + + - name: Get physical network adapter details when auth_token is defined + block: + + - ilo_network: + category: Systems + command: GetNetworkAdapters + baseuri: '{{ baseuri }}' + auth_token: '{{ auth_token }}' + register: network_adapter_details + + - name: Physical network adapter details in the server + ansible.builtin.debug: + msg: '{{ network_adapter_details }}' + when: auth_token is defined diff --git a/test/testdata/inline_replace_data/block_and_when_play_fixed.yml b/test/testdata/inline_replace_data/block_and_when_play_fixed.yml new file mode 100644 index 000000000..b50c57984 --- /dev/null +++ b/test/testdata/inline_replace_data/block_and_when_play_fixed.yml @@ -0,0 +1,71 @@ +- hosts: myhosts + connection: local + gather_facts: false + tasks: + + - name: Validate server authentication input provided by user + when: + - (username is not defined or password is not defined) and (cert_file is not defined + or key_file is not defined) and (auth_token is not defined) + ansible.builtin.fail: + msg: username/password or cert_file/key_file or auth_token is mandatory + + - name: Fail when more than one valid authentication method is provided + when: + - ((username is defined or password is defined) and (cert_file is defined or key_file + is defined) and auth_token is defined) or ((username is defined or password + is defined) and (cert_file is defined or key_file is defined)) or ((username + is defined or password is defined) and auth_token is defined) or ((cert_file + is defined or key_file is defined) and auth_token is defined) + ansible.builtin.fail: + msg: Only one authentication method is allowed. Provide either username/password + or cert_file/key_file or auth_token. + + - name: Get physical network adapter details when username and password are defined + block: + + - ilo_network: + category: Systems + command: GetNetworkAdapters + baseuri: '{{ baseuri }}' + username: '{{ username }}' + password: '{{ password }}' + register: network_adapter_details + + - name: Physical network adapter details in the server + ansible.builtin.debug: + msg: '{{ network_adapter_details }}' + + when: username is defined and password is defined + + - name: Get physical network adapter details when cert_file and key_file are defined + block: + + - ilo_network: + category: Systems + command: GetNetworkAdapters + baseuri: '{{ baseuri }}' + cert_file: '{{ cert_file }}' + key_file: '{{ key_file }}' + register: network_adapter_details + + - name: Physical network adapter details present in the server + ansible.builtin.debug: + msg: '{{ network_adapter_details }}' + + when: cert_file is defined and key_file is defined + + - name: Get physical network adapter details when auth_token is defined + block: + + - ilo_network: + category: Systems + command: GetNetworkAdapters + baseuri: '{{ baseuri }}' + auth_token: '{{ auth_token }}' + register: network_adapter_details + + - name: Physical network adapter details in the server + ansible.builtin.debug: + msg: '{{ network_adapter_details }}' + when: auth_token is defined