Use the certificate role to create the cert and the key #78

nhosoi · 2022-10-10T04:37:07Z

Introduce a variable cockpit_certificates to set the certificate_requests.
Add a test test script tests/tests_certificate2.yml

I borrowed (again :) the mssql README in linux-system-roles/mssql#125 written by @spetrosi. Thanks, Sergei!

@spetrosi, @martinpitt , this is another piece of the effort to use other roles in the cockpit role. If you could review it, I'd greatly appreciate it.

nhosoi · 2022-10-10T16:05:39Z

[citest]

spetrosi · 2022-10-11T12:05:34Z

README.md

+You can also use the `certificate` role inside the `cockpit` role to create
+certificates by providing `cockpit_certificates`.
+
+Use this variable to generate certificate and private key for TLS encryption


Suggested change

Use this variable to generate certificate and private key for TLS encryption

Use the `cockpit_certificates` variable to generate certificate and private key for TLS encryption

Thanks, @spetrosi!

nhosoi · 2022-10-11T17:07:11Z

[citest]

nhosoi · 2022-10-13T05:17:23Z

[citest pending]

nhosoi · 2022-10-14T22:06:38Z

@spetrosi, @martinpitt, all the CI tests have passed. Could you please review one more time? Thanks!

spetrosi

lgtm

nhosoi · 2022-10-17T16:13:57Z

Thanks a lot for your reviews, @spetrosi.
@martinpitt, could you please take another look? If it looks ok to you, may I ask you to merge this pr? Thanks, in advance.

martinpitt

Thank you! I'd like to see just one approach, not two. I also have some grammar/style fixes for the documentation.

martinpitt · 2022-10-18T08:17:39Z

README.md

@@ -170,6 +172,35 @@ You can also use `ca: self-sign` or `ca: local` depending on your certmonger usa

 Note that this does *not* work on RHEL/CentOS 7.

+#### Generate a new certificate in the role


I find this a bit nicer, and our role retains more control over what actually happens. So 👍 from me for moving to that approach.

But please let's actually move -- I find it confusing to offer "you can do it like this or that" in the README. Which one is better? (from the admin's POV). IMHO we should recommend just one approach.

The setype: hack is indeed obsolete. RHEL 8.6 has Cockpit 264, and CentOS 8 has something much newer. So we don't need this any more, neither with the "builtin" role.

I find this a bit nicer, and our role retains more control over what actually happens. So +1 from me for moving to that approach.

But please let's actually move -- I find it confusing to offer "you can do it like this or that" in the README. Which one is better? (from the admin's POV). IMHO we should recommend just one approach.

The setype: hack is indeed obsolete. RHEL 8.6 has Cockpit 264, and CentOS 8 has something much newer. So we don't need this any more, neither with the "builtin" role.

Hmmm, a good point... Maybe, we can say something like this?

### Generate a new certificate Now you can do it in the cockpit role by providing the cockpit_certificates variable as follows (in the nicer English, of course ;) - name: Install cockpit with Cockpit web server certificate include_role: name: linux-system-roles.cockpit vars: cockpit_certificates: - name: monger-cockpit dns: ['localhost', 'www.example.com'] ca: ipa group: cockpit-ws If you already have a playbook using the certificate role and the cockpit role separately, that's supported, too. - name: Generate Cockpit web server certificate include_role: name: linux-system-roles.certificate vars: certificate_requests: - name: /etc/cockpit/ws-certs.d/monger-cockpit dns: ['localhost', 'www.example.com'] ca: ipa group: cockpit-ws

Agreed -- documenting this new way as the primary method, and just giving this a fallback is fine.

README.md

tests/tests_certificate2.yml

nhosoi · 2022-10-20T19:12:24Z

[citest]

nhosoi · 2022-10-20T19:15:04Z

Thanks a lot, @martinpitt. I've pushed a commit fixing the issues you found (I hope :).
Could you please take one more look at the latest commit? Thanks!

nhosoi · 2022-10-20T23:14:02Z

