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.

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

build: Remove cockpit(-ws-instance) user/group config option #20402

Merged
merged 1 commit into from
May 2, 2024

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Apr 30, 2024

All distributions use the same value anyway, and this paves the way for unifying them further with moving to systemd-sysusers.

We never test the upstream default of running them as root, so it might not even work and we also don't want to support/encourage that.


Broken out of #20365 and fixed. I want to test/land this separately, as it's quite intrusive.

@travier FYI

The release note also covers the changes from #20425 and #20447.

webserver: System user changes

This release drops the ./configure options for customizing the two system users of cockpit's web server:
--with-cockpit-user, --with-cockpit-group, --with-cockpit-ws-instance-user, and --with-cockpit-ws-instance-user. All supported distributions use the same name anyway. We don't test, support, or recommend running the web server as root, which so far has been the default with ./configure; make; make install.

The cockpit-ws system user is not created statically at all, but created transiently and on demand via systemd's DynamicUser feature. If you want to, you can safely remove the static system user with e.g. userdel -r cockpit-ws after the upgrade to this version and after systemctl stop cockpit.

The cockpit-wsinstance system user is now declared through systemd-sysusers.

All distributions use the same value anyway, and this paves the way for
unifying them further with moving to systemd-sysusers.

We never test the upstream default of running them as root, so it might
not even work and we also don't want to support/encourage that.

Co-Authored-By: Martin Pitt <mpitt@redhat.com>
@mvollmer
Copy link
Member

mvollmer commented May 2, 2024

Initial reaction: Why not just change the defaults to cockpit-ws and cockpit-wsinstance and keep the configurability?

But then again it's nice to reduce complexity.

@martinpitt
Copy link
Member Author

@mvollmer This was spawned from #20365 -- It's mostly cleaning up variability which we don't need anywhere or even test, and that autoconf seddery doesn't translate well into a world of "Linuxes should converge" and "use systemd sysusers everywhere". In particular, I'm not happy about defaulting "root". We could change the defaults, but it just feels like accidental complexity to me.

@allisonkarlitskaya
Copy link
Member

Thanks for this very nice cleanup. I always thought we had to keep this because debian or something had historically different values. If they're all the same anyway, then ya, definitely burn it down!

@allisonkarlitskaya allisonkarlitskaya merged commit 77fe203 into cockpit-project:main May 2, 2024
82 checks passed
@martinpitt martinpitt deleted the user-cleanup branch May 2, 2024 09:37
@travier
Copy link
Contributor

travier commented May 2, 2024

Thanks. Looks good.

@martinpitt
Copy link
Member Author

@allisonkarlitskaya I updated the release note here to also contain the DynamicUser and sysuser changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants