Skip to content

Commit

Permalink
Update validation set assertions only when updating all snaps.
Browse files Browse the repository at this point in the history
  • Loading branch information
@stolowski
stolowski committed Sep 22, 2021
1 parent 673cd60 commit 354a19d
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 23 deletions.
12 changes: 8 additions & 4 deletions daemon/api_snaps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/snapcore/snapd/client"
"github.com/snapcore/snapd/i18n"
"github.com/snapcore/snapd/logger"
"github.com/snapcore/snapd/overlord/assertstate"
"github.com/snapcore/snapd/overlord/auth"
"github.com/snapcore/snapd/overlord/servicestate"
"github.com/snapcore/snapd/overlord/snapstate"
Expand Down Expand Up @@ -336,7 +337,7 @@ func snapUpdate(inst *snapInstruction, st *state.State) (string, []*state.TaskSe
}

// we need refreshed snap-declarations to enforce refresh-control as best as we can
if err = assertstateRefreshSnapAssertions(st, inst.userID); err != nil {
if err = assertstateRefreshSnapAssertions(st, inst.userID, nil); err != nil {
return "", nil, err
}

Expand Down Expand Up @@ -586,9 +587,12 @@ func snapInstallMany(inst *snapInstruction, st *state.State) (*snapInstructionRe
func snapUpdateMany(inst *snapInstruction, st *state.State) (*snapInstructionResult, error) {
// we need refreshed snap-declarations to enforce refresh-control as best as
// we can, this also ensures that snap-declarations and their prerequisite
// assertions are updated regularly, as well as updates validation sets
// assertions.
if err := assertstateRefreshSnapAssertions(st, inst.userID); err != nil {
// assertions are updated regularly; update validation sets assertions only
// if refreshing all snaps (no snap names explicitly requested).
opts := &assertstate.RefreshAssertionsOptions{
IsRefreshOfAllSnaps: len(inst.Snaps) == 0,
}
if err := assertstateRefreshSnapAssertions(st, inst.userID, opts); err != nil {
return nil, err
}

Expand Down
39 changes: 24 additions & 15 deletions daemon/api_snaps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ func (s *snapsSuite) TestPostSnapsOpMoreComplexContentType(c *check.C) {
}

func (s *snapsSuite) testPostSnapsOp(c *check.C, contentType string) {
defer daemon.MockAssertstateRefreshSnapAssertions(func(*state.State, int) error { return nil })()
defer daemon.MockAssertstateRefreshSnapAssertions(func(*state.State, int, *assertstate.RefreshAssertionsOptions) error { return nil })()
defer daemon.MockSnapstateUpdateMany(func(_ context.Context, s *state.State, names []string, userID int, flags *snapstate.Flags) ([]string, []*state.TaskSet, error) {
c.Check(names, check.HasLen, 0)
t := s.NewTask("fake-refresh-all", "Refreshing everything")
Expand Down Expand Up @@ -519,9 +519,11 @@ func (s *snapsSuite) TestPostSnapsOpInvalidCharset(c *check.C) {

func (s *snapsSuite) TestRefreshAll(c *check.C) {
refreshSnapAssertions := false
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
var refreshAssertionsOpts *assertstate.RefreshAssertionsOptions
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
refreshSnapAssertions = true
return assertstate.RefreshSnapAssertions(s, userID)
refreshAssertionsOpts = opts
return assertstate.RefreshSnapAssertions(s, userID, opts)
})()

d := s.daemon(c)
Expand All @@ -535,6 +537,7 @@ func (s *snapsSuite) TestRefreshAll(c *check.C) {
{[]string{"fake1", "fake2"}, `Refresh snaps "fake1", "fake2"`},
} {
refreshSnapAssertions = false
refreshAssertionsOpts = nil

defer daemon.MockSnapstateUpdateMany(func(_ context.Context, s *state.State, names []string, userID int, flags *snapstate.Flags) ([]string, []*state.TaskSet, error) {
c.Check(names, check.HasLen, 0)
Expand All @@ -550,14 +553,16 @@ func (s *snapsSuite) TestRefreshAll(c *check.C) {
c.Assert(err, check.IsNil)
c.Check(res.Summary, check.Equals, tst.msg)
c.Check(refreshSnapAssertions, check.Equals, true)
c.Assert(refreshAssertionsOpts, check.NotNil)
c.Check(refreshAssertionsOpts.IsRefreshOfAllSnaps, check.Equals, true)
}
}

func (s *snapsSuite) TestRefreshAllNoChanges(c *check.C) {
refreshSnapAssertions := false
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
refreshSnapAssertions = true
return assertstate.RefreshSnapAssertions(s, userID)
return assertstate.RefreshSnapAssertions(s, userID, opts)
})()

defer daemon.MockSnapstateUpdateMany(func(_ context.Context, s *state.State, names []string, userID int, flags *snapstate.Flags) ([]string, []*state.TaskSet, error) {
Expand All @@ -578,8 +583,10 @@ func (s *snapsSuite) TestRefreshAllNoChanges(c *check.C) {

func (s *snapsSuite) TestRefreshMany(c *check.C) {
refreshSnapAssertions := false
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
var refreshAssertionsOpts *assertstate.RefreshAssertionsOptions
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
refreshSnapAssertions = true
refreshAssertionsOpts = opts
return nil
})()

Expand All @@ -599,11 +606,13 @@ func (s *snapsSuite) TestRefreshMany(c *check.C) {
c.Check(res.Summary, check.Equals, `Refresh snaps "foo", "bar"`)
c.Check(res.Affected, check.DeepEquals, inst.Snaps)
c.Check(refreshSnapAssertions, check.Equals, true)
c.Assert(refreshAssertionsOpts, check.NotNil)
c.Check(refreshAssertionsOpts.IsRefreshOfAllSnaps, check.Equals, false)
}

func (s *snapsSuite) TestRefreshMany1(c *check.C) {
refreshSnapAssertions := false
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
refreshSnapAssertions = true
return nil
})()
Expand Down Expand Up @@ -1434,7 +1443,7 @@ func (s *snapsSuite) TestInstallIgnoreValidation(c *check.C) {
t := s.NewTask("fake-install-snap", "Doing a fake install")
return state.NewTaskSet(t), nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
return nil
})()

Expand Down Expand Up @@ -1600,7 +1609,7 @@ func (s *snapsSuite) TestRefresh(c *check.C) {
t := s.NewTask("fake-refresh-snap", "Doing a fake install")
return state.NewTaskSet(t), nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
assertstateCalledUserID = userID
return nil
})()
Expand Down Expand Up @@ -1639,7 +1648,7 @@ func (s *snapsSuite) TestRefreshDevMode(c *check.C) {
t := s.NewTask("fake-refresh-snap", "Doing a fake install")
return state.NewTaskSet(t), nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
return nil
})()

Expand Down Expand Up @@ -1673,7 +1682,7 @@ func (s *snapsSuite) TestRefreshClassic(c *check.C) {
calledFlags = flags
return nil, nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
return nil
})()

Expand Down Expand Up @@ -1707,7 +1716,7 @@ func (s *snapsSuite) TestRefreshIgnoreValidation(c *check.C) {
t := s.NewTask("fake-refresh-snap", "Doing a fake install")
return state.NewTaskSet(t), nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
return nil
})()

Expand Down Expand Up @@ -1746,7 +1755,7 @@ func (s *snapsSuite) TestRefreshIgnoreRunning(c *check.C) {
t := s.NewTask("fake-refresh-snap", "Doing a fake install")
return state.NewTaskSet(t), nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
return nil
})()

Expand Down Expand Up @@ -1781,7 +1790,7 @@ func (s *snapsSuite) TestRefreshCohort(c *check.C) {
t := s.NewTask("fake-refresh-snap", "Doing a fake install")
return state.NewTaskSet(t), nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
return nil
})()

Expand Down Expand Up @@ -1811,7 +1820,7 @@ func (s *snapsSuite) TestRefreshLeaveCohort(c *check.C) {
t := s.NewTask("fake-refresh-snap", "Doing a fake install")
return state.NewTaskSet(t), nil
})()
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int) error {
defer daemon.MockAssertstateRefreshSnapAssertions(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error {
return nil
})()

Expand Down
3 changes: 2 additions & 1 deletion daemon/export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/gorilla/mux"

"github.com/snapcore/snapd/overlord"
"github.com/snapcore/snapd/overlord/assertstate"
"github.com/snapcore/snapd/overlord/snapstate"
"github.com/snapcore/snapd/overlord/state"
"github.com/snapcore/snapd/snap"
Expand Down Expand Up @@ -105,7 +106,7 @@ func MockUnsafeReadSnapInfo(mock func(string) (*snap.Info, error)) (restore func
}
}