[citest]

martinpitt

Thank you!

nhosoi · 2022-10-21T15:25:17Z

[citest pending]

nhosoi · 2022-10-21T22:08:38Z

Thank you for your reviews and suggestions, @martinpitt and @spetrosi.
All the CI tests have passed.
If you have the privilege and merge this pr, I'd appreciate it. Thanks!

nhosoi · 2022-10-24T23:20:28Z

@richm, I'm adding your note to the cockpit README, as well.
Do you think my addition is appropriate?

- Introduce a variable cockpit_certificates to set the certificate_requests. - Add a test script tests_certificate_internal.yml. - Rename tests_certificate.yml to tests_certificate_external.yml. - Update README so that using the certificate role is recommended.

nhosoi · 2022-10-25T03:47:47Z

[citest]

nhosoi · 2022-10-25T16:01:22Z

[citest pending]

nhosoi · 2022-10-25T18:09:07Z

I believe all the CI tests have passed.
This is a leftover from the old attempt...

RHEL-8.8.0-20221020.2/ansible-2.9/(citool) Pending — Test started

@richm, @martinpitt, could you please review one more time (if needed) and merge this pr? Thanks, in advance.

richm · 2022-10-27T22:18:44Z

tasks/main.yml

+- name: Create certificates
+  when:
+    - cockpit_certificates | length > 0
+    - ansible_facts['os_family'] == 'RedHat'


The node in the readme says NOTE: This does *not* work on RHEL/CentOS 7. - do we need to add a condition for that e.g.

- ansible_facts['distribution_version'] is version('>=', '8')

?

Ahh, sorry. The note is not needed. The issue was if we try to generate the new cert directly in the target location /etc/cockpit/ws-certs.d like this:

cockpit_certificates: - name: /etc/cockpit/ws-certs.d/cockpit_cert <<snip>>

it fails due to this bug on RHEL/CentOS-7 with an invalid mode.

In this PR, it generates the private key and cert in the standard location and copies them to the directories Cockpit expects. So, it is free from the issue. And there's no problem running on RHEL/CentOS-7. (CI successfully passed on the platforms.) ~~I'm removing the note.~~ Thanks for finding it out, @richm!

Oops, the note is needed. It is for this example (located above in README), in which the full path /etc/cockpit/ws-certs.d/monger-cockpit is specified. THAT does not work on RHEL/CentOS 7.

- name: Generate Cockpit web server certificate include_role: name: linux-system-roles.certificate vars: certificate_requests: - name: /etc/cockpit/ws-certs.d/monger-cockpit dns: ['localhost', 'www.example.com'] ca: ipa group: cockpit-ws

Do we want to describe in a little more detailed manner something like this?

NOTE: This specifying the certificate location by full path does *not* work on RHEL/CentOS 7.

Do we want to describe in a little more detailed manner something like this?

Yes - and - do we have a bz or some other issue tracker for this issue? Or maybe it is a "feature" of cockpit/certmonger?

nhosoi · 2022-10-28T21:09:13Z

[citest]

nhosoi · 2022-10-28T21:41:25Z

Do we want to describe in a little more detailed manner something like this?

Yes - and - do we have a bz or some other issue tracker for this issue? Or maybe it is a "feature" of the cockpit/certmonger?

These 2 issues were filed by @martinpitt against RHEL/CentOS-7 last year, which are still open.
linux-system-roles/certificate#98
linux-system-roles/certificate#99

And as you suggested, I've added a condition - if the distribution is RedHat and the version is 7, and the ca is self-sign, it fails with an error Creating a self-signed certificate is not supported on RHEL-7 or CentOS-7.

richm · 2022-10-28T22:45:45Z

tasks/main.yml

+      include_role:
+        name: fedora.linux_system_roles.certificate
+      vars:
+        __cert_name: "{{ cockpit_certificates.0.name  | basename }}"


where is __cert_name used?

Thank you, @richm. Removed it.

certificate on RHEL/CentOS-7.

