Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This advice is wrong and even dangerous -- files should never be owned by cockpit-wsinstance. cockpit-tls needs to read the certificates, but the actual cockpit-ws units really shouldn't. There were only two situations:

  • with cockpit < 257 the certificate needed to readable by group cockpit-ws
  • after that, the certificate ownership does not matter any more (it's copied by a tiny root helper process), so you can share it with other services; but it should not be owned by system groups.

It also keeps setting the group by default, which has been obsolete for a loooong time. It's only necessary on RHEL 7.

```

Note: Generating a new certificate using the `certificate` system role in the playbook remains supported.
Expand All @@ -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
```

Expand Down
41 changes: 24 additions & 17 deletions tests/tasks/cleanup.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
39 changes: 39 additions & 0 deletions tests/tasks/setup_for_certificate_tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
57 changes: 14 additions & 43 deletions tests/tests_certificate_external.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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
54 changes: 21 additions & 33 deletions tests/tests_certificate_internal.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
Loading
Loading