test: omit setting group for certificate tests on cockpit 330 and later

cockpit 330 uses DynamicUser instead of the hard coded user/group in the package, so omit setting group, but keep group for other platforms. refactor the certificate test code to DRY it a bit Signed-off-by: Rich Megginson <[email protected]>
linux-system-roles · Dec 12, 2024 · 28d1fd0 · 28d1fd0
1 parent 83f3c9b
commit 28d1fd0
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 98 deletions.
diff --git a/README.md b/README.md
@@ -212,7 +212,8 @@ assuming your machines are joined to a FreeIPA domain.
           - name: monger-cockpit
             dns: ['localhost', 'www.example.com']
             ca: ipa
-            group: cockpit-ws
+            # NOTE: omit group when using cockpit 330 and later (Fedora and EL10)
+            group: cockpit-ws  # or cockpit-wsinstance on newer cockpit versions
 ```
 
 Note: Generating a new certificate using the `certificate` system role in the playbook remains supported.
@@ -235,6 +236,7 @@ This example also installs Cockpit with an IdM-issued web server certificate.
           - name: /etc/cockpit/ws-certs.d/monger-cockpit
             dns: ['localhost', 'www.example.com']
             ca: ipa
+            # NOTE: omit group when using cockpit 330 and later (Fedora and EL10)
             group: cockpit-ws  # or cockpit-wsinstance on newer cockpit versions
 ```
 

diff --git a/tests/tasks/cleanup.yml b/tests/tasks/cleanup.yml
@@ -32,7 +32,29 @@
     - always
     - tests::cleanup
 
-- name: Cleanup - certificates
+- name: Cleanup - stop tracking certificates
+  command: getcert stop-tracking -f {{ item.path | quote }}
+  changed_when: false
+  with_items: "{{ certs_to_delete.files }}"
+  register: __cockpit_stop_tracking
+  failed_when:
+    - __cockpit_stop_tracking is failed
+    - "'stdout' in __cockpit_stop_tracking"
+    - not __cockpit_stop_tracking.stdout is
+      search("No request found that matched arguments")
+  tags:
+    - always
+    - tests::cleanup
+
+- name: Cleanup - stop tracking other certificate
+  command: getcert stop-tracking -f {{ __cockpit_other_certificate | quote }}.crt
+  changed_when: true
+  when: __cockpit_other_certificate | d("") | length > 0
+  tags:
+    - always
+    - tests::cleanup
+
+- name: Cleanup - remove certificate files
   file:
     path: "{{ item.path }}"
     state: absent
@@ -41,6 +63,26 @@
     - always
     - tests::cleanup
 
+- name: Cleanup - remove other certificate files
+  file:
+    path: "{{ __cockpit_other_certificate }}{{ __suffix }}"
+    state: absent
+  loop: [".crt", ".key"]
+  loop_control:
+    loop_var: __suffix
+  when: __cockpit_other_certificate | d("") | length > 0
+  tags:
+    - always
+    - tests::cleanup
+
+- name: Cleanup - certificates directory
+  file:
+    path: /etc/cockpit/ws-certs.d
+    state: absent
+  when:
+    - __cockpit_test_cert_dir is defined
+    - __cockpit_test_cert_dir is changed
+
 - name: Cleanup - config file
   file:
     path: /etc/cockpit/cockpit.conf

