Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
overlord/ifacestate,daemon: setup security on conect and disconnect #948
+134
−34
Conversation
zyga
added some commits
Apr 14, 2016
zyga
referenced this pull request
Apr 14, 2016
Merged
overlord/ifacestate: reload snap connections when setting up security for a given snap #950
niemeyer
reviewed
Apr 14, 2016
| @@ -108,10 +110,15 @@ func (s *apiSuite) SetUpTest(c *check.C) { | ||
| configs: map[string]string{}, | ||
| } | ||
| s.d = nil | ||
| + // Disable real security backends for all API tests | ||
| + s.restoreBackends = ifacestate.MockSecurityBackendsForSnap( |
niemeyer
Apr 14, 2016
Contributor
How about making that simply MockSecurityBackends(nil)? I don't think any call site is using per-snap backends, which means all of that boilerplate is just unnecessary trouble.
niemeyer
reviewed
Apr 14, 2016
| +// MockSecurityBackendsForSnap mocks the list of security backends that are used for setting up security. | ||
| +// | ||
| +// This function is public because it is referenced in the daemon | ||
| +func MockSecurityBackendsForSnap(fn func(snapInfo *snap.Info) []interfaces.SecurityBackend) func() { |
niemeyer
Apr 14, 2016
Contributor
I actually meant this:
func MockSecurityBackends(backends []interfaces.SecurityBackend)
niemeyer
reviewed
Apr 14, 2016
| + task.Errorf("cannot get state of snap %q: %s", snapInfo.Name(), err) | ||
| + return state.Retry | ||
| + } | ||
| + for _, backend := range securityBackendsForSnap(snapInfo) { |
zyga
added some commits
Apr 14, 2016
niemeyer
reviewed
Apr 14, 2016
| @@ -365,8 +365,25 @@ func (m *InterfaceManager) doConnect(task *state.Task, _ *tomb.Tomb) error { | ||
| } | ||
| plug := m.repo.Plug(plugRef.Snap, plugRef.Name) | ||
| + slot := m.repo.Slot(slotRef.Snap, slotRef.Name) | ||
| + | ||
| + for _, snapInfo := range []*snap.Info{plug.Snap, slot.Snap} { |
niemeyer
Apr 14, 2016
Contributor
Can we please do this here instead:
err := setupSnapSecurity(task, plug.Snap, m.repo)
if err != nil {
return err
}
err = setupSnapSecurity(task, slot.Snap, m.repo)
if err != nil {
return err
}
niemeyer
reviewed
Apr 14, 2016
| + plug := m.repo.Plug(plugRef.Snap, plugRef.Name) | ||
| + slot := m.repo.Slot(slotRef.Snap, slotRef.Name) | ||
| + | ||
| + for _, snapInfo := range []*snap.Info{plug.Snap, slot.Snap} { |
niemeyer
reviewed
Apr 14, 2016
| +// | ||
| +// This function is public because it is referenced in the daemon | ||
| +func MockSecurityBackends(backends []interfaces.SecurityBackend) func() { | ||
| + securityBackends = func(snapInfo *snap.Info) []interfaces.SecurityBackend { |
niemeyer
Apr 14, 2016
Contributor
This should be
oldBackends = securityBackends
securityBackends = backends
return func() { securityBackends = oldBackends }
zyga
added some commits
Apr 14, 2016
niemeyer
reviewed
Apr 14, 2016
| + } | ||
| + for _, backend := range securityBackends { | ||
| + if err := backend.Setup(snapInfo, snapState.DevMode, repo); err != nil { | ||
| + task.Errorf("cannot setup security of snap %q (backend %s): %s", snapName, backend.Name(), err) |
niemeyer
Apr 14, 2016
Contributor
("cannot setup %s for snap %q: %s", backend.Name(), snapName, err)
|
Int tests infra is broken. Merging based on unit tests. |
niemeyer
merged commit 18fd1c0
into
snapcore:master
Apr 14, 2016
zyga
deleted the
zyga:overlord-connect-disconnect-setup-security
branch
Apr 14, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
zyga commentedApr 14, 2016
This branch makes connect/disconnect setup security of both affected snaps.