Add support for exit codes returned by snapctl commands #97
Conversation
jhenstridge
force-pushed
the
snapctl-exit-code
branch
from
698457a to
bc9492a
Compare
|
An alternative would be to continue returning a GError when we receive an "unsuccessful" response, but set stdout, stderr, and exit code. This would allow the old API to be a simple wrapper around the new one (rather than having to generate an error where the new one doesn't), and do similar for the Qt API. @robert-ancell: Do you have any preferences about this? |
|
I'd prefer to only use a GError when the error is a true exception and no futher processing can be done. I think that better matches the way other GLib APIs use it. Please continue with the original proposal. |
jhenstridge
force-pushed
the
snapctl-exit-code
branch
from
bc9492a to
e23a9d3
Compare
|
I was just looking through the snapd side for snapctl, and noticed that canonical/snapd#9692 had been merged. If we're adding a new prototype for snapctl, then it should probably support stdin as an argument too so we don't need |
|
After chatting with Michael, he suggests we ignore the current stdin support: it's only currently used by a full disk encryption setup command, and they don't like the current API because it requires draining stdin completely before making the REST API request. Adding a |
|
Just a couple of small fixes, then LGTM. Thanks @jhenstridge! |
Let the scheduler check whether if an administrative request (create/modify print queue, delete someone else's jobs, ...) from a client is from a fully confined Snap and then only grant access if the client Snap plugs "cups-control". If client Snap plugs "cups" instead it can only print, check status, or remove the caller's own jobs. For requests from classically confined Snaps or unsnapped clients access is always granted. This is to protect arbitrary Snaps from the Snap Store to do administrative CUPS tasks. The Snap Store allows automatic connection only of the "cups" interface, not of the cups-control interface. This facility is optional, to be activated by configure options, "--enable-snapped-clients" for unsnapped CUPS and "--enable-snapped-cupsd" when CUPS itself is also in a Snap. The former accesses the needed information about the client using libsnapd-glib (which cannot be used from within a Snap) and latter uses the "snapctl" utility (which only works from within a Snap). In both cases also libapparmor is needed to determine whether the client is actually a Snap.
This builds on top of PR #95, which adds support for inspecting the value embedded in an error response. The diff will be somewhat smaller once that is merged.
This PR adds new
snapd_client_run_snapctl2_*functions that add a newexit_codeout parameter. These differ from the previous version in that if an error response is received with kind "unsuccessful", then the stdout, stderr, and exit code will be decoded from the response. At present,snapctl is-connectedis the only command that returns these responses, but more could in the future.I've tried to keep the old
run_snapctl_*functions the same, such that they will still set the GError if they receive an unsuccessful error. Things would be a bit simpler if we didn't maintain that part of the API, but there would be no way for users of the old API to check error returns from is-connected otherwise.I haven't updated the Qt binding yet, with similar concerns. Switching QSnapdRunSnapCtlRequest to use the new API and adding an exitCode() method would be easy, but now code that was expecting a QSnapdError would no longer see it.
I do kind of wonder whether anyone has ever used this part of the snapd-glib API though, so it might not be so bad to take the simpler option.