diff --git a/tests/tasks/setup_for_certificate_tests.yml b/tests/tasks/setup_for_certificate_tests.yml
@@ -0,0 +1,39 @@
+---
+# install so that we can get version, and create group on platforms
+# where we need the group
+- name: Install cockpit
+  include_role:
+    name: linux-system-roles.cockpit
+    public: true
+  vars:
+    cockpit_packages: minimal
+
+- name: Collect installed package versions
+  package_facts:
+
+# NOTE: There is no way to set __cockpit_version to be a native int
+# without using ANSIBLE_JINJA2_NATIVE
+# https://docs.ansible.com/ansible/latest/reference_appendices/config.html#default-jinja2-native
+- name: Set cockpit version for tests
+  set_fact:
+    __cockpit_version: "{{ ansible_facts.packages['cockpit-ws'][0].version }}"
+
+# Fixed in cockpit 255 (https://github.com/cockpit-project/cockpit/commit/6ec4881856e)
+- name: Allow certmonger to write into Cockpit's certificate directory
+  when: __cockpit_version | int < 255
+  file:
+    path: /etc/cockpit/ws-certs.d
+    state: directory
+    setype: cert_t
+    mode: "0755"
+  register: __cockpit_test_cert_dir
+
+# returns global variable __cockpit_test_group
+- name: Get name of cockpit group to use
+  include_tasks: tasks/get_cockpit_group.yml
+  when: __cockpit_version | int < 330
+
+- name: Set name of cockpit group to use to None
+  set_fact:
+    __cockpit_test_group: null
+  when: __cockpit_version | int >= 330
diff --git a/tests/tests_certificate_external.yml b/tests/tests_certificate_external.yml
@@ -4,32 +4,12 @@
   tasks:
     - name: Tests
       block:
-        - name: Include role
-          include_role:
-            name: linux-system-roles.cockpit
-            public: true
-          vars:
-            cockpit_packages: minimal
-
-        - name: Collect installed package versions
-          package_facts:
+        - name: Set up for certificate tests
+          include_tasks: tasks/setup_for_certificate_tests.yml
 
         - name: Check if cockpit is new enough (at least 211) to support certmonger
-          when: ansible_facts.packages['cockpit-ws'][0].version | int >= 211
+          when: __cockpit_version | int >= 211
           block:
-            # Fixed in cockpit 255 (https://github.com/cockpit-project/cockpit/commit/6ec4881856e)
-            - name: >-
-                Allow certmonger to write into Cockpit's certificate directory
-              file:
-                path: /etc/cockpit/ws-certs.d/
-                state: directory
-                setype: cert_t
-                mode: "0755"
-
-            # returns global variable __cockpit_test_group
-            - name: Get name of cockpit group to use
-              include_tasks: tasks/get_cockpit_group.yml
-
             # has to be done dynamically, as the first step checks it out
             - name: Generate certificate with certificate system role
               include_role:
@@ -39,7 +19,8 @@
                   - name: /etc/cockpit/ws-certs.d/monger-cockpit
                     dns: ['localhost', 'www.example.com']
                     ca: self-sign
-                    group: "{{ __cockpit_test_group }}"
+                    group: "{{ __cockpit_test_group
+                      if __cockpit_test_group else omit }}"
 
             # ostree cannot remove packages and cannot cleanup properly
             # this works around that issue
@@ -74,22 +55,6 @@
               assert:
                 that: "'status: MONITORING' in result.stdout"
 
-            - name: Test - clean up tracked certificate
-              command: >
-                getcert stop-tracking
-                -f /etc/cockpit/ws-certs.d/monger-cockpit.crt
-              changed_when: false
-
       always:
-        - name: Test - clean up generated certificate
-          file:
-            path: /etc/cockpit/ws-certs.d/monger-cockpit.crt
-            state: absent
-
-        - name: Test - clean up generated private key
-          file:
-            path: /etc/cockpit/ws-certs.d/monger-cockpit.key
-            state: absent
-
         - name: Test - generic cleanup
           include_tasks: tasks/cleanup.yml
diff --git a/tests/tests_certificate_internal.yml b/tests/tests_certificate_internal.yml
@@ -9,17 +9,8 @@
         - name: >-
             Install cockpit using the certificate role to create a certificate
           block:
-            # install cockpit package to get group
-            - name: Include role
-              include_role:
-                name: linux-system-roles.cockpit
-                public: true
-              vars:
-                cockpit_packages: minimal
-
-            # returns global variable __cockpit_test_group
-            - name: Get name of cockpit group to use
-              include_tasks: tasks/get_cockpit_group.yml
+            - name: Set up for certificate tests
+              include_tasks: tasks/setup_for_certificate_tests.yml
 
             - name: Install cockpit with cockpit_certificates request
               vars:
