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 <[email protected]> * fix test Signed-off-by: Sumit Jaiswal <[email protected]> * fix test Signed-off-by: Sumit Jaiswal <[email protected]> * fix new line Signed-off-by: Sumit Jaiswal <[email protected]> --------- Signed-off-by: Sumit Jaiswal <[email protected]>
ansible · Jul 11, 2024 · 69f1917 · 69f1917
1 parent b28ba60
commit 69f1917
Show file tree

Hide file tree

Showing 5 changed files with 215 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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