Skip to content

Commit

Permalink
overlord/snapstate: fix handling of classic on updates & install, ext…
Browse files Browse the repository at this point in the history 
…end unit tests

Allow upgrading from classic to strict on update, but keep blocking the
transition from strict to classic.

Update unit tests for installation and updates.

Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
  • Loading branch information
@bboozzoo
bboozzoo committed Oct 26, 2018
1 parent a82c8ba commit ccd4623
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 22 deletions.
26 changes: 21 additions & 5 deletions overlord/snapstate/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ func (f *fakeStore) snap(spec snapSpec, user *auth.UserState) (*snap.Info, error
typ = snap.TypeGadget
case "some-snapd":
typ = snap.TypeSnapd
case "some-snap-now-classic":
confinement = "classic"
}

if spec.Name == "snap-unknown" {
Expand Down Expand Up @@ -263,6 +265,10 @@ func (f *fakeStore) lookupRefresh(cand refreshCand) (*snap.Info, error) {
name = "services-snap"
case "some-snap-id":
name = "some-snap"
case "some-snap-now-classic-id":
name = "some-snap-now-classic"
case "some-snap-was-classic-id":
name = "some-snap-was-classic"
case "core-snap-id":
name = "core"
typ = snap.TypeOS
Expand Down Expand Up @@ -305,6 +311,9 @@ func (f *fakeStore) lookupRefresh(cand refreshCand) (*snap.Info, error) {
case "channel-for-devmode":
confinement = snap.DevModeConfinement
}
if name == "some-snap-now-classic" {
confinement = "classic"
}

info := &snap.Info{
Type: typ,
Expand Down Expand Up @@ -589,31 +598,38 @@ func (f *fakeSnappyBackend) OpenSnapFile(snapFilePath string, si *snap.SideInfo)
op.sinfo = *si
}

var name string
var info *snap.Info
if !osutil.IsDirectory(snapFilePath) {
name = filepath.Base(snapFilePath)
name := filepath.Base(snapFilePath)
split := strings.Split(name, "_")
if len(split) >= 2 {
// <snap>_<rev>.snap
// <snap>_<instance-key>_<rev>.snap
name = split[0]
}

info = &snap.Info{SuggestedName: name, Architectures: []string{"all"}}
if name == "some-snap-now-classic" {
info.Confinement = "classic"
}
} else {
// for snap try only
snapf, err := snap.Open(snapFilePath)
if err != nil {
return nil, nil, err
}

info, err := snap.ReadInfoFromSnapFile(snapf, si)
info, err = snap.ReadInfoFromSnapFile(snapf, si)
if err != nil {
return nil, nil, err
}
name = info.SuggestedName
}

if info == nil {
return nil, nil, fmt.Errorf("internal error: no mocked snap for %q", snapFilePath)
}
f.appendOp(&op)
return &snap.Info{SuggestedName: name, Architectures: []string{"all"}}, f.emptyContainer, nil
return info, f.emptyContainer, nil
}

func (f *fakeSnappyBackend) SetupSnap(snapFilePath, instanceName string, si *snap.SideInfo, p progress.Meter) (snap.Type, error) {
Expand Down
18 changes: 17 additions & 1 deletion overlord/snapstate/snapstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,11 @@ func UpdateMany(ctx context.Context, st *state.State, names []string, userID int

params := func(update *snap.Info) (string, Flags, *SnapState) {
snapst := stateByInstanceName[update.InstanceName()]
updateFlags := snapst.Flags
if !update.NeedsClassic() && updateFlags.Classic {
// allow updating from classic to strict
updateFlags.Classic = false
}
return snapst.Channel, snapst.Flags, snapst

}
Expand Down Expand Up @@ -1118,7 +1123,12 @@ func Update(st *state.State, name, channel string, revision snap.Revision, userI
}

params := func(update *snap.Info) (string, Flags, *SnapState) {
return channel, flags, &snapst
updateFlags := flags
if !update.NeedsClassic() && updateFlags.Classic {
// allow updating from classic to strict
updateFlags.Classic = false
}
return channel, updateFlags, &snapst
}

_, tts, err := doUpdate(context.TODO(), st, []string{name}, updates, params, userID, &flags)
Expand Down Expand Up @@ -1192,6 +1202,12 @@ func infoForUpdate(st *state.State, snapst *SnapState, name, channel string, rev
if err != nil {
return nil, err
}
if !info.NeedsClassic() && flags.Classic {
// thw new revision is no longer classic, but the one
// installed is, allow such transition so do not fail
// validation
flags.Classic = false
}
if err := validateInfoAndFlags(info, snapst, flags); err != nil {
return nil, err
}
Expand Down
42 changes: 26 additions & 16 deletions overlord/snapstate/snapstate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package snapstate_test

import (
"bytes"
"encoding/json"
"errors"
"fmt"
Expand Down Expand Up @@ -510,9 +511,9 @@ func (s *snapmgrTestSuite) TestInstallClassicConfinementFiltering(c *C) {
_, err = snapstate.Install(s.state, "some-snap", "channel-for-classic", snap.R(0), s.user.ID, snapstate.Flags{Classic: true})
c.Assert(err, IsNil)

// if a snap is *not* classic, you can still install it with --classic
// if a snap is *not* classic, you cannot install it with --classic
_, err = snapstate.Install(s.state, "some-snap", "channel-for-strict", snap.R(0), s.user.ID, snapstate.Flags{Classic: true})
c.Assert(err, IsNil)
c.Assert(err, ErrorMatches, "classic confinement requested for a non classic confined snap")
}

func (s *snapmgrTestSuite) TestInstallFailsWhenClassicSnapsAreNotSupported(c *C) {
Expand Down Expand Up @@ -1832,21 +1833,20 @@ func (s *snapmgrTestSuite) TestUpdateClassicConfinementFiltering(c *C) {
s.state.Lock()
defer s.state.Unlock()

snapstate.Set(s.state, "some-snap", &snapstate.SnapState{
snapstate.Set(s.state, "some-snap-now-classic", &snapstate.SnapState{
Active: true,
Channel: "channel-for-classic",
Sequence: []*snap.SideInfo{{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(7)}},
Sequence: []*snap.SideInfo{{RealName: "some-snap-now-classic", SnapID: "some-snap-now-classic-id", Revision: snap.R(7)}},
Current: snap.R(7),
SnapType: "app",
})

// updated snap is classic, refresh without --classic, do nothing
// TODO: better error message here
_, err := snapstate.Update(s.state, "some-snap", "", snap.R(0), s.user.ID, snapstate.Flags{})
_, err := snapstate.Update(s.state, "some-snap-now-classic", "", snap.R(0), s.user.ID, snapstate.Flags{})
c.Assert(err, ErrorMatches, `.* requires classic confinement`)

// updated snap is classic, refresh with --classic
ts, err := snapstate.Update(s.state, "some-snap", "", snap.R(0), s.user.ID, snapstate.Flags{Classic: true})
ts, err := snapstate.Update(s.state, "some-snap-now-classic", "", snap.R(0), s.user.ID, snapstate.Flags{Classic: true})
c.Assert(err, IsNil)

chg := s.state.NewChange("refresh", "refresh snap")
Expand All @@ -1857,9 +1857,12 @@ func (s *snapmgrTestSuite) TestUpdateClassicConfinementFiltering(c *C) {
s.settle(c)
s.state.Lock()

c.Assert(chg.Err(), IsNil)
c.Assert(chg.IsReady(), Equals, true)

// verify snap is in classic
var snapst snapstate.SnapState
err = snapstate.Get(s.state, "some-snap", &snapst)
err = snapstate.Get(s.state, "some-snap-now-classic", &snapst)
c.Assert(err, IsNil)
c.Check(snapst.Classic, Equals, true)
}
Expand Down Expand Up @@ -1942,21 +1945,21 @@ func (s *snapmgrTestSuite) TestUpdateStrictFromClassic(c *C) {
s.state.Lock()
defer s.state.Unlock()

snapstate.Set(s.state, "some-snap", &snapstate.SnapState{
snapstate.Set(s.state, "some-snap-was-classic", &snapstate.SnapState{
Active: true,
Channel: "channel",
Sequence: []*snap.SideInfo{{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(7)}},
Sequence: []*snap.SideInfo{{RealName: "some-snap-was-classic", SnapID: "some-snap-was-classic-id", Revision: snap.R(7)}},
Current: snap.R(7),
SnapType: "app",
Flags: snapstate.Flags{Classic: true},
})

// snap installed with --classic, update does not need classic, refresh works without --classic
_, err := snapstate.Update(s.state, "some-snap", "", snap.R(0), s.user.ID, snapstate.Flags{})
_, err := snapstate.Update(s.state, "some-snap-was-classic", "", snap.R(0), s.user.ID, snapstate.Flags{})
c.Assert(err, IsNil)

// snap installed with --classic, update does not need classic, refresh works with --classic
_, err = snapstate.Update(s.state, "some-snap", "", snap.R(0), s.user.ID, snapstate.Flags{Classic: true})
_, err = snapstate.Update(s.state, "some-snap-was-classic", "", snap.R(0), s.user.ID, snapstate.Flags{Classic: true})
c.Assert(err, IsNil)
}

Expand Down Expand Up @@ -9127,10 +9130,10 @@ func (s *snapmgrTestSuite) TestTrySetsTryModeClassic(c *C) {
restore := maybeMockClassicSupport(c)
defer restore()

s.testTrySetsTryMode(snapstate.Flags{Classic: true}, c)
s.testTrySetsTryMode(snapstate.Flags{Classic: true}, c, "confinement: classic\n")
}

func (s *snapmgrTestSuite) testTrySetsTryMode(flags snapstate.Flags, c *C) {
func (s *snapmgrTestSuite) testTrySetsTryMode(flags snapstate.Flags, c *C, extraYaml ...string) {
s.state.Lock()
defer s.state.Unlock()

Expand All @@ -9140,7 +9143,14 @@ func (s *snapmgrTestSuite) testTrySetsTryMode(flags snapstate.Flags, c *C) {
tryYaml := filepath.Join(d, "meta", "snap.yaml")
err := os.MkdirAll(filepath.Dir(tryYaml), 0755)
c.Assert(err, IsNil)
err = ioutil.WriteFile(tryYaml, []byte("name: foo\nversion: 1.0"), 0644)
buf := bytes.Buffer{}
buf.WriteString("name: foo\nversion: 1.0\n")
if len(extraYaml) > 0 {
for _, extra := range extraYaml {
buf.WriteString(extra)
}
}
err = ioutil.WriteFile(tryYaml, buf.Bytes(), 0644)
c.Assert(err, IsNil)

chg := s.state.NewChange("try", "try snap")
Expand Down Expand Up @@ -9197,7 +9207,7 @@ func (s *snapmgrTestSuite) TestTryUndoRemovesTryFlagLeavesJailMode(c *C) {
func (s *snapmgrTestSuite) TestTryUndoRemovesTryFlagLeavesClassic(c *C) {
restore := maybeMockClassicSupport(c)
defer restore()
s.testTrySetsTryMode(snapstate.Flags{Classic: true}, c)
s.testTrySetsTryMode(snapstate.Flags{Classic: true}, c, "confinement: classic\n")
}

func (s *snapmgrTestSuite) testTryUndoRemovesTryFlag(flags snapstate.Flags, c *C) {
Expand Down

0 comments on commit ccd4623

Please sign in to comment.