From 28d1fd0d13fa0b524d3e137e7885ab00f33173a0 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Thu, 12 Dec 2024 11:01:36 -0700 Subject: [PATCH] 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 --- README.md | 4 +- tests/tasks/cleanup.yml | 44 +++++++++++++++++++- tests/tasks/setup_for_certificate_tests.yml | 39 ++++++++++++++++++ tests/tests_certificate_external.yml | 45 +++------------------ tests/tests_certificate_internal.yml | 36 ++++------------- tests/tests_certificate_runafter.yml | 35 ++++------------ 6 files changed, 105 insertions(+), 98 deletions(-) create mode 100644 tests/tasks/setup_for_certificate_tests.yml diff --git a/README.md b/README.md index 36da52d..fca925d 100644 --- 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 index 359267c..f625a1d 100644 --- 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 new file mode 100644 index 0000000..a00ceea --- /dev/null +++ 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 index 3aeab53..fec843d 100644 --- 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 index b287184..b744081 100644 --- 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 index f86d61b..e432809 100644 --- 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