test: omit setting group for certificate tests on cockpit 330 and later #188
Conversation
richm
force-pushed
the
group-2
branch
3 times, most recently
from
28d1fd0 to
1b6fbb6
Compare
|
[citest] |
|
[citest] |
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]>
|
[citest] |
|
Not sure what's going on with the integration tests. The error is that the cockpit service/socket restarts too quickly too many times, and systemd is shutting it down. But I'm not sure why it is happening with my PR - perhaps I changed the timing? At any rate, we have seen similar issues in other tests, and have had to fix them like https://github.com/linux-system-roles/journald/blob/main/tests/tests_example.yml#L28 - I will submit another PR to address that in the tests. |
|
and this version doesn't clean up /etc/cockpit/ws-certs.d/50-system-role.* - because I didn't want to remove any files installed by the package, I changed the code that removed /etc/cockpit/ws-certs.d/* |
There was a problem hiding this comment.
Thanks! I really don't like the proliferation of the group: setting. #187 gets rid of that in a more thorough and forward-looking way.
However, the factorization of cert setup for the three test_certificate* is nice -- perhaps you can dedicate this PR to that, and we land both?
| @@ -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 | |||
There was a problem hiding this comment.
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.
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]