interfaces: add auto-connection information to interfaces #801

Merged
merged 4 commits into from Apr 7, 2016

Conversation

Projects
None yet
3 participants
Contributor

zyga commented Apr 6, 2016

This patch adds a new method to indicate if a plug / slot of a given
interface should be automatically connected.

Currently only the following interfaces are marked for auto-connection:

  • network
  • network-bind

Other interfaces may be added later, once assertions are in place to
ensure that those are allowed for a given snap (e.g. firewall-control).

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

interfaces: add auto-connection information to interfaces
This patch adds a new method to indicate if a plug / slot of a given
interface should be automatically connected.

Currently only the following interfaces are marked for auto-connection:

 - network
 - network-bind

Other interfaces may be added later, once assertions are in place to
ensure that those are allowed for a given snap (e.g. firewall-control).

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Contributor

jdstrand commented Apr 6, 2016

As discussed on IRC, I think it is fine to autoconnect network and network-bind until we work out how assertions will play into autoconnects and the UX is designed for when things are requested but not autoconnected.

Looking at the patch, I'm somewhat puzzled: you are explicitly setting autoconnect to false for bool file, but assuming it is false for the other builtins (ie, you set network and network-bind to true, but don't set any of the others). For defense in depth, I think autoconnect should default to false unless specifically set to true in the interface, but I don't see where that is happening in the patch.

Contributor

zyga commented Apr 6, 2016

@jdstrand this happens courtesy to go's zero-value defaults. The zero-value for bool is simply false (there are no uninitialized variables in go)

interfaces/core.go
@@ -133,6 +133,10 @@ type Interface interface {
// is returned when the plug cannot deal with the requested security
// system.
ConnectedSlotSnippet(plug *Plug, slot *Slot, securitySystem SecuritySystem) ([]byte, error)
+
+ // AutoConnect returns true if plugs and slots should be implicitly
@niemeyer

niemeyer Apr 7, 2016

Contributor

s/true if/whether/

interfaces/core.go
@@ -133,6 +133,10 @@ type Interface interface {
// is returned when the plug cannot deal with the requested security
// system.
ConnectedSlotSnippet(plug *Plug, slot *Slot, securitySystem SecuritySystem) ([]byte, error)
+
+ // AutoConnect returns true if plugs and slots should be implicitly
+ // auto-connected when an unambiguous connection candidate is available.
@niemeyer

niemeyer Apr 7, 2016

Contributor

... is available in the OS snap.

interfaces/testtype.go
@@ -40,6 +40,8 @@ type TestInterface struct {
PlugSnippetCallback func(plug *Plug, slot *Slot, securitySystem SecuritySystem) ([]byte, error)
// PermanentPlugSnippetCallback is the callback invoked inside PermanentPlugSnippet()
PermanentPlugSnippetCallback func(plug *Plug, securitySystem SecuritySystem) ([]byte, error)
+ // AutoConnectCallback is the callback invoked in inside AutoConnect()
+ AutoConnectCallback func() bool
@niemeyer

niemeyer Apr 7, 2016

Contributor

Can this be a bool, similar to InterfaceName?

@zyga

zyga Apr 7, 2016

Contributor

Done

Contributor

niemeyer commented Apr 7, 2016

LGTM.. just a few trivials. Please feel free to merge once these comments are in, with @jdstrand's being the second review.

zyga added some commits Apr 7, 2016

interfaces: tweak comment on Interface.AutoConnect
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@niemeyer niemeyer added the Reviewed label Apr 7, 2016

interfaces: use variable for TestInterface.AutoConnect
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@zyga zyga merged commit c6f4176 into snapcore:master Apr 7, 2016

2 of 4 checks passed

Integration tests Triggered
Details
autopkgtest Triggered
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 76.414%
Details

@zyga zyga deleted the zyga:ifaces-autoconnect-flag branch Apr 7, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment