Skip to content

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

Merged
necouchman merged 1 commit intoapache:patchfrom
morganwillcock:systemd-user
Aug 16, 2025
Merged

GUACAMOLE-1981: Add configure argument for systemd user#543
necouchman merged 1 commit intoapache:patchfrom
morganwillcock:systemd-user

Conversation

@morganwillcock
Copy link
Copy Markdown
Contributor

@morganwillcock morganwillcock commented Sep 3, 2024

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
Copy Markdown
Contributor

necouchman commented Sep 3, 2024

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

@morganwillcock
Copy link
Copy Markdown
Contributor Author

morganwillcock commented Sep 3, 2024

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
Copy Markdown
Contributor

necouchman commented Sep 13, 2024

@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
Copy Markdown
Contributor Author

morganwillcock commented Sep 14, 2024

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.

@Vebryn
Copy link
Copy Markdown

Vebryn commented Aug 8, 2025

On debian bookworm with latest 1.6.0:

août 08 11:59:45 sv1 guacd[1629]: guacd[1629]: INFO:        Creating new client for protocol "rdp"
août 08 11:59:45 sv1 guacd[1629]: Connection ID is "$6cc13365-1523-40ab-aa49-120c318af223"
août 08 11:59:45 sv1 guacd[1629]: guacd[1629]: INFO:        Connection ID is "$6cc13365-1523-40ab-aa49-120c318af223"
août 08 11:59:45 sv1 guacd[14781]: FreeRDP initialization may fail: The current user's home directory ("/usr/sbin") is not writable, but FreeRDP generally requires a writable home directory for storage of configuration files and certificates.
août 08 11:59:45 sv1 guacd[14781]: guacd[14781]: WARNING:        FreeRDP initialization may fail: The current user's home directory ("/usr/sbin") is not writable, but FreeRDP generally requires a writable home directory for storage of configuration files and certificates.
août 08 11:59:45 sv1 guacd[1629]: Connection "$6cc13365-1523-40ab-aa49-120c318af223" removed.
août 08 11:59:45 sv1 guacd[1629]: guacd[1629]: INFO:        Connection "$6cc13365-1523-40ab-aa49-120c318af223" removed.

After switching to guacd user into systemd unit, no more warning, it connect:

août 08 12:04:25 sv1 guacd[15600]: Creating new client for protocol "rdp"
août 08 12:04:25 sv1 guacd[15600]: guacd[15600]: INFO:        Creating new client for protocol "rdp"
août 08 12:04:25 sv1 guacd[15600]: guacd[15600]: INFO:        Connection ID is "$fffc02f2-c80a-4dc0-bbe3-12b242f53a36"
août 08 12:04:25 sv1 guacd[15600]: Connection ID is "$fffc02f2-c80a-4dc0-bbe3-12b242f53a36"
août 08 12:04:25 sv1 guacd[15609]: No security mode specified. Defaulting to security mode negotiation with server.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        No security mode specified. Defaulting to security mode negotiation with server.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        Resize method: none
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        No clipboard line-ending normalization specified. Defaulting to preserving the format of all line endings.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        User "@a5697992-9f08-459f-8fdf-23cc38f1025b" joined connection "$fffc02f2-c80a-4dc0-bbe3-12b242f53a36" (1 users now present)
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        Local system reports 6 processor(s) are available.
août 08 12:04:25 sv1 guacd[15609]: guacd[15609]: INFO:        Graphical updates will be encoded using 6 worker thread(s).

Any news on this PR ?

Best regards

@necouchman
Copy link
Copy Markdown
Contributor

necouchman commented Aug 8, 2025

@morganwillcock I'm good with these changes - could you re-base this on the patch branch?

@morganwillcock morganwillcock changed the base branch from main to patch August 16, 2025 09:11
@morganwillcock
Copy link
Copy Markdown
Contributor Author

morganwillcock commented Aug 16, 2025

@necouchman It should now be mergable on the patch branch.

@necouchman necouchman merged commit 3e1eb3c into apache:patch Aug 16, 2025
1 check passed
@morganwillcock morganwillcock deleted the systemd-user branch August 16, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@necouchman necouchman necouchman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@morganwillcock @necouchman @Vebryn