From c715d3aad22e965aad0eadc2697a110df0f192ec Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Mon, 20 Feb 2023 16:02:56 -0300
Subject: [PATCH] ansible-lint: Fix key order on upstream tests

In latest ansible-lint versions, the use of "blocks" has a required
order to be implemented. According to ansible-lint error mesage, the
order is name, when, block, rescue, always.

As not following this rule is now an error, this patch fixes all tests
for the 'key-order[task]' error.
---
 tests/config/test_config.yml                  |  8 ++--
 tests/config/test_config_sid.yml              |  4 +-
 tests/env_freeipa_facts.yml                   |  2 +-
 tests/group/test_group.yml                    |  3 +-
 tests/group/test_group_external_members.yml   |  3 +-
 tests/group/test_group_external_nonposix.yml  |  3 +-
 tests/group/test_group_idoverrideuser.yml     |  3 +-
 tests/group/test_group_membermanager.yml      |  3 +-
 .../test_hostgroup_membermanager.yml          |  2 +-
 tests/hostgroup/test_hostgroup_rename.yml     |  3 +-
 tests/idrange/test_idrange.yml                |  3 +-
 tests/role/env_facts.yml                      |  2 +-
 tests/server/test_server.yml                  |  4 +-
 tests/service/test_service.yml                |  2 +-
 ...st_servicedelegationrule_hostprincipal.yml |  3 +-
 ..._servicedelegationtarget_hostprincipal.yml |  3 +-
 tests/trust/test_trust.yml                    |  5 +--
 tests/vault/test_vault_change_type.yml        | 40 ++++++++-----------
 18 files changed, 38 insertions(+), 58 deletions(-)

diff --git a/tests/config/test_config.yml b/tests/config/test_config.yml
index ced34211..555a142e 100644
--- a/tests/config/test_config.yml
+++ b/tests/config/test_config.yml
@@ -59,13 +59,13 @@
           pac_type: ""
 
       - name: Execute tests if ipa_version >= 4.8.0
+        when: ipa_version is version('4.8.0', '>=')
         block:
           - name: Set maxhostname to 255
             ipaconfig:
               ipaadmin_password: SomeADMINpassword
               ipaapi_context: "{{ ipa_context | default(omit) }}"
               maxhostname: 255
-        when: ipa_version is version('4.8.0', '>=')
 
       - name: Set maxusername to 45
         ipaconfig:
@@ -225,6 +225,7 @@
         failed_when: result.changed or result.failed
 
       - name: Execute tests if ipa_version >= 4.8.0
+        when: ipa_version is version('4.8.0', '>=')
         block:
           - name: Set maxhostname to 77
             ipaconfig:
@@ -241,7 +242,6 @@
               maxhostname: 77
             register: result
             failed_when: result.changed or result.failed
-        when: ipa_version is version('4.8.0', '>=')
 
       - name: Set pwdexpnotify to 17
         ipaconfig:
@@ -415,13 +415,13 @@
         failed_when: not result.changed or result.failed
 
       - name: Execute tests if ipa_version >= 4.8.0
+        when: ipa_version is version('4.8.0', '>=')
         block:
           - name: Reset maxhostname
             ipaconfig:
               ipaadmin_password: SomeADMINpassword
               ipaapi_context: "{{ ipa_context | default(omit) }}"
               maxhostname: '{{ previousconfig.config.maxhostname | default(omit) }}'
-        when: ipa_version is version('4.8.0', '>=')
 
       - name: Reset changed fields, again
         ipaconfig:
@@ -451,13 +451,13 @@
         failed_when: result.changed or result.failed
 
       - name: Execute tests if ipa_version >= 4.8.0
+        when: ipa_version is version('4.8.0', '>=')
         block:
           - name: Reset maxhostname
             ipaconfig:
               ipaadmin_password: SomeADMINpassword
               ipaapi_context: "{{ ipa_context | default(omit) }}"
               maxhostname: '{{ previousconfig.config.maxhostname | default(omit) }}'
-        when: ipa_version is version('4.8.0', '>=')
 
     rescue:
       - name: Set fields to IPA default, due to error
