diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 2300002c857..440f84a3501 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -10,7 +10,9 @@ on: jobs: snap-builds: runs-on: ubuntu-20.04 - if: ${{ github.event_name != 'push' && github.ref != 'refs/heads/master' }} + # only build the snap for pull requests, it's not needed on release branches + # or on master since we have launchpad build recipes which do this already + if: ${{ github.event_name == 'pull_request' }} steps: - name: Checkout code uses: actions/checkout@v2 @@ -208,7 +210,6 @@ jobs: uses: codecov/codecov-action@v2 if: steps.cached-results.outputs.already-ran != 'true' with: - # token: ${{ secrets.CODECOV_TOKEN }} # needed ? fail_ci_if_error: true flags: unittests name: codecov-umbrella @@ -221,7 +222,9 @@ jobs: spread: needs: [unit-tests] - if: ${{ github.event_name != 'push' && github.ref != 'refs/heads/master' }} + # have spread jobs run on master on PRs only, but on both PRs and pushes to + # release branches + if: ${{ github.event_name != 'push' || github.ref != 'refs/heads/master' }} runs-on: self-hosted strategy: # FIXME: enable fail-fast mode once spread can cancel an executing job. @@ -296,7 +299,9 @@ jobs: spread-nested: needs: [unit-tests] - if: ${{ github.event_name != 'push' && github.ref != 'refs/heads/master' }} + # have spread jobs run on master on PRs only, but on both PRs and pushes to + # release branches + if: ${{ github.event_name != 'push' || github.ref != 'refs/heads/master' }} runs-on: self-hosted strategy: # FIXME: enable fail-fast mode once spread can cancel an executing job. diff --git a/cmd/snap/cmd_snap_op.go b/cmd/snap/cmd_snap_op.go index 0d3085f862a..4f68096db74 100644 --- a/cmd/snap/cmd_snap_op.go +++ b/cmd/snap/cmd_snap_op.go @@ -763,9 +763,9 @@ func (x *cmdRefresh) listRefresh() error { defer w.Flush() // TRANSLATORS: the %s is to insert a filler escape sequence (please keep it flush to the column header, with no extra spaces) - fmt.Fprintf(w, i18n.G("Name\tVersion\tRev\tPublisher%s\tNotes\n"), fillerPublisher(esc)) + fmt.Fprintf(w, i18n.G("Name\tVersion\tRev\tSize\tPublisher%s\tNotes\n"), fillerPublisher(esc)) for _, snap := range snaps { - fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", snap.Name, snap.Version, snap.Revision, shortPublisher(esc, snap.Publisher), NotesFromRemote(snap, nil)) + fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%s\n", snap.Name, snap.Version, snap.Revision, strutil.SizeToStr(snap.DownloadSize), shortPublisher(esc, snap.Publisher), NotesFromRemote(snap, nil)) } return nil diff --git a/cmd/snap/cmd_snap_op_test.go b/cmd/snap/cmd_snap_op_test.go index a5b75c323f2..4aa831664db 100644 --- a/cmd/snap/cmd_snap_op_test.go +++ b/cmd/snap/cmd_snap_op_test.go @@ -1078,7 +1078,7 @@ func (s *SnapSuite) TestRefreshList(c *check.C) { c.Check(r.Method, check.Equals, "GET") c.Check(r.URL.Path, check.Equals, "/v2/find") c.Check(r.URL.Query().Get("select"), check.Equals, "refresh") - fmt.Fprintln(w, `{"type": "sync", "result": [{"name": "foo", "status": "active", "version": "4.2update1", "developer": "bar", "publisher": {"id": "bar-id", "username": "bar", "display-name": "Bar", "validation": "unproven"}, "revision":17,"summary":"some summary"}]}`) + fmt.Fprintln(w, `{"type": "sync", "result": [{"name": "foo", "status": "active", "version": "4.2update1", "developer": "bar", "download-size": 436375552, "publisher": {"id": "bar-id", "username": "bar", "display-name": "Bar", "validation": "unproven"}, "revision":17,"summary":"some summary"}]}`) default: c.Fatalf("expected to get 1 requests, now on %d", n+1) } @@ -1088,8 +1088,8 @@ func (s *SnapSuite) TestRefreshList(c *check.C) { rest, err := snap.Parser(snap.Client()).ParseArgs([]string{"refresh", "--list"}) c.Assert(err, check.IsNil) c.Assert(rest, check.DeepEquals, []string{}) - c.Check(s.Stdout(), check.Matches, `Name +Version +Rev +Publisher +Notes -foo +4.2update1 +17 +bar +-.* + c.Check(s.Stdout(), check.Matches, `Name +Version +Rev +Size +Publisher +Notes +foo +4.2update1 +17 +436MB +bar +-.* `) c.Check(s.Stderr(), check.Equals, "") // ensure that the fake server api was actually hit diff --git a/daemon/api_snaps.go b/daemon/api_snaps.go index 07ea6d57196..c8282c331c3 100644 --- a/daemon/api_snaps.go +++ b/daemon/api_snaps.go @@ -336,7 +336,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 = assertstateRefreshSnapDeclarations(st, inst.userID); err != nil { + if err = assertstateRefreshSnapDeclarations(st, inst.userID, nil); err != nil { return "", nil, err } @@ -585,7 +585,7 @@ 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 - if err := assertstateRefreshSnapDeclarations(st, inst.userID); err != nil { + if err := assertstateRefreshSnapDeclarations(st, inst.userID, nil); err != nil { return nil, err } diff --git a/daemon/api_snaps_test.go b/daemon/api_snaps_test.go index 067a52426d8..84229ff00cb 100644 --- a/daemon/api_snaps_test.go +++ b/daemon/api_snaps_test.go @@ -478,7 +478,7 @@ func (s *snapsSuite) TestPostSnapsOpMoreComplexContentType(c *check.C) { } func (s *snapsSuite) testPostSnapsOp(c *check.C, contentType string) { - defer daemon.MockAssertstateRefreshSnapDeclarations(func(*state.State, int) error { return nil })() + defer daemon.MockAssertstateRefreshSnapDeclarations(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") @@ -519,9 +519,9 @@ func (s *snapsSuite) TestPostSnapsOpInvalidCharset(c *check.C) { func (s *snapsSuite) TestRefreshAll(c *check.C) { refreshSnapDecls := false - defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error { refreshSnapDecls = true - return assertstate.RefreshSnapDeclarations(s, userID) + return assertstate.RefreshSnapDeclarations(s, userID, opts) })() d := s.daemon(c) @@ -555,9 +555,9 @@ func (s *snapsSuite) TestRefreshAll(c *check.C) { func (s *snapsSuite) TestRefreshAllNoChanges(c *check.C) { refreshSnapDecls := false - defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error { refreshSnapDecls = true - return assertstate.RefreshSnapDeclarations(s, userID) + return assertstate.RefreshSnapDeclarations(s, userID, opts) })() defer daemon.MockSnapstateUpdateMany(func(_ context.Context, s *state.State, names []string, userID int, flags *snapstate.Flags) ([]string, []*state.TaskSet, error) { @@ -578,7 +578,7 @@ func (s *snapsSuite) TestRefreshAllNoChanges(c *check.C) { func (s *snapsSuite) TestRefreshMany(c *check.C) { refreshSnapDecls := false - defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { refreshSnapDecls = true return nil })() @@ -603,7 +603,7 @@ func (s *snapsSuite) TestRefreshMany(c *check.C) { func (s *snapsSuite) TestRefreshMany1(c *check.C) { refreshSnapDecls := false - defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { refreshSnapDecls = true return nil })() @@ -1434,7 +1434,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.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error { return nil })() @@ -1600,7 +1600,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.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, opts *assertstate.RefreshAssertionsOptions) error { assertstateCalledUserID = userID return nil })() @@ -1639,7 +1639,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.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { return nil })() @@ -1673,7 +1673,7 @@ func (s *snapsSuite) TestRefreshClassic(c *check.C) { calledFlags = flags return nil, nil })() - defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { return nil })() @@ -1707,7 +1707,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.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { return nil })() @@ -1746,7 +1746,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.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { return nil })() @@ -1781,7 +1781,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.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { return nil })() @@ -1811,7 +1811,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.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int) error { + defer daemon.MockAssertstateRefreshSnapDeclarations(func(s *state.State, userID int, _ *assertstate.RefreshAssertionsOptions) error { return nil })() diff --git a/daemon/export_test.go b/daemon/export_test.go index 72e44dbffdf..62b66fea7fa 100644 --- a/daemon/export_test.go +++ b/daemon/export_test.go @@ -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" @@ -105,7 +106,7 @@ func MockUnsafeReadSnapInfo(mock func(string) (*snap.Info, error)) (restore func } } -func MockAssertstateRefreshSnapDeclarations(mock func(*state.State, int) error) (restore func()) { +func MockAssertstateRefreshSnapDeclarations(mock func(*state.State, int, *assertstate.RefreshAssertionsOptions) error) (restore func()) { oldAssertstateRefreshSnapDeclarations := assertstateRefreshSnapDeclarations assertstateRefreshSnapDeclarations = mock return func() { diff --git a/get-deps.sh b/get-deps.sh index 65f593691e1..4c1729c5f05 100755 --- a/get-deps.sh +++ b/get-deps.sh @@ -2,7 +2,6 @@ set -e - echo Obtaining dependencies go mod vendor diff --git a/overlord/assertstate/assertstate.go b/overlord/assertstate/assertstate.go index 15e12e7304e..440d66b69ab 100644 --- a/overlord/assertstate/assertstate.go +++ b/overlord/assertstate/assertstate.go @@ -55,8 +55,16 @@ func findError(format string, ref *asserts.Ref, err error) error { } } +type RefreshAssertionsOptions struct { + IsAutoRefresh bool +} + // RefreshSnapDeclarations refetches all the current snap declarations and their prerequisites. -func RefreshSnapDeclarations(s *state.State, userID int) error { +func RefreshSnapDeclarations(s *state.State, userID int, opts *RefreshAssertionsOptions) error { + if opts == nil { + opts = &RefreshAssertionsOptions{} + } + deviceCtx, err := snapstate.DevicePastSeeding(s, nil) if err != nil { return err @@ -67,7 +75,7 @@ func RefreshSnapDeclarations(s *state.State, userID int) error { return nil } - err = bulkRefreshSnapDeclarations(s, snapStates, userID, deviceCtx) + err = bulkRefreshSnapDeclarations(s, snapStates, userID, deviceCtx, opts) if err == nil { // done return nil @@ -336,15 +344,20 @@ func delayedCrossMgrInit() { // AutoRefreshAssertions tries to refresh all assertions func AutoRefreshAssertions(s *state.State, userID int) error { - if err := RefreshSnapDeclarations(s, userID); err != nil { + opts := &RefreshAssertionsOptions{IsAutoRefresh: true} + if err := RefreshSnapDeclarations(s, userID, opts); err != nil { return err } - return RefreshValidationSetAssertions(s, userID) + return RefreshValidationSetAssertions(s, userID, opts) } // RefreshValidationSetAssertions tries to refresh all validation set // assertions. -func RefreshValidationSetAssertions(s *state.State, userID int) error { +func RefreshValidationSetAssertions(s *state.State, userID int, opts *RefreshAssertionsOptions) error { + if opts == nil { + opts = &RefreshAssertionsOptions{} + } + deviceCtx, err := snapstate.DevicePastSeeding(s, nil) if err != nil { return err @@ -391,7 +404,7 @@ func RefreshValidationSetAssertions(s *state.State, userID int) error { return nil } - if err := bulkRefreshValidationSetAsserts(s, monitorModeSets, nil, userID, deviceCtx); err != nil { + if err := bulkRefreshValidationSetAsserts(s, monitorModeSets, nil, userID, deviceCtx, opts); err != nil { return err } if err := updateTracking(monitorModeSets); err != nil { @@ -430,7 +443,7 @@ func RefreshValidationSetAssertions(s *state.State, userID int) error { return vsets.Conflict() } - if err := bulkRefreshValidationSetAsserts(s, enforceModeSets, checkForConflicts, userID, deviceCtx); err != nil { + if err := bulkRefreshValidationSetAsserts(s, enforceModeSets, checkForConflicts, userID, deviceCtx, opts); err != nil { if _, ok := err.(*snapasserts.ValidationSetsConflictError); ok { logger.Noticef("cannot refresh to conflicting validation set assertions: %v", err) return nil @@ -514,7 +527,8 @@ func ValidationSetAssertionForMonitor(st *state.State, accountID, name string, s } } - if err := resolvePoolNoFallback(st, pool, nil, userID, deviceCtx); err != nil { + refreshOpts := &RefreshAssertionsOptions{IsAutoRefresh: false} + if err := resolvePoolNoFallback(st, pool, nil, userID, deviceCtx, refreshOpts); err != nil { rerr, ok := err.(*resolvePoolError) if ok && as != nil && opts.AllowLocalFallback { if e := rerr.errors[atSeq.Unique()]; asserts.IsNotFound(e) { @@ -549,9 +563,11 @@ func ValidationSetAssertionForEnforce(st *state.State, accountID, name string, s return nil, err } + opts := &RefreshAssertionsOptions{IsAutoRefresh: false} + // refresh all currently tracked validation set assertions (this may or may not // include the one requested by the caller). - if err = RefreshValidationSetAssertions(st, userID); err != nil { + if err = RefreshValidationSetAssertions(st, userID, opts); err != nil { return nil, err } @@ -664,7 +680,7 @@ func ValidationSetAssertionForEnforce(st *state.State, accountID, name string, s return nil } - if err := resolvePoolNoFallback(st, pool, checkBeforeCommit, userID, deviceCtx); err != nil { + if err := resolvePoolNoFallback(st, pool, checkBeforeCommit, userID, deviceCtx, opts); err != nil { return nil, err } diff --git a/overlord/assertstate/assertstate_test.go b/overlord/assertstate/assertstate_test.go index b6b52627d48..ffdce8b3c74 100644 --- a/overlord/assertstate/assertstate_test.go +++ b/overlord/assertstate/assertstate_test.go @@ -84,6 +84,7 @@ type fakeStore struct { maxValidationSetSupportedFormat int requestedTypes [][]string + opts *store.RefreshOptions snapActionErr error downloadAssertionsErr error @@ -122,6 +123,8 @@ func (sto *fakeStore) SnapAction(_ context.Context, currentSnaps []*store.Curren return nil, nil, sto.snapActionErr } + sto.opts = opts + restore := asserts.MockMaxSupportedFormat(asserts.SnapDeclarationType, sto.maxDeclSupportedFormat) defer restore() @@ -951,7 +954,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsTooEarly(c *C) { r := snapstatetest.MockDeviceModel(nil) defer r() - err := assertstate.RefreshSnapDeclarations(s.state, 0) + err := assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Check(err, FitsTypeOf, &snapstate.ChangeConflictError{}) } @@ -961,8 +964,9 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNop(c *C) { s.setModel(sysdb.GenericClassicModel()) - err := assertstate.RefreshSnapDeclarations(s.state, 0) + err := assertstate.RefreshSnapDeclarations(s.state, 0, &assertstate.RefreshAssertionsOptions{IsAutoRefresh: true}) c.Assert(err, IsNil) + c.Check(s.fakeStore.(*fakeStore).opts.IsAutoRefresh, Equals, true) } func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStore(c *C) { @@ -1008,7 +1012,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStore(c *C) { err = s.storeSigning.Add(snapDeclFoo1) c.Assert(err, IsNil) - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, IsNil) a, err := assertstate.DB(s.state).Find(asserts.SnapDeclarationType, map[string]string{ @@ -1028,7 +1032,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStore(c *C) { err = s.storeSigning.Add(dev1Acct1) c.Assert(err, IsNil) - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, IsNil) a, err = assertstate.DB(s.state).Find(asserts.AccountType, map[string]string{ @@ -1060,7 +1064,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsNoStore(c *C) { })() // no error, kept the old one - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, IsNil) a, err = assertstate.DB(s.state).Find(asserts.SnapDeclarationType, map[string]string{ @@ -1118,7 +1122,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsChangingKey(c *C) { _, err = storeKey2.Ref().Resolve(assertstate.DB(s.state).Find) c.Check(asserts.IsNotFound(err), Equals, true) - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, IsNil) a, err := assertstate.DB(s.state).Find(asserts.SnapDeclarationType, map[string]string{ @@ -1167,7 +1171,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsWithStore(c *C) { c.Assert(err, IsNil) // store assertion is missing - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, IsNil) a, err := assertstate.DB(s.state).Find(asserts.SnapDeclarationType, map[string]string{ @@ -1195,7 +1199,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsWithStore(c *C) { err = s.storeSigning.Add(storeAs) c.Assert(err, IsNil) - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, IsNil) a, err = assertstate.DB(s.state).Find(asserts.SnapDeclarationType, map[string]string{ @@ -1224,7 +1228,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsWithStore(c *C) { err = s.storeSigning.Add(storeAs) c.Assert(err, IsNil) - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, IsNil) a, err = assertstate.DB(s.state).Find(asserts.StoreType, map[string]string{ "store": "my-brand-store", @@ -1267,7 +1271,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsDownloadError(c *C) { s.fakeStore.(*fakeStore).downloadAssertionsErr = errors.New("download error") - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, ErrorMatches, `cannot refresh snap-declarations for snaps: - foo: download error`) } @@ -1307,7 +1311,7 @@ func (s *assertMgrSuite) TestRefreshSnapDeclarationsPersistentNetworkError(c *C) pne := new(httputil.PersistentNetworkError) s.fakeStore.(*fakeStore).snapActionErr = pne - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) c.Assert(err, Equals, pne) } @@ -1393,7 +1397,7 @@ func (s *assertMgrSuite) testRefreshSnapDeclarationsMany(c *C, n int) error { c.Assert(err, IsNil) } - err = assertstate.RefreshSnapDeclarations(s.state, 0) + err = assertstate.RefreshSnapDeclarations(s.state, 0, nil) if err != nil { // fot the caller to check return err @@ -2150,7 +2154,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsNop(c *C) { s.setModel(sysdb.GenericClassicModel()) - err := assertstate.RefreshValidationSetAssertions(s.state, 0) + err := assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) } @@ -2182,6 +2186,7 @@ func (s *assertMgrSuite) TestValidationSetAssertionsAutoRefresh(c *C) { assertstate.UpdateValidationSet(s.state, &tr) c.Assert(assertstate.AutoRefreshAssertions(s.state, 0), IsNil) + c.Check(s.fakeStore.(*fakeStore).opts.IsAutoRefresh, Equals, true) a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{ "series": "16", @@ -2235,7 +2240,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsStoreError(c *C) { } assertstate.UpdateValidationSet(s.state, &tr) - err := assertstate.RefreshValidationSetAssertions(s.state, 0) + err := assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, ErrorMatches, `cannot refresh validation set assertions: cannot : got unexpected HTTP status code 400.*`) } @@ -2268,7 +2273,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertions(c *C) { } assertstate.UpdateValidationSet(s.state, &tr) - err = assertstate.RefreshValidationSetAssertions(s.state, 0) + err = assertstate.RefreshValidationSetAssertions(s.state, 0, &assertstate.RefreshAssertionsOptions{IsAutoRefresh: true}) c.Assert(err, IsNil) a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{ @@ -2284,6 +2289,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertions(c *C) { c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ {"account", "account-key", "validation-set"}, }) + c.Check(s.fakeStore.(*fakeStore).opts.IsAutoRefresh, Equals, true) // sequence changed in the store to 4 vsetAs3 := s.validationSetAssert(c, "bar", "4", "3", "required") @@ -2300,7 +2306,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertions(c *C) { c.Assert(asserts.IsNotFound(err), Equals, true) s.fakeStore.(*fakeStore).requestedTypes = nil - err = assertstate.RefreshValidationSetAssertions(s.state, 0) + err = assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ @@ -2354,7 +2360,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsPinned(c *C) { } assertstate.UpdateValidationSet(s.state, &tr) - err = assertstate.RefreshValidationSetAssertions(s.state, 0) + err = assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{ @@ -2378,7 +2384,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsPinned(c *C) { c.Assert(err, IsNil) s.fakeStore.(*fakeStore).requestedTypes = nil - err = assertstate.RefreshValidationSetAssertions(s.state, 0) + err = assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) c.Check(s.fakeStore.(*fakeStore).requestedTypes, DeepEquals, [][]string{ @@ -2443,7 +2449,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsLocalOnlyFailed(c *C) assertstate.UpdateValidationSet(s.state, &tr1) assertstate.UpdateValidationSet(s.state, &tr2) - err = assertstate.RefreshValidationSetAssertions(s.state, 0) + err = assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) // sanity - local assertion vsetAs1 is the latest @@ -2515,7 +2521,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeHappyNot } assertstate.UpdateValidationSet(s.state, &tr) - err = assertstate.RefreshValidationSetAssertions(s.state, 0) + err = assertstate.RefreshValidationSetAssertions(s.state, 0, nil) c.Assert(err, IsNil) a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{ @@ -2580,7 +2586,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeHappyPin } assertstate.UpdateValidationSet(s.state, &tr) - c.Assert(assertstate.RefreshValidationSetAssertions(s.state, 0), IsNil) + c.Assert(assertstate.RefreshValidationSetAssertions(s.state, 0, nil), IsNil) a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{ "series": "16", @@ -2644,7 +2650,7 @@ func (s *assertMgrSuite) TestRefreshValidationSetAssertionsEnforcingModeConflict } assertstate.UpdateValidationSet(s.state, &tr) - c.Assert(assertstate.RefreshValidationSetAssertions(s.state, 0), IsNil) + c.Assert(assertstate.RefreshValidationSetAssertions(s.state, 0, nil), IsNil) c.Assert(logbuf.String(), Matches, `.*cannot refresh to conflicting validation set assertions: validation sets are in conflict:\n- cannot constrain snap "foo" as both invalid .* and required at revision 1.*\n`) a, err := assertstate.DB(s.state).Find(asserts.ValidationSetType, map[string]string{ @@ -2844,6 +2850,7 @@ func (s *assertMgrSuite) TestValidationSetAssertionForEnforcePinnedHappy(c *C) { "sequence": "2", }) c.Assert(err, IsNil) + c.Check(s.fakeStore.(*fakeStore).opts.IsAutoRefresh, Equals, false) } func (s *assertMgrSuite) TestValidationSetAssertionForEnforceNotPinnedUnhappyMissingSnap(c *C) { diff --git a/overlord/assertstate/bulk.go b/overlord/assertstate/bulk.go index d4620b0a03f..6be768c07eb 100644 --- a/overlord/assertstate/bulk.go +++ b/overlord/assertstate/bulk.go @@ -44,14 +44,14 @@ const storeGroup = "store assertion" // that. Most systems should be done in one request anyway. var maxGroups = 256 -func bulkRefreshSnapDeclarations(s *state.State, snapStates map[string]*snapstate.SnapState, userID int, deviceCtx snapstate.DeviceContext) error { +func bulkRefreshSnapDeclarations(s *state.State, snapStates map[string]*snapstate.SnapState, userID int, deviceCtx snapstate.DeviceContext, opts *RefreshAssertionsOptions) error { db := cachedDB(s) pool := asserts.NewPool(db, maxGroups) var mergedRPErr *resolvePoolError tryResolvePool := func() error { - err := resolvePool(s, pool, nil, userID, deviceCtx) + err := resolvePool(s, pool, nil, userID, deviceCtx, opts) if rpe, ok := err.(*resolvePoolError); ok { if mergedRPErr == nil { mergedRPErr = rpe @@ -140,7 +140,7 @@ func bulkRefreshSnapDeclarations(s *state.State, snapStates map[string]*snapstat return nil } -func bulkRefreshValidationSetAsserts(s *state.State, vsets map[string]*ValidationSetTracking, beforeCommitChecker func(*asserts.Database, asserts.Backstore) error, userID int, deviceCtx snapstate.DeviceContext) error { +func bulkRefreshValidationSetAsserts(s *state.State, vsets map[string]*ValidationSetTracking, beforeCommitChecker func(*asserts.Database, asserts.Backstore) error, userID int, deviceCtx snapstate.DeviceContext, opts *RefreshAssertionsOptions) error { db := cachedDB(s) pool := asserts.NewPool(db, maxGroups) @@ -175,7 +175,7 @@ func bulkRefreshValidationSetAsserts(s *state.State, vsets map[string]*Validatio } } - err := resolvePoolNoFallback(s, pool, beforeCommitChecker, userID, deviceCtx) + err := resolvePoolNoFallback(s, pool, beforeCommitChecker, userID, deviceCtx, opts) if err == nil { return nil } @@ -242,7 +242,7 @@ func (rpe *resolvePoolError) Error() string { return strings.Join(s, "\n") } -func resolvePool(s *state.State, pool *asserts.Pool, checkBeforeCommit func(*asserts.Database, asserts.Backstore) error, userID int, deviceCtx snapstate.DeviceContext) error { +func resolvePool(s *state.State, pool *asserts.Pool, checkBeforeCommit func(*asserts.Database, asserts.Backstore) error, userID int, deviceCtx snapstate.DeviceContext, opts *RefreshAssertionsOptions) error { user, err := userFromUserID(s, userID) if err != nil { return err @@ -252,9 +252,9 @@ func resolvePool(s *state.State, pool *asserts.Pool, checkBeforeCommit func(*ass unsupported := handleUnsupported(db) for { - // TODO: pass refresh options? + storeOpts := &store.RefreshOptions{IsAutoRefresh: opts.IsAutoRefresh} s.Unlock() - _, aresults, err := sto.SnapAction(context.TODO(), nil, nil, pool, user, nil) + _, aresults, err := sto.SnapAction(context.TODO(), nil, nil, pool, user, storeOpts) s.Lock() if err != nil { // request fallback on @@ -313,8 +313,8 @@ func resolvePool(s *state.State, pool *asserts.Pool, checkBeforeCommit func(*ass return nil } -func resolvePoolNoFallback(s *state.State, pool *asserts.Pool, checkBeforeCommit func(*asserts.Database, asserts.Backstore) error, userID int, deviceCtx snapstate.DeviceContext) error { - err := resolvePool(s, pool, checkBeforeCommit, userID, deviceCtx) +func resolvePoolNoFallback(s *state.State, pool *asserts.Pool, checkBeforeCommit func(*asserts.Database, asserts.Backstore) error, userID int, deviceCtx snapstate.DeviceContext, opts *RefreshAssertionsOptions) error { + err := resolvePool(s, pool, checkBeforeCommit, userID, deviceCtx, opts) if err != nil { // no fallback, report inner error. if ferr, ok := err.(*bulkAssertionFallbackError); ok { diff --git a/overlord/hookstate/ctlcmd/ctlcmd.go b/overlord/hookstate/ctlcmd/ctlcmd.go index 21cb6c37996..a24d72d1371 100644 --- a/overlord/hookstate/ctlcmd/ctlcmd.go +++ b/overlord/hookstate/ctlcmd/ctlcmd.go @@ -113,41 +113,32 @@ func (f ForbiddenCommandError) Error() string { return f.Message } -// ForbiddenCommand contains information about an attempt to use a command in a context where it is not allowed. -type ForbiddenCommand struct { - Uid uint32 - Name string -} - -func (f *ForbiddenCommand) Execute(args []string) error { - return &ForbiddenCommandError{Message: fmt.Sprintf("cannot use %q with uid %d, try with sudo", f.Name, f.Uid)} -} - // nonRootAllowed lists the commands that can be performed even when snapctl // is invoked not by root. var nonRootAllowed = []string{"get", "services", "set-health", "is-connected", "system-mode"} // Run runs the requested command. func Run(context *hookstate.Context, args []string, uid uint32) (stdout, stderr []byte, err error) { + if len(args) == 0 { + return nil, nil, fmt.Errorf("internal error: snapctl cannot run without args") + } + + if !isAllowedToRun(uid, args) { + return nil, nil, &ForbiddenCommandError{Message: fmt.Sprintf("cannot use %q with uid %d, try with sudo", args[0], uid)} + } + parser := flags.NewNamedParser("snapctl", flags.PassDoubleDash|flags.HelpFlag) // Create stdout/stderr buffers, and make sure commands use them. var stdoutBuffer bytes.Buffer var stderrBuffer bytes.Buffer for name, cmdInfo := range commands { - var data interface{} - // commands listed here will be allowed for regular users - // note: commands still need valid context and snaps can only access own config. - if uid == 0 || strutil.ListContains(nonRootAllowed, name) { - cmd := cmdInfo.generator() - cmd.setStdout(&stdoutBuffer) - cmd.setStderr(&stderrBuffer) - cmd.setContext(context) - data = cmd - } else { - data = &ForbiddenCommand{Uid: uid, Name: name} - } - theCmd, err := parser.AddCommand(name, cmdInfo.shortHelp, cmdInfo.longHelp, data) + cmd := cmdInfo.generator() + cmd.setStdout(&stdoutBuffer) + cmd.setStderr(&stderrBuffer) + cmd.setContext(context) + + theCmd, err := parser.AddCommand(name, cmdInfo.shortHelp, cmdInfo.longHelp, cmd) theCmd.Hidden = cmdInfo.hidden if err != nil { logger.Panicf("cannot add command %q: %s", name, err) @@ -157,3 +148,15 @@ func Run(context *hookstate.Context, args []string, uid uint32) (stdout, stderr _, err = parser.ParseArgs(args) return stdoutBuffer.Bytes(), stderrBuffer.Bytes(), err } + +func isAllowedToRun(uid uint32, args []string) bool { + // A command can run if any of the following are true: + // * It runs as root + // * It's contained in nonRootAllowed + // * It's used with the -h or --help flags + // note: commands still need valid context and snaps can only access own config. + return uid == 0 || + strutil.ListContains(nonRootAllowed, args[0]) || + strutil.ListContains(args, "-h") || + strutil.ListContains(args, "--help") +} diff --git a/overlord/hookstate/ctlcmd/ctlcmd_test.go b/overlord/hookstate/ctlcmd/ctlcmd_test.go index a1c8f37cb40..28cc6f33710 100644 --- a/overlord/hookstate/ctlcmd/ctlcmd_test.go +++ b/overlord/hookstate/ctlcmd/ctlcmd_test.go @@ -21,6 +21,7 @@ package ctlcmd_test import ( "fmt" + "strings" "testing" "github.com/jessevdk/go-flags" @@ -112,3 +113,43 @@ func (s *ctlcmdSuite) TestHiddenCommand(c *C) { // mock-hidden is not in the help message c.Check(err.Error(), Not(testutil.Contains), " mock-hidden\n") } + +func (s *ctlcmdSuite) TestRootRequiredCommandFailure(c *C) { + _, _, err := ctlcmd.Run(s.mockContext, []string{"start"}, 1000) + + c.Check(err, FitsTypeOf, &ctlcmd.ForbiddenCommandError{}) + c.Check(err.Error(), Equals, `cannot use "start" with uid 1000, try with sudo`) +} + +func (s *ctlcmdSuite) TestRunNoArgsFailure(c *C) { + _, _, err := ctlcmd.Run(s.mockContext, []string{}, 0) + c.Check(err, NotNil) +} + +func (s *ctlcmdSuite) TestRunOnlyHelp(c *C) { + _, _, err := ctlcmd.Run(s.mockContext, []string{"-h"}, 1000) + c.Check(err, NotNil) + c.Assert(strings.HasPrefix(err.Error(), "Usage:"), Equals, true) + + _, _, err = ctlcmd.Run(s.mockContext, []string{"--help"}, 1000) + c.Check(err, NotNil) + c.Assert(strings.HasPrefix(err.Error(), "Usage:"), Equals, true) +} + +func (s *ctlcmdSuite) TestRunHelpAtAnyPosition(c *C) { + _, _, err := ctlcmd.Run(s.mockContext, []string{"start", "a", "-h"}, 1000) + c.Check(err, NotNil) + c.Assert(strings.HasPrefix(err.Error(), "Usage:"), Equals, true) + + _, _, err = ctlcmd.Run(s.mockContext, []string{"start", "a", "b", "--help"}, 1000) + c.Check(err, NotNil) + c.Assert(strings.HasPrefix(err.Error(), "Usage:"), Equals, true) +} + +func (s *ctlcmdSuite) TestRunNonRootAllowedCommandWithAllowedCmdAsArg(c *C) { + // this test protects us against a future refactor introducing a bug that allows + // a root-only command to run without root if an arg is in the nonRootAllowed list + _, _, err := ctlcmd.Run(s.mockContext, []string{"set", "get", "a"}, 1000) + c.Check(err, FitsTypeOf, &ctlcmd.ForbiddenCommandError{}) + c.Check(err.Error(), Equals, `cannot use "set" with uid 1000, try with sudo`) +} diff --git a/overlord/hookstate/hooks_test.go b/overlord/hookstate/hooks_test.go index 2c624434e6d..b9e02f74947 100644 --- a/overlord/hookstate/hooks_test.go +++ b/overlord/hookstate/hooks_test.go @@ -459,7 +459,8 @@ func (s *gateAutoRefreshHookSuite) TestGateAutorefreshHookErrorHoldErrorLogged(c // and snap-b is not held (due to hold error). var held map[string]map[string]interface{} - c.Assert(st.Get("snaps-hold", &held), Equals, state.ErrNoState) + c.Assert(st.Get("snaps-hold", &held), IsNil) + c.Check(held, HasLen, 0) // no runinhibit because the refresh-app-awareness feature is disabled. hint, err := runinhibit.IsLocked("snap-a") diff --git a/overlord/managers_test.go b/overlord/managers_test.go index 3cb407158e1..a8d5fc1c897 100644 --- a/overlord/managers_test.go +++ b/overlord/managers_test.go @@ -3149,7 +3149,7 @@ apps: s.serveSnap(fooPath, "15") // refresh all - err = assertstate.RefreshSnapDeclarations(st, 0) + err = assertstate.RefreshSnapDeclarations(st, 0, nil) c.Assert(err, IsNil) updated, tss, err := snapstate.UpdateMany(context.TODO(), st, nil, 0, nil) @@ -3302,7 +3302,7 @@ assumes: [something-that-is-not-provided] defer st.Unlock() _, err := snapstate.Install(context.TODO(), st, "some-snap", nil, 0, snapstate.Flags{}) - c.Assert(err, ErrorMatches, `snap "some-snap" assumes unsupported features: something-that-is-not-provided \(try to update snapd and refresh the core snap\)`) + c.Assert(err, ErrorMatches, `snap "some-snap" assumes unsupported features: something-that-is-not-provided \(try to refresh snapd\)`) } func (s *mgrsSuite) TestUpdateWithAssumesIsRefusedEarly(c *C) { @@ -3332,7 +3332,7 @@ assumes: [something-that-is-not-provided] }) _, err := snapstate.Update(st, "some-snap", nil, 0, snapstate.Flags{}) - c.Assert(err, ErrorMatches, `snap "some-snap" assumes unsupported features: something-that-is-not-provided \(try to update snapd and refresh the core snap\)`) + c.Assert(err, ErrorMatches, `snap "some-snap" assumes unsupported features: something-that-is-not-provided \(try to refresh snapd\)`) } func (s *mgrsSuite) TestUpdateManyWithAssumesIsRefusedEarly(c *C) { @@ -3754,7 +3754,7 @@ version: @VERSION@` c.Assert(repo.AddSnap(coreInfo), IsNil) // refresh all - err := assertstate.RefreshSnapDeclarations(st, 0) + err := assertstate.RefreshSnapDeclarations(st, 0, nil) c.Assert(err, IsNil) updates, tts, err := snapstate.UpdateMany(context.TODO(), st, []string{"core", "some-snap", "other-snap"}, 0, nil) @@ -3856,7 +3856,7 @@ version: 1` c.Assert(repo.AddSnap(coreInfo), IsNil) // refresh all - err := assertstate.RefreshSnapDeclarations(st, 0) + err := assertstate.RefreshSnapDeclarations(st, 0, nil) c.Assert(err, IsNil) updates, tts, err := snapstate.UpdateMany(context.TODO(), st, []string{"some-snap"}, 0, nil) @@ -3936,7 +3936,7 @@ func (s *mgrsSuite) testUpdateWithAutoconnectRetry(c *C, updateSnapName, removeS c.Assert(repo.AddSnap(otherInfo), IsNil) // refresh all - err := assertstate.RefreshSnapDeclarations(st, 0) + err := assertstate.RefreshSnapDeclarations(st, 0, nil) c.Assert(err, IsNil) ts, err := snapstate.Update(st, updateSnapName, nil, 0, snapstate.Flags{}) @@ -6704,7 +6704,7 @@ func (s *mgrsSuite) TestCheckRefreshFailureWithConcurrentRemoveOfConnectedSnap(c c.Assert(err, IsNil) // refresh all - c.Assert(assertstate.RefreshSnapDeclarations(st, 0), IsNil) + c.Assert(assertstate.RefreshSnapDeclarations(st, 0, nil), IsNil) ts, err := snapstate.Update(st, "some-snap", nil, 0, snapstate.Flags{}) c.Assert(err, IsNil) @@ -8795,3 +8795,54 @@ volumes: // gadget content is not updated because there is no edition update c.Check(filepath.Join(dirs.GlobalRootDir, "/run/mnt/", structureName, "start.elf"), testutil.FileContains, "start.elf rev1") } + +// deal with the missing "update-gadget-assets" tasks, see LP:#1940553 +func (ms *gadgetUpdatesSuite) TestGadgetKernelRefreshFromOldBrokenSnap(c *C) { + st := ms.o.State() + st.Lock() + defer st.Unlock() + + // we have an install kernel/gadget + gadgetSnapYaml := "name: pi\nversion: 1.0\ntype: gadget" + ms.mockInstalledSnapWithFiles(c, gadgetSnapYaml, [][]string{ + {"meta/gadget.yaml", "volumes:\n volume-id:\n bootloader: grub"}, + }) + kernelSnapYaml := "name: pi-kernel\nversion: 1.0\ntype: kernel" + ms.mockInstalledSnapWithFiles(c, kernelSnapYaml, nil) + + // add new kernel/gadget snap + newKernelSnapYaml := kernelSnapYaml + ms.mockSnapUpgradeWithFiles(c, newKernelSnapYaml, nil) + ms.mockSnapUpgradeWithFiles(c, gadgetSnapYaml, [][]string{ + {"meta/gadget.yaml", "volumes:\n volume-id:\n bootloader: grub"}, + }) + + // now a refresh is simulated that does *not* contain an + // "update-gadget-assets" task, see LP:#1940553 + snapstate.TestingLeaveOutKernelUpdateGadgetAssets = true + defer func() { snapstate.TestingLeaveOutKernelUpdateGadgetAssets = false }() + affected, tasksets, err := snapstate.UpdateMany(context.TODO(), st, nil, 0, &snapstate.Flags{}) + c.Assert(err, IsNil) + sort.Strings(affected) + c.Check(affected, DeepEquals, []string{"pi", "pi-kernel"}) + + // here we need to manipulate the change to simulate that there + // is no "update-gadget-assets" task for the kernel, unfortunately + // there is no "state.TaskSet.RemoveTask" nor a "state.Task.Unwait()" + chg := st.NewChange("upgrade-snaps", "...") + for _, ts := range tasksets { + // skip the taskset of UpdateMany that does the + // check-rerefresh, see tsWithoutReRefresh for details + if ts.Tasks()[0].Kind() == "check-rerefresh" { + continue + } + + chg.AddAll(ts) + } + + st.Unlock() + err = ms.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Check(chg.Err(), ErrorMatches, "cannot perform the following tasks:\n.*Mount snap \"pi-kernel\" \\(2\\) \\(cannot refresh kernel with change created by old snapd that is missing gadget update task\\)") +} diff --git a/overlord/snapstate/autorefresh_gating.go b/overlord/snapstate/autorefresh_gating.go index 35c17d9e0e6..d70a64b2bd8 100644 --- a/overlord/snapstate/autorefresh_gating.go +++ b/overlord/snapstate/autorefresh_gating.go @@ -216,9 +216,18 @@ func HoldRefresh(st *state.State, gatingSnap string, holdDuration time.Duration, gating[heldSnap][gatingSnap] = hold } - if len(herr.SnapsInError) != len(affectingSnaps) { - st.Set("snaps-hold", gating) + if len(herr.SnapsInError) > 0 { + // if some of the affecting snaps couldn't be held anymore then it + // doesn't make sense to hold other affecting snaps (because the gating + // snap is going to be disrupted anyway); go over all affectingSnaps + // again and remove gating info for them - this also deletes old holdings + // (if the hook run on previous refresh attempt) therefore we need to + // update snaps-hold state below. + for _, heldSnap := range affectingSnaps { + delete(gating[heldSnap], gatingSnap) + } } + st.Set("snaps-hold", gating) if len(herr.SnapsInError) > 0 { return herr } diff --git a/overlord/snapstate/autorefresh_gating_test.go b/overlord/snapstate/autorefresh_gating_test.go index c66c4295b7e..eb9be08d6c0 100644 --- a/overlord/snapstate/autorefresh_gating_test.go +++ b/overlord/snapstate/autorefresh_gating_test.go @@ -581,6 +581,53 @@ func (s *autorefreshGatingSuite) TestHoldAndProceedWithRefreshHelper(c *C) { c.Check(held, IsNil) } +// Test that if all snaps cannot be held anymore, we don't hold only some of them +// e.g. is a snap and its base snap have updates and the snap wants to hold (itself +// and the base) but the base cannot be held, it doesn't make sense to refresh the +// base but hold the affected snap. +func (s *autorefreshGatingSuite) TestDontHoldSomeSnapsIfSomeFail(c *C) { + st := s.state + st.Lock() + defer st.Unlock() + + // snap-b and snap-bb have base-snap-b base + mockInstalledSnap(c, st, snapByaml, useHook) + mockInstalledSnap(c, st, snapBByaml, useHook) + mockInstalledSnap(c, st, baseSnapByaml, noHook) + + mockInstalledSnap(c, st, snapCyaml, useHook) + mockInstalledSnap(c, st, snapDyaml, useHook) + + now := "2021-05-01T10:00:00Z" + restore := snapstate.MockTimeNow(func() time.Time { + t, err := time.Parse(time.RFC3339, now) + c.Assert(err, IsNil) + return t + }) + defer restore() + + // snap-b, base-snap-b get refreshed and affect snap-b (gating snap) + c.Assert(snapstate.HoldRefresh(st, "snap-b", 0, "snap-b", "base-snap-b"), IsNil) + // unrealted snap-d gets refreshed and holds itself + c.Assert(snapstate.HoldRefresh(st, "snap-d", 0, "snap-d"), IsNil) + + // advance time by 49h + now = "2021-05-03T11:00:00Z" + // snap-b, base-snap-b and snap-c get refreshed and snap-a (gating snap) wants to hold them + c.Assert(snapstate.HoldRefresh(st, "snap-b", 0, "snap-b", "base-snap-b", "snap-c"), ErrorMatches, `cannot hold some snaps:\n - snap "snap-b" cannot hold snap "base-snap-b" anymore, maximum refresh postponement exceeded`) + // snap-bb (gating snap) wants to hold base-snap-b as well and succeeds since it didn't exceed its holding time yet + c.Assert(snapstate.HoldRefresh(st, "snap-bb", 0, "base-snap-b"), IsNil) + + held, err := snapstate.HeldSnaps(st) + c.Assert(err, IsNil) + // note, snap-b couldn't hold base-snap-b anymore so we didn't hold snap-b + // and snap-c. base-snap-b was held by snap-bb. + c.Check(held, DeepEquals, map[string]bool{ + "snap-d": true, + "base-snap-b": true, + }) +} + func (s *autorefreshGatingSuite) TestPruneGatingHelper(c *C) { st := s.state st.Lock() diff --git a/overlord/snapstate/check_snap.go b/overlord/snapstate/check_snap.go index aacce3f11e1..3d5646320c8 100644 --- a/overlord/snapstate/check_snap.go +++ b/overlord/snapstate/check_snap.go @@ -63,11 +63,7 @@ func checkAssumes(si *snap.Info) error { } } if len(missing) > 0 { - hint := "try to refresh the core or snapd snaps" - if release.OnClassic { - hint = "try to update snapd and refresh the core snap" - } - return fmt.Errorf("snap %q assumes unsupported features: %s (%s)", si.InstanceName(), strings.Join(missing, ", "), hint) + return fmt.Errorf("snap %q assumes unsupported features: %s (try to refresh snapd)", si.InstanceName(), strings.Join(missing, ", ")) } return nil } diff --git a/overlord/snapstate/check_snap_test.go b/overlord/snapstate/check_snap_test.go index d7c12033067..f5026811b18 100644 --- a/overlord/snapstate/check_snap_test.go +++ b/overlord/snapstate/check_snap_test.go @@ -101,11 +101,11 @@ var assumesTests = []struct { assumes: "[common-data-dir]", }, { assumes: "[f1, f2]", - error: `snap "foo" assumes unsupported features: f1, f2 \(try to refresh the core or snapd snaps\)`, + error: `snap "foo" assumes unsupported features: f1, f2 \(try to refresh snapd\)`, }, { assumes: "[f1, f2]", classic: true, - error: `snap "foo" assumes unsupported features: f1, f2 \(try to update snapd and refresh the core snap\)`, + error: `snap "foo" assumes unsupported features: f1, f2 \(try to refresh snapd\)`, }, { assumes: "[snapd2.15]", version: "unknown", diff --git a/overlord/snapstate/handlers.go b/overlord/snapstate/handlers.go index 1942670dea9..12f9f2cec99 100644 --- a/overlord/snapstate/handlers.go +++ b/overlord/snapstate/handlers.go @@ -22,6 +22,7 @@ package snapstate import ( "context" "encoding/json" + "errors" "fmt" "os" "path/filepath" @@ -642,6 +643,28 @@ func hasOtherInstances(st *state.State, instanceName string) (bool, error) { return false, nil } +var ErrKernelGadgetUpdateTaskMissing = errors.New("cannot refresh kernel with change created by old snapd that is missing gadget update task") + +func checkKernelHasUpdateAssetsTask(t *state.Task) error { + for _, other := range t.Change().Tasks() { + snapsup, err := TaskSnapSetup(other) + if err == state.ErrNoState { + // XXX: hooks have no snapsup, is this detection okay? + continue + } + if err != nil { + return err + } + if snapsup.Type != "kernel" { + continue + } + if other.Kind() == "update-gadget-assets" { + return nil + } + } + return ErrKernelGadgetUpdateTaskMissing +} + func (m *SnapManager) doMountSnap(t *state.Task, _ *tomb.Tomb) error { st := t.State() st.Lock() @@ -666,6 +689,17 @@ func (m *SnapManager) doMountSnap(t *state.Task, _ *tomb.Tomb) error { return err } + // check that there is a "update-gadget-assets" task for kernels too, + // see https://bugs.launchpad.net/snapd/+bug/1940553 + if snapsup.Type == snap.TypeKernel { + st.Lock() + err = checkKernelHasUpdateAssetsTask(t) + st.Unlock() + if err != nil { + return err + } + } + timings.Run(perfTimings, "check-snap", fmt.Sprintf("check snap %q", snapsup.InstanceName()), func(timings.Measurer) { err = checkSnap(st, snapsup.SnapPath, snapsup.InstanceName(), snapsup.SideInfo, curInfo, snapsup.Flags, deviceCtx) }) diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index 24f466aa7be..70dc3c73f0c 100644 --- a/overlord/snapstate/snapstate.go +++ b/overlord/snapstate/snapstate.go @@ -74,6 +74,11 @@ var ErrNothingToDo = errors.New("nothing to do") var osutilCheckFreeSpace = osutil.CheckFreeSpace +// TestingLeaveOutKernelUpdateGadgetAssets can be used to simulate an upgrade +// from a broken snapd that does not generate a "update-gadget-assets" task. +// See LP:#1940553 +var TestingLeaveOutKernelUpdateGadgetAssets bool = false + type minimalInstallInfo interface { InstanceName() string Type() snap.Type @@ -360,7 +365,7 @@ func doInstall(st *state.State, snapst *SnapState, snapsup *SnapSetup, flags int prev = unlink } - if !release.OnClassic && (snapsup.Type == snap.TypeGadget || snapsup.Type == snap.TypeKernel) { + if !release.OnClassic && (snapsup.Type == snap.TypeGadget || (snapsup.Type == snap.TypeKernel && !TestingLeaveOutKernelUpdateGadgetAssets)) { // XXX: gadget update currently for core systems only gadgetUpdate := st.NewTask("update-gadget-assets", fmt.Sprintf(i18n.G("Update assets from %s %q%s"), snapsup.Type, snapsup.InstanceName(), revisionStr)) addTask(gadgetUpdate) @@ -2525,6 +2530,10 @@ func removeInactiveRevision(st *state.State, name, snapID string, revision snap. // RemoveMany removes everything from the given list of names. // Note that the state must be locked by the caller. func RemoveMany(st *state.State, names []string) ([]string, []*state.TaskSet, error) { + if err := validateSnapNames(names); err != nil { + return nil, nil, err + } + removed := make([]string, 0, len(names)) tasksets := make([]*state.TaskSet, 0, len(names)) @@ -2565,6 +2574,22 @@ func RemoveMany(st *state.State, names []string) ([]string, []*state.TaskSet, er return removed, tasksets, nil } +func validateSnapNames(names []string) error { + var invalidNames []string + + for _, name := range names { + if err := snap.ValidateInstanceName(name); err != nil { + invalidNames = append(invalidNames, name) + } + } + + if len(invalidNames) > 0 { + return fmt.Errorf("cannot remove invalid snap names: %v", strings.Join(invalidNames, ", ")) + } + + return nil +} + // Revert returns a set of tasks for reverting to the previous version of the snap. // Note that the state must be locked by the caller. func Revert(st *state.State, name string, flags Flags) (*state.TaskSet, error) { diff --git a/overlord/snapstate/snapstate_remove_test.go b/overlord/snapstate/snapstate_remove_test.go index d8ec4c85f29..4f74a5ddc21 100644 --- a/overlord/snapstate/snapstate_remove_test.go +++ b/overlord/snapstate/snapstate_remove_test.go @@ -1707,3 +1707,23 @@ func (s *snapmgrTestSuite) TestRemoveKeepsGatingDataIfNotLastRevision(c *C) { c.Assert(candidates, HasLen, 1) c.Check(candidates["some-snap"], NotNil) } + +func (s *snapmgrTestSuite) TestRemoveFailsWithInvalidSnapName(c *C) { + s.state.Lock() + defer s.state.Unlock() + + removed, ts, err := snapstate.RemoveMany(s.state, []string{"some-snap", "rev=", "123"}) + c.Check(removed, HasLen, 0) + c.Check(ts, HasLen, 0) + c.Check(err.Error(), Equals, "cannot remove invalid snap names: rev=, 123") +} + +func (s *snapmgrTestSuite) TestRemoveSucceedsWithInstanceName(c *C) { + s.state.Lock() + defer s.state.Unlock() + + removed, ts, err := snapstate.RemoveMany(s.state, []string{"some-snap", "ab_c"}) + c.Check(removed, NotNil) + c.Check(ts, NotNil) + c.Check(err, IsNil) +} diff --git a/overlord/snapstate/snapstate_test.go b/overlord/snapstate/snapstate_test.go index 29bf8e94d0c..5dc51d9f32c 100644 --- a/overlord/snapstate/snapstate_test.go +++ b/overlord/snapstate/snapstate_test.go @@ -269,6 +269,7 @@ func AddForeignTaskHandlers(runner *state.TaskRunner, tracker ForeignTaskTracker runner.AddHandler("validate-snap", fakeHandler, nil) runner.AddHandler("transition-ubuntu-core", fakeHandler, nil) runner.AddHandler("transition-to-snapd-snap", fakeHandler, nil) + runner.AddHandler("update-gadget-assets", fakeHandler, nil) // Add handler to test full aborting of changes erroringHandler := func(task *state.Task, _ *tomb.Tomb) error { diff --git a/overlord/snapstate/snapstate_update_test.go b/overlord/snapstate/snapstate_update_test.go index 07502456c6e..c9f326878e7 100644 --- a/overlord/snapstate/snapstate_update_test.go +++ b/overlord/snapstate/snapstate_update_test.go @@ -1556,6 +1556,9 @@ func (s *snapmgrTestSuite) TestUpdateRememberedUserRunThrough(c *C) { } func (s *snapmgrTestSuite) TestUpdateModelKernelSwitchTrackRunThrough(c *C) { + restore := release.MockOnClassic(false) + defer restore() + // use services-snap here to make sure services would be stopped/started appropriately si := snap.SideInfo{ RealName: "kernel", diff --git a/packaging/snapd.mk b/packaging/snapd.mk index f04ace0b196..d19f8120971 100644 --- a/packaging/snapd.mk +++ b/packaging/snapd.mk @@ -59,7 +59,7 @@ all: $(go_binaries) snap: GO_TAGS += nomanagers snap snap-seccomp: - go build $(if $(GO_TAGS),-tags $(GO_TAGS)) -buildmode=pie $(import_path)/cmd/$@ + go build $(if $(GO_TAGS),-tags $(GO_TAGS)) -buildmode=pie -ldflags=-w $(import_path)/cmd/$@ # Those three need to be built as static binaries. They run on the inside of a # nearly-arbitrary mount namespace that does not contain anything we can depend @@ -73,9 +73,9 @@ snap-update-ns snap-exec snapctl: # suite to add test assertions. Do not enable this in distribution packages. snapd: ifeq ($(with_testkeys),1) - go build -buildmode=pie -tags withtestkeys $(import_path)/cmd/$@ + go build -buildmode=pie -ldflags=-w -tags withtestkeys $(import_path)/cmd/$@ else - go build -buildmode=pie $(import_path)/cmd/$@ + go build -buildmode=pie -ldflags=-w $(import_path)/cmd/$@ endif # Know how to create certain directories. diff --git a/packaging/ubuntu-14.04/tests/integrationtests b/packaging/ubuntu-14.04/tests/integrationtests index 530034cdad9..b1ef4af3f9c 100644 --- a/packaging/ubuntu-14.04/tests/integrationtests +++ b/packaging/ubuntu-14.04/tests/integrationtests @@ -36,6 +36,6 @@ fi . /etc/os-release apt-get install golang-1.6-go export GOPATH=/tmp/go -export PATH=/usr/lib/go-1.6/bin:${PATH} +export PATH=/usr/lib/go-1.6/bin:"${PATH}" go get -u github.com/snapcore/spread/cmd/spread /tmp/go/bin/spread -v "autopkgtest:${ID}-${VERSION_ID}-$(dpkg --print-architecture)" diff --git a/packaging/ubuntu-16.04/rules b/packaging/ubuntu-16.04/rules index 60b546a72e5..f2376cb3f56 100755 --- a/packaging/ubuntu-16.04/rules +++ b/packaging/ubuntu-16.04/rules @@ -43,6 +43,7 @@ SYSTEMD_UNITS_DESTDIR="lib/systemd/system/" GCCGO := $(shell go tool dist env > /dev/null 2>&1 && echo no || echo yes) BUILDFLAGS:=-pkgdir=$(CURDIR)/_build/std +BUILDFLAGS+=-ldflags=-w # Disable -buildmode=pie mode on all our 32bit platforms # (i386 and armhf). For i386 because of LP: #1711052 and for # armhf because of LP: #1822738 diff --git a/run-checks b/run-checks index efd7158e368..65a0c9f3fcb 100755 --- a/run-checks +++ b/run-checks @@ -12,7 +12,7 @@ COVERMODE=${COVERMODE:-atomic} if [ -z "${GITHUB_WORKFLOW:-}" ]; then # when *not* running inside github, ensure we use go-1.13 by default - export PATH=/usr/lib/go-1.13/bin:${PATH} + export PATH=/usr/lib/go-1.13/bin:"${PATH}" fi # add workaround for https://github.com/golang/go/issues/24449 diff --git a/strutil/strutil.go b/strutil/strutil.go index dc674fd0bfb..00c70100396 100644 --- a/strutil/strutil.go +++ b/strutil/strutil.go @@ -27,7 +27,7 @@ import ( "unicode/utf8" ) -// Convert the given size in btes to a readable string +// SizeToStr converts the given size in bytes to a readable string func SizeToStr(size int64) string { suffixes := []string{"B", "kB", "MB", "GB", "TB", "PB", "EB"} for _, suf := range suffixes { diff --git a/tests/lib/snaps/config-versions-v2/meta/hooks/configure b/tests/lib/snaps/config-versions-v2/meta/hooks/configure index c76c901b9d2..fa3b344050b 100755 --- a/tests/lib/snaps/config-versions-v2/meta/hooks/configure +++ b/tests/lib/snaps/config-versions-v2/meta/hooks/configure @@ -3,7 +3,7 @@ snapctl set configure-marker="executed-for-v2" command=$(snapctl get fail-configure) -if [ "x$command" = "xyes" ]; then +if [ "$command" = "yes" ]; then echo "failing configure hook as requested" exit 1 fi diff --git a/tests/lib/snaps/snapctl-hooks/meta/hooks/configure b/tests/lib/snaps/snapctl-hooks/meta/hooks/configure index 1ebb72e2c64..d3386662812 100755 --- a/tests/lib/snaps/snapctl-hooks/meta/hooks/configure +++ b/tests/lib/snaps/snapctl-hooks/meta/hooks/configure @@ -68,9 +68,9 @@ test_get_nested() { fi expected_output='{\n\t"key1": "a",\n\t"key2": "b"\n}' # note: "echo" is a built-in of sh and doesn't support -e flag, use /bin/echo. - # shellcheck disable=SC2039 + # shellcheck disable=SC3037 if [ "$output" != "$(/bin/echo -e "$expected_output")" ]; then - echo "Expected output to be '$expected_output' but it was '$output'" + echo "Expected output to be '$(/bin/echo -e "$expected_output")' but it was '$output'" exit 1 fi } diff --git a/tests/lib/snaps/store/test-snapd-upower/bin/upowerd.sh b/tests/lib/snaps/store/test-snapd-upower/bin/upowerd.sh index beab24627aa..e2f4e0e1223 100644 --- a/tests/lib/snaps/store/test-snapd-upower/bin/upowerd.sh +++ b/tests/lib/snaps/store/test-snapd-upower/bin/upowerd.sh @@ -32,6 +32,6 @@ if [ ! -e "$SNAP_DATA/UPower.conf" ]; then cp "$SNAP/etc/UPower/UPower.conf" "$SNAP_DATA" fi -export UPOWER_CONF_FILE_NAME=$SNAP_DATA/UPower.conf +export UPOWER_CONF_FILE_NAME="$SNAP_DATA"/UPower.conf exec "$SNAP/usr/libexec/upowerd" $UPOWER_OPTS diff --git a/tests/main/lxd-no-fuse/task.yaml b/tests/main/lxd-no-fuse/task.yaml index 737b2cdaca7..5112d050ccf 100644 --- a/tests/main/lxd-no-fuse/task.yaml +++ b/tests/main/lxd-no-fuse/task.yaml @@ -5,7 +5,7 @@ systems: [ubuntu-18.04-64] restore: | lxc delete --force my-ubuntu - snap remove ---purge lxd + snap remove --purge lxd "$TESTSTOOLS"/lxd-state undo-mount-changes # Remove manually the snap.lxd.workaround.service systemd unit. This unit is needed to diff --git a/tests/main/lxd-snapfuse/task.yaml b/tests/main/lxd-snapfuse/task.yaml index 0f57a35f10d..0cd5862cc8a 100644 --- a/tests/main/lxd-snapfuse/task.yaml +++ b/tests/main/lxd-snapfuse/task.yaml @@ -5,7 +5,7 @@ systems: [ubuntu-18.04-64] restore: | lxc delete --force my-ubuntu - snap remove ---purge lxd + snap remove --purge lxd "$TESTSTOOLS"/lxd-state undo-mount-changes # Remove manually the snap.lxd.workaround.service systemd unit. This unit is needed to diff --git a/tests/main/snapctl/task.yaml b/tests/main/snapctl/task.yaml index aac98441f25..54ae13b3b61 100644 --- a/tests/main/snapctl/task.yaml +++ b/tests/main/snapctl/task.yaml @@ -37,6 +37,9 @@ execute: | echo "Verify that snapctl set is forbidden for regular user" su -c "snapctl set snapctl-hooks foo=bar" test 2>&1 | MATCH "cannot use \"set\" with uid .*, try with sudo" + echo "Verify that snapctl fails with correct error message using flag if regular user" + su -c "snapctl start --enable" test 2>&1 | MATCH "cannot use \"start\" with uid [0-9]+, try with sudo" + echo "Verify that the snapd API is only available via the snapd socket" if ! printf 'GET /v2/snaps HTTP/1.0\r\n\r\n' | nc -U -w 1 /run/snapd.socket | grep '200 OK'; then echo "Expected snapd API to be available on the snapd socket"