@@ -28,10 +19,15 @@
                   - name: "{{ cert_name }}"
                     dns: ['localhost', 'www.example.com']
                     ca: self-sign
-                    group: "{{ __cockpit_test_group }}"
+                    group: "{{ __cockpit_test_group
+                      if  __cockpit_test_group else omit }}"
               include_role:
                 name: linux-system-roles.cockpit
                 public: true
+
+            - name: Set var to tell test to cleanup certificate
+              set_fact:
+                __cockpit_other_certificate: /etc/pki/tls/certs/{{ cert_name }}
           rescue:
             - name: Check the error message
               vars:
@@ -85,22 +81,6 @@
               assert:
                 that: "'status: MONITORING' in result.stdout"
 
-            - name: Test - clean up tracked certificate
-              command: >
-                getcert stop-tracking
-                -f "/etc/pki/tls/certs/{{ cert_name }}.crt"
-              changed_when: false
-
       always:
-        - name: Test - clean up generated certificate
-          file:
-            path: "/etc/pki/tls/certs/{{ cert_name }}.crt"
-            state: absent
-
-        - name: Test - clean up generated private key
-          file:
-            path: "/etc/pki/tls/private/{{ cert_name }}.key"
-            state: absent
-
         - name: Test - generic cleanup
           include_tasks: tasks/cleanup.yml
diff --git a/tests/tests_certificate_runafter.yml b/tests/tests_certificate_runafter.yml
@@ -6,27 +6,8 @@
 - name: Test certificate issuance with run_after shell script
   hosts: all
   tasks:
-    - name: Install cockpit
-      vars:
-        cockpit_packages: minimal
-      include_role:
-        name: linux-system-roles.cockpit
-        public: true
-
-    # self-signed is broken (https://github.com/linux-system-roles/certificate/issues/98),
-    # and has too restrictive keyUsage so that using the certificate as CA is not allowed
-    # (https://github.com/linux-system-roles/certificate/issues/99), thus use "local"
-    # Fixed in cockpit 255 (https://github.com/cockpit-project/cockpit/commit/6ec4881856e)
-    - name: Allow certmonger to write into Cockpit's certificate directory
-      file:
-        path: /etc/cockpit/ws-certs.d/
-        state: directory
-        setype: cert_t
-        mode: "0755"
-
-    # returns global variable __cockpit_test_group
-    - name: Get name of cockpit group to use
-      include_tasks: tasks/get_cockpit_group.yml
+    - name: Set up for certificate tests
+      include_tasks: tasks/setup_for_certificate_tests.yml
 
     # has to be done dynamically, as the first step checks it out
     - name: Generate certificate with certificate system role
@@ -38,7 +19,8 @@
           - name: monger-cockpit
             dns: ['localhost', 'www.example.com']
             ca: local
-            group: "{{ __cockpit_test_group }}"
+            group: "{{ __cockpit_test_group
+              if  __cockpit_test_group else omit }}"
             # ideally we'd put the cert directly into /etc/cockpit/ws-certs.d;
             # however, cockpit in RHEL/CentOS 7 does not yet support a separate
             # key file, and lsr.certificate sets wrong permissions
@@ -48,7 +30,9 @@
               cat {{ __certificate_default_directory }}/certs/monger-cockpit.crt \
               {{ __certificate_default_directory }}/private/monger-cockpit.key > $DEST
               chmod 640 $DEST
-              chown root:{{ __cockpit_test_group }} $DEST
+              if [ -n "{{ __cockpit_test_group }}" ]; then
+                chown root:{{ __cockpit_test_group }} $DEST
+              fi
 
     - name: Validate installation
       block:
@@ -107,10 +91,5 @@
             path: "{{ __certificate_default_directory }}/private/monger-cockpit.key"
             state: absent
 
-        - name: Test - clean up copied certificate
-          file:
-            path: /etc/cockpit/ws-certs.d/monger-cockpit.cert
-            state: absent
-
         - name: Test - generic cleanup
           include_tasks: tasks/cleanup.yml