diff --git a/tests/config/test_config_sid.yml b/tests/config/test_config_sid.yml
index e288f014..cd3ce4fb 100644
--- a/tests/config/test_config_sid.yml
+++ b/tests/config/test_config_sid.yml
@@ -19,6 +19,8 @@
 
   # TESTS
   - name: Test config sid
+    # only run tests if version supports enable-sid
+    when: ipa_version is version("4.9.8", ">=")
     block:
     - name: Check if SID is enabled.
       ipaconfig:
@@ -115,8 +117,6 @@
         ipaapi_context: "{{ ipa_context | default(omit) }}"
         add_sids: yes
 
-    # only run tests if version supports enable-sid
-    when: ipa_version is version("4.9.8", ">=")
     # REVERT TO PREVIOUS CONFIG
     always:
     # Once SID is enabled, it cannot be reverted.
diff --git a/tests/env_freeipa_facts.yml b/tests/env_freeipa_facts.yml
index d6f65f3b..280e8efa 100644
--- a/tests/env_freeipa_facts.yml
+++ b/tests/env_freeipa_facts.yml
@@ -31,6 +31,7 @@
     trust_test_is_supported: no
 
 - name: Ensure ipaserver_domain is set
+  when: ipaserver_domain is not defined
   block:
   - name: Get Domain from server name
     ansible.builtin.set_fact:
@@ -41,4 +42,3 @@
     ansible.builtin.set_fact:
       ipaserver_domain: "ipa.test"
     when: "'fqdn' not in ansible_facts"
-  when: ipaserver_domain is not defined
diff --git a/tests/group/test_group.yml b/tests/group/test_group.yml
index f3f1c521..8f6a0fa9 100644
--- a/tests/group/test_group.yml
+++ b/tests/group/test_group.yml
@@ -138,6 +138,7 @@
     # service
 
   - name: Execute tests if ipa_verison >= 4.7.0
+    when: ipa_version is version('4.7.0', '>=')
     block:
 
     - name: Ensure service "{{ 'HTTP/' + fqdn_at_domain }}" is present in group group1
@@ -282,8 +283,6 @@
       register: result
       failed_when: result.changed or result.failed
 
-    when: ipa_version is version('4.7.0', '>=')
-
     # user
 
   - name: Ensure users user1, user2 and user3 are present in group group1
diff --git a/tests/group/test_group_external_members.yml b/tests/group/test_group_external_members.yml
index db926cf4..7541f876 100644
--- a/tests/group/test_group_external_members.yml
+++ b/tests/group/test_group_external_members.yml
@@ -10,6 +10,7 @@
     ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
   - name: Execute group tests if trust test environment is supported
+    when: trust_test_is_supported | default(false)
     block:
 
     - name: Add nonposix group.
@@ -111,5 +112,3 @@
         ipaadmin_password: SomeADMINpassword
         name: extgroup
         state: absent
-
-    when: trust_test_is_supported | default(false)
diff --git a/tests/group/test_group_external_nonposix.yml b/tests/group/test_group_external_nonposix.yml
index 83e009cf..51d2e755 100644
--- a/tests/group/test_group_external_nonposix.yml
+++ b/tests/group/test_group_external_nonposix.yml
@@ -205,6 +205,7 @@
     # EXTERNAL MEMBER TEST (REQUIRES AD)
 
     - name: Execute group tests if trust test environment is supported
+      when: trust_test_is_supported | default(false)
       block:
 
       - name: Ensure users testuser1, testuser2 and testuser3 are present in group externalgroup
@@ -231,8 +232,6 @@
         register: result
         failed_when: result.changed or result.failed
 
-      when: trust_test_is_supported | default(false)
-
     # CONVERT NONPOSIX TO POSIX GROUP WITH USERS
 
     - name: Ensure nonposix group nonposixgroup as posix
diff --git a/tests/group/test_group_idoverrideuser.yml b/tests/group/test_group_idoverrideuser.yml
index 71d640a6..2a096201 100644
--- a/tests/group/test_group_idoverrideuser.yml
+++ b/tests/group/test_group_idoverrideuser.yml
@@ -13,6 +13,7 @@
       ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
     - name: Execute tests if ipa_verison >= 4.8.7 and trust test environment is supported
