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 13, 2024 · b7b5450 · b7b5450
1 parent 83f3c9b
commit b7b5450
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 138 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
@@ -22,24 +22,31 @@
     - always
     - tests::cleanup
 
-- name: Cleanup - find certificates
-  find:
-    paths: /etc/cockpit/ws-certs.d/
-    file_type: any
-    patterns: "*"
-  register: certs_to_delete
-  tags:
-    - always
-    - tests::cleanup
-
 - name: Cleanup - certificates
-  file:
-    path: "{{ item.path }}"
-    state: absent
-  with_items: "{{ certs_to_delete.files }}"
-  tags:
-    - always
-    - tests::cleanup
+  when: __test_cert_files | d([]) | length > 0
+  block:
+    - name: Cleanup - stop tracking certificates
+      command: getcert stop-tracking -f {{ item }}
+      changed_when: false
+      with_items: "{{ __test_cert_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 - remove certificate and key files
+      file:
+        path: "{{ item }}"
+        state: absent
+      with_items: "{{ __test_cert_files + __test_key_files }}"
+      tags:
+        - always
+        - tests::cleanup
 
 - name: Cleanup - config file
   file:

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
@@ -1,45 +1,32 @@
 ---
 - name: Test with generated self-signed certmonger certificate
   hosts: all
+  vars:
+    __test_cert_name: /etc/cockpit/ws-certs.d/monger-cockpit
+    __test_cert_files:
+      - "{{ __test_cert_name }}.crt"
+    __test_key_files:
+      - "{{ __test_cert_name }}.key"
   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:
                 name: fedora.linux_system_roles.certificate
               vars:
                 certificate_requests:
-                  - name: /etc/cockpit/ws-certs.d/monger-cockpit
+                  - name: "{{ __test_cert_name }}"
                     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
@@ -57,7 +44,7 @@
               # noqa command-instead-of-module
               command:
                 cmd: >
-                  curl --cacert /etc/cockpit/ws-certs.d/monger-cockpit.crt
+                  curl --cacert {{ __test_cert_name }}.crt
                   https://localhost:9090
                 # ansible 2.11's uri module has ca_path,
                 # but that's still too new for us
@@ -66,30 +53,14 @@
             - name: Test - get certmonger tracking status
               command: >
                 getcert list  --tracking-only
-                -f /etc/cockpit/ws-certs.d/monger-cockpit.crt
+                -f {{ __test_cert_name }}.crt
               register: result
               changed_when: false
 
             - name: Test - ensure certificate generation succeeded
               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
@@ -1,37 +1,41 @@
 ---
 - name: Test the cockpit role calling the certificate role internally
   hosts: all
+  vars:
+    __test_cert_name: cockpit_cert
+    __test_cert_files:
+      - /etc/cockpit/ws-certs.d/{{ __test_cert_name }}.crt
+      - /etc/pki/tls/certs/certs/{{ __test_cert_name }}.crt
+    __test_key_files:
+      - /etc/cockpit/ws-certs.d/{{ __test_cert_name }}.key
+      - /etc/pki/tls/certs/private/{{ __test_cert_name }}.key
   tasks:
     - name: Tests
-      vars:
-        cert_name: cockpit_cert
       block:
         - 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:
                 cockpit_packages: minimal
                 cockpit_certificates:
-                  - name: "{{ cert_name }}"
+                  - name: "{{ __test_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: See if cert file exists
+              stat:
+                path: /etc/pki/tls/certs/{{ __test_cert_name }}.crt
+              register: __cockpit_other_certificate
+
           rescue:
             - name: Check the error message
               vars:
@@ -68,7 +72,7 @@
               # noqa command-instead-of-module
               command:
                 cmd: >
-                  curl --cacert "/etc/pki/tls/certs/{{ cert_name }}.crt"
+                  curl --cacert "/etc/pki/tls/certs/{{ __test_cert_name }}.crt"
                   https://localhost:9090
                 # ansible 2.11's uri module has ca_path,
                 # but that's still too new for us
@@ -77,30 +81,14 @@
             - name: Test - get certmonger tracking status
               command: >
                 getcert list  --tracking-only
-                -f "/etc/pki/tls/certs/{{ cert_name }}.crt"
+                -f "/etc/pki/tls/certs/{{ __test_cert_name }}.crt"
               register: result
               changed_when: false
 
             - name: Test - ensure certificate generation succeeded
               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