nhosoi · 2022-10-29T00:21:50Z

[citest]

[1.4.0] - 2022-11-01 -------------------- ### New Features - Use the firewall role and the selinux role from the cockpit role (linux-system-roles#76) Since cockpit_port is a public api of the cockpit role, define it in defaults/main.yml as null. - Introduce cockpit_manage_firewall to use the firewall role to manage the cockpit service. Default to false - means the firewall role is not used. - Add the test check task tasks/check_port.yml for verifying the ports status. - Add meta/collection-requirements.yml. - Introduce cockpit_manage_selinux to use the selinux role to manage the ports in the cockpit service. Assign websm_port_t to the cockpit service ports. Default to false - means the selinux role is not used. - Use the certificate role to create the cert and the key (linux-system-roles#78) - Introduce a variable cockpit_certificates to set the certificate_requests. - Update README so that using the certificate role is recommended. Add a check and README note for not supporting creating a self-signed certificate on RHEL/CentOS-7. ### Bug Fixes - none ### Other Changes - workflows: Add integration-tests for Ubuntu 22.04 (linux-system-roles#68) The current LTS 22.04 is the more interesting target. This detects bugs like [1]. Keep 20.04 running as well for the time being. [1] linux-system-roles/certificate#130 - Clone the certificate role in the temporary dir. (linux-system-roles#77) Signed-off-by: Rich Megginson <rmeggins@redhat.com>

[1.4.0] - 2022-11-01 -------------------- ### New Features - Use the firewall role and the selinux role from the cockpit role (#76) Since cockpit_port is a public api of the cockpit role, define it in defaults/main.yml as null. - Introduce cockpit_manage_firewall to use the firewall role to manage the cockpit service. Default to false - means the firewall role is not used. - Add the test check task tasks/check_port.yml for verifying the ports status. - Add meta/collection-requirements.yml. - Introduce cockpit_manage_selinux to use the selinux role to manage the ports in the cockpit service. Assign websm_port_t to the cockpit service ports. Default to false - means the selinux role is not used. - Use the certificate role to create the cert and the key (#78) - Introduce a variable cockpit_certificates to set the certificate_requests. - Update README so that using the certificate role is recommended. Add a check and README note for not supporting creating a self-signed certificate on RHEL/CentOS-7. ### Bug Fixes - none ### Other Changes - workflows: Add integration-tests for Ubuntu 22.04 (#68) The current LTS 22.04 is the more interesting target. This detects bugs like [1]. Keep 20.04 running as well for the time being. [1] linux-system-roles/certificate#130 - Clone the certificate role in the temporary dir. (#77) Signed-off-by: Rich Megginson <rmeggins@redhat.com> Signed-off-by: Rich Megginson <rmeggins@redhat.com>

nhosoi changed the title ~~Use the certificate role to create the cert and the key~~ [WIP] Use the certificate role to create the cert and the key Oct 10, 2022

nhosoi force-pushed the certificate branch from cde8ca4 to 073a19a Compare October 10, 2022 15:59

spetrosi reviewed Oct 11, 2022

View reviewed changes

nhosoi force-pushed the certificate branch from 073a19a to 16491c5 Compare October 11, 2022 16:59

nhosoi changed the title ~~[WIP] Use the certificate role to create the cert and the key~~ Use the certificate role to create the cert and the key Oct 11, 2022

spetrosi previously approved these changes Oct 17, 2022

View reviewed changes

martinpitt requested changes Oct 18, 2022

View reviewed changes

nhosoi dismissed spetrosi’s stale review via 21b3019 October 20, 2022 19:05

nhosoi force-pushed the certificate branch from 21b3019 to 990a235 Compare October 20, 2022 21:57

martinpitt previously approved these changes Oct 21, 2022

View reviewed changes

nhosoi dismissed martinpitt’s stale review via 78b071f October 24, 2022 23:17

nhosoi force-pushed the certificate branch from 78b071f to f25ee78 Compare October 25, 2022 03:28