+      when: ipa_version is version("4.8.7", ">=") and trust_test_is_supported | default(false)
       block:
       - name: Create idoverrideuser.
         ansible.builtin.shell: |
@@ -102,5 +103,3 @@
           ipa idoverrideuser-del "Default Trust View" {{ ad_user }}
           kdestroy -A -q -c idoverride_cache
         when:
-
-      when: ipa_version is version("4.8.7", ">=") and trust_test_is_supported | default(false)
diff --git a/tests/group/test_group_membermanager.yml b/tests/group/test_group_membermanager.yml
index ecb3fe09..e4214f60 100644
--- a/tests/group/test_group_membermanager.yml
+++ b/tests/group/test_group_membermanager.yml
@@ -9,6 +9,7 @@
     ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
   - name: Execute tests if ipa_verison >= 4.8.4
+    when: ipa_version is version('4.8.4', '>=')
     block:
       - name: Ensure user manangeruser1 and manageruser2 is absent
         ipauser:
@@ -206,5 +207,3 @@
           state: absent
         register: result
         failed_when: not result.changed or result.failed
-
-    when: ipa_version is version('4.8.4', '>=')
diff --git a/tests/hostgroup/test_hostgroup_membermanager.yml b/tests/hostgroup/test_hostgroup_membermanager.yml
index eb82b566..a26a9a4a 100644
--- a/tests/hostgroup/test_hostgroup_membermanager.yml
+++ b/tests/hostgroup/test_hostgroup_membermanager.yml
@@ -9,6 +9,7 @@
     ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
   - name: Tests requiring IPA version 4.8.4+
+    when: ipa_version is version('4.8.4', '>=')
     block:
       - name: Ensure host-group testhostgroup is absent
         ipahostgroup:
@@ -224,4 +225,3 @@
           state: absent
         register: result
         failed_when: not result.changed or result.failed
-    when: ipa_version is version('4.8.4', '>=')
diff --git a/tests/hostgroup/test_hostgroup_rename.yml b/tests/hostgroup/test_hostgroup_rename.yml
index 5cde2705..b41cd902 100644
--- a/tests/hostgroup/test_hostgroup_rename.yml
+++ b/tests/hostgroup/test_hostgroup_rename.yml
@@ -9,6 +9,7 @@
     ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
   - name: Tests requiring IPA version 4.8.7+
+    when: ipa_version is version('4.8.7', '>=')
     block:
       - name: Ensure testing host-group are absent
         ipahostgroup:
@@ -108,5 +109,3 @@
           - databases
           - datalake
           state: absent
-
-    when: ipa_version is version('4.8.7', '>=')
diff --git a/tests/idrange/test_idrange.yml b/tests/idrange/test_idrange.yml
index 92ea3aff..2becb9c1 100644
--- a/tests/idrange/test_idrange.yml
+++ b/tests/idrange/test_idrange.yml
@@ -120,6 +120,7 @@
           name: local_id_range
 
   - name: Execute idrange tests if trust test environment is supported
+    when: trust_test_is_supported | default(false)
     block:
       # Create trust with range_type: ipa-ad-trust
       - name: Create trust with range_type 'ipa-ad-trust'
@@ -367,5 +368,3 @@
             - ad_posix_id_range
           continue: yes
           state: absent
-
-    when: trust_test_is_supported | default(false)
diff --git a/tests/role/env_facts.yml b/tests/role/env_facts.yml
index 87c27743..398354b2 100644
--- a/tests/role/env_facts.yml
+++ b/tests/role/env_facts.yml
@@ -1,5 +1,6 @@
 ---
 - name: Ensure ipaserver_domain is set
+  when: ipaserver_domain is not defined
   block:
   - name: Get Domain from server name
     ansible.builtin.set_fact:
@@ -9,7 +10,6 @@
     ansible.builtin.set_fact:
       ipaserver_domain: "ipa.test"
     when: "'fqdn' not in ansible_facts"
-  when: ipaserver_domain is not defined
 
 - name: Set ipaserver_realm.
   ansible.builtin.set_fact:
diff --git a/tests/server/test_server.yml b/tests/server/test_server.yml
index 75560194..d2990324 100644
--- a/tests/server/test_server.yml
+++ b/tests/server/test_server.yml
@@ -8,6 +8,7 @@
 
   # CLEANUP TEST ITEMS
   - name: Ensure ipa_server_name is set
