Skip to content

Commit

Permalink
Allow snapctl refresh --proceed from snaps.
Browse files Browse the repository at this point in the history
  • Loading branch information
stolowski committed Aug 17, 2021
1 parent 4c67b39 commit 66dfd51
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 11 deletions.
8 changes: 8 additions & 0 deletions overlord/hookstate/ctlcmd/export_test.go
Expand Up @@ -108,3 +108,11 @@ func MockCgroupSnapNameFromPid(f func(int) (string, error)) (restore func()) {
cgroupSnapNameFromPid = old
}
}

func MockAutoRefreshForGatingSnap(f func(st *state.State, gatingSnap string) error) (restore func()) {
old := autoRefreshForGatingSnap
autoRefreshForGatingSnap = f
return func() {
autoRefreshForGatingSnap = old
}
}
31 changes: 26 additions & 5 deletions overlord/hookstate/ctlcmd/refresh.go
Expand Up @@ -25,12 +25,16 @@ import (

"gopkg.in/yaml.v2"

"github.com/snapcore/snapd/features"
"github.com/snapcore/snapd/i18n"
"github.com/snapcore/snapd/overlord/configstate/config"
"github.com/snapcore/snapd/overlord/hookstate"
"github.com/snapcore/snapd/overlord/snapstate"
"github.com/snapcore/snapd/snap"
)

var autoRefreshForGatingSnap = snapstate.AutoRefreshForGatingSnap

type refreshCommand struct {
baseCommand

Expand Down Expand Up @@ -89,12 +93,8 @@ func (c *refreshCommand) Execute(args []string) error {
if context == nil {
return fmt.Errorf("cannot run without a context")
}
if context.IsEphemeral() {
// TODO: handle this
return fmt.Errorf("cannot run outside of gate-auto-refresh hook")
}

if context.HookName() != "gate-auto-refresh" {
if !context.IsEphemeral() && context.HookName() != "gate-auto-refresh" {
return fmt.Errorf("can only be used from gate-auto-refresh hook")
}

Expand Down Expand Up @@ -214,6 +214,9 @@ func (c *refreshCommand) printPendingInfo() error {

func (c *refreshCommand) hold() error {
ctx := c.context()
if ctx.IsEphemeral() {
return fmt.Errorf("cannot hold outside of gate-auto-refresh hook")
}
ctx.Lock()
defer ctx.Unlock()
st := ctx.State()
Expand Down Expand Up @@ -241,6 +244,24 @@ func (c *refreshCommand) proceed() error {
ctx.Lock()
defer ctx.Unlock()

// running outside of hook
if ctx.IsEphemeral() {
st := ctx.State()
// we need to check if GateAutoRefreshHook feature is enabled when
// running by the snap (we don't need to do this when running from the
// hook because in that case hook task won't be created if not enabled).
tr := config.NewTransaction(st)
gateAutoRefreshHook, err := features.Flag(tr, features.GateAutoRefreshHook)
if err != nil && !config.IsNoOption(err) {
return err
}
if !gateAutoRefreshHook {
return fmt.Errorf("cannot proceed without experimental.gate-auto-refresh feature enabled")
}

return autoRefreshForGatingSnap(st, ctx.InstanceName())
}

// cache the action, hook handler will trigger proceed logic; we cannot
// call snapstate.ProceedWithRefresh() immediately as this would reset
// holdState, allowing the snap to --hold with fresh duration limit.
Expand Down
62 changes: 56 additions & 6 deletions overlord/hookstate/ctlcmd/refresh_test.go
Expand Up @@ -20,11 +20,13 @@
package ctlcmd_test

import (
"fmt"
"time"

. "gopkg.in/check.v1"

"github.com/snapcore/snapd/dirs"
"github.com/snapcore/snapd/overlord/configstate/config"
"github.com/snapcore/snapd/overlord/hookstate"
"github.com/snapcore/snapd/overlord/hookstate/ctlcmd"
"github.com/snapcore/snapd/overlord/hookstate/hooktest"
Expand Down Expand Up @@ -239,17 +241,65 @@ func (s *refreshSuite) TestRefreshFromUnsupportedHook(c *C) {
c.Check(err, ErrorMatches, `can only be used from gate-auto-refresh hook`)
}

// TODO: support this case
func (s *refreshSuite) TestRefreshFromApp(c *C) {
func (s *refreshSuite) TestRefreshProceedFromSnap(c *C) {
var called bool
restore := ctlcmd.MockAutoRefreshForGatingSnap(func(st *state.State, gatingSnap string) error {
called = true
c.Check(gatingSnap, Equals, "foo")
return nil
})
defer restore()

s.st.Lock()
defer s.st.Unlock()
mockInstalledSnap(c, s.st, `name: foo
version: 1
`)

setup := &hookstate.HookSetup{Snap: "snap", Revision: snap.R(1)}
mockContext, err := hookstate.NewContext(nil, s.st, setup, s.mockHandler, "")
// enable gate-auto-refresh-hook feature
tr := config.NewTransaction(s.st)
tr.Set("core", "experimental.gate-auto-refresh-hook", true)
tr.Commit()

// foo is the snap that is going to call --proceed.
setup := &hookstate.HookSetup{Snap: "foo", Revision: snap.R(1)}
mockContext, err := hookstate.NewContext(nil, s.st, setup, nil, "")
c.Check(err, IsNil)
s.st.Unlock()
defer s.st.Lock()

_, _, err = ctlcmd.Run(mockContext, []string{"refresh"}, 0)
c.Check(err, ErrorMatches, `cannot run outside of gate-auto-refresh hook`)
_, _, err = ctlcmd.Run(mockContext, []string{"refresh", "--proceed"}, 0)
c.Assert(err, IsNil)
c.Check(called, Equals, true)
}

func (s *refreshSuite) TestRefreshProceedFromSnapError(c *C) {
restore := ctlcmd.MockAutoRefreshForGatingSnap(func(st *state.State, gatingSnap string) error {
c.Check(gatingSnap, Equals, "foo")
return fmt.Errorf("boom")
})
defer restore()

s.st.Lock()
defer s.st.Unlock()
mockInstalledSnap(c, s.st, `name: foo
version: 1
`)

// enable gate-auto-refresh-hook feature
tr := config.NewTransaction(s.st)
tr.Set("core", "experimental.gate-auto-refresh-hook", true)
tr.Commit()

// foo is the snap that is going to call --proceed.
setup := &hookstate.HookSetup{Snap: "foo", Revision: snap.R(1)}
mockContext, err := hookstate.NewContext(nil, s.st, setup, nil, "")
c.Check(err, IsNil)
s.st.Unlock()
defer s.st.Lock()

_, _, err = ctlcmd.Run(mockContext, []string{"refresh", "--proceed"}, 0)
c.Assert(err, ErrorMatches, "boom")
}

func (s *refreshSuite) TestRefreshRegularUserForbidden(c *C) {
Expand Down

0 comments on commit 66dfd51

Please sign in to comment.