richm previously approved these changes Oct 25, 2022

View reviewed changes

richm reviewed Oct 27, 2022

View reviewed changes

nhosoi dismissed richm’s stale review via c876b59 October 28, 2022 20:27

nhosoi force-pushed the certificate branch from c876b59 to 2c644fd Compare October 28, 2022 21:02

richm reviewed Oct 28, 2022

View reviewed changes

Add a check and README note for not supporting creating a self-signed

a118f45

certificate on RHEL/CentOS-7.

nhosoi force-pushed the certificate branch from 2c644fd to a118f45 Compare October 29, 2022 00:06

richm approved these changes Oct 29, 2022

View reviewed changes

richm merged commit 706a601 into linux-system-roles:master Nov 1, 2022

richm mentioned this pull request Nov 1, 2022

Version 1.4.0 - CHANGELOG.md [citest skip] #79

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use the certificate role to create the cert and the key #78

Use the certificate role to create the cert and the key #78

nhosoi commented Oct 10, 2022 •

edited

Loading

nhosoi commented Oct 10, 2022

spetrosi Oct 11, 2022

nhosoi Oct 11, 2022

nhosoi commented Oct 11, 2022

nhosoi commented Oct 13, 2022

nhosoi commented Oct 14, 2022

spetrosi left a comment

nhosoi commented Oct 17, 2022

martinpitt left a comment

martinpitt Oct 18, 2022

nhosoi Oct 18, 2022

martinpitt Oct 20, 2022

nhosoi commented Oct 20, 2022

nhosoi commented Oct 20, 2022

nhosoi commented Oct 20, 2022

martinpitt left a comment

nhosoi commented Oct 21, 2022

nhosoi commented Oct 21, 2022

nhosoi commented Oct 24, 2022

nhosoi commented Oct 25, 2022

nhosoi commented Oct 25, 2022

nhosoi commented Oct 25, 2022

richm Oct 27, 2022

nhosoi Oct 27, 2022 •

edited

Loading

richm Oct 28, 2022 •

edited

Loading

nhosoi commented Oct 28, 2022

nhosoi commented Oct 28, 2022

richm Oct 28, 2022

nhosoi Oct 29, 2022

nhosoi commented Oct 29, 2022

	Use this variable to generate certificate and private key for TLS encryption
	Use the `cockpit_certificates` variable to generate certificate and private key for TLS encryption

		@@ -170,6 +172,35 @@ You can also use `ca: self-sign` or `ca: local` depending on your certmonger usa

		Note that this does not work on RHEL/CentOS 7.

		#### Generate a new certificate in the role

Use the certificate role to create the cert and the key #78

Use the certificate role to create the cert and the key #78

Conversation

nhosoi commented Oct 10, 2022 • edited Loading

nhosoi commented Oct 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhosoi commented Oct 11, 2022

nhosoi commented Oct 13, 2022

nhosoi commented Oct 14, 2022

spetrosi left a comment

Choose a reason for hiding this comment

nhosoi commented Oct 17, 2022

martinpitt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhosoi commented Oct 20, 2022

nhosoi commented Oct 20, 2022

nhosoi commented Oct 20, 2022

martinpitt left a comment

Choose a reason for hiding this comment

nhosoi commented Oct 21, 2022

nhosoi commented Oct 21, 2022

nhosoi commented Oct 24, 2022

nhosoi commented Oct 25, 2022

nhosoi commented Oct 25, 2022

nhosoi commented Oct 25, 2022

Choose a reason for hiding this comment

nhosoi Oct 27, 2022 • edited Loading

Choose a reason for hiding this comment

richm Oct 28, 2022 • edited Loading

Choose a reason for hiding this comment

nhosoi commented Oct 28, 2022

nhosoi commented Oct 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nhosoi commented Oct 29, 2022

nhosoi commented Oct 10, 2022 •

edited

Loading

nhosoi Oct 27, 2022 •

edited

Loading

richm Oct 28, 2022 •

edited

Loading