+    when: ipa_server_name is not defined
     block:
       - name: Get server name from hostname
         ansible.builtin.set_fact:
@@ -16,9 +17,9 @@
       - name: Fallback to 'ipaserver'
         ansible.builtin.set_fact:
           ipa_server_name: ipaserver
-    when: ipa_server_name is not defined
 
   - name: Ensure ipaserver_domain is set
+    when: ipaserver_domain is not defined
     block:
       - name: Get domain name from hostname.
         ansible.builtin.set_fact:
@@ -27,7 +28,6 @@
       - name: Fallback to 'ipa.test'
         ansible.builtin.set_fact:
           ipaserver_domain: "ipa.test"
-    when: ipaserver_domain is not defined
 
   - name: Ensure server "{{ ipa_server_name + '.' + ipaserver_domain }}" without location
     ipaserver:
diff --git a/tests/service/test_service.yml b/tests/service/test_service.yml
index 8c6c4d8b..a2460803 100644
--- a/tests/service/test_service.yml
+++ b/tests/service/test_service.yml
@@ -22,6 +22,7 @@
 
   # tests
   - name: Tests with skip_host_check, require IPA version 4.8.0+.
+    when: ipa_version is version('4.7.0', '>=')
     block:
       - name: Setup test environment
         ansible.builtin.include_tasks: env_setup.yml
@@ -577,4 +578,3 @@
       # cleanup
       - name: Cleanup test environment
         ansible.builtin.include_tasks: env_cleanup.yml
-    when: ipa_version is version('4.7.0', '>=')
diff --git a/tests/servicedelegationrule/test_servicedelegationrule_hostprincipal.yml b/tests/servicedelegationrule/test_servicedelegationrule_hostprincipal.yml
index 8df4274a..bac500e1 100644
--- a/tests/servicedelegationrule/test_servicedelegationrule_hostprincipal.yml
+++ b/tests/servicedelegationrule/test_servicedelegationrule_hostprincipal.yml
@@ -10,6 +10,7 @@
     ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
   - name: Host principals are only possible with IPA 4.9.0+
+    when: ipa_version is version('4.9.0', '>=')
     block:
 
     # SET FACTS
@@ -145,5 +146,3 @@
         state: absent
       register: result
       failed_when: not result.changed or result.failed
-
-    when: ipa_version is version('4.9.0', '>=')
diff --git a/tests/servicedelegationtarget/test_servicedelegationtarget_hostprincipal.yml b/tests/servicedelegationtarget/test_servicedelegationtarget_hostprincipal.yml
index 111608d8..77987764 100644
--- a/tests/servicedelegationtarget/test_servicedelegationtarget_hostprincipal.yml
+++ b/tests/servicedelegationtarget/test_servicedelegationtarget_hostprincipal.yml
@@ -10,6 +10,7 @@
     ansible.builtin.include_tasks: ../env_freeipa_facts.yml
 
   - name: Host principals are only possible with IPA 4.9.0+
+    when: ipa_version is version('4.9.0', '>=')
     block:
 
     # SET FACTS
@@ -145,5 +146,3 @@
         state: absent
       register: result
       failed_when: not result.changed or result.failed
-
-    when: ipa_version is version('4.9.0', '>=')
diff --git a/tests/trust/test_trust.yml b/tests/trust/test_trust.yml
index e0e63386..8c0afb97 100644
--- a/tests/trust/test_trust.yml
+++ b/tests/trust/test_trust.yml
@@ -17,10 +17,9 @@
     ipa_range_exists: 'Range name: {{ ipaserver.realm }}_subid_range'
 
   tasks:
-
   - name: Run tust tests, if supported by environment
+    when: trust_test_is_supported | default(false)
     block:
-
     - name: Delete test trust
       ipatrust:
         ipaadmin_password: SomeADMINpassword
@@ -165,5 +164,3 @@
         ipa idrange-del {{ adserver.realm }}_id_range || true
         ipa idrange-del {{ ipaserver.realm }}_subid_range || true
         kdestroy -c test_krb5_cache -q -A
