Try both --output=json and --json=short when running loginctl list-sessions output
#1759
Conversation
| return nil, fmt.Errorf("creating loginctl command --no-legend --no-pager --output=json: %w", err) | ||
| } | ||
| legacyOut, err := legacyCmd.Output() | ||
| if err == nil { |
There was a problem hiding this comment.
No point in trying to unmarshal if the command failed, right -- or have I missed something?
There was a problem hiding this comment.
If the command failed, it returns on line 76
There was a problem hiding this comment.
That's the creation of the exec.Cmd struct -- we return on 76 if we fail to validate the command (e.g. if we can't find the loginctl binary). But we don't exec the command till line 78, so we're checking for exec failures here.
There was a problem hiding this comment.
Oh, I see it now. This is inverted logic -- no error early returns. You could probably combine, if legacyOut, err := legacyCmd.Output(); err == nil { but I don' think that changes anything real.
Thank you for putting up with my fuzziness.
| return nil, fmt.Errorf("creating loginctl command --no-legend --no-pager --output=json: %w", err) | ||
| } | ||
| legacyOut, err := legacyCmd.Output() | ||
| if err == nil { |
There was a problem hiding this comment.
Oh, I see it now. This is inverted logic -- no error early returns. You could probably combine, if legacyOut, err := legacyCmd.Output(); err == nil { but I don' think that changes anything real.
Thank you for putting up with my fuzziness.
The output flag for
loginctl list-sessionshas changed in more recent versions of systemd: https://www.freedesktop.org/software/systemd/man/latest/loginctl.html#Session%20CommandsOn Ubuntu at least up to 24.04, systemd doesn't use this new
--jsonflag -- I think at this point, the older flag will be much more widely supported in our userbase. So I have defaulted to trying with the older flag first, and falling back to the newer flag in case of error. Eventually we will want to swap the order that we try the flags in.Confirmed
--json=shortoutput is compatible with what we previously expected (session, uid, user, seat all present):Relates to #1758
Notes on setting up a test environment
I set up an arch VM in Google Cloud to test this -- here are the steps.
I used a base image from this project and created a VM:
I SSHed to the VM and got it updated via these instructions
Validated that it's adequately updated via
loginctl --versionandloginctl list-sessions --no-legend --no-pager --json=short.I stopped the VM, enabled
display device, and restarted the VM – I think this is what got me aRemote=no Active=yessession to be able to get a desktop process going.I uploaded launcher binary from this PR's artifacts to VM; I uploaded osqueryd binary from the releases to the VM;
chmod +xboth.I made a root dir for testing, then ran launcher and let it start up:
In another window, I confirmed I saw the launcher desktop process eventually --
ps -eaf | grep launcher. There were some errors (e.g. no tray is running, so we get errors about no tray running), but seeing the desktop process running should be enough to prove this fix.