func MockAssertstateRefreshSnapAssertions(mock func(*state.State, int) error) (restore func()) {
func MockAssertstateRefreshSnapAssertions(mock func(*state.State, int, *assertstate.RefreshAssertionsOptions) error) (restore func()) {
oldAssertstateRefreshSnapAssertions := assertstateRefreshSnapAssertions
assertstateRefreshSnapAssertions = mock
return func() {
Expand Down
14 changes: 12 additions & 2 deletions overlord/assertstate/assertstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ func findError(format string, ref *asserts.Ref, err error) error {

type RefreshAssertionsOptions struct {
IsAutoRefresh bool
// IsRefreshOfAllSnaps indicates if assertions are refreshed together with
// all installed snaps, which means validation set assertions can be refreshed
// as well. It is implied if IsAutoRefresh is true.
IsRefreshOfAllSnaps bool
}

// RefreshSnapDeclarations refetches all the current snap declarations and their prerequisites.
Expand Down Expand Up @@ -352,11 +356,17 @@ func AutoRefreshAssertions(s *state.State, userID int) error {
}

// RefreshSnapAssertions tries to refresh all snap-centered assertions
func RefreshSnapAssertions(s *state.State, userID int) error {
opts := &RefreshAssertionsOptions{IsAutoRefresh: false}
func RefreshSnapAssertions(s *state.State, userID int, opts *RefreshAssertionsOptions) error {
if opts == nil {
opts = &RefreshAssertionsOptions{}
}
opts.IsAutoRefresh = false
if err := RefreshSnapDeclarations(s, userID, opts); err != nil {
return err
}
if !opts.IsRefreshOfAllSnaps {
return nil
}
return RefreshValidationSetAssertions(s, userID, opts)
}

Expand Down
19 changes: 18 additions & 1 deletion overlord/assertstate/assertstate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ func (s *assertMgrSuite) TestRefreshAssertionsRefreshSnapDeclarationsAndValidati
vsetAs2 := s.validationSetAssert(c, "bar", "2", "3", "required", "1")
c.Assert(s.storeSigning.Add(vsetAs2), IsNil)

err = assertstate.RefreshSnapAssertions(s.state, 0)
err = assertstate.RefreshSnapAssertions(s.state, 0, &assertstate.RefreshAssertionsOptions{IsRefreshOfAllSnaps: true})
c.Assert(err, IsNil)

a, err := assertstate.DB(s.state).Find(asserts.SnapDeclarationType, map[string]string{
Expand All @@ -1012,6 +1012,23 @@ func (s *assertMgrSuite) TestRefreshAssertionsRefreshSnapDeclarationsAndValidati

c.Assert(err, IsNil)
c.Check(s.fakeStore.(*fakeStore).opts.IsAutoRefresh, Equals, false)

// changed validation set assertion again
vsetAs3 := s.validationSetAssert(c, "bar", "4", "5", "required")
c.Assert(s.storeSigning.Add(vsetAs3), IsNil)

// but pretend it's not a refresh of all snaps
err = assertstate.RefreshSnapAssertions(s.state, 0, &assertstate.RefreshAssertionsOptions{IsRefreshOfAllSnaps: false})
c.Assert(err, IsNil)

// so the assertion is not updated
_, err = assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{
"series": "16",
"account-id": s.dev1Acct.AccountID(),
"name": "bar",
"sequence": "4",
})
c.Check(asserts.IsNotFound(err), Equals, true)
}

func (s *assertMgrSuite) TestRefreshSnapDeclarationsTooEarly(c *C) {
Expand Down

0 comments on commit 354a19d

Please sign in to comment.