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

GUACAMOLE-1981: Add configure argument for systemd user #543

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

morganwillcock
Copy link

This adds an additional configure argument --with-systemd-user.

I've tried to follow the pattern of how other configure arguments and variables are defined. The indentation style for the changes in Makefile.am is taken from an example in the Automake manual, using an additional space character to indent a multi-line command.

Here are some examples which show the additional configure output:

$ ./configure --help | grep -A2 with-systemd-user
  --with-systemd-user=<username>
                          the user configured in the systemd unit to run guacd
                          [default=daemon]

$ 2>/dev/null ./configure | grep -B1 'Systemd user:'
   Systemd units: no
   Systemd user: no

$ 2>/dev/null ./configure --with-systemd-dir=/etc/systemd/system | grep -B1 'Systemd user:'
   Systemd units: /etc/systemd/system
   Systemd user: daemon

$ 2>/dev/null ./configure --with-systemd-dir=/etc/systemd/system --with-systemd-user=guacd | grep -B1 'Systemd user:'
   Systemd units: /etc/systemd/system
   Systemd user: guacd

@necouchman
Copy link
Contributor

Thanks @morganwillcock, this looks pretty good. Does it make sense to also make the group configurable, and add that in?

@morganwillcock
Copy link
Author

I'm not sure about doing the same for the group. If no group is specified it should automatically use the default group of the user.

It probably isn't a good idea to try and guess ahead of time which Systemd settings will be useful and then start adding additional configure arguments for each. It could be argued that setting the user with a configure argument is already an abuse of the build system - personally I think this single case is justified to avoid running [a remote access service] as root by default, while still allowing the unit file to be installed without additional changes.

@necouchman
Copy link
Contributor

@morganwillcock : That's fair - I was just thinking of other software builds that I've seen, and often when a user option is available at build-time a group option is also available. But I'm not feeling particularly strongly that it has to be there.

@morganwillcock
Copy link
Author

I think the problem would be that creating the unit file with a default group name set in it will probably break people's existing deployment mechanisms which wouldn't expect it to be there.

i.e. There may already be something in place to modify the user before starting the service, but there wouldn't necessarily be anything configured to set the group.

Just on the grounds of compatibility with previous releases, it is probably best to omit the group unless there was some compelling case to always set it.

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.

2 participants