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

o/hookstate/ctlcmd: add a new exit code for is-connected for cases when the peer is from the same snap #10024

Closed

Conversation

jhenstridge
Copy link
Contributor

This PR grew out of discussions about the CUPS snap on the forum:

https://forum.snapcraft.io/t/request-cups-snap-cups-auto-connection-to-of-cups-cups-control-to-cups-admin-and-also-of-the-network-manager-observe-interface/22802/25?u=jamesh

At present, Till's CUPS snap has both a plug and slot for the cups-control interface, with a request that they be auto-connected. At present the plug is not actually necessary to connect to the CUPS socket (that permission comes with the slot), but is needed to verify that utilities like lpadmin to pass the snapctl is-connected check.

As discussed in the forum thread, we could avoid the need for the extra plug if snapctl is-connected grew a new exit code representing "peer is not connected, but belongs to the same snap". So that is what this PR implements.

Copy link
Contributor

@stolowski stolowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

@pedronis pedronis self-requested a review March 10, 2021 13:20
@pedronis pedronis self-assigned this Jun 3, 2021
@pedronis
Copy link
Collaborator

as discussed, self-connections are not fully unusual, if a snap ships both the server side of a service and client tool, it's possibly a better pattern for the slot permissions to be given only to the services that constitute the server side, in which case the client applications might need a self-connection anyway to talk to the server bits. This code here helps when not doing this, so until there's a clearer use case I would prefer not to go into this direction.

@pedronis pedronis closed this Jun 10, 2021
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.

3 participants