-
-    when: trust_test_is_supported | default(false)
diff --git a/tests/vault/test_vault_change_type.yml b/tests/vault/test_vault_change_type.yml
index 7e4ca44f..3b8a332d 100644
--- a/tests/vault/test_vault_change_type.yml
+++ b/tests/vault/test_vault_change_type.yml
@@ -31,6 +31,8 @@
     failed_when: result.failed or not result.changed
 
   - name: Change vault type from asymmetric to symmetric
+    vars:
+      krb5ccname: verify_change_from_asymmetric
     block:
     - name: Change from asymmetric to symmetric
       ipavault:
@@ -50,10 +52,9 @@
       register: result
       failed_when: result.failed or "Public Key:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_asymmetric
-
   - name: Change vault type from symmetric to standard
+    vars:
+      krb5ccname: verify_change_from_symmetric
     block:
     - name: Change from symmetric to standard
       ipavault:
@@ -72,9 +73,6 @@
       register: result
       failed_when: result.failed or "Salt:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_symmetric
-
   - name: Change from standard to symmetric
     ipavault:
       ipaadmin_password: SomeADMINpassword
@@ -85,6 +83,8 @@
     failed_when: result.failed or not result.changed
 
   - name: Change vault type from symmetric to asymmetric
+    vars:
+      krb5ccname: verify_change_from_symmetric
     block:
     - name: Change from symmetric to asymmetric
       ipavault:
@@ -104,10 +104,9 @@
       register: result
       failed_when: result.failed or "Salt:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_symmetric
-
   - name: Change vault type from asymmetric to standard
+    vars:
+      krb5ccname: verify_change_from_asymmetric
     block:
     - name: Change from asymmetric to standard
       ipavault:
@@ -126,9 +125,6 @@
       register: result
       failed_when: result.failed or "Public Key:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_asymmetric
-
   - name: Ensure test_vault is absent.
     ipavault:
       ipaadmin_password: SomeADMINpassword
@@ -161,6 +157,8 @@
     failed_when: result.failed or result.changed or result.vault.data != 'hello'
 
   - name: Change vault type from asymmetric to symmetric, with data
+    vars:
+      krb5ccname: verify_change_from_asymmetric
     block:
     - name: Change from asymmetric to symmetric, with data
       ipavault:
@@ -180,9 +178,6 @@
       register: result
       failed_when: result.failed or "Public Key:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_asymmetric
-
   - name: Retrieve data from symmetric vault.
     ipavault:
       ipaadmin_password: SomeADMINpassword
@@ -193,6 +188,8 @@
     failed_when: result.failed or result.changed or result.vault.data != 'hello'
 
   - name: Change vault type from symmetric to standard, with data
+    vars:
+      krb5ccname: verify_change_from_symmetric
     block:
     - name: Change from symmetric to standard, with data
       ipavault:
@@ -211,9 +208,6 @@
       register: result
       failed_when: result.failed or "Salt:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_symmetric
-
   - name: Retrieve data from standard vault.
     ipavault:
       ipaadmin_password: SomeADMINpassword
@@ -241,6 +235,8 @@
     failed_when: result.failed or result.changed or result.vault.data != 'hello'
 
   - name: Change vault type from symmetric to asymmetric, with data
+    vars:
+      krb5ccname: verify_change_from_symmetric
     block:
     - name: Change from symmetric to asymmetric, with data
       ipavault:
@@ -260,9 +256,6 @@
       register: result
       failed_when: result.failed or "Salt:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_symmetric
-
   - name: Retrieve data from asymmetric vault.
     ipavault:
       ipaadmin_password: SomeADMINpassword
@@ -273,6 +266,8 @@
     failed_when: result.failed or result.changed or result.vault.data != 'hello'
 
   - name: Change vault type from asymmetric to standard, with data
+    vars:
+      krb5ccname: verify_change_from_asymmetric
     block:
     - name: Change from asymmetric to standard, with data
       ipavault:
@@ -291,9 +286,6 @@
       register: result
       failed_when: result.failed or "Public Key:" in result.stdout
 
-    vars:
-      krb5ccname: verify_change_from_asymmetric
-
   - name: Retrieve data from standard vault.
     ipavault:
       ipaadmin_password: SomeADMINpassword
-- 
GitLab