diff options
| author | Pawel Stolowski <stolowski@gmail.com> | 2019-11-08 14:27:17 +0100 |
|---|---|---|
| committer | Pawel Stolowski <stolowski@gmail.com> | 2019-11-08 14:27:17 +0100 |
| commit | 298546316b882bd9f939b7130371d4d2d3094114 (patch) | |
| tree | 7ef93aa901450e394388fc2da95e31e3b13b708f | |
| parent | d2bea39a2b220fd52757dc25c8919c4e587a11df (diff) | |
| parent | f3eaed44de03cfed6079f2d3d88419fdb7930af9 (diff) | |
Merge branch 'master' into prebaking/is-preseed-modeprebaking/is-preseed-mode
65 files changed, 4304 insertions, 581 deletions
diff --git a/asserts/ifacedecls.go b/asserts/ifacedecls.go index d2394ad8d2..aa36bf1260 100644 --- a/asserts/ifacedecls.go +++ b/asserts/ifacedecls.go @@ -361,10 +361,11 @@ func (c *AttributeConstraints) Check(attrer Attrer, ctx AttrMatchContext) error } // SideArityConstraint specifies a constraint for the overall arity of -// the opposite connected set of slots for a plug, respectively plugs -// for a slot. -// It is used to express parsed slots-per-plug, respectively plugs-per-slot +// the set of connected slots for a given plug or the set of +// connected plugs for a given slot. +// It is used to express parsed slots-per-plug and plugs-per-slot // constraints. +// See https://forum.snapcraft.io/t/plug-slot-declaration-rules-greedy-plugs/12438 type SideArityConstraint struct { // N can be: // =>1 @@ -386,7 +387,7 @@ func compileSideArityConstraint(context *subruleContext, which string, v interfa return a, fmt.Errorf("%s cannot specify a %s constraint, they apply only to allow-*connection", context, which) } x, ok := v.(string) - if !ok { + if !ok || len(x) == 0 { return a, fmt.Errorf("%s in %s must be an integer >=1 or *", which, context) } if x == "*" { @@ -412,7 +413,7 @@ func normalizeSideArityConstraints(context *subruleContext, c sideArityConstrain return } any := SideArityConstraint{N: -1} - // normalized plugs-per-slots is always * + // normalized plugs-per-slot is always * c.setPlugsPerSlot(any) slotsPerPlug := c.slotsPerPlug() if context.autoConnection() { @@ -646,10 +647,21 @@ type constraintsDef struct { invert bool } +// subruleContext carries queryable context information about one the +// {allow,deny}-* subrules that end up compiled as +// Plug|Slot*Constraints. The information includes the parent rule, +// the introductory subrule key ({allow,deny}-*) and which alternative +// it corresponds to if any. +// The information is useful for constraints compilation now that we +// have constraints with different behavior depending on the kind of +// subrule that hosts them (e.g. slots-per-plug, plugs-per-slot). type subruleContext struct { - rule string + // rule is the parent rule context description + rule string + // subrule is the subrule key subrule string - alt int + // alt is which alternative this is (if > 0) + alt int } func (c *subruleContext) String() string { @@ -660,14 +672,17 @@ func (c *subruleContext) String() string { return subctxt } +// allow returns whether the subrule is an allow-* subrule. func (c *subruleContext) allow() bool { return strings.HasPrefix(c.subrule, "allow-") } +// installation returns whether the subrule is an *-installation subrule. func (c *subruleContext) installation() bool { return strings.HasSuffix(c.subrule, "-installation") } +// autoConnection returns whether the subrule is an *-auto-connection subrule. func (c *subruleContext) autoConnection() bool { return strings.HasSuffix(c.subrule, "-auto-connection") } diff --git a/asserts/ifacedecls_test.go b/asserts/ifacedecls_test.go index 6d1c571d86..105c27af35 100644 --- a/asserts/ifacedecls_test.go +++ b/asserts/ifacedecls_test.go @@ -497,6 +497,11 @@ func checkAttrs(c *C, attrs *asserts.AttributeConstraints, witness, expected str c.Check(attrs.Check(plug, nil), IsNil) } +var ( + sideArityAny = asserts.SideArityConstraint{N: -1} + sideArityOne = asserts.SideArityConstraint{N: 1} +) + func checkBoolPlugConnConstraints(c *C, subrule string, cstrs []*asserts.PlugConnectionConstraints, always bool) { expected := asserts.NeverMatchAttributes if always { @@ -511,13 +516,11 @@ func checkBoolPlugConnConstraints(c *C, subrule string, cstrs []*asserts.PlugCon c.Check(cstrs1.SlotsPerPlug, Equals, undef) c.Check(cstrs1.PlugsPerSlot, Equals, undef) } else { - any := asserts.SideArityConstraint{N: -1} - one := asserts.SideArityConstraint{N: 1} - c.Check(cstrs1.PlugsPerSlot, Equals, any) + c.Check(cstrs1.PlugsPerSlot, Equals, sideArityAny) if strings.HasSuffix(subrule, "-auto-connection") { - c.Check(cstrs1.SlotsPerPlug, Equals, one) + c.Check(cstrs1.SlotsPerPlug, Equals, sideArityOne) } else { - c.Check(cstrs1.SlotsPerPlug, Equals, any) + c.Check(cstrs1.SlotsPerPlug, Equals, sideArityAny) } } c.Check(cstrs1.SlotSnapIDs, HasLen, 0) @@ -539,13 +542,11 @@ func checkBoolSlotConnConstraints(c *C, subrule string, cstrs []*asserts.SlotCon c.Check(cstrs1.SlotsPerPlug, Equals, undef) c.Check(cstrs1.PlugsPerSlot, Equals, undef) } else { - any := asserts.SideArityConstraint{N: -1} - one := asserts.SideArityConstraint{N: 1} - c.Check(cstrs1.PlugsPerSlot, Equals, any) + c.Check(cstrs1.PlugsPerSlot, Equals, sideArityAny) if strings.HasSuffix(subrule, "-auto-connection") { - c.Check(cstrs1.SlotsPerPlug, Equals, one) + c.Check(cstrs1.SlotsPerPlug, Equals, sideArityOne) } else { - c.Check(cstrs1.SlotsPerPlug, Equals, any) + c.Check(cstrs1.SlotsPerPlug, Equals, sideArityAny) } } c.Check(cstrs1.PlugSnapIDs, HasLen, 0) @@ -1000,7 +1001,9 @@ func (s *plugSlotRulesSuite) TestCompilePlugRuleConnectionConstraintsSideArityCo c.Check(rule.AllowConnection[0].SlotsPerPlug.Any(), Equals, true) c.Check(rule.AllowConnection[0].PlugsPerSlot.Any(), Equals, true) - // allow-connection => * + // test that the arity constraints get normalized away to any + // under allow-connection + // see https://forum.snapcraft.io/t/plug-slot-declaration-rules-greedy-plugs/12438 allowConnTests := []string{ `iface: allow-connection: @@ -1027,23 +1030,26 @@ func (s *plugSlotRulesSuite) TestCompilePlugRuleConnectionConstraintsSideArityCo c.Check(rule.AllowConnection[0].PlugsPerSlot.Any(), Equals, true) } - // allow-auto-connection => * + // test that under allow-auto-connection: + // slots-per-plug can be * (any) or otherwise gets normalized to 1 + // plugs-per-slot gets normalized to any (*) + // see https://forum.snapcraft.io/t/plug-slot-declaration-rules-greedy-plugs/12438 allowAutoConnTests := []struct { rule string - slotsPerPlug int + slotsPerPlug asserts.SideArityConstraint }{ {`iface: allow-auto-connection: slots-per-plug: 1 - plugs-per-slot: 2`, 1}, + plugs-per-slot: 2`, sideArityOne}, {`iface: allow-auto-connection: slots-per-plug: * - plugs-per-slot: 1`, -1}, + plugs-per-slot: 1`, sideArityAny}, {`iface: allow-auto-connection: slots-per-plug: 2 - plugs-per-slot: *`, 1}, + plugs-per-slot: *`, sideArityOne}, } for _, t := range allowAutoConnTests { @@ -1053,7 +1059,7 @@ func (s *plugSlotRulesSuite) TestCompilePlugRuleConnectionConstraintsSideArityCo rule, err = asserts.CompilePlugRule("iface", m["iface"].(map[string]interface{})) c.Assert(err, IsNil) - c.Check(rule.AllowAutoConnection[0].SlotsPerPlug, Equals, asserts.SideArityConstraint{N: t.slotsPerPlug}) + c.Check(rule.AllowAutoConnection[0].SlotsPerPlug, Equals, t.slotsPerPlug) c.Check(rule.AllowAutoConnection[0].PlugsPerSlot.Any(), Equals, true) } } @@ -1706,7 +1712,9 @@ func (s *plugSlotRulesSuite) TestCompileSlotRuleConnectionConstraintsSideArityCo c.Check(rule.AllowConnection[0].SlotsPerPlug.Any(), Equals, true) c.Check(rule.AllowConnection[0].PlugsPerSlot.Any(), Equals, true) - // allow-connection => * + // test that the arity constraints get normalized away to any + // under allow-connection + // see https://forum.snapcraft.io/t/plug-slot-declaration-rules-greedy-plugs/12438 allowConnTests := []string{ `iface: allow-connection: @@ -1733,23 +1741,26 @@ func (s *plugSlotRulesSuite) TestCompileSlotRuleConnectionConstraintsSideArityCo c.Check(rule.AllowConnection[0].PlugsPerSlot.Any(), Equals, true) } - // allow-auto-connection => * + // test that under allow-auto-connection: + // slots-per-plug can be * (any) or otherwise gets normalized to 1 + // plugs-per-slot gets normalized to any (*) + // see https://forum.snapcraft.io/t/plug-slot-declaration-rules-greedy-plugs/12438 allowAutoConnTests := []struct { rule string - slotsPerPlug int + slotsPerPlug asserts.SideArityConstraint }{ {`iface: allow-auto-connection: slots-per-plug: 1 - plugs-per-slot: 2`, 1}, + plugs-per-slot: 2`, sideArityOne}, {`iface: allow-auto-connection: slots-per-plug: * - plugs-per-slot: 1`, -1}, + plugs-per-slot: 1`, sideArityAny}, {`iface: allow-auto-connection: slots-per-plug: 2 - plugs-per-slot: *`, 1}, + plugs-per-slot: *`, sideArityOne}, } for _, t := range allowAutoConnTests { @@ -1759,7 +1770,7 @@ func (s *plugSlotRulesSuite) TestCompileSlotRuleConnectionConstraintsSideArityCo rule, err = asserts.CompileSlotRule("iface", m["iface"].(map[string]interface{})) c.Assert(err, IsNil) - c.Check(rule.AllowAutoConnection[0].SlotsPerPlug, Equals, asserts.SideArityConstraint{N: t.slotsPerPlug}) + c.Check(rule.AllowAutoConnection[0].SlotsPerPlug, Equals, t.slotsPerPlug) c.Check(rule.AllowAutoConnection[0].PlugsPerSlot.Any(), Equals, true) } } diff --git a/asserts/model.go b/asserts/model.go index e448a6f6ff..c6f4e534eb 100644 --- a/asserts/model.go +++ b/asserts/model.go @@ -174,7 +174,7 @@ func checkExtendedSnaps(extendedSnaps interface{}, base string, grade ModelGrade } var ( - validSnapTypes = []string{"app", "base", "gadget", "kernel", "core"} + validSnapTypes = []string{"app", "base", "gadget", "kernel", "core", "snapd"} validSnapMode = regexp.MustCompile("^[a-z][-a-z]+$") validSnapPresences = []string{"required", "optional"} ) @@ -214,7 +214,7 @@ func checkModelSnap(snap map[string]interface{}, grade ModelGrade) (*ModelSnap, typ = "app" } if !strutil.ListContains(validSnapTypes, typ) { - return nil, fmt.Errorf("type of snap %q must be one of app|base|gadget|kernel|core", name) + return nil, fmt.Errorf("type of snap %q must be one of %s", name, strings.Join(validSnapTypes, "|")) } modes, err := checkStringListInMap(snap, "modes", fmt.Sprintf("%q %s", "modes", what), validSnapMode) diff --git a/asserts/model_test.go b/asserts/model_test.go index 10e73f2fc8..9b9436401a 100644 --- a/asserts/model_test.go +++ b/asserts/model_test.go @@ -686,6 +686,41 @@ func (mods *modelSuite) TestCore20ExplictBootBase(c *C) { }) } +func (mods *modelSuite) TestCore20ExplictSnapd(c *C) { + encoded := strings.Replace(core20ModelExample, "TSLINE", mods.tsLine, 1) + encoded = strings.Replace(encoded, "OTHER", ` - + name: snapd + id: snapdidididididididididididididd + type: snapd + modes: + - run + - ephemeral + default-channel: latest/edge +`, 1) + a, err := asserts.Decode([]byte(encoded)) + c.Assert(err, IsNil) + c.Check(a.Type(), Equals, asserts.ModelType) + model := a.(*asserts.Model) + find := func(snapName string, haystack []*asserts.ModelSnap) *asserts.ModelSnap { + for _, s := range haystack { + if s.Name == snapName { + return s + } + } + return nil + } + snapdSnap := find("snapd", model.AllSnaps()) + c.Assert(snapdSnap, NotNil) + c.Check(snapdSnap, DeepEquals, &asserts.ModelSnap{ + Name: "snapd", + SnapID: "snapdidididididididididididididd", + SnapType: "snapd", + Modes: []string{"run", "ephemeral"}, + DefaultChannel: "latest/edge", + Presence: "required", + }) +} + func (mods *modelSuite) TestCore20GradeOptionalDefaultSigned(c *C) { encoded := strings.Replace(core20ModelExample, "TSLINE", mods.tsLine, 1) encoded = strings.Replace(encoded, "OTHER", "", 1) @@ -749,7 +784,7 @@ func (mods *modelSuite) TestCore20DecodeInvalid(c *C) { {"id: myappdididididididididididididid\n", "id: 2\n", `"id" of snap "myapp" contains invalid characters: "2"`}, {" id: myappdididididididididididididid\n", "", `"id" of snap "myapp" is mandatory for stable model`}, {"type: gadget\n", "type:\n - g\n", `"type" of snap "brand-gadget" must be a string`}, - {"type: app\n", "type: thing\n", `"type" of snap "myappopt" must be one of must be one of app|base|gadget|kernel|core`}, + {"type: app\n", "type: thing\n", `"type" of snap "myappopt" must be one of must be one of app|base|gadget|kernel|core|snapd`}, {"modes:\n - run\n", "modes: run\n", `"modes" of snap "other-base" must be a list of strings`}, {"default-channel: 20\n", "default-channel: edge\n", `default channel for snap "baz-linux" must specify a track`}, {"default-channel: 2.0\n", "default-channel:\n - x\n", `"default-channel" of snap "myapp" must be a string`}, diff --git a/cmd/snap-bootstrap/bootstrap/bootstrap.go b/cmd/snap-bootstrap/bootstrap/bootstrap.go index abe4b2d11b..c55847a627 100644 --- a/cmd/snap-bootstrap/bootstrap/bootstrap.go +++ b/cmd/snap-bootstrap/bootstrap/bootstrap.go @@ -37,15 +37,22 @@ func Run(gadgetRoot, device string, options *Options) error { return fmt.Errorf("cannot use empty device node") } - // XXX: ensure we test that the current partition table is - // compatible with the gadget lv, err := gadget.PositionedVolumeFromGadget(gadgetRoot) if err != nil { return fmt.Errorf("cannot layout the volume: %v", err) } - sfdisk := partition.NewSFDisk(device) - created, err := sfdisk.Create(lv) + diskLayout, err := partition.DeviceLayoutFromDisk(device) + if err != nil { + return fmt.Errorf("cannot read %v partitions: %v", device, err) + } + + // check if the current partition table is compatible with the gadget + if err := ensureLayoutCompatibility(lv, diskLayout); err != nil { + return fmt.Errorf("gadget and %v partition table not compatible: %v", device, err) + } + + created, err := diskLayout.CreateMissing(lv) if err != nil { return fmt.Errorf("cannot create the partitions: %v", err) } @@ -59,3 +66,48 @@ func Run(gadgetRoot, device string, options *Options) error { return nil } + +func ensureLayoutCompatibility(gadgetLayout *gadget.LaidOutVolume, diskLayout *partition.DeviceLayout) error { + eq := func(ds partition.DeviceStructure, gs gadget.LaidOutStructure) bool { + dv := ds.VolumeStructure + gv := gs.VolumeStructure + return dv.Name == gv.Name && ds.StartOffset == gs.StartOffset && dv.Size == gv.Size && dv.Filesystem == gv.Filesystem + } + contains := func(haystack []gadget.LaidOutStructure, needle partition.DeviceStructure) bool { + for _, h := range haystack { + if eq(needle, h) { + return true + } + } + return false + } + + // Check if top level properties match + if !isCompatibleSchema(gadgetLayout.Volume.Schema, diskLayout.Schema) { + return fmt.Errorf("disk partitioning schema %q doesn't match gadget schema %q", diskLayout.Schema, gadgetLayout.Volume.Schema) + } + if gadgetLayout.Volume.ID != "" && gadgetLayout.Volume.ID != diskLayout.ID { + return fmt.Errorf("disk ID %q doesn't match gadget volume ID %q", diskLayout.ID, gadgetLayout.Volume.ID) + } + + // Check if all existing device partitions are also in gadget + for _, ds := range diskLayout.Structure { + if !contains(gadgetLayout.LaidOutStructure, ds) { + return fmt.Errorf("cannot find disk partition %q (starting at %d) in gadget", ds.VolumeStructure.Label, ds.StartOffset) + } + } + + return nil +} + +func isCompatibleSchema(gadgetSchema, diskSchema string) bool { + switch gadgetSchema { + // XXX: "mbr,gpt" is currently unsupported + case "", "gpt": + return diskSchema == "gpt" + case "mbr": + return diskSchema == "dos" + default: + return false + } +} diff --git a/cmd/snap-bootstrap/bootstrap/bootstrap_test.go b/cmd/snap-bootstrap/bootstrap/bootstrap_test.go index 3ae101c7ef..e48277e88f 100644 --- a/cmd/snap-bootstrap/bootstrap/bootstrap_test.go +++ b/cmd/snap-bootstrap/bootstrap/bootstrap_test.go @@ -19,11 +19,16 @@ package bootstrap_test import ( + "io/ioutil" + "os" + "path/filepath" "testing" . "gopkg.in/check.v1" "github.com/snapcore/snapd/cmd/snap-bootstrap/bootstrap" + "github.com/snapcore/snapd/cmd/snap-bootstrap/partition" + "github.com/snapcore/snapd/gadget" ) func TestBootstrap(t *testing.T) { TestingT(t) } @@ -43,3 +48,164 @@ func (s *bootstrapSuite) TestBootstrapRunError(c *C) { err = bootstrap.Run("some-dir", "", nil) c.Assert(err, ErrorMatches, "cannot use empty device node") } + +const mockGadgetYaml = `volumes: + pc: + bootloader: grub + structure: + - name: mbr + type: mbr + size: 440 + - name: BIOS Boot + type: DA,21686148-6449-6E6F-744E-656564454649 + size: 1M + offset: 1M + offset-write: mbr+92 +` + +const mockExtraStructure = ` + - name: Writable + role: system-data + filesystem-label: writable + filesystem: ext4 + type: 83,0FC63DAF-8483-4772-8E79-3D69D8477DE4 + size: 1200M +` + +var mockDeviceLayout = partition.DeviceLayout{ + Structure: []partition.DeviceStructure{ + { + LaidOutStructure: gadget.LaidOutStructure{ + VolumeStructure: &gadget.VolumeStructure{ + Name: "mbr", + Size: 440, + }, + StartOffset: 0, + }, + Node: "/dev/node1", + }, + { + LaidOutStructure: gadget.LaidOutStructure{ + VolumeStructure: &gadget.VolumeStructure{ + Name: "BIOS Boot", + Size: 1 * gadget.SizeMiB, + }, + StartOffset: 1 * gadget.SizeMiB, + }, + Node: "/dev/node2", + }, + }, + ID: "anything", + Device: "/dev/node", + Schema: "gpt", + Size: 0x500000, + SectorSize: 512, +} + +func (s *bootstrapSuite) TestLayoutCompatibility(c *C) { + // same contents + gadgetLayout := layoutFromYaml(c, mockGadgetYaml, "pc") + err := bootstrap.EnsureLayoutCompatibility(gadgetLayout, &mockDeviceLayout) + c.Assert(err, IsNil) + + // missing structure (that's ok) + gadgetLayoutWithExtras := layoutFromYaml(c, mockGadgetYaml+mockExtraStructure, "pc") + err = bootstrap.EnsureLayoutCompatibility(gadgetLayoutWithExtras, &mockDeviceLayout) + c.Assert(err, IsNil) + + deviceLayoutWithExtras := mockDeviceLayout + deviceLayoutWithExtras.Structure = append(deviceLayoutWithExtras.Structure, + partition.DeviceStructure{ + LaidOutStructure: gadget.LaidOutStructure{ + VolumeStructure: &gadget.VolumeStructure{ + Name: "Extra partition", + Size: 10 * gadget.SizeMiB, + Label: "extra", + }, + StartOffset: 2 * gadget.SizeMiB, + }, + Node: "/dev/node3", + }, + ) + // extra structure (should fail) + err = bootstrap.EnsureLayoutCompatibility(gadgetLayout, &deviceLayoutWithExtras) + c.Assert(err, ErrorMatches, `cannot find disk partition "extra".* in gadget`) +} + +func (s *bootstrapSuite) TestSchemaCompatibility(c *C) { + gadgetLayout := layoutFromYaml(c, mockGadgetYaml, "pc") + deviceLayout := mockDeviceLayout + + error_msg := "disk partitioning.* doesn't match gadget.*" + + for i, tc := range []struct { + gs string + ds string + e string + }{ + {"", "dos", error_msg}, + {"", "gpt", ""}, + {"", "xxx", error_msg}, + {"mbr", "dos", ""}, + {"mbr", "gpt", error_msg}, + {"mbr", "xxx", error_msg}, + {"gpt", "dos", error_msg}, + {"gpt", "gpt", ""}, + {"gpt", "xxx", error_msg}, + // XXX: "mbr,gpt" is currently unsupported + {"mbr,gpt", "dos", error_msg}, + {"mbr,gpt", "gpt", error_msg}, + {"mbr,gpt", "xxx", error_msg}, + } { + c.Logf("%d: %q %q\n", i, tc.gs, tc.ds) + gadgetLayout.Volume.Schema = tc.gs + deviceLayout.Schema = tc.ds + err := bootstrap.EnsureLayoutCompatibility(gadgetLayout, &deviceLayout) + if tc.e == "" { + c.Assert(err, IsNil) + } else { + c.Assert(err, ErrorMatches, tc.e) + } + } + c.Logf("-----") +} + +func (s *bootstrapSuite) TestIDCompatibility(c *C) { + gadgetLayout := layoutFromYaml(c, mockGadgetYaml, "pc") + deviceLayout := mockDeviceLayout + + error_msg := "disk ID.* doesn't match gadget volume ID.*" + + for i, tc := range []struct { + gid string + did string + e string + }{ + {"", "", ""}, + {"", "123", ""}, + {"123", "345", error_msg}, + {"123", "123", ""}, + } { + c.Logf("%d: %q %q\n", i, tc.gid, tc.did) + gadgetLayout.Volume.ID = tc.gid + deviceLayout.ID = tc.did + err := bootstrap.EnsureLayoutCompatibility(gadgetLayout, &deviceLayout) + if tc.e == "" { + c.Assert(err, IsNil) + } else { + c.Assert(err, ErrorMatches, tc.e) + } + } + c.Logf("-----") +} + +func layoutFromYaml(c *C, gadgetYaml, volume string) *gadget.LaidOutVolume { + gadgetRoot := filepath.Join(c.MkDir(), "gadget") + err := os.MkdirAll(filepath.Join(gadgetRoot, "meta"), 0755) + c.Assert(err, IsNil) + err = ioutil.WriteFile(filepath.Join(gadgetRoot, "meta", "gadget.yaml"), []byte(gadgetYaml), 0644) + c.Assert(err, IsNil) + pv, err := gadget.PositionedVolumeFromGadget(gadgetRoot) + c.Assert(err, IsNil) + return pv +} diff --git a/cmd/snap-bootstrap/bootstrap/export_test.go b/cmd/snap-bootstrap/bootstrap/export_test.go index f95db68ec6..d14c02800f 100644 --- a/cmd/snap-bootstrap/bootstrap/export_test.go +++ b/cmd/snap-bootstrap/bootstrap/export_test.go @@ -17,3 +17,7 @@ * */ package bootstrap + +var ( + EnsureLayoutCompatibility = ensureLayoutCompatibility +) diff --git a/cmd/snap-bootstrap/partition/sfdisk.go b/cmd/snap-bootstrap/partition/sfdisk.go index 8dc223e181..223c1238b0 100644 --- a/cmd/snap-bootstrap/partition/sfdisk.go +++ b/cmd/snap-bootstrap/partition/sfdisk.go @@ -24,6 +24,7 @@ import ( "fmt" "os/exec" "strings" + "time" "github.com/snapcore/snapd/gadget" "github.com/snapcore/snapd/osutil" @@ -57,27 +58,26 @@ type sfdiskPartition struct { Name string `json:"name"` } +type DeviceLayout struct { + Structure []DeviceStructure + ID string + Device string + Schema string + Size gadget.Size + SectorSize gadget.Size + partitionTable *sfdiskPartitionTable +} + type DeviceStructure struct { gadget.LaidOutStructure Node string } -type SFDisk struct { - device string - partitionTable *sfdiskPartitionTable -} - -func NewSFDisk(device string) *SFDisk { - return &SFDisk{ - device: device, - } -} - -// Layout obtains the partitioning and filesystem information from the block -// device and expresses it as a laid out volume. -func (sf *SFDisk) Layout() (*gadget.LaidOutVolume, error) { - output, err := exec.Command("sfdisk", "--json", "-d", sf.device).CombinedOutput() +// NewDeviceLayout obtains the partitioning and filesystem information from the +// block device. +func DeviceLayoutFromDisk(device string) (*DeviceLayout, error) { + output, err := exec.Command("sfdisk", "--json", "-d", device).CombinedOutput() if err != nil { return nil, osutil.OutputErr(output, err) } @@ -87,39 +87,37 @@ func (sf *SFDisk) Layout() (*gadget.LaidOutVolume, error) { return nil, fmt.Errorf("cannot parse sfdisk output: %v", err) } - pv, err := positionedVolumeFromDump(&dump) + dl, err := deviceLayoutFromDump(&dump) if err != nil { return nil, err } + dl.Device = device - // Hold the partition table for later use when creating partitions - sf.partitionTable = &dump.PartitionTable - - return pv, nil + return dl, nil } -// Create creates the partitions listed in positionedVolume -func (sf *SFDisk) Create(pv *gadget.LaidOutVolume) ([]DeviceStructure, error) { - // Layout() will update sf.partitionTable - if _, err := sf.Layout(); err != nil { - return nil, err - } - buf, created := buildPartitionList(sf.partitionTable, pv) +// CreateMissing creates the partitions listed in the positioned volume pv +// that are missing from the existing device layout. +func (dl *DeviceLayout) CreateMissing(pv *gadget.LaidOutVolume) ([]DeviceStructure, error) { + buf, created := buildPartitionList(dl.partitionTable, pv) // Write the partition table, note that sfdisk will re-read the // partition table by itself: see disk-utils/sfdisk.c:write_changes() - cmd := exec.Command("sfdisk", sf.device) + cmd := exec.Command("sfdisk", dl.Device) cmd.Stdin = buf if output, err := cmd.CombinedOutput(); err != nil { return created, osutil.OutputErr(output, err) } + // Add an extra delay to prevent reads to take place immediately + time.Sleep(250 * time.Millisecond) + return created, nil } -// positionedVolumeFromDump takes an sfdisk dump format and returns the partitioning -// information as a laid out volume. -func positionedVolumeFromDump(dump *sfdiskDeviceDump) (*gadget.LaidOutVolume, error) { +// deviceLayoutFromDump takes an sfdisk dump format and returns the partitioning +// information as a device layout. +func deviceLayoutFromDump(dump *sfdiskDeviceDump) (*DeviceLayout, error) { ptable := dump.PartitionTable if ptable.Unit != "sectors" { @@ -127,7 +125,7 @@ func positionedVolumeFromDump(dump *sfdiskDeviceDump) (*gadget.LaidOutVolume, er } structure := make([]gadget.VolumeStructure, len(ptable.Partitions)) - ps := make([]gadget.LaidOutStructure, len(ptable.Partitions)) + ds := make([]DeviceStructure, len(ptable.Partitions)) for i, p := range ptable.Partitions { info, err := filesystemInfo(p.Node) @@ -149,24 +147,27 @@ func positionedVolumeFromDump(dump *sfdiskDeviceDump) (*gadget.LaidOutVolume, er Type: p.Type, Filesystem: bd.FSType, } - ps[i] = gadget.LaidOutStructure{ - VolumeStructure: &structure[i], - StartOffset: gadget.Size(p.Start) * sectorSize, - Index: i + 1, + ds[i] = DeviceStructure{ + LaidOutStructure: gadget.LaidOutStructure{ + VolumeStructure: &structure[i], + StartOffset: gadget.Size(p.Start) * sectorSize, + Index: i + 1, + }, + Node: p.Node, } } - pv := &gadget.LaidOutVolume{ - Volume: &gadget.Volume{ - ID: ptable.ID, - Structure: structure, - }, - Size: gadget.Size(ptable.LastLBA), - SectorSize: sectorSize, - LaidOutStructure: ps, + dl := &DeviceLayout{ + Structure: ds, + ID: ptable.ID, + Device: ptable.Device, + Schema: ptable.Label, + Size: gadget.Size(ptable.LastLBA), + SectorSize: sectorSize, + partitionTable: &ptable, } - return pv, nil + return dl, nil } func deviceName(name string, index int) string { diff --git a/cmd/snap-bootstrap/partition/sfdisk_test.go b/cmd/snap-bootstrap/partition/sfdisk_test.go index 34220e9858..edc4f62818 100644 --- a/cmd/snap-bootstrap/partition/sfdisk_test.go +++ b/cmd/snap-bootstrap/partition/sfdisk_test.go @@ -117,8 +117,8 @@ func (s *partitionTestSuite) TestDeviceInfo(c *C) { exit 0`) defer cmdLsblk.Restore() - sf := partition.NewSFDisk("/dev/node") - pv, err := sf.Layout() + dl, err := partition.DeviceLayoutFromDisk("/dev/node") + c.Assert(err, IsNil) c.Assert(cmdSfdisk.Calls(), DeepEquals, [][]string{ {"sfdisk", "--json", "-d", "/dev/node"}, }) @@ -127,23 +127,38 @@ exit 0`) {"lsblk", "--fs", "--json", "/dev/node2"}, }) c.Assert(err, IsNil) - c.Assert(pv.Volume.ID, Equals, "9151F25B-CDF0-48F1-9EDE-68CBD616E2CA") - c.Assert(len(pv.Structure), Equals, 2) + c.Assert(dl.ID, Equals, "9151F25B-CDF0-48F1-9EDE-68CBD616E2CA") + c.Assert(dl.Device, Equals, "/dev/node") + c.Assert(len(dl.Structure), Equals, 2) - c.Assert(pv.Structure, DeepEquals, []gadget.VolumeStructure{ + c.Assert(dl.Structure, DeepEquals, []partition.DeviceStructure{ { - Name: "BIOS Boot", - Size: 0x100000, - Label: "", - Type: "21686148-6449-6E6F-744E-656564454649", - Filesystem: "", + LaidOutStructure: gadget.LaidOutStructure{ + VolumeStructure: &gadget.VolumeStructure{ + Name: "BIOS Boot", + Size: 0x100000, + Label: "", + Type: "21686148-6449-6E6F-744E-656564454649", + Filesystem: "", + }, + StartOffset: 0x100000, + Index: 1, + }, + Node: "/dev/node1", }, { - Name: "Recovery", - Size: 0x4b000000, - Label: "ubuntu-seed", - Type: "C12A7328-F81F-11D2-BA4B-00A0C93EC93B", - Filesystem: "vfat", + LaidOutStructure: gadget.LaidOutStructure{ + VolumeStructure: &gadget.VolumeStructure{ + Name: "Recovery", + Size: 0x4b000000, + Label: "ubuntu-seed", + Type: "C12A7328-F81F-11D2-BA4B-00A0C93EC93B", + Filesystem: "vfat", + }, + StartOffset: 0x200000, + Index: 2, + }, + Node: "/dev/node2", }, }) } @@ -164,8 +179,7 @@ func (s *partitionTestSuite) TestDeviceInfoNotSectors(c *C) { }'`) defer cmdSfdisk.Restore() - sf := partition.NewSFDisk("/dev/node") - _, err := sf.Layout() + _, err := partition.DeviceLayoutFromDisk("/dev/node") c.Assert(err, ErrorMatches, "cannot position partitions: unknown unit .*") } @@ -188,8 +202,7 @@ func (s *partitionTestSuite) TestDeviceInfoFilesystemInfoError(c *C) { cmdLsblk := testutil.MockCommand(c, "lsblk", "echo lsblk error; exit 1") defer cmdLsblk.Restore() - sf := partition.NewSFDisk("/dev/node") - _, err := sf.Layout() + _, err := partition.DeviceLayoutFromDisk("/dev/node") c.Assert(err, ErrorMatches, "cannot obtain filesystem information: lsblk error") } @@ -197,20 +210,18 @@ func (s *partitionTestSuite) TestDeviceInfoJsonError(c *C) { cmd := testutil.MockCommand(c, "sfdisk", `echo 'This is not a json'`) defer cmd.Restore() - sf := partition.NewSFDisk("/dev/node") - info, err := sf.Layout() + dl, err := partition.DeviceLayoutFromDisk("/dev/node") c.Assert(err, ErrorMatches, "cannot parse sfdisk output: invalid .*") - c.Assert(info, IsNil) + c.Assert(dl, IsNil) } func (s *partitionTestSuite) TestDeviceInfoError(c *C) { cmd := testutil.MockCommand(c, "sfdisk", "echo 'sfdisk: not found'; exit 127") defer cmd.Restore() - sf := partition.NewSFDisk("/dev/node") - info, err := sf.Layout() + dl, err := partition.DeviceLayoutFromDisk("/dev/node") c.Assert(err, ErrorMatches, "sfdisk: not found") - c.Assert(info, IsNil) + c.Assert(dl, IsNil) } func (s *partitionTestSuite) TestBuildPartitionList(c *C) { @@ -273,8 +284,9 @@ func (s *partitionTestSuite) TestCreatePartitions(c *C) { pv, err := gadget.PositionedVolumeFromGadget(gadgetRoot) c.Assert(err, IsNil) - sf := partition.NewSFDisk("/dev/node") - created, err := sf.Create(pv) + dl, err := partition.DeviceLayoutFromDisk("/dev/node") + c.Assert(err, IsNil) + created, err := dl.CreateMissing(pv) c.Assert(err, IsNil) c.Assert(created, DeepEquals, []partition.DeviceStructure{ mockDeviceStructureSystemSeed, diff --git a/cmd/snap-confine/snap-confine.c b/cmd/snap-confine/snap-confine.c index 3bc27532f6..83db3ee6a7 100644 --- a/cmd/snap-confine/snap-confine.c +++ b/cmd/snap-confine/snap-confine.c @@ -291,13 +291,18 @@ static void sc_cleanup_preserved_process_state(sc_preserved_process_state * sc_cleanup_close(&proc_state->orig_cwd_fd); } -static void enter_classic_execution_environment(const sc_invocation * inv); +static void enter_classic_execution_environment(const sc_invocation * inv, + gid_t real_gid, + gid_t saved_gid); static void enter_non_classic_execution_environment(sc_invocation * inv, struct sc_apparmor *aa, uid_t real_uid, gid_t real_gid, gid_t saved_gid); +static void maybe_join_tracking_cgroup(const sc_invocation * inv, + gid_t real_gid, gid_t saved_gid); + int main(int argc, char **argv) { // Use our super-defensive parser to figure out what we've been asked to do. @@ -421,13 +426,15 @@ int main(int argc, char **argv) } if (invocation.classic_confinement) { - enter_classic_execution_environment(&invocation); + enter_classic_execution_environment(&invocation, real_gid, + saved_gid); } else { enter_non_classic_execution_environment(&invocation, &apparmor, real_uid, real_gid, saved_gid); } + // Temporarily drop privs back to calling user (we'll permanently drop // after loading seccomp). if (setegid(real_gid) != 0) @@ -543,7 +550,8 @@ int main(int argc, char **argv) return 1; } -static void enter_classic_execution_environment(const sc_invocation * inv) +static void enter_classic_execution_environment(const sc_invocation * inv, + gid_t real_gid, gid_t saved_gid) { /* with parallel-instances enabled, main() reassociated with the mount ns of * PID 1 to make /run/snapd/ns visible */ @@ -555,8 +563,12 @@ static void enter_classic_execution_environment(const sc_invocation * inv) * - snapd sets up a lenient AppArmor profile for snap-confine to use * - snapd sets up a lenient seccomp profile for snap-confine to use */ + debug("preparing classic execution environment"); - debug("skipping sandbox setup, classic confinement in use"); + /* Join a tracking cgroup if appropriate feature is enabled. */ + int snap_lock_fd = sc_lock_snap(inv->snap_instance); + maybe_join_tracking_cgroup(inv, real_gid, saved_gid); + sc_unlock(snap_lock_fd); if (!sc_feature_enabled(SC_FEATURE_PARALLEL_INSTANCES)) { return; @@ -733,9 +745,6 @@ static void enter_non_classic_execution_environment(sc_invocation * inv, } if (!sc_cgroup_is_v2()) { sc_cgroup_freezer_join(inv->snap_instance, getpid()); - if (sc_feature_enabled(SC_FEATURE_REFRESH_APP_AWARENESS)) { - sc_cgroup_pids_join(inv->security_tag, getpid()); - } } if (geteuid() == 0 && real_gid != 0) { if (setegid(real_gid) != 0) { @@ -743,6 +752,8 @@ static void enter_non_classic_execution_environment(sc_invocation * inv, } } + maybe_join_tracking_cgroup(inv, real_gid, saved_gid); + sc_unlock(snap_lock_fd); sc_close_mount_ns(group); @@ -771,3 +782,24 @@ static void enter_non_classic_execution_environment(sc_invocation * inv, } } } + +static void maybe_join_tracking_cgroup(const sc_invocation * inv, + gid_t real_gid, gid_t saved_gid) +{ + if (getegid() != 0 && saved_gid == 0) { + /* Temporarily raise egid so we can chown the cgroup under LXD. */ + if (setegid(0) != 0) { + die("cannot set effective group id to root"); + } + } + if (!sc_cgroup_is_v2()) { + if (sc_feature_enabled(SC_FEATURE_REFRESH_APP_AWARENESS)) { + sc_cgroup_pids_join(inv->security_tag, getpid()); + } + } + if (geteuid() == 0 && real_gid != 0) { + if (setegid(real_gid) != 0) { + die("cannot set effective group id to %d", real_gid); + } + } +} diff --git a/cmd/snap/cmd_snap_op.go b/cmd/snap/cmd_snap_op.go index b3f332caa1..19d4d063ce 100644 --- a/cmd/snap/cmd_snap_op.go +++ b/cmd/snap/cmd_snap_op.go @@ -252,10 +252,25 @@ func (mx *channelMixin) setChannelFromCommandline() error { mx.Channel = ch.chName } - if !strings.Contains(mx.Channel, "/") && mx.Channel != "" && mx.Channel != "edge" && mx.Channel != "beta" && mx.Channel != "candidate" && mx.Channel != "stable" { - // shortcut to jump to a different track, e.g. - // snap install foo --channel=3.4 # implies 3.4/stable - mx.Channel += "/stable" + if mx.Channel != "" { + if ch, err := channel.Parse(mx.Channel, ""); err != nil { + full, er := channel.Full(mx.Channel) + if er != nil { + // the parse error has more detailed info + return err + } + + // TODO: get escapes in here so we can bold the Warning + head := i18n.G("Warning:") + msg := i18n.G("Specifying a channel %q is relying on undefined behaviour. Interpreting it as %q for now, but this will be an error later.\n") + warn := fill(fmt.Sprintf(msg, mx.Channel, full), utf8.RuneCountInString(head)+1) // +1 for the space + fmt.Fprint(Stderr, head, " ", warn, "\n\n") + mx.Channel = full // so a malformed-but-eh channel will always be full, i.e. //stable// -> latest/stable + } else { + // because snap.ParseChannel calls Clean() on the channel, + // the name will be the short form name that we want + mx.Channel = ch.Name + } } return nil diff --git a/cmd/snap/cmd_snap_op_test.go b/cmd/snap/cmd_snap_op_test.go index db9fd67546..4fc61ebbeb 100644 --- a/cmd/snap/cmd_snap_op_test.go +++ b/cmd/snap/cmd_snap_op_test.go @@ -276,7 +276,7 @@ func (s *SnapOpSuite) TestInstallSameRiskInTrack(c *check.C) { c.Check(r.URL.Path, check.Equals, "/v2/snaps/foo") c.Check(DecodedRequestBody(c, r), check.DeepEquals, map[string]interface{}{ "action": "install", - "channel": "latest/stable", + "channel": "stable", }) s.srv.channel = "stable" s.srv.trackingChannel = "latest/stable" @@ -662,21 +662,11 @@ error: snap "foo" is not available on this architecture (arm64) but exists on func (s *SnapOpSuite) TestInstallSnapRevisionNotAvailableInvalidChannel(c *check.C) { s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, `{"type": "error", "result": {"message": "no snap revision on specified channel", "value": { - "snap-name": "foo", - "action": "install", - "architecture": "amd64", - "channel": "a/b/c/d", - "releases": [{"architecture": "amd64", "channel": "stable"}] -}, "kind": "snap-channel-not-available"}, "status-code": 404}`) + c.Fatal("unexpected call to server") }) _, err := snap.Parser(snap.Client()).ParseArgs([]string{"install", "--channel=a/b/c/d", "foo"}) - c.Assert(err, check.NotNil) - c.Check(fmt.Sprintf("\nerror: %v\n", err), check.Equals, ` -error: requested channel "a/b/c/d" is not valid (see 'snap info foo' for valid - ones) -`) + c.Assert(err, check.ErrorMatches, "channel name has too many components: a/b/c/d") c.Check(s.Stdout(), check.Equals, "") c.Check(s.Stderr(), check.Equals, "") @@ -1301,6 +1291,25 @@ func (s *SnapOpSuite) TestRefreshOneRebooting(c *check.C) { } +func (s *SnapOpSuite) TestRefreshOneChanDeprecated(c *check.C) { + var in, out string + s.RedirectClientToTestServer(func(w http.ResponseWriter, r *http.Request) { + c.Check(DecodedRequestBody(c, r), check.DeepEquals, map[string]interface{}{"action": "refresh", "channel": out}) + fmt.Fprintln(w, `{"type": "error", "result": {"message": "snap not found", "value": "foo", "kind": "snap-not-found"}, "status-code": 404}`) + }) + + for in, out = range map[string]string{ + "/foo": "foo/stable", + "/stable": "latest/stable", + "///foo/stable//": "foo/stable", + } { + s.stderr.Reset() + _, err := snap.Parser(snap.Client()).ParseArgs([]string{"refresh", "--channel=" + in, "one"}) + c.Assert(err, check.ErrorMatches, "snap \"one\" not found") + c.Check(s.Stderr(), testutil.EqualsWrapped, `Warning: Specifying a channel "`+in+`" is relying on undefined behaviour. Interpreting it as "`+out+`" for now, but this will be an error later.`) + } +} + func (s *SnapOpSuite) TestRefreshOneModeErr(c *check.C) { s.RedirectClientToTestServer(nil) _, err := snap.Parser(snap.Client()).ParseArgs([]string{"refresh", "--jailmode", "--devmode", "one"}) diff --git a/cmd/snap/error.go b/cmd/snap/error.go index 0fe2ac2595..363e18230b 100644 --- a/cmd/snap/error.go +++ b/cmd/snap/error.go @@ -244,6 +244,7 @@ func snapRevisionNotAvailableMessage(kind, snapName, action, arch, snapChannel s // as reported by the store through the daemon req, err := channel.Parse(snapChannel, arch) if err != nil { + // XXX: this is no longer possible (should be caught before hitting the store), unless the state itself has an invalid channel // TRANSLATORS: %q is the invalid request channel, %s is the snap name msg := fmt.Sprintf(i18n.G("requested channel %q is not valid (see 'snap info %s' for valid ones)"), snapChannel, snapName) return msg diff --git a/daemon/api.go b/daemon/api.go index e98e7bf74a..6bf48b33ad 100644 --- a/daemon/api.go +++ b/daemon/api.go @@ -68,6 +68,7 @@ import ( "github.com/snapcore/snapd/progress" "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/channel" "github.com/snapcore/snapd/store" "github.com/snapcore/snapd/strutil" "github.com/snapcore/snapd/systemd" @@ -753,6 +754,26 @@ type snapRevisionOptions struct { LeaveCohort bool `json:"leave-cohort"` } +func (ropt *snapRevisionOptions) validate() error { + if ropt.CohortKey != "" { + if ropt.LeaveCohort { + return fmt.Errorf("cannot specify both cohort-key and leave-cohort") + } + if !ropt.Revision.Unset() { + return fmt.Errorf("cannot specify both cohort-key and revision") + } + } + + if ropt.Channel != "" { + ch, err := channel.Parse(ropt.Channel, "-") + if err != nil { + return err + } + ropt.Channel = ch.Name + } + return nil +} + type snapInstruction struct { progress.NullMeter @@ -801,6 +822,30 @@ func (inst *snapInstruction) installFlags() (snapstate.Flags, error) { return flags, nil } +func (inst *snapInstruction) validate() error { + if inst.CohortKey != "" { + if inst.Action != "install" && inst.Action != "refresh" && inst.Action != "switch" { + return fmt.Errorf("cohort-key can only be specified for install, refresh, or switch") + } + } + if inst.LeaveCohort { + if inst.Action != "refresh" && inst.Action != "switch" { + return fmt.Errorf("leave-cohort can only be specified for refresh or switch") + } + } + if inst.Action == "install" { + for _, snapName := range inst.Snaps { + // FIXME: alternatively we could simply mutate *inst + // and s/ubuntu-core/core/ ? + if snapName == "ubuntu-core" { + return fmt.Errorf(`cannot install "ubuntu-core", please use "core" instead`) + } + } + } + + return inst.snapRevisionOptions.validate() +} + type snapInstructionResult struct { Summary string Affected []string @@ -896,37 +941,6 @@ func snapUpdateMany(inst *snapInstruction, st *state.State) (*snapInstructionRes }, nil } -func verifySnapInstructions(inst *snapInstruction) error { - if inst.CohortKey != "" { - if inst.LeaveCohort { - return fmt.Errorf("cannot specify both cohort-key and leave-cohort") - } - if !inst.Revision.Unset() { - return fmt.Errorf("cannot specify both cohort-key and revision") - } - if inst.Action != "install" && inst.Action != "refresh" && inst.Action != "switch" { - return fmt.Errorf("cohort-key can only be specified for install, refresh, or switch") - } - } - if inst.LeaveCohort { - if inst.Action != "refresh" && inst.Action != "switch" { - return fmt.Errorf("leave-cohort can only be specified for refresh or switch") - } - } - switch inst.Action { - case "install": - for _, snapName := range inst.Snaps { - // FIXME: alternatively we could simply mutate *inst - // and s/ubuntu-core/core/ ? - if snapName == "ubuntu-core" { - return fmt.Errorf(`cannot install "ubuntu-core", please use "core" instead`) - } - } - } - - return nil -} - func snapInstallMany(inst *snapInstruction, st *state.State) (*snapInstructionResult, error) { for _, name := range inst.Snaps { if len(name) == 0 { @@ -1200,7 +1214,7 @@ func postSnap(c *Command, r *http.Request, user *auth.UserState) Response { vars := muxVars(r) inst.Snaps = []string{vars["name"]} - if err := verifySnapInstructions(&inst); err != nil { + if err := inst.validate(); err != nil { return BadRequest("%s", err) } @@ -1305,7 +1319,7 @@ func snapsOp(c *Command, r *http.Request, user *auth.UserState) Response { if inst.Channel != "" || !inst.Revision.Unset() || inst.DevMode || inst.JailMode || inst.CohortKey != "" || inst.LeaveCohort { return BadRequest("unsupported option provided for multi-snap operation") } - if err := verifySnapInstructions(&inst); err != nil { + if err := inst.validate(); err != nil { return BadRequest("%v", err) } diff --git a/daemon/api_download.go b/daemon/api_download.go index c4602873dc..e3c14ab9bf 100644 --- a/daemon/api_download.go +++ b/daemon/api_download.go @@ -22,6 +22,7 @@ package daemon import ( "context" "encoding/json" + "errors" "net/http" "path/filepath" @@ -41,6 +42,15 @@ type snapDownloadAction struct { snapRevisionOptions } +var errDownloadNameRequired = errors.New("download operation requires one snap name") + +func (action *snapDownloadAction) validate() error { + if action.SnapName == "" { + return errDownloadNameRequired + } + return action.snapRevisionOptions.validate() +} + func postSnapDownload(c *Command, r *http.Request, user *auth.UserState) Response { var action snapDownloadAction decoder := json.NewDecoder(r.Body) @@ -51,8 +61,8 @@ func postSnapDownload(c *Command, r *http.Request, user *auth.UserState) Respons return BadRequest("extra content found after download operation") } - if action.SnapName == "" { - return BadRequest("download operation requires one snap name") + if err := action.validate(); err != nil { + return BadRequest(err.Error()) } return streamOneSnap(c, action, user) diff --git a/daemon/api_download_test.go b/daemon/api_download_test.go index fdd122f227..c8d3c28289 100644 --- a/daemon/api_download_test.go +++ b/daemon/api_download_test.go @@ -162,6 +162,11 @@ func (s *snapDownloadSuite) TestDownloadSnapErrors(c *check.C) { status: 400, err: `cannot decode request body into download operation: unexpected EOF`, }, + { + dataJSON: `{"snap-name": "doom","channel":"latest/potato"}`, + status: 400, + err: `invalid risk in channel name: latest/potato`, + }, } { var err error data := []byte(scen.dataJSON) diff --git a/daemon/api_test.go b/daemon/api_test.go index 8aad781c04..637b89440d 100644 --- a/daemon/api_test.go +++ b/daemon/api_test.go @@ -2428,7 +2428,79 @@ func (s *apiSuite) TestPostSnapBadAction(c *check.C) { c.Check(rsp.Result, check.NotNil) } +func (s *apiSuite) TestPostSnapBadChannel(c *check.C) { + buf := bytes.NewBufferString(`{"channel": "1/2/3/4"}`) + req, err := http.NewRequest("POST", "/v2/snaps/hello-world", buf) + c.Assert(err, check.IsNil) + + rsp := postSnap(snapCmd, req, nil).(*resp) + + c.Check(rsp.Type, check.Equals, ResponseTypeError) + c.Check(rsp.Status, check.Equals, 400) + c.Check(rsp.Result, check.NotNil) +} + func (s *apiSuite) TestPostSnap(c *check.C) { + s.testPostSnap(c, false) +} + +func (s *apiSuite) TestPostSnapWithChannel(c *check.C) { + s.testPostSnap(c, true) +} + +func (s *apiSuite) testPostSnap(c *check.C, withChannel bool) { + d := s.daemonWithOverlordMock(c) + + soon := 0 + ensureStateSoon = func(st *state.State) { + soon++ + ensureStateSoonImpl(st) + } + + s.vars = map[string]string{"name": "foo"} + + snapInstructionDispTable["install"] = func(inst *snapInstruction, _ *state.State) (string, []*state.TaskSet, error) { + if withChannel { + // channel in -> it was parsed + c.Check(inst.Channel, check.Equals, "xyzzy/stable") + } else { + // no channel in -> no channel out + c.Check(inst.Channel, check.Equals, "") + } + return "foooo", nil, nil + } + defer func() { + snapInstructionDispTable["install"] = snapInstall + }() + + var buf *bytes.Buffer + if withChannel { + buf = bytes.NewBufferString(`{"action": "install", "channel": "xyzzy"}`) + } else { + buf = bytes.NewBufferString(`{"action": "install"}`) + } + req, err := http.NewRequest("POST", "/v2/snaps/hello-world", buf) + c.Assert(err, check.IsNil) + + rsp := postSnap(snapCmd, req, nil).(*resp) + + c.Check(rsp.Type, check.Equals, ResponseTypeAsync) + + st := d.overlord.State() + st.Lock() + defer st.Unlock() + chg := st.Change(rsp.Change) + c.Assert(chg, check.NotNil) + c.Check(chg.Summary(), check.Equals, "foooo") + var names []string + err = chg.Get("snap-names", &names) + c.Assert(err, check.IsNil) + c.Check(names, check.DeepEquals, []string{"foo"}) + + c.Check(soon, check.Equals, 1) +} + +func (s *apiSuite) TestPostSnapChannel(c *check.C) { d := s.daemonWithOverlordMock(c) soon := 0 diff --git a/features/features.go b/features/features.go index 29f4244494..08b081422b 100644 --- a/features/features.go +++ b/features/features.go @@ -43,6 +43,8 @@ const ( PerUserMountNamespace // RefreshAppAwareness controls refresh being aware of running applications. RefreshAppAwareness + // ClassicPreservesXdgRuntimeDir controls $XDG_RUNTIME_DIR in snaps with classic confinement. + ClassicPreservesXdgRuntimeDir // lastFeature is the final known feature, it is only used for testing. lastFeature ) @@ -65,6 +67,8 @@ var featureNames = map[SnapdFeature]string{ SnapdSnap: "snapd-snap", PerUserMountNamespace: "per-user-mount-namespace", RefreshAppAwareness: "refresh-app-awareness", + + ClassicPreservesXdgRuntimeDir: "classic-preserves-xdg-runtime-dir", } // featuresEnabledWhenUnset contains a set of features that are enabled when not explicitly configured. @@ -77,6 +81,8 @@ var featuresExported = map[SnapdFeature]bool{ PerUserMountNamespace: true, RefreshAppAwareness: true, ParallelInstances: true, + + ClassicPreservesXdgRuntimeDir: true, } // String returns the name of a snapd feature. diff --git a/features/features_test.go b/features/features_test.go index ae2eecadd6..9188e21b7f 100644 --- a/features/features_test.go +++ b/features/features_test.go @@ -43,6 +43,7 @@ func (*featureSuite) TestName(c *C) { c.Check(features.SnapdSnap.String(), Equals, "snapd-snap") c.Check(features.PerUserMountNamespace.String(), Equals, "per-user-mount-namespace") c.Check(features.RefreshAppAwareness.String(), Equals, "refresh-app-awareness") + c.Check(features.ClassicPreservesXdgRuntimeDir.String(), Equals, "classic-preserves-xdg-runtime-dir") c.Check(func() { _ = features.SnapdFeature(1000).String() }, PanicMatches, "unknown feature flag code 1000") } @@ -63,6 +64,7 @@ func (*featureSuite) TestIsExported(c *C) { c.Check(features.ParallelInstances.IsExported(), Equals, true) c.Check(features.PerUserMountNamespace.IsExported(), Equals, true) c.Check(features.RefreshAppAwareness.IsExported(), Equals, true) + c.Check(features.ClassicPreservesXdgRuntimeDir.IsExported(), Equals, true) } func (*featureSuite) TestIsEnabled(c *C) { @@ -91,6 +93,7 @@ func (*featureSuite) TestIsEnabledWhenUnset(c *C) { c.Check(features.SnapdSnap.IsEnabledWhenUnset(), Equals, false) c.Check(features.PerUserMountNamespace.IsEnabledWhenUnset(), Equals, false) c.Check(features.RefreshAppAwareness.IsEnabledWhenUnset(), Equals, false) + c.Check(features.ClassicPreservesXdgRuntimeDir.IsEnabledWhenUnset(), Equals, false) } func (*featureSuite) TestControlFile(c *C) { diff --git a/interfaces/policy/basedeclaration_test.go b/interfaces/policy/basedeclaration_test.go index 3df856ae59..04740d77c7 100644 --- a/interfaces/policy/basedeclaration_test.go +++ b/interfaces/policy/basedeclaration_test.go @@ -183,9 +183,10 @@ func (s *baseDeclSuite) TestAutoConnection(c *C) { // check base declaration cand := s.connectCand(c, iface.Name(), "", "") - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() if expected { c.Check(err, IsNil, comm) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } else { c.Check(err, NotNil, comm) } @@ -216,11 +217,12 @@ func (s *baseDeclSuite) TestInterimAutoConnectionHome(c *C) { restore := release.MockOnClassic(true) defer restore() cand := s.connectCand(c, "home", "", "") - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) release.OnClassic = false - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, ErrorMatches, `auto-connection denied by slot rule of interface \"home\"`) } @@ -237,14 +239,14 @@ plugs: err := cand.Check() c.Check(err, NotNil) - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, NotNil) release.OnClassic = false err = cand.Check() c.Check(err, NotNil) - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, NotNil) } @@ -261,21 +263,22 @@ plugs: c.Check(err, IsNil) // Same as TestInterimAutoConnectionHome() - err = cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) release.OnClassic = false err = cand.Check() c.Check(err, IsNil) // Same as TestInterimAutoConnectionHome() - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, NotNil) } func (s *baseDeclSuite) TestAutoConnectionSnapdControl(c *C) { cand := s.connectCand(c, "snapd-control", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"snapd-control\"") @@ -287,15 +290,16 @@ plugs: lxdDecl := s.mockSnapDecl(c, "some-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = lxdDecl - err = cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } func (s *baseDeclSuite) TestAutoConnectionContent(c *C) { // random snaps cannot connect with content // (Sanitize* will now also block this) cand := s.connectCand(c, "content", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) slotDecl1 := s.mockSnapDecl(c, "slot-snap", "slot-snap-id", "pub1", "") @@ -320,13 +324,14 @@ plugs: `) cand.SlotSnapDeclaration = slotDecl1 cand.PlugSnapDeclaration = plugDecl1 - err = cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) // different publisher, same content cand.SlotSnapDeclaration = slotDecl1 cand.PlugSnapDeclaration = plugDecl2 - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, NotNil) // same publisher, different content @@ -346,14 +351,14 @@ plugs: `) cand.SlotSnapDeclaration = slotDecl1 cand.PlugSnapDeclaration = plugDecl1 - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, NotNil) } func (s *baseDeclSuite) TestAutoConnectionLxdSupportOverride(c *C) { // by default, don't auto-connect cand := s.connectCand(c, "lxd-support", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) plugsSlots := ` @@ -364,7 +369,7 @@ plugs: lxdDecl := s.mockSnapDecl(c, "lxd", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = lxdDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } @@ -378,14 +383,14 @@ plugs: lxdDecl := s.mockSnapDecl(c, "notlxd", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = lxdDecl - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection not allowed by plug rule of interface \"lxd-support\" for \"notlxd\" snap") } func (s *baseDeclSuite) TestAutoConnectionKernelModuleControlOverride(c *C) { cand := s.connectCand(c, "kernel-module-control", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"kernel-module-control\"") @@ -397,13 +402,13 @@ plugs: snapDecl := s.mockSnapDecl(c, "some-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } func (s *baseDeclSuite) TestAutoConnectionDockerSupportOverride(c *C) { cand := s.connectCand(c, "docker-support", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"docker-support\"") @@ -415,13 +420,13 @@ plugs: snapDecl := s.mockSnapDecl(c, "some-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } func (s *baseDeclSuite) TestAutoConnectionClassicSupportOverride(c *C) { cand := s.connectCand(c, "classic-support", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"classic-support\"") @@ -433,13 +438,13 @@ plugs: snapDecl := s.mockSnapDecl(c, "classic", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } func (s *baseDeclSuite) TestAutoConnectionKubernetesSupportOverride(c *C) { cand := s.connectCand(c, "kubernetes-support", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"kubernetes-support\"") @@ -451,13 +456,13 @@ plugs: snapDecl := s.mockSnapDecl(c, "some-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } func (s *baseDeclSuite) TestAutoConnectionGreengrassSupportOverride(c *C) { cand := s.connectCand(c, "greengrass-support", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"greengrass-support\"") @@ -469,13 +474,13 @@ plugs: snapDecl := s.mockSnapDecl(c, "some-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } func (s *baseDeclSuite) TestAutoConnectionMultipassSupportOverride(c *C) { cand := s.connectCand(c, "multipass-support", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"multipass-support\"") @@ -487,13 +492,13 @@ plugs: snapDecl := s.mockSnapDecl(c, "multipass-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } func (s *baseDeclSuite) TestAutoConnectionBlockDevicesOverride(c *C) { cand := s.connectCand(c, "block-devices", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"block-devices\"") @@ -505,13 +510,13 @@ plugs: snapDecl := s.mockSnapDecl(c, "some-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } func (s *baseDeclSuite) TestAutoConnectionPackagekitControlOverride(c *C) { cand := s.connectCand(c, "packagekit-control", "", "") - err := cand.CheckAutoConnect() + _, err := cand.CheckAutoConnect() c.Check(err, NotNil) c.Assert(err, ErrorMatches, "auto-connection denied by plug rule of interface \"packagekit-control\"") @@ -523,7 +528,7 @@ plugs: snapDecl := s.mockSnapDecl(c, "some-snap", "J60k4JY0HppjwOjW8dZdYc8obXKxujRu", "canonical", plugsSlots) cand.PlugSnapDeclaration = snapDecl - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, IsNil) } @@ -560,8 +565,9 @@ plugs: cand := s.connectCand(c, iface.Name(), "", "") cand.PlugSnapDeclaration = snapDecl - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } } @@ -945,7 +951,7 @@ plugs: err := cand.Check() c.Check(err, NotNil) - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, NotNil) } @@ -992,7 +998,7 @@ plugs: cand := s.connectCand(c, "optical-drive", "", plugYaml) err := cand.Check() c.Check(err, checker) - err = cand.CheckAutoConnect() + _, err = cand.CheckAutoConnect() c.Check(err, checker) } diff --git a/interfaces/policy/helpers.go b/interfaces/policy/helpers.go index f8a18ced8a..3ddd09b286 100644 --- a/interfaces/policy/helpers.go +++ b/interfaces/policy/helpers.go @@ -152,19 +152,19 @@ func checkPlugConnectionConstraints1(connc *ConnectCandidate, constraints *asser return nil } -func checkPlugConnectionAltConstraints(connc *ConnectCandidate, altConstraints []*asserts.PlugConnectionConstraints) error { +func checkPlugConnectionAltConstraints(connc *ConnectCandidate, altConstraints []*asserts.PlugConnectionConstraints) (*asserts.PlugConnectionConstraints, error) { var firstErr error // OR of constraints for _, constraints := range altConstraints { err := checkPlugConnectionConstraints1(connc, constraints) if err == nil { - return nil + return constraints, nil } if firstErr == nil { firstErr = err } } - return firstErr + return nil, firstErr } func checkSlotConnectionConstraints1(connc *ConnectCandidate, constraints *asserts.SlotConnectionConstraints) error { @@ -195,19 +195,19 @@ func checkSlotConnectionConstraints1(connc *ConnectCandidate, constraints *asser return nil } -func checkSlotConnectionAltConstraints(connc *ConnectCandidate, altConstraints []*asserts.SlotConnectionConstraints) error { +func checkSlotConnectionAltConstraints(connc *ConnectCandidate, altConstraints []*asserts.SlotConnectionConstraints) (*asserts.SlotConnectionConstraints, error) { var firstErr error // OR of constraints for _, constraints := range altConstraints { err := checkSlotConnectionConstraints1(connc, constraints) if err == nil { - return nil + return constraints, nil } if firstErr == nil { firstErr = err } } - return firstErr + return nil, firstErr } func checkSnapTypeSlotInstallationConstraints1(ic *InstallCandidateMinimalCheck, slot *snap.SlotInfo, constraints *asserts.SlotInstallationConstraints) error { @@ -303,3 +303,20 @@ func checkPlugInstallationAltConstraints(ic *InstallCandidate, plug *snap.PlugIn } return firstErr } + +// sideArity carries relevant arity constraints for successful +// allow-auto-connection rules. It implements policy.SideArity. +// ATM only slots-per-plug might have an interesting non-default +// value. +// See: https://forum.snapcraft.io/t/plug-slot-declaration-rules-greedy-plugs/12438 +type sideArity struct { + slotsPerPlug asserts.SideArityConstraint +} + +func (a sideArity) SlotsPerPlugOne() bool { + return a.slotsPerPlug.N == 1 +} + +func (a sideArity) SlotsPerPlugAny() bool { + return a.slotsPerPlug.Any() +} diff --git a/interfaces/policy/policy.go b/interfaces/policy/policy.go index d546770fdf..9e2f157f41 100644 --- a/interfaces/policy/policy.go +++ b/interfaces/policy/policy.go @@ -176,7 +176,7 @@ func (connc *ConnectCandidate) slotPublisherID() string { return "" // never a valid publisher-id } -func (connc *ConnectCandidate) checkPlugRule(kind string, rule *asserts.PlugRule, snapRule bool) error { +func (connc *ConnectCandidate) checkPlugRule(kind string, rule *asserts.PlugRule, snapRule bool) (interfaces.SideArity, error) { context := "" if snapRule { context = fmt.Sprintf(" for %q snap", connc.PlugSnapDeclaration.SnapName()) @@ -187,16 +187,18 @@ func (connc *ConnectCandidate) checkPlugRule(kind string, rule *asserts.PlugRule denyConst = rule.DenyAutoConnection allowConst = rule.AllowAutoConnection } - if checkPlugConnectionAltConstraints(connc, denyConst) == nil { - return fmt.Errorf("%s denied by plug rule of interface %q%s", kind, connc.Plug.Interface(), context) + if _, err := checkPlugConnectionAltConstraints(connc, denyConst); err == nil { + return nil, fmt.Errorf("%s denied by plug rule of interface %q%s", kind, connc.Plug.Interface(), context) } - if checkPlugConnectionAltConstraints(connc, allowConst) != nil { - return fmt.Errorf("%s not allowed by plug rule of interface %q%s", kind, connc.Plug.Interface(), context) + + allowedConstraints, err := checkPlugConnectionAltConstraints(connc, allowConst) + if err != nil { + return nil, fmt.Errorf("%s not allowed by plug rule of interface %q%s", kind, connc.Plug.Interface(), context) } - return nil + return sideArity{allowedConstraints.SlotsPerPlug}, nil } -func (connc *ConnectCandidate) checkSlotRule(kind string, rule *asserts.SlotRule, snapRule bool) error { +func (connc *ConnectCandidate) checkSlotRule(kind string, rule *asserts.SlotRule, snapRule bool) (interfaces.SideArity, error) { context := "" if snapRule { context = fmt.Sprintf(" for %q snap", connc.SlotSnapDeclaration.SnapName()) @@ -207,25 +209,27 @@ func (connc *ConnectCandidate) checkSlotRule(kind string, rule *asserts.SlotRule denyConst = rule.DenyAutoConnection allowConst = rule.AllowAutoConnection } - if checkSlotConnectionAltConstraints(connc, denyConst) == nil { - return fmt.Errorf("%s denied by slot rule of interface %q%s", kind, connc.Plug.Interface(), context) + if _, err := checkSlotConnectionAltConstraints(connc, denyConst); err == nil { + return nil, fmt.Errorf("%s denied by slot rule of interface %q%s", kind, connc.Plug.Interface(), context) } - if checkSlotConnectionAltConstraints(connc, allowConst) != nil { - return fmt.Errorf("%s not allowed by slot rule of interface %q%s", kind, connc.Plug.Interface(), context) + + allowedConstraints, err := checkSlotConnectionAltConstraints(connc, allowConst) + if err != nil { + return nil, fmt.Errorf("%s not allowed by slot rule of interface %q%s", kind, connc.Plug.Interface(), context) } - return nil + return sideArity{allowedConstraints.SlotsPerPlug}, nil } -func (connc *ConnectCandidate) check(kind string) error { +func (connc *ConnectCandidate) check(kind string) (interfaces.SideArity, error) { baseDecl := connc.BaseDeclaration if baseDecl == nil { - return fmt.Errorf("internal error: improperly initialized ConnectCandidate") + return nil, fmt.Errorf("internal error: improperly initialized ConnectCandidate") } iface := connc.Plug.Interface() if connc.Slot.Interface() != iface { - return fmt.Errorf("cannot connect mismatched plug interface %q to slot interface %q", iface, connc.Slot.Interface()) + return nil, fmt.Errorf("cannot connect mismatched plug interface %q to slot interface %q", iface, connc.Slot.Interface()) } if plugDecl := connc.PlugSnapDeclaration; plugDecl != nil { @@ -244,17 +248,27 @@ func (connc *ConnectCandidate) check(kind string) error { if rule := baseDecl.SlotRule(iface); rule != nil { return connc.checkSlotRule(kind, rule, false) } - return nil + return nil, nil } // Check checks whether the connection is allowed. func (connc *ConnectCandidate) Check() error { - return connc.check("connection") + _, err := connc.check("connection") + return err } // CheckAutoConnect checks whether the connection is allowed to auto-connect. -func (connc *ConnectCandidate) CheckAutoConnect() error { - return connc.check("auto-connection") +func (connc *ConnectCandidate) CheckAutoConnect() (interfaces.SideArity, error) { + arity, err := connc.check("auto-connection") + if err != nil { + return nil, err + } + if arity == nil { + // shouldn't happen but be safe, the callers should be able + // to assume arity to be non nil + arity = sideArity{asserts.SideArityConstraint{N: 1}} + } + return arity, nil } // InstallCandidateMinimalCheck represents a candidate snap installed with --dangerous flag that should pass minimum checks diff --git a/interfaces/policy/policy_test.go b/interfaces/policy/policy_test.go index 6b1ee984b1..3e189855d4 100644 --- a/interfaces/policy/policy_test.go +++ b/interfaces/policy/policy_test.go @@ -298,6 +298,20 @@ slots: - debian install-slot-device-scope: allow-installation: false + slots-arity-default: + allow-auto-connection: true + slots-arity-slot-any: + deny-auto-connection: true + slots-arity-plug-any: + deny-auto-connection: true + slots-arity-slot-any-plug-one: + deny-auto-connection: true + slots-arity-slot-any-plug-two: + deny-auto-connection: true + slots-arity-slot-any-plug-default: + deny-auto-connection: true + slots-arity-slot-one-plug-any: + deny-auto-connection: true timestamp: 2016-09-30T12:00:00Z sign-key-sha3-384: Jv8_JiHiIzJVcO9M55pPdqSDWUvuhfDIBJUS-3VW7F_idjix7Ffn5qMxB21ZQuij @@ -477,6 +491,14 @@ plugs: plug-on-classic-true: plug-on-classic-distros: plug-on-classic-false: + + slots-arity-default: + slots-arity-slot-any: + slots-arity-plug-any: + slots-arity-slot-any-plug-one: + slots-arity-slot-any-plug-two: + slots-arity-slot-any-plug-default: + slots-arity-slot-one-plug-any: `, nil) s.slotSnap = snaptest.MockInfo(c, ` @@ -642,6 +664,14 @@ slots: plug-on-classic-true: plug-on-classic-distros: plug-on-classic-false: + + slots-arity-default: + slots-arity-slot-any: + slots-arity-plug-any: + slots-arity-slot-any-plug-one: + slots-arity-slot-any-plug-two: + slots-arity-slot-any-plug-default: + slots-arity-slot-one-plug-any: `, nil) a, err = asserts.Decode([]byte(`type: snap-declaration @@ -703,6 +733,20 @@ plugs: on-model: - my-brand/my-model1 - my-brand-subbrand/my-model2 + slots-arity-plug-any: + allow-auto-connection: + slots-per-plug: * + slots-arity-slot-any-plug-one: + allow-auto-connection: + slots-per-plug: 1 + slots-arity-slot-any-plug-two: + allow-auto-connection: + slots-per-plug: 2 + slots-arity-slot-any-plug-default: + allow-auto-connection: true + slots-arity-slot-one-plug-any: + allow-auto-connection: + slots-per-plug: * timestamp: 2016-09-30T12:00:00Z sign-key-sha3-384: Jv8_JiHiIzJVcO9M55pPdqSDWUvuhfDIBJUS-3VW7F_idjix7Ffn5qMxB21ZQuij @@ -767,6 +811,21 @@ slots: on-model: - my-brand/my-model1 - my-brand-subbrand/my-model2 + slots-arity-slot-any: + allow-auto-connection: + slots-per-plug: * + slots-arity-slot-any-plug-one: + allow-auto-connection: + slots-per-plug: * + slots-arity-slot-any-plug-two: + allow-auto-connection: + slots-per-plug: * + slots-arity-slot-any-plug-default: + allow-auto-connection: + slots-per-plug: * + slots-arity-slot-one-plug-any: + allow-auto-connection: + slots-per-plug: 1 timestamp: 2016-09-30T12:00:00Z sign-key-sha3-384: Jv8_JiHiIzJVcO9M55pPdqSDWUvuhfDIBJUS-3VW7F_idjix7Ffn5qMxB21ZQuij @@ -814,7 +873,9 @@ func (s *policySuite) TestBaselineDefaultIsAllow(c *C) { } c.Check(cand.Check(), IsNil) - c.Check(cand.CheckAutoConnect(), IsNil) + arity, err := cand.CheckAutoConnect() + c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } func (s *policySuite) TestInterfaceMismatch(c *C) { @@ -896,9 +957,10 @@ func (s *policySuite) TestBaseDeclAllowDenyAutoConnection(c *C) { BaseDeclaration: s.baseDecl, } - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() if t.expected == "" { c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } else { c.Check(err, ErrorMatches, t.expected) } @@ -968,9 +1030,10 @@ func (s *policySuite) TestSnapDeclAllowDenyAutoConnection(c *C) { BaseDeclaration: s.baseDecl, } - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() if t.expected == "" { c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } else { c.Check(err, ErrorMatches, t.expected) } @@ -1866,9 +1929,10 @@ func (s *policySuite) TestPlugDeviceScopeCheckAutoConnection(c *C) { Model: t.model, } - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() if t.err == "" { c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } else { c.Check(err, ErrorMatches, t.err) } @@ -1900,9 +1964,10 @@ func (s *policySuite) TestPlugDeviceScopeFriendlyStoreCheckAutoConnection(c *C) Model: t.model, Store: t.store, } - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() if t.err == "" { c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } else { c.Check(err, ErrorMatches, t.err) } @@ -1942,9 +2007,10 @@ func (s *policySuite) TestSlotDeviceScopeCheckAutoConnection(c *C) { Model: t.model, } - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() if t.err == "" { c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } else { c.Check(err, ErrorMatches, t.err) } @@ -1976,9 +2042,10 @@ func (s *policySuite) TestSlotDeviceScopeFriendlyStoreCheckAutoConnection(c *C) Model: t.model, Store: t.store, } - err := cand.CheckAutoConnect() + arity, err := cand.CheckAutoConnect() if t.err == "" { c.Check(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, false) } else { c.Check(err, ErrorMatches, t.err) } @@ -2220,3 +2287,32 @@ func (s *policySuite) TestPlugDollarSlotDynamicAttrConnection(c *C) { } c.Check(cand.Check(), IsNil) } + +func (s *policySuite) TestSlotsArityAutoConnection(c *C) { + tests := []struct { + iface string + any bool + }{ + {"slots-arity-default", false}, + {"slots-arity-slot-any", true}, + {"slots-arity-plug-any", true}, + {"slots-arity-slot-any-plug-one", false}, + {"slots-arity-slot-any-plug-two", false}, + {"slots-arity-slot-any-plug-default", false}, + {"slots-arity-slot-one-plug-any", true}, + } + + for _, t := range tests { + cand := policy.ConnectCandidate{ + Plug: interfaces.NewConnectedPlug(s.plugSnap.Plugs[t.iface], nil, nil), + Slot: interfaces.NewConnectedSlot(s.slotSnap.Slots[t.iface], nil, nil), + PlugSnapDeclaration: s.plugDecl, + SlotSnapDeclaration: s.slotDecl, + + BaseDeclaration: s.baseDecl, + } + arity, err := cand.CheckAutoConnect() + c.Assert(err, IsNil) + c.Check(arity.SlotsPerPlugAny(), Equals, t.any) + } +} diff --git a/interfaces/repo.go b/interfaces/repo.go index 232c732a59..bdc38be9d0 100644 --- a/interfaces/repo.go +++ b/interfaces/repo.go @@ -1110,18 +1110,28 @@ func (r *Repository) DisconnectSnap(snapName string) ([]string, error) { return result, nil } +// SideArity conveys the arity constraints for an allowed auto-connection. +// ATM only slots-per-plug might have an interesting non-default +// value. +// See: https://forum.snapcraft.io/t/plug-slot-declaration-rules-greedy-plugs/12438 +type SideArity interface { + SlotsPerPlugAny() bool + // TODO: consider PlugsPerSlot* +} + // AutoConnectCandidateSlots finds and returns viable auto-connection candidates // for a given plug. -func (r *Repository) AutoConnectCandidateSlots(plugSnapName, plugName string, policyCheck func(*ConnectedPlug, *ConnectedSlot) (bool, error)) []*snap.SlotInfo { +func (r *Repository) AutoConnectCandidateSlots(plugSnapName, plugName string, policyCheck func(*ConnectedPlug, *ConnectedSlot) (bool, SideArity, error)) ([]*snap.SlotInfo, []SideArity) { r.m.Lock() defer r.m.Unlock() plugInfo := r.plugs[plugSnapName][plugName] if plugInfo == nil { - return nil + return nil, nil } var candidates []*snap.SlotInfo + var arities []SideArity for _, slotsForSnap := range r.slots { for _, slotInfo := range slotsForSnap { if slotInfo.Interface != plugInfo.Interface { @@ -1130,22 +1140,23 @@ func (r *Repository) AutoConnectCandidateSlots(plugSnapName, plugName string, po iface := slotInfo.Interface // declaration based checks disallow - ok, err := policyCheck(NewConnectedPlug(plugInfo, nil, nil), NewConnectedSlot(slotInfo, nil, nil)) + ok, arity, err := policyCheck(NewConnectedPlug(plugInfo, nil, nil), NewConnectedSlot(slotInfo, nil, nil)) if !ok || err != nil { continue } if r.ifaces[iface].AutoConnect(plugInfo, slotInfo) { candidates = append(candidates, slotInfo) + arities = append(arities, arity) } } } - return candidates + return candidates, arities } // AutoConnectCandidatePlugs finds and returns viable auto-connection candidates // for a given slot. -func (r *Repository) AutoConnectCandidatePlugs(slotSnapName, slotName string, policyCheck func(*ConnectedPlug, *ConnectedSlot) (bool, error)) []*snap.PlugInfo { +func (r *Repository) AutoConnectCandidatePlugs(slotSnapName, slotName string, policyCheck func(*ConnectedPlug, *ConnectedSlot) (bool, SideArity, error)) []*snap.PlugInfo { r.m.Lock() defer r.m.Unlock() @@ -1163,7 +1174,7 @@ func (r *Repository) AutoConnectCandidatePlugs(slotSnapName, slotName string, po iface := slotInfo.Interface // declaration based checks disallow - ok, err := policyCheck(NewConnectedPlug(plugInfo, nil, nil), NewConnectedSlot(slotInfo, nil, nil)) + ok, _, err := policyCheck(NewConnectedPlug(plugInfo, nil, nil), NewConnectedSlot(slotInfo, nil, nil)) if !ok || err != nil { continue } diff --git a/interfaces/repo_test.go b/interfaces/repo_test.go index e6a6ad28a9..2a6f25ba16 100644 --- a/interfaces/repo_test.go +++ b/interfaces/repo_test.go @@ -21,6 +21,7 @@ package interfaces_test import ( "fmt" + "strings" . "gopkg.in/check.v1" @@ -1683,6 +1684,14 @@ func (s *RepositorySuite) TestSnapSpecificationFailureWithPermanentSnippets(c *C c.Assert(spec, IsNil) } +type testSideArity struct { + sideSnapName string +} + +func (a *testSideArity) SlotsPerPlugAny() bool { + return strings.HasSuffix(a.sideSnapName, "2") +} + func (s *RepositorySuite) TestAutoConnectCandidatePlugsAndSlots(c *C) { // Add two interfaces, one with automatic connections, one with manual repo := s.emptyRepo @@ -1691,8 +1700,8 @@ func (s *RepositorySuite) TestAutoConnectCandidatePlugsAndSlots(c *C) { err = repo.AddInterface(&ifacetest.TestInterface{InterfaceName: "manual"}) c.Assert(err, IsNil) - policyCheck := func(plug *ConnectedPlug, slot *ConnectedSlot) (bool, error) { - return slot.Interface() == "auto", nil + policyCheck := func(plug *ConnectedPlug, slot *ConnectedSlot) (bool, SideArity, error) { + return slot.Interface() == "auto", &testSideArity{plug.Snap().InstanceName()}, nil } // Add a pair of snaps with plugs/slots using those two interfaces @@ -1716,11 +1725,13 @@ slots: err = repo.AddSnap(consumer) c.Assert(err, IsNil) - candidateSlots := repo.AutoConnectCandidateSlots("consumer", "auto", policyCheck) + candidateSlots, arities := repo.AutoConnectCandidateSlots("consumer", "auto", policyCheck) c.Assert(candidateSlots, HasLen, 1) c.Check(candidateSlots[0].Snap.InstanceName(), Equals, "producer") c.Check(candidateSlots[0].Interface, Equals, "auto") c.Check(candidateSlots[0].Name, Equals, "auto") + c.Assert(arities, HasLen, 1) + c.Check(arities[0].SlotsPerPlugAny(), Equals, false) candidatePlugs := repo.AutoConnectCandidatePlugs("producer", "auto", policyCheck) c.Assert(candidatePlugs, HasLen, 1) @@ -1735,8 +1746,8 @@ func (s *RepositorySuite) TestAutoConnectCandidatePlugsAndSlotsSymmetry(c *C) { err := repo.AddInterface(&ifacetest.TestInterface{InterfaceName: "auto"}) c.Assert(err, IsNil) - policyCheck := func(plug *ConnectedPlug, slot *ConnectedSlot) (bool, error) { - return slot.Interface() == "auto", nil + policyCheck := func(plug *ConnectedPlug, slot *ConnectedSlot) (bool, SideArity, error) { + return slot.Interface() == "auto", &testSideArity{plug.Snap().InstanceName()}, nil } // Add a producer snap for "auto" @@ -1773,17 +1784,21 @@ plugs: c.Assert(err, IsNil) // Both can auto-connect - candidateSlots := repo.AutoConnectCandidateSlots("consumer1", "auto", policyCheck) + candidateSlots, arities := repo.AutoConnectCandidateSlots("consumer1", "auto", policyCheck) c.Assert(candidateSlots, HasLen, 1) c.Check(candidateSlots[0].Snap.InstanceName(), Equals, "producer") c.Check(candidateSlots[0].Interface, Equals, "auto") c.Check(candidateSlots[0].Name, Equals, "auto") + c.Assert(arities, HasLen, 1) + c.Check(arities[0].SlotsPerPlugAny(), Equals, false) - candidateSlots = repo.AutoConnectCandidateSlots("consumer2", "auto", policyCheck) + candidateSlots, arities = repo.AutoConnectCandidateSlots("consumer2", "auto", policyCheck) c.Assert(candidateSlots, HasLen, 1) c.Check(candidateSlots[0].Snap.InstanceName(), Equals, "producer") c.Check(candidateSlots[0].Interface, Equals, "auto") c.Check(candidateSlots[0].Name, Equals, "auto") + c.Assert(arities, HasLen, 1) + c.Check(arities[0].SlotsPerPlugAny(), Equals, true) // Plugs candidates seen from the producer (for example if // it's installed after) should be the same @@ -1791,6 +1806,69 @@ plugs: c.Assert(candidatePlugs, HasLen, 2) } +func (s *RepositorySuite) TestAutoConnectCandidateSlotsSideArity(c *C) { + repo := s.emptyRepo + // Add a "auto" interface + err := repo.AddInterface(&ifacetest.TestInterface{InterfaceName: "auto"}) + c.Assert(err, IsNil) + + policyCheck := func(plug *ConnectedPlug, slot *ConnectedSlot) (bool, SideArity, error) { + return slot.Interface() == "auto", &testSideArity{slot.Snap().InstanceName()}, nil + } + + // Add two producer snaps for "auto" + producer1 := snaptest.MockInfo(c, ` +name: producer1 +version: 0 +slots: + auto: +`, nil) + err = repo.AddSnap(producer1) + c.Assert(err, IsNil) + + producer2 := snaptest.MockInfo(c, ` +name: producer2 +version: 0 +slots: + auto: +`, nil) + err = repo.AddSnap(producer2) + c.Assert(err, IsNil) + + // Add a consumer snap for "auto" + consumer := snaptest.MockInfo(c, ` +name: consumer +version: 0 +plugs: + auto: +`, nil) + err = repo.AddSnap(consumer) + c.Assert(err, IsNil) + + // Both slots could auto-connect + seenProducers := make(map[string]bool) + candidateSlots, arities := repo.AutoConnectCandidateSlots("consumer", "auto", policyCheck) + c.Assert(candidateSlots, HasLen, 2) + c.Assert(arities, HasLen, 2) + for i, candSlot := range candidateSlots { + c.Check(candSlot.Interface, Equals, "auto") + c.Check(candSlot.Name, Equals, "auto") + producerName := candSlot.Snap.InstanceName() + // SideArities match + switch producerName { + case "producer1": + c.Check(arities[i].SlotsPerPlugAny(), Equals, false) + case "producer2": + c.Check(arities[i].SlotsPerPlugAny(), Equals, true) + } + seenProducers[producerName] = true + } + c.Check(seenProducers, DeepEquals, map[string]bool{ + "producer1": true, + "producer2": true, + }) +} + // Tests for AddSnap and RemoveSnap type AddRemoveSuite struct { @@ -2064,8 +2142,8 @@ func (s *DisconnectSnapSuite) TestParallelInstances(c *C) { c.Check(affected, testutil.Contains, "s2_instance") } -func contentPolicyCheck(plug *ConnectedPlug, slot *ConnectedSlot) (bool, error) { - return plug.Snap().Publisher.ID == slot.Snap().Publisher.ID, nil +func contentPolicyCheck(plug *ConnectedPlug, slot *ConnectedSlot) (bool, SideArity, error) { + return plug.Snap().Publisher.ID == slot.Snap().Publisher.ID, nil, nil } func contentAutoConnect(plug *snap.PlugInfo, slot *snap.SlotInfo) bool { @@ -2106,7 +2184,7 @@ slots: func (s *RepositorySuite) TestAutoConnectContentInterfaceSimple(c *C) { repo, _, _ := makeContentConnectionTestSnaps(c, "mylib", "mylib") - candidateSlots := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) + candidateSlots, _ := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) c.Assert(candidateSlots, HasLen, 1) c.Check(candidateSlots[0].Name, Equals, "exported-content") candidatePlugs := repo.AutoConnectCandidatePlugs("content-slot-snap", "exported-content", contentPolicyCheck) @@ -2118,7 +2196,7 @@ func (s *RepositorySuite) TestAutoConnectContentInterfaceOSWorksCorrectly(c *C) repo, _, slotSnap := makeContentConnectionTestSnaps(c, "mylib", "otherlib") slotSnap.SnapType = snap.TypeOS - candidateSlots := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) + candidateSlots, _ := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) c.Check(candidateSlots, HasLen, 0) candidatePlugs := repo.AutoConnectCandidatePlugs("content-slot-snap", "exported-content", contentPolicyCheck) c.Assert(candidatePlugs, HasLen, 0) @@ -2126,7 +2204,7 @@ func (s *RepositorySuite) TestAutoConnectContentInterfaceOSWorksCorrectly(c *C) func (s *RepositorySuite) TestAutoConnectContentInterfaceNoMatchingContent(c *C) { repo, _, _ := makeContentConnectionTestSnaps(c, "mylib", "otherlib") - candidateSlots := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) + candidateSlots, _ := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) c.Check(candidateSlots, HasLen, 0) candidatePlugs := repo.AutoConnectCandidatePlugs("content-slot-snap", "exported-content", contentPolicyCheck) c.Assert(candidatePlugs, HasLen, 0) @@ -2138,7 +2216,7 @@ func (s *RepositorySuite) TestAutoConnectContentInterfaceNoMatchingDeveloper(c * plugSnap.Publisher.ID = "fooid" slotSnap.Publisher.ID = "barid" - candidateSlots := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) + candidateSlots, _ := repo.AutoConnectCandidateSlots("content-plug-snap", "imported-content", contentPolicyCheck) c.Check(candidateSlots, HasLen, 0) candidatePlugs := repo.AutoConnectCandidatePlugs("content-slot-snap", "exported-content", contentPolicyCheck) c.Assert(candidatePlugs, HasLen, 0) diff --git a/overlord/devicestate/devicestate.go b/overlord/devicestate/devicestate.go index 16be3d73fb..9f09fb0f61 100644 --- a/overlord/devicestate/devicestate.go +++ b/overlord/devicestate/devicestate.go @@ -319,11 +319,20 @@ func extractDownloadInstallEdgesFromTs(ts *state.TaskSet) (firstDl, lastDl, firs return firstDl, tasks[edgeTaskIndex], tasks[edgeTaskIndex+1], lastInst, nil } +func notInstalled(st *state.State, name string) (bool, error) { + _, err := snapstate.CurrentInfo(st, name) + _, isNotInstalled := err.(*snap.NotInstalledError) + if isNotInstalled { + return true, nil + } + return false, err +} + func remodelTasks(ctx context.Context, st *state.State, current, new *asserts.Model, deviceCtx snapstate.DeviceContext, fromChange string) ([]*state.TaskSet, error) { userID := 0 var tss []*state.TaskSet - // adjust kernel track + // kernel if current.Kernel() == new.Kernel() && current.KernelTrack() != new.KernelTrack() { ts, err := snapstateUpdateWithDeviceContext(st, new.Kernel(), &snapstate.RevisionOptions{Channel: new.KernelTrack()}, userID, snapstate.Flags{NoReRefresh: true}, deviceCtx, fromChange) if err != nil { @@ -331,15 +340,48 @@ func remodelTasks(ctx context.Context, st *state.State, current, new *asserts.Mo } tss = append(tss, ts) } - // add new kernel + + var ts *state.TaskSet if current.Kernel() != new.Kernel() { - // TODO: we need to support corner cases here like: - // 0. start with "old-kernel" - // 1. remodel to "new-kernel" - // 2. remodel back to "old-kernel" - // In step (2) we will get a "already-installed" error - // here right now (workaround: remove "old-kernel") - ts, err := snapstateInstallWithDeviceContext(ctx, st, new.Kernel(), &snapstate.RevisionOptions{Channel: new.KernelTrack()}, userID, snapstate.Flags{}, deviceCtx, fromChange) + needsInstall, err := notInstalled(st, new.Kernel()) + if err != nil { + return nil, err + } + if needsInstall { + ts, err = snapstateInstallWithDeviceContext(ctx, st, new.Kernel(), &snapstate.RevisionOptions{Channel: new.KernelTrack()}, userID, snapstate.Flags{}, deviceCtx, fromChange) + } else { + ts, err = snapstate.LinkNewBaseOrKernel(st, new.Base()) + } + if err != nil { + return nil, err + } + tss = append(tss, ts) + } + if current.Base() != new.Base() { + needsInstall, err := notInstalled(st, new.Base()) + if err != nil { + return nil, err + } + if needsInstall { + ts, err = snapstateInstallWithDeviceContext(ctx, st, new.Base(), nil, userID, snapstate.Flags{}, deviceCtx, fromChange) + } else { + ts, err = snapstate.LinkNewBaseOrKernel(st, new.Base()) + } + if err != nil { + return nil, err + } + tss = append(tss, ts) + } + // gadget + if current.Gadget() == new.Gadget() && current.GadgetTrack() != new.GadgetTrack() { + ts, err := snapstateUpdateWithDeviceContext(st, new.Gadget(), &snapstate.RevisionOptions{Channel: new.GadgetTrack()}, userID, snapstate.Flags{NoReRefresh: true}, deviceCtx, fromChange) + if err != nil { + return nil, err + } + tss = append(tss, ts) + } + if current.Gadget() != new.Gadget() { + ts, err := snapstateInstallWithDeviceContext(ctx, st, new.Gadget(), &snapstate.RevisionOptions{Channel: new.GadgetTrack()}, userID, snapstate.Flags{}, deviceCtx, fromChange) if err != nil { return nil, err } @@ -351,16 +393,17 @@ func remodelTasks(ctx context.Context, st *state.State, current, new *asserts.Mo for _, snapRef := range new.RequiredNoEssentialSnaps() { // TODO|XXX: have methods that take refs directly // to respect the snap ids - _, err := snapstate.CurrentInfo(st, snapRef.SnapName()) - // If the snap is not installed we need to install it now. - if _, ok := err.(*snap.NotInstalledError); ok { + needsInstall, err := notInstalled(st, snapRef.SnapName()) + if err != nil { + return nil, err + } + if needsInstall { + // If the snap is not installed we need to install it now. ts, err := snapstateInstallWithDeviceContext(ctx, st, snapRef.SnapName(), nil, userID, snapstate.Flags{Required: true}, deviceCtx, fromChange) if err != nil { return nil, err } tss = append(tss, ts) - } else if err != nil { - return nil, err } } // TODO: Validate that all bases and default-providers are part @@ -476,12 +519,9 @@ func Remodel(st *state.State, new *asserts.Model) (*state.Change, error) { } // calculate snap differences between the two models - // FIXME: this needs work to switch the base to boot as well - if current.Base() != new.Base() { - return nil, fmt.Errorf("cannot remodel to different bases yet") - } - if current.Gadget() != new.Gadget() { - return nil, fmt.Errorf("cannot remodel to different gadgets yet") + // FIXME: this needs work to switch from core->bases + if current.Base() == "" && new.Base() != "" { + return nil, fmt.Errorf("cannot remodel from core to bases yet") } // TODO: should we run a remodel only while no other change is diff --git a/overlord/devicestate/devicestate_gadget_test.go b/overlord/devicestate/devicestate_gadget_test.go index a54bcc3578..1f6243be46 100644 --- a/overlord/devicestate/devicestate_gadget_test.go +++ b/overlord/devicestate/devicestate_gadget_test.go @@ -31,8 +31,11 @@ import ( "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/gadget" "github.com/snapcore/snapd/osutil" + "github.com/snapcore/snapd/overlord/auth" "github.com/snapcore/snapd/overlord/devicestate" + "github.com/snapcore/snapd/overlord/devicestate/devicestatetest" "github.com/snapcore/snapd/overlord/snapstate" + "github.com/snapcore/snapd/overlord/snapstate/snapstatetest" "github.com/snapcore/snapd/overlord/state" "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" @@ -57,7 +60,21 @@ volumes: bootloader: grub ` -func setupGadgetUpdate(c *C, st *state.State) (chg *state.Change, tsk *state.Task) { +func (s *deviceMgrGadgetSuite) setupModelWithGadget(c *C, gadget string) { + s.makeModelAssertionInState(c, "canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "gadget": gadget, + "base": "core18", + }) + devicestatetest.SetDevice(s.state, &auth.DeviceState{ + Brand: "canonical", + Model: "pc-model", + Serial: "serial", + }) +} + +func (s *deviceMgrGadgetSuite) setupGadgetUpdate(c *C) (chg *state.Change, tsk *state.Task) { siCurrent := &snap.SideInfo{ RealName: "foo-gadget", Revision: snap.R(33), @@ -75,25 +92,26 @@ func setupGadgetUpdate(c *C, st *state.State) (chg *state.Change, tsk *state.Tas {"meta/gadget.yaml", gadgetYaml}, }) - st.Lock() + s.state.Lock() + defer s.state.Unlock() + + s.setupModelWithGadget(c, "foo-gadget") - snapstate.Set(st, "foo-gadget", &snapstate.SnapState{ + snapstate.Set(s.state, "foo-gadget", &snapstate.SnapState{ SnapType: "gadget", Sequence: []*snap.SideInfo{siCurrent}, Current: siCurrent.Revision, Active: true, }) - tsk = st.NewTask("update-gadget-assets", "update gadget") + tsk = s.state.NewTask("update-gadget-assets", "update gadget") tsk.Set("snap-setup", &snapstate.SnapSetup{ SideInfo: si, Type: snap.TypeGadget, }) - chg = st.NewChange("dummy", "...") + chg = s.state.NewChange("dummy", "...") chg.AddTask(tsk) - st.Unlock() - return chg, tsk } @@ -112,12 +130,13 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreSimple(c *C) { }) defer restore() - chg, t := setupGadgetUpdate(c, s.state) + chg, t := s.setupGadgetUpdate(c) - for i := 0; i < 6; i++ { - s.se.Ensure() - s.se.Wait() - } + s.state.Lock() + s.state.Set("seeded", true) + s.state.Unlock() + + s.settle(c) s.state.Lock() defer s.state.Unlock() @@ -140,7 +159,7 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreNoUpdateNeeded(c *C) { }) defer restore() - chg, t := setupGadgetUpdate(c, s.state) + chg, t := s.setupGadgetUpdate(c) s.se.Ensure() s.se.Wait() @@ -166,16 +185,17 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreRollbackDirCreateFailed(c * }) defer restore() - chg, t := setupGadgetUpdate(c, s.state) + chg, t := s.setupGadgetUpdate(c) rollbackDir := filepath.Join(dirs.SnapRollbackDir, "foo-gadget_34") err := os.MkdirAll(dirs.SnapRollbackDir, 0000) c.Assert(err, IsNil) - for i := 0; i < 6; i++ { - s.se.Ensure() - s.se.Wait() - } + s.state.Lock() + s.state.Set("seeded", true) + s.state.Unlock() + + s.settle(c) s.state.Lock() defer s.state.Unlock() @@ -191,12 +211,13 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreUpdateFailed(c *C) { return errors.New("gadget exploded") }) defer restore() - chg, t := setupGadgetUpdate(c, s.state) + chg, t := s.setupGadgetUpdate(c) - for i := 0; i < 6; i++ { - s.se.Ensure() - s.se.Wait() - } + s.state.Lock() + s.state.Set("seeded", true) + s.state.Unlock() + + s.settle(c) s.state.Lock() defer s.state.Unlock() @@ -227,6 +248,9 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreNotDuringFirstboot(c *C) { }) s.state.Lock() + s.state.Set("seeded", true) + + s.setupModelWithGadget(c, "foo-gadget") t := s.state.NewTask("update-gadget-assets", "update gadget") t.Set("snap-setup", &snapstate.SnapSetup{ @@ -238,10 +262,7 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreNotDuringFirstboot(c *C) { s.state.Unlock() - for i := 0; i < 6; i++ { - s.se.Ensure() - s.se.Wait() - } + s.settle(c) s.state.Lock() defer s.state.Unlock() @@ -277,6 +298,9 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreBadGadgetYaml(c *C) { }) s.state.Lock() + s.state.Set("seeded", true) + + s.setupModelWithGadget(c, "foo-gadget") snapstate.Set(s.state, "foo-gadget", &snapstate.SnapState{ SnapType: "gadget", @@ -295,21 +319,67 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreBadGadgetYaml(c *C) { s.state.Unlock() - for i := 0; i < 6; i++ { - s.se.Ensure() - s.se.Wait() - } + s.settle(c) s.state.Lock() defer s.state.Unlock() c.Assert(chg.IsReady(), Equals, true) - c.Check(chg.Err(), ErrorMatches, `(?s).*update gadget \(cannot read candidate gadget snap details: .*\).*`) + c.Check(chg.Err(), ErrorMatches, `(?s).*update gadget \(cannot read candidate snap gadget metadata: .*\).*`) c.Check(t.Status(), Equals, state.ErrorStatus) rollbackDir := filepath.Join(dirs.SnapRollbackDir, "foo-gadget") c.Check(osutil.IsDirectory(rollbackDir), Equals, false) c.Check(s.restartRequests, HasLen, 0) } +func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnCoreParanoidChecks(c *C) { + restore := devicestate.MockGadgetUpdate(func(current, update gadget.GadgetData, path string, policy gadget.UpdatePolicyFunc) error { + return errors.New("unexpected call") + }) + defer restore() + siCurrent := &snap.SideInfo{ + RealName: "foo-gadget", + Revision: snap.R(33), + SnapID: "foo-id", + } + si := &snap.SideInfo{ + RealName: "foo-gadget-unexpected", + Revision: snap.R(34), + SnapID: "foo-id", + } + + s.state.Lock() + + s.state.Set("seeded", true) + + s.setupModelWithGadget(c, "foo-gadget") + + snapstate.Set(s.state, "foo-gadget", &snapstate.SnapState{ + SnapType: "gadget", + Sequence: []*snap.SideInfo{siCurrent}, + Current: siCurrent.Revision, + Active: true, + }) + + t := s.state.NewTask("update-gadget-assets", "update gadget") + t.Set("snap-setup", &snapstate.SnapSetup{ + SideInfo: si, + Type: snap.TypeGadget, + }) + chg := s.state.NewChange("dummy", "...") + chg.AddTask(t) + + s.state.Unlock() + + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + c.Assert(chg.IsReady(), Equals, true) + c.Assert(chg.Err(), ErrorMatches, `(?s).*\(cannot apply gadget assets update from non-model gadget snap "foo-gadget-unexpected", expected "foo-gadget" snap\)`) + c.Check(t.Status(), Equals, state.ErrorStatus) + c.Check(s.restartRequests, HasLen, 0) +} + func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnClassicErrorsOut(c *C) { restore := release.MockOnClassic(true) defer restore() @@ -321,25 +391,15 @@ func (s *deviceMgrGadgetSuite) TestUpdateGadgetOnClassicErrorsOut(c *C) { s.state.Lock() + s.state.Set("seeded", true) + t := s.state.NewTask("update-gadget-assets", "update gadget") chg := s.state.NewChange("dummy", "...") chg.AddTask(t) s.state.Unlock() - // we cannot use "s.o.Settle()" here because this change has an - // error which means that the settle will never converge - for i := 0; i < 50; i++ { - s.se.Ensure() - s.se.Wait() - - s.state.Lock() - ready := chg.IsReady() - s.state.Unlock() - if ready { - break - } - } + s.settle(c) s.state.Lock() defer s.state.Unlock() @@ -402,7 +462,7 @@ volumes: expectedRollbackDir := filepath.Join(dirs.SnapRollbackDir, "foo-gadget_34") updaterForStructureCalls := 0 - gadget.MockUpdaterForStructure(func(ps *gadget.LaidOutStructure, rootDir, rollbackDir string) (gadget.Updater, error) { + restore := gadget.MockUpdaterForStructure(func(ps *gadget.LaidOutStructure, rootDir, rollbackDir string) (gadget.Updater, error) { updaterForStructureCalls++ c.Assert(ps.Name, Equals, "foo") @@ -412,8 +472,12 @@ volumes: c.Assert(osutil.IsDirectory(rollbackDir), Equals, true) return &mockUpdater{}, nil }) + defer restore() s.state.Lock() + s.state.Set("seeded", true) + + s.setupModelWithGadget(c, "foo-gadget") snapstate.Set(s.state, "foo-gadget", &snapstate.SnapState{ SnapType: "gadget", @@ -432,10 +496,7 @@ volumes: s.state.Unlock() - for i := 0; i < 6; i++ { - s.se.Ensure() - s.se.Wait() - } + s.settle(c) s.state.Lock() defer s.state.Unlock() @@ -465,10 +526,17 @@ func (s *deviceMgrGadgetSuite) TestCurrentAndUpdateInfo(c *C) { Type: snap.TypeGadget, } - current, update, err := devicestate.GadgetCurrentAndUpdate(s.state, snapsup) + model := s.brands.Model("canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "gadget": "foo-gadget", + "base": "core18", + }) + deviceCtx := &snapstatetest.TrivialDeviceContext{DeviceModel: model} + + current, err := devicestate.CurrentGadgetInfo(s.state, deviceCtx) c.Assert(current, IsNil) - c.Assert(update, IsNil) - c.Assert(err, IsNil) + c.Check(err, IsNil) snapstate.Set(s.state, "foo-gadget", &snapstate.SnapState{ SnapType: "gadget", @@ -480,26 +548,37 @@ func (s *deviceMgrGadgetSuite) TestCurrentAndUpdateInfo(c *C) { // mock current first, but gadget.yaml is still missing ci := snaptest.MockSnapWithFiles(c, snapYaml, siCurrent, nil) - current, update, err = devicestate.GadgetCurrentAndUpdate(s.state, snapsup) + current, err = devicestate.CurrentGadgetInfo(s.state, deviceCtx) + c.Assert(current, IsNil) - c.Assert(update, IsNil) c.Assert(err, ErrorMatches, "cannot read current gadget snap details: .*/33/meta/gadget.yaml: no such file or directory") // drop gadget.yaml for current snap ioutil.WriteFile(filepath.Join(ci.MountDir(), "meta/gadget.yaml"), []byte(gadgetYaml), 0644) - // update missing snap.yaml - current, update, err = devicestate.GadgetCurrentAndUpdate(s.state, snapsup) - c.Assert(current, IsNil) + current, err = devicestate.CurrentGadgetInfo(s.state, deviceCtx) + c.Assert(err, IsNil) + c.Assert(current, DeepEquals, &gadget.GadgetData{ + Info: &gadget.Info{ + Volumes: map[string]gadget.Volume{ + "pc": { + Bootloader: "grub", + }, + }, + }, + RootDir: ci.MountDir(), + }) + + // pending update + update, err := devicestate.PendingGadgetInfo(snapsup) c.Assert(update, IsNil) c.Assert(err, ErrorMatches, "cannot read candidate gadget snap details: cannot find installed snap .* .*/34/meta/snap.yaml") ui := snaptest.MockSnapWithFiles(c, snapYaml, si, nil) - current, update, err = devicestate.GadgetCurrentAndUpdate(s.state, snapsup) - c.Assert(current, IsNil) + update, err = devicestate.PendingGadgetInfo(snapsup) c.Assert(update, IsNil) - c.Assert(err, ErrorMatches, "cannot read candidate gadget snap details: .*/34/meta/gadget.yaml: no such file or directory") + c.Assert(err, ErrorMatches, "cannot read candidate snap gadget metadata: .*/34/meta/gadget.yaml: no such file or directory") var updateGadgetYaml = ` volumes: @@ -511,18 +590,8 @@ volumes: // drop gadget.yaml for update snap ioutil.WriteFile(filepath.Join(ui.MountDir(), "meta/gadget.yaml"), []byte(updateGadgetYaml), 0644) - current, update, err = devicestate.GadgetCurrentAndUpdate(s.state, snapsup) + update, err = devicestate.PendingGadgetInfo(snapsup) c.Assert(err, IsNil) - c.Assert(current, DeepEquals, &gadget.GadgetData{ - Info: &gadget.Info{ - Volumes: map[string]gadget.Volume{ - "pc": { - Bootloader: "grub", - }, - }, - }, - RootDir: ci.MountDir(), - }) c.Assert(update, DeepEquals, &gadget.GadgetData{ Info: &gadget.Info{ Volumes: map[string]gadget.Volume{ diff --git a/overlord/devicestate/devicestate_remodel_test.go b/overlord/devicestate/devicestate_remodel_test.go index 5e63461402..f2058b424f 100644 --- a/overlord/devicestate/devicestate_remodel_test.go +++ b/overlord/devicestate/devicestate_remodel_test.go @@ -25,8 +25,10 @@ import ( "fmt" "io/ioutil" "path/filepath" + "reflect" . "gopkg.in/check.v1" + "gopkg.in/tomb.v2" "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/asserts/assertstest" @@ -78,7 +80,6 @@ func (s *deviceMgrRemodelSuite) TestRemodelUnhappy(c *C) { "architecture": "amd64", "kernel": "pc-kernel", "gadget": "pc", - "base": "core18", } s.makeModelAssertionInState(c, cur["brand"], cur["model"], map[string]interface{}{ "architecture": cur["architecture"], @@ -97,8 +98,7 @@ func (s *deviceMgrRemodelSuite) TestRemodelUnhappy(c *C) { errStr string }{ {map[string]string{"architecture": "pdp-7"}, "cannot remodel to different architectures yet"}, - {map[string]string{"base": "core20"}, "cannot remodel to different bases yet"}, - {map[string]string{"gadget": "other-gadget"}, "cannot remodel to different gadgets yet"}, + {map[string]string{"base": "core20"}, "cannot remodel from core to bases yet"}, } { // copy current model unless new model test data is different for k, v := range cur { @@ -119,7 +119,19 @@ func (s *deviceMgrRemodelSuite) TestRemodelUnhappy(c *C) { } } +func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchGadgetTrack(c *C) { + s.testRemodelTasksSwitchTrack(c, "pc", map[string]interface{}{ + "gadget": "pc=18", + }) +} + func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchKernelTrack(c *C) { + s.testRemodelTasksSwitchTrack(c, "pc-kernel", map[string]interface{}{ + "kernel": "pc-kernel=18", + }) +} + +func (s *deviceMgrRemodelSuite) testRemodelTasksSwitchTrack(c *C, whatRefreshes string, newModelOverrides map[string]interface{}) { s.state.Lock() defer s.state.Unlock() s.state.Set("seeded", true) @@ -148,6 +160,8 @@ func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchKernelTrack(c *C) { c.Check(flags.NoReRefresh, Equals, true) c.Check(deviceCtx, Equals, testDeviceCtx) c.Check(fromChange, Equals, "99") + c.Check(name, Equals, whatRefreshes) + c.Check(opts.Channel, Equals, "18") tDownload := s.state.NewTask("fake-download", fmt.Sprintf("Download %s to track %s", name, opts.Channel)) tValidate := s.state.NewTask("validate-snap", fmt.Sprintf("Validate %s", name)) @@ -174,14 +188,18 @@ func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchKernelTrack(c *C) { Model: "pc-model", }) - new := s.brands.Model("canonical", "pc-model", map[string]interface{}{ + headers := map[string]interface{}{ "architecture": "amd64", - "kernel": "pc-kernel=18", + "kernel": "pc-kernel", "gadget": "pc", "base": "core18", "required-snaps": []interface{}{"new-required-snap-1", "new-required-snap-2"}, "revision": "1", - }) + } + for k, v := range newModelOverrides { + headers[k] = v + } + new := s.brands.Model("canonical", "pc-model", headers) testDeviceCtx = &snapstatetest.TrivialDeviceContext{Remodeling: true} @@ -192,7 +210,20 @@ func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchKernelTrack(c *C) { c.Assert(tss, HasLen, 4) } +func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchGadget(c *C) { + s.testRemodelSwitchTasks(c, "other-gadget", "18", map[string]interface{}{ + "gadget": "other-gadget=18", + }) +} + func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchKernel(c *C) { + s.testRemodelSwitchTasks(c, "other-kernel", "18", map[string]interface{}{ + "kernel": "other-kernel=18", + }) +} + +func (s *deviceMgrRemodelSuite) testRemodelSwitchTasks(c *C, whatsNew, whatNewTrack string, newModelOverrides map[string]interface{}) { + c.Check(newModelOverrides, HasLen, 1, Commentf("test expects a single model property to change")) s.state.Lock() defer s.state.Unlock() s.state.Set("seeded", true) @@ -200,10 +231,13 @@ func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchKernel(c *C) { var testDeviceCtx snapstate.DeviceContext + var snapstateInstallWithDeviceContextCalled int restore := devicestate.MockSnapstateInstallWithDeviceContext(func(ctx context.Context, st *state.State, name string, opts *snapstate.RevisionOptions, userID int, flags snapstate.Flags, deviceCtx snapstate.DeviceContext, fromChange string) (*state.TaskSet, error) { - c.Check(deviceCtx, Equals, testDeviceCtx) - c.Check(name, Equals, "other-kernel") - c.Check(opts.Channel, Equals, "18") + snapstateInstallWithDeviceContextCalled++ + c.Check(name, Equals, whatsNew) + if whatNewTrack != "" { + c.Check(opts.Channel, Equals, whatNewTrack) + } tDownload := s.state.NewTask("fake-download", fmt.Sprintf("Download %s", name)) tValidate := s.state.NewTask("validate-snap", fmt.Sprintf("Validate %s", name)) @@ -230,20 +264,26 @@ func (s *deviceMgrRemodelSuite) TestRemodelTasksSwitchKernel(c *C) { Model: "pc-model", }) - new := s.brands.Model("canonical", "pc-model", map[string]interface{}{ + headers := map[string]interface{}{ "architecture": "amd64", - "kernel": "other-kernel=18", + "kernel": "pc-kernel", "gadget": "pc", "base": "core18", "revision": "1", - }) + } + for k, v := range newModelOverrides { + headers[k] = v + } + new := s.brands.Model("canonical", "pc-model", headers) testDeviceCtx = &snapstatetest.TrivialDeviceContext{Remodeling: true} tss, err := devicestate.RemodelTasks(context.Background(), s.state, current, new, testDeviceCtx, "99") c.Assert(err, IsNil) - // 1 new kernel plus the remodel task + // 1 of switch-kernel/base/gadget plus the remodel task c.Assert(tss, HasLen, 2) + // API was hit + c.Assert(snapstateInstallWithDeviceContextCalled, Equals, 1) } func (s *deviceMgrRemodelSuite) TestRemodelRequiredSnaps(c *C) { @@ -999,3 +1039,421 @@ volumes: err = devicestate.CheckGadgetRemodelCompatible(s.state, info, nil, snapf, snapstate.Flags{}, remodelCtx) c.Check(err, IsNil) } + +var ( + compatibleTestMockOkGadget = ` +type: gadget +name: gadget +volumes: + volume: + schema: gpt + bootloader: grub + structure: + - name: foo + size: 10M + type: 00000000-0000-0000-0000-0000deadbeef +` +) + +func (s *deviceMgrRemodelSuite) testCheckGadgetRemodelCompatibleWithYaml(c *C, currentGadgetYaml, newGadgetYaml string, expErr string) { + s.state.Lock() + defer s.state.Unlock() + + currentSnapYaml := ` +name: gadget +type: gadget +version: 123 +` + remodelSnapYaml := ` +name: new-gadget +type: gadget +version: 123 +` + + currInfo := snaptest.MockSnapWithFiles(c, currentSnapYaml, &snap.SideInfo{Revision: snap.R(123)}, [][]string{ + {"meta/gadget.yaml", currentGadgetYaml}, + }) + // gadget we're remodeling to is identical + info := snaptest.MockSnapWithFiles(c, remodelSnapYaml, &snap.SideInfo{Revision: snap.R(1)}, [][]string{ + {"meta/gadget.yaml", newGadgetYaml}, + }) + snapf, err := snap.Open(info.MountDir()) + c.Assert(err, IsNil) + + s.setupBrands(c) + // model assertion in device context + model := fakeMyModel(map[string]interface{}{ + "architecture": "amd64", + "gadget": "gadget", + "kernel": "krnl", + }) + remodelCtx := &snapstatetest.TrivialDeviceContext{DeviceModel: model, Remodeling: true} + + err = devicestate.CheckGadgetRemodelCompatible(s.state, info, currInfo, snapf, snapstate.Flags{}, remodelCtx) + if expErr == "" { + c.Check(err, IsNil) + } else { + c.Check(err, ErrorMatches, expErr) + } + +} + +func (s *deviceMgrRemodelSuite) TestCheckGadgetRemodelCompatibleWithYamlHappy(c *C) { + s.testCheckGadgetRemodelCompatibleWithYaml(c, compatibleTestMockOkGadget, compatibleTestMockOkGadget, "") +} + +func (s *deviceMgrRemodelSuite) TestCheckGadgetRemodelCompatibleWithYamlBad(c *C) { + mockBadGadgetYaml := ` +type: gadget +name: gadget +volumes: + volume: + schema: gpt + bootloader: grub + structure: + - name: foo + size: 20M + type: 00000000-0000-0000-0000-0000deadbeef +` + + errMatch := `cannot remodel to an incompatible gadget: incompatible layout change: incompatible structure #0 \("foo"\) change: cannot change structure size from 10485760 to 20971520` + s.testCheckGadgetRemodelCompatibleWithYaml(c, compatibleTestMockOkGadget, mockBadGadgetYaml, errMatch) +} + +func (s *deviceMgrRemodelSuite) TestRemodelGadgetAssetsUpdate(c *C) { + var currentGadgetYaml = ` +volumes: + pc: + bootloader: grub + structure: + - name: foo + type: 00000000-0000-0000-0000-0000deadcafe + filesystem: ext4 + size: 10M + content: + - source: foo-content + target: / + - name: bare-one + type: bare + size: 1M + content: + - image: bare.img +` + + var remodelGadgetYaml = ` +volumes: + pc: + bootloader: grub + structure: + - name: foo + type: 00000000-0000-0000-0000-0000deadcafe + filesystem: ext4 + size: 10M + content: + - source: new-foo-content + target: / + - name: bare-one + type: bare + size: 1M + content: + - image: new-bare-content.img +` + + s.state.Lock() + s.state.Set("seeded", true) + s.state.Set("refresh-privacy-key", "some-privacy-key") + + nopHandler := func(task *state.Task, _ *tomb.Tomb) error { + return nil + } + s.o.TaskRunner().AddHandler("fake-download", nopHandler, nil) + s.o.TaskRunner().AddHandler("validate-snap", nopHandler, nil) + s.o.TaskRunner().AddHandler("set-model", nopHandler, nil) + + // set a model assertion we remodel from + s.makeModelAssertionInState(c, "canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "gadget": "pc", + "base": "core18", + }) + devicestatetest.SetDevice(s.state, &auth.DeviceState{ + Brand: "canonical", + Model: "pc-model", + Serial: "serial", + }) + + // the target model + new := s.brands.Model("canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "base": "core18", + "revision": "1", + // remodel to new gadget + "gadget": "new-gadget", + }) + + // current gadget + siModelGadget := &snap.SideInfo{ + RealName: "pc", + Revision: snap.R(33), + SnapID: "foo-id", + } + currentGadgetInfo := snaptest.MockSnapWithFiles(c, snapYaml, siModelGadget, [][]string{ + {"meta/gadget.yaml", currentGadgetYaml}, + }) + snapstate.Set(s.state, "pc", &snapstate.SnapState{ + SnapType: "gadget", + Sequence: []*snap.SideInfo{siModelGadget}, + Current: siModelGadget.Revision, + Active: true, + }) + + // new gadget snap + siNewModelGadget := &snap.SideInfo{ + RealName: "new-gadget", + Revision: snap.R(34), + } + newGadgetInfo := snaptest.MockSnapWithFiles(c, snapYaml, siNewModelGadget, [][]string{ + {"meta/gadget.yaml", remodelGadgetYaml}, + }) + + restore := devicestate.MockSnapstateInstallWithDeviceContext(func(ctx context.Context, st *state.State, name string, opts *snapstate.RevisionOptions, userID int, flags snapstate.Flags, deviceCtx snapstate.DeviceContext, fromChange string) (*state.TaskSet, error) { + tDownload := s.state.NewTask("fake-download", fmt.Sprintf("Download %s", name)) + tValidate := s.state.NewTask("validate-snap", fmt.Sprintf("Validate %s", name)) + tValidate.WaitFor(tDownload) + tGadgetUpdate := s.state.NewTask("update-gadget-assets", fmt.Sprintf("Update gadget %s", name)) + tGadgetUpdate.Set("snap-setup", &snapstate.SnapSetup{ + SideInfo: siNewModelGadget, + Type: snap.TypeGadget, + }) + tGadgetUpdate.WaitFor(tValidate) + ts := state.NewTaskSet(tDownload, tValidate, tGadgetUpdate) + ts.MarkEdge(tValidate, snapstate.DownloadAndChecksDoneEdge) + return ts, nil + }) + defer restore() + restore = release.MockOnClassic(false) + defer restore() + + gadgetUpdateCalled := false + restore = devicestate.MockGadgetUpdate(func(current, update gadget.GadgetData, path string, policy gadget.UpdatePolicyFunc) error { + gadgetUpdateCalled = true + c.Check(policy, NotNil) + c.Check(reflect.ValueOf(policy).Pointer(), Equals, reflect.ValueOf(gadget.RemodelUpdatePolicy).Pointer()) + c.Check(current, DeepEquals, gadget.GadgetData{ + Info: &gadget.Info{ + Volumes: map[string]gadget.Volume{ + "pc": { + Bootloader: "grub", + Structure: []gadget.VolumeStructure{{ + Name: "foo", + Type: "00000000-0000-0000-0000-0000deadcafe", + Size: 10 * gadget.SizeMiB, + Filesystem: "ext4", + Content: []gadget.VolumeContent{ + {Source: "foo-content", Target: "/"}, + }, + }, { + Name: "bare-one", + Type: "bare", + Size: gadget.SizeMiB, + Content: []gadget.VolumeContent{ + {Image: "bare.img"}, + }, + }}, + }, + }, + }, + RootDir: currentGadgetInfo.MountDir(), + }) + c.Check(update, DeepEquals, gadget.GadgetData{ + Info: &gadget.Info{ + Volumes: map[string]gadget.Volume{ + "pc": { + Bootloader: "grub", + Structure: []gadget.VolumeStructure{{ + Name: "foo", + Type: "00000000-0000-0000-0000-0000deadcafe", + Size: 10 * gadget.SizeMiB, + Filesystem: "ext4", + Content: []gadget.VolumeContent{ + {Source: "new-foo-content", Target: "/"}, + }, + }, { + Name: "bare-one", + Type: "bare", + Size: gadget.SizeMiB, + Content: []gadget.VolumeContent{ + {Image: "new-bare-content.img"}, + }, + }}, + }, + }, + }, + RootDir: newGadgetInfo.MountDir(), + }) + return nil + }) + defer restore() + + chg, err := devicestate.Remodel(s.state, new) + c.Check(err, IsNil) + s.state.Unlock() + + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + c.Check(chg.IsReady(), Equals, true) + c.Check(chg.Err(), IsNil) + c.Check(gadgetUpdateCalled, Equals, true) + c.Check(s.restartRequests, DeepEquals, []state.RestartType{state.RestartSystem}) +} + +func (s *deviceMgrRemodelSuite) TestRemodelGadgetAssetsParanoidCheck(c *C) { + s.state.Lock() + s.state.Set("seeded", true) + s.state.Set("refresh-privacy-key", "some-privacy-key") + + nopHandler := func(task *state.Task, _ *tomb.Tomb) error { + return nil + } + s.o.TaskRunner().AddHandler("fake-download", nopHandler, nil) + s.o.TaskRunner().AddHandler("validate-snap", nopHandler, nil) + s.o.TaskRunner().AddHandler("set-model", nopHandler, nil) + + // set a model assertion we remodel from + s.makeModelAssertionInState(c, "canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "gadget": "pc", + "base": "core18", + }) + devicestatetest.SetDevice(s.state, &auth.DeviceState{ + Brand: "canonical", + Model: "pc-model", + Serial: "serial", + }) + + // the target model + new := s.brands.Model("canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "base": "core18", + "revision": "1", + // remodel to new gadget + "gadget": "new-gadget", + }) + + // current gadget + siModelGadget := &snap.SideInfo{ + RealName: "pc", + Revision: snap.R(33), + SnapID: "foo-id", + } + snapstate.Set(s.state, "pc", &snapstate.SnapState{ + SnapType: "gadget", + Sequence: []*snap.SideInfo{siModelGadget}, + Current: siModelGadget.Revision, + Active: true, + }) + + // new gadget snap, name does not match the new model + siUnexpectedModelGadget := &snap.SideInfo{ + RealName: "new-gadget-unexpected", + Revision: snap.R(34), + } + restore := devicestate.MockSnapstateInstallWithDeviceContext(func(ctx context.Context, st *state.State, name string, opts *snapstate.RevisionOptions, userID int, flags snapstate.Flags, deviceCtx snapstate.DeviceContext, fromChange string) (*state.TaskSet, error) { + tDownload := s.state.NewTask("fake-download", fmt.Sprintf("Download %s", name)) + tValidate := s.state.NewTask("validate-snap", fmt.Sprintf("Validate %s", name)) + tValidate.WaitFor(tDownload) + tGadgetUpdate := s.state.NewTask("update-gadget-assets", fmt.Sprintf("Update gadget %s", name)) + tGadgetUpdate.Set("snap-setup", &snapstate.SnapSetup{ + SideInfo: siUnexpectedModelGadget, + Type: snap.TypeGadget, + }) + tGadgetUpdate.WaitFor(tValidate) + ts := state.NewTaskSet(tDownload, tValidate, tGadgetUpdate) + ts.MarkEdge(tValidate, snapstate.DownloadAndChecksDoneEdge) + return ts, nil + }) + defer restore() + restore = release.MockOnClassic(false) + defer restore() + + gadgetUpdateCalled := false + restore = devicestate.MockGadgetUpdate(func(current, update gadget.GadgetData, path string, policy gadget.UpdatePolicyFunc) error { + return errors.New("unexpected call") + }) + defer restore() + + chg, err := devicestate.Remodel(s.state, new) + c.Check(err, IsNil) + s.state.Unlock() + + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + c.Check(chg.IsReady(), Equals, true) + c.Assert(chg.Err(), ErrorMatches, `(?s).*\(cannot apply gadget assets update from non-model gadget snap "new-gadget-unexpected", expected "new-gadget" snap\)`) + c.Check(gadgetUpdateCalled, Equals, false) + c.Check(s.restartRequests, HasLen, 0) +} + +func (s *deviceMgrSuite) TestRemodelSwitchBase(c *C) { + s.state.Lock() + defer s.state.Unlock() + s.state.Set("seeded", true) + s.state.Set("refresh-privacy-key", "some-privacy-key") + + var testDeviceCtx snapstate.DeviceContext + + var snapstateInstallWithDeviceContextCalled int + restore := devicestate.MockSnapstateInstallWithDeviceContext(func(ctx context.Context, st *state.State, name string, opts *snapstate.RevisionOptions, userID int, flags snapstate.Flags, deviceCtx snapstate.DeviceContext, fromChange string) (*state.TaskSet, error) { + snapstateInstallWithDeviceContextCalled++ + c.Check(name, Equals, "core20") + + tDownload := s.state.NewTask("fake-download", fmt.Sprintf("Download %s", name)) + tValidate := s.state.NewTask("validate-snap", fmt.Sprintf("Validate %s", name)) + tValidate.WaitFor(tDownload) + tInstall := s.state.NewTask("fake-install", fmt.Sprintf("Install %s", name)) + tInstall.WaitFor(tValidate) + ts := state.NewTaskSet(tDownload, tValidate, tInstall) + ts.MarkEdge(tValidate, snapstate.DownloadAndChecksDoneEdge) + return ts, nil + }) + defer restore() + + // set a model assertion + current := s.brands.Model("canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "gadget": "pc", + "base": "core18", + }) + err := assertstate.Add(s.state, current) + c.Assert(err, IsNil) + devicestatetest.SetDevice(s.state, &auth.DeviceState{ + Brand: "canonical", + Model: "pc-model", + }) + + new := s.brands.Model("canonical", "pc-model", map[string]interface{}{ + "architecture": "amd64", + "kernel": "pc-kernel", + "gadget": "pc", + "base": "core20", + "revision": "1", + }) + + testDeviceCtx = &snapstatetest.TrivialDeviceContext{Remodeling: true} + + tss, err := devicestate.RemodelTasks(context.Background(), s.state, current, new, testDeviceCtx, "99") + c.Assert(err, IsNil) + // 1 switch to a new base plus the remodel task + c.Assert(tss, HasLen, 2) + // API was hit + c.Assert(snapstateInstallWithDeviceContextCalled, Equals, 1) +} diff --git a/overlord/devicestate/export_test.go b/overlord/devicestate/export_test.go index 8c42deb7dd..5c7939833b 100644 --- a/overlord/devicestate/export_test.go +++ b/overlord/devicestate/export_test.go @@ -162,8 +162,9 @@ var ( CleanupRemodelCtx = cleanupRemodelCtx CachedRemodelCtx = cachedRemodelCtx - GadgetUpdateBlocked = gadgetUpdateBlocked - GadgetCurrentAndUpdate = gadgetCurrentAndUpdate + GadgetUpdateBlocked = gadgetUpdateBlocked + CurrentGadgetInfo = currentGadgetInfo + PendingGadgetInfo = pendingGadgetInfo ) func MockGadgetUpdate(mock func(current, update gadget.GadgetData, path string, policy gadget.UpdatePolicyFunc) error) (restore func()) { diff --git a/overlord/devicestate/handlers_gadget.go b/overlord/devicestate/handlers_gadget.go index 1c423753fd..1c05f17bd5 100644 --- a/overlord/devicestate/handlers_gadget.go +++ b/overlord/devicestate/handlers_gadget.go @@ -34,15 +34,6 @@ import ( "github.com/snapcore/snapd/snap" ) -func snapState(st *state.State, name string) (*snapstate.SnapState, error) { - var snapst snapstate.SnapState - err := snapstate.Get(st, name, &snapst) - if err != nil && err != state.ErrNoState { - return nil, err - } - return &snapst, nil -} - func makeRollbackDir(name string) (string, error) { rollbackDir := filepath.Join(dirs.SnapRollbackDir, name) @@ -53,9 +44,9 @@ func makeRollbackDir(name string) (string, error) { return rollbackDir, nil } -func currentGadgetInfo(snapst *snapstate.SnapState) (*gadget.GadgetData, error) { - currentInfo, err := snapst.CurrentInfo() - if err != nil && err != snapstate.ErrNoCurrent { +func currentGadgetInfo(st *state.State, deviceCtx snapstate.DeviceContext) (*gadget.GadgetData, error) { + currentInfo, err := snapstate.GadgetInfo(st, deviceCtx) + if err != nil && err != state.ErrNoState { return nil, err } if currentInfo == nil { @@ -63,53 +54,24 @@ func currentGadgetInfo(snapst *snapstate.SnapState) (*gadget.GadgetData, error) return nil, nil } - constraints := &gadget.ModelConstraints{ - Classic: false, - } - gi, err := gadget.ReadInfo(currentInfo.MountDir(), constraints) + ci, err := gadgetDataFromInfo(currentInfo, coreGadgetConstraints) if err != nil { - return nil, err + return nil, fmt.Errorf("cannot read current gadget snap details: %v", err) } - return &gadget.GadgetData{Info: gi, RootDir: currentInfo.MountDir()}, nil + return ci, nil } func pendingGadgetInfo(snapsup *snapstate.SnapSetup) (*gadget.GadgetData, error) { info, err := snap.ReadInfo(snapsup.InstanceName(), snapsup.SideInfo) if err != nil { - return nil, err + return nil, fmt.Errorf("cannot read candidate gadget snap details: %v", err) } - constraints := &gadget.ModelConstraints{ - Classic: false, - } - update, err := gadget.ReadInfo(info.MountDir(), constraints) + gi, err := gadgetDataFromInfo(info, coreGadgetConstraints) if err != nil { - return nil, err + return nil, fmt.Errorf("cannot read candidate snap gadget metadata: %v", err) } - return &gadget.GadgetData{Info: update, RootDir: info.MountDir()}, nil -} - -func gadgetCurrentAndUpdate(st *state.State, snapsup *snapstate.SnapSetup) (current *gadget.GadgetData, update *gadget.GadgetData, err error) { - snapst, err := snapState(st, snapsup.InstanceName()) - if err != nil { - return nil, nil, err - } - - currentData, err := currentGadgetInfo(snapst) - if err != nil { - return nil, nil, fmt.Errorf("cannot read current gadget snap details: %v", err) - } - if currentData == nil { - // don't bother reading update if there is no current - return nil, nil, nil - } - - newData, err := pendingGadgetInfo(snapsup) - if err != nil { - return nil, nil, fmt.Errorf("cannot read candidate gadget snap details: %v", err) - } - - return currentData, newData, nil + return gi, nil } var ( @@ -130,7 +92,33 @@ func (m *DeviceManager) doUpdateGadgetAssets(t *state.Task, _ *tomb.Tomb) error return err } - currentData, updateData, err := gadgetCurrentAndUpdate(t.State(), snapsup) + remodelCtx, err := DeviceCtx(st, t, nil) + if err != nil && err != state.ErrNoState { + return err + } + isRemodel := remodelCtx != nil && remodelCtx.ForRemodeling() + + groundDeviceCtx, err := DeviceCtx(st, nil, nil) + if err != nil { + return fmt.Errorf("cannot identify the current model") + } + + // be extra paranoid when checking we are installing the right gadget + expectedGadgetSnap := groundDeviceCtx.Model().Gadget() + if isRemodel { + expectedGadgetSnap = remodelCtx.Model().Gadget() + } + if snapsup.InstanceName() != expectedGadgetSnap { + return fmt.Errorf("cannot apply gadget assets update from non-model gadget snap %q, expected %q snap", + snapsup.InstanceName(), expectedGadgetSnap) + } + + updateData, err := pendingGadgetInfo(snapsup) + if err != nil { + return err + } + + currentData, err := currentGadgetInfo(t.State(), groundDeviceCtx) if err != nil { return err } @@ -144,8 +132,16 @@ func (m *DeviceManager) doUpdateGadgetAssets(t *state.Task, _ *tomb.Tomb) error return fmt.Errorf("cannot prepare update rollback directory: %v", err) } + var updatePolicy gadget.UpdatePolicyFunc = nil + + if isRemodel { + // use the remodel policy which triggers an update of all + // structures + updatePolicy = gadget.RemodelUpdatePolicy + } + st.Unlock() - err = gadgetUpdate(*currentData, *updateData, snapRollbackDir, nil) + err = gadgetUpdate(*currentData, *updateData, snapRollbackDir, updatePolicy) st.Lock() if err != nil { if err == gadget.ErrNoUpdate { diff --git a/overlord/devicestate/handlers_remodel.go b/overlord/devicestate/handlers_remodel.go index c0e8dedaf5..5f1bde06e2 100644 --- a/overlord/devicestate/handlers_remodel.go +++ b/overlord/devicestate/handlers_remodel.go @@ -178,8 +178,7 @@ var ( Classic: false, } - // TODO: replace with gadget.IsCompatible() once we are ready - gadgetIsCompatible = func(current, new *gadget.Info) error { return nil } + gadgetIsCompatible = gadget.IsCompatible ) func checkGadgetRemodelCompatible(st *state.State, snapInfo, curInfo *snap.Info, snapf snap.Container, flags snapstate.Flags, deviceCtx snapstate.DeviceContext) error { diff --git a/overlord/ifacestate/handlers.go b/overlord/ifacestate/handlers.go index 8ea11532d6..98ee7465e4 100644 --- a/overlord/ifacestate/handlers.go +++ b/overlord/ifacestate/handlers.go @@ -465,7 +465,10 @@ func (m *InterfaceManager) doConnect(task *state.Task, _ *tomb.Tomb) error { if err != nil { return err } - policyChecker = autochecker.check + policyChecker = func(plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) (bool, error) { + ok, _, err := autochecker.check(plug, slot) + return ok, err + } } else { policyCheck, err := newConnectChecker(st, deviceCtx) if err != nil { diff --git a/overlord/ifacestate/helpers.go b/overlord/ifacestate/helpers.go index 9192642b94..a083fdcd1e 100644 --- a/overlord/ifacestate/helpers.go +++ b/overlord/ifacestate/helpers.go @@ -269,13 +269,20 @@ func (m *InterfaceManager) reloadConnections(snapName string) ([]string, error) continue } - // Some versions of snapd may have left stray connections that don't - // have the corresponding plug or slot anymore. Before we choose how to - // deal with this data we want to silently ignore that error not to - // worry the users. plugInfo := m.repo.Plug(connRef.PlugRef.Snap, connRef.PlugRef.Name) slotInfo := m.repo.Slot(connRef.SlotRef.Snap, connRef.SlotRef.Name) + + // The connection refers to a plug or slot that doesn't exist anymore, e.g. because of a refresh + // to a new snap revision that doesn't have the given plug/slot. if plugInfo == nil || slotInfo == nil { + // automatic connection can simply be removed (it will be re-created automatically if needed) + // as long as it wasn't disconnected manually; note that undesired flag is taken care of at + // the beginning of the loop. + if connState.Auto && !connState.ByGadget && connState.Interface != "core-support" { + delete(conns, connId) + connStateChanged = true + } + // otherwise keep it and silently ignore, e.g. in case of a revert. continue } @@ -486,7 +493,7 @@ func (c *autoConnectChecker) snapDeclaration(snapID string) (*asserts.SnapDeclar return snapDecl, nil } -func (c *autoConnectChecker) check(plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) (bool, error) { +func (c *autoConnectChecker) check(plug *interfaces.ConnectedPlug, slot *interfaces.ConnectedSlot) (bool, interfaces.SideArity, error) { modelAs := c.deviceCtx.Model() var storeAs *asserts.Store @@ -494,7 +501,7 @@ func (c *autoConnectChecker) check(plug *interfaces.ConnectedPlug, slot *interfa var err error storeAs, err = assertstate.Store(c.st, modelAs.Store()) if err != nil && !asserts.IsNotFound(err) { - return false, err + return false, nil, err } } @@ -504,7 +511,7 @@ func (c *autoConnectChecker) check(plug *interfaces.ConnectedPlug, slot *interfa plugDecl, err = c.snapDeclaration(plug.Snap().SnapID) if err != nil { logger.Noticef("error: cannot find snap declaration for %q: %v", plug.Snap().InstanceName(), err) - return false, nil + return false, nil, nil } } @@ -514,7 +521,7 @@ func (c *autoConnectChecker) check(plug *interfaces.ConnectedPlug, slot *interfa slotDecl, err = c.snapDeclaration(slot.Snap().SnapID) if err != nil { logger.Noticef("error: cannot find snap declaration for %q: %v", slot.Snap().InstanceName(), err) - return false, nil + return false, nil, nil } } @@ -529,22 +536,29 @@ func (c *autoConnectChecker) check(plug *interfaces.ConnectedPlug, slot *interfa Store: storeAs, } - return ic.CheckAutoConnect() == nil, nil + arity, err := ic.CheckAutoConnect() + if err == nil { + return true, arity, nil + } + + return false, nil, nil } // filterUbuntuCoreSlots filters out any ubuntu-core slots, // if there are both ubuntu-core and core slots. This would occur // during a ubuntu-core -> core transition. -func filterUbuntuCoreSlots(candidates []*snap.SlotInfo) []*snap.SlotInfo { +func filterUbuntuCoreSlots(candidates []*snap.SlotInfo, arities []interfaces.SideArity) ([]*snap.SlotInfo, []interfaces.SideArity) { hasCore := false hasUbuntuCore := false var withoutUbuntuCore []*snap.SlotInfo + var withoutUbuntuCoreArities []interfaces.SideArity for i, candSlot := range candidates { switch candSlot.Snap.InstanceName() { case "ubuntu-core": if !hasUbuntuCore { hasUbuntuCore = true withoutUbuntuCore = append(withoutUbuntuCore, candidates[:i]...) + withoutUbuntuCoreArities = append(withoutUbuntuCoreArities, arities[:i]...) } case "core": hasCore = true @@ -552,13 +566,15 @@ func filterUbuntuCoreSlots(candidates []*snap.SlotInfo) []*snap.SlotInfo { default: if hasUbuntuCore { withoutUbuntuCore = append(withoutUbuntuCore, candSlot) + withoutUbuntuCoreArities = append(withoutUbuntuCoreArities, arities[i]) } } } if hasCore && hasUbuntuCore { candidates = withoutUbuntuCore + arities = withoutUbuntuCoreArities } - return candidates + return candidates, arities } // addAutoConnections adds to newconns any applicable auto-connections @@ -569,7 +585,7 @@ func filterUbuntuCoreSlots(candidates []*snap.SlotInfo) []*snap.SlotInfo { // to handle checkAutoconnectConflicts errors. func (c *autoConnectChecker) addAutoConnections(newconns map[string]*interfaces.ConnRef, plugs []*snap.PlugInfo, filter func([]*snap.SlotInfo) []*snap.SlotInfo, conns map[string]*connState, cannotAutoConnectLog func(plug *snap.PlugInfo, candRefs []string) string, conflictError func(*state.Retry, error) error) error { for _, plug := range plugs { - candSlots := c.repo.AutoConnectCandidateSlots(plug.Snap.InstanceName(), plug.Name, c.check) + candSlots, arities := c.repo.AutoConnectCandidateSlots(plug.Snap.InstanceName(), plug.Name, c.check) if len(candSlots) == 0 { continue @@ -580,12 +596,18 @@ func (c *autoConnectChecker) addAutoConnections(newconns map[string]*interfaces. // providing the same interface. In that situation we // want to ignore any candidates in ubuntu-core and // simply go with those from the new core snap. - candSlots = filterUbuntuCoreSlots(candSlots) + candSlots, arities = filterUbuntuCoreSlots(candSlots, arities) applicable := candSlots // candidate arity check - if len(candSlots) != 1 { - applicable = nil + for _, arity := range arities { + if !arity.SlotsPerPlugAny() { + // ATM not any (*) => none or exactly one + if len(candSlots) != 1 { + applicable = nil + } + break + } } if filter != nil { diff --git a/overlord/ifacestate/ifacestate_test.go b/overlord/ifacestate/ifacestate_test.go index 12f00ed2c4..1b95a5dc69 100644 --- a/overlord/ifacestate/ifacestate_test.go +++ b/overlord/ifacestate/ifacestate_test.go @@ -1855,10 +1855,24 @@ version: 1 type: os ` +var ubuntuCoreSnapYaml2 = ` +name: ubuntu-core +version: 1 +type: os +slots: + test1: + interface: test1 + test2: + interface: test2 +` + var coreSnapYaml = ` name: core version: 1 type: os +slots: + unrelated: + interface: unrelated ` var sampleSnapYaml = ` @@ -1870,6 +1884,8 @@ apps: plugs: network: interface: network + unrelated: + interface: unrelated ` var sampleSnapYamlManyPlugs = ` @@ -1989,6 +2005,41 @@ hooks: disconnect-slot-slot: ` +var refreshedSnapYaml = ` +name: snap +version: 2 +apps: + app: + command: foo +plugs: + test2: + interface: test2 +` + +var refreshedSnapYaml2 = ` +name: snap +version: 2 +apps: + app: + command: foo +plugs: + test1: + interface: test1 + test2: + interface: test2 +` + +var slotSnapYaml = ` +name: snap2 +version: 2 +apps: + app: + command: bar +slots: + test2: + interface: test2 +` + // The auto-connect task will not auto-connect a plug that was previously // explicitly disconnected by the user. func (s *interfaceManagerSuite) TestDoSetupSnapSecurityHonorsUndesiredFlag(c *C) { @@ -3177,6 +3228,116 @@ func (s *interfaceManagerSuite) TestSetupProfilesUsesFreshSnapInfo(c *C) { c.Check(s.secBackend.SetupCalls[1].SnapInfo.Revision, Equals, coreSnapInfo.Revision) } +func (s *interfaceManagerSuite) TestSetupProfilesKeepsUndesiredConnection(c *C) { + undesired := true + byGadget := false + s.testAutoconnectionsRemovedForMissingPlugs(c, undesired, byGadget, map[string]interface{}{ + "snap:test1 ubuntu-core:test1": map[string]interface{}{"interface": "test1", "auto": true, "undesired": true}, + "snap:test2 ubuntu-core:test2": map[string]interface{}{"interface": "test2", "auto": true}, + }) +} + +func (s *interfaceManagerSuite) TestSetupProfilesRemovesMissingAutoconnectedPlugs(c *C) { + s.testAutoconnectionsRemovedForMissingPlugs(c, false, false, map[string]interface{}{ + "snap:test2 ubuntu-core:test2": map[string]interface{}{"interface": "test2", "auto": true}, + }) +} + +func (s *interfaceManagerSuite) TestSetupProfilesKeepsMissingGadgetAutoconnectedPlugs(c *C) { + undesired := false + byGadget := true + s.testAutoconnectionsRemovedForMissingPlugs(c, undesired, byGadget, map[string]interface{}{ + "snap:test1 ubuntu-core:test1": map[string]interface{}{"interface": "test1", "auto": true, "by-gadget": true}, + "snap:test2 ubuntu-core:test2": map[string]interface{}{"interface": "test2", "auto": true}, + }) +} + +func (s *interfaceManagerSuite) testAutoconnectionsRemovedForMissingPlugs(c *C, undesired, byGadget bool, expectedConns map[string]interface{}) { + s.MockModel(c, nil) + + // Mock the interface that will be used by the test + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test1"}, &ifacetest.TestInterface{InterfaceName: "test2"}) + + // Put the OS and the sample snap in place. + _ = s.mockSnap(c, ubuntuCoreSnapYaml2) + newSnapInfo := s.mockSnap(c, refreshedSnapYaml) + + s.state.Lock() + s.state.Set("conns", map[string]interface{}{ + "snap:test1 ubuntu-core:test1": map[string]interface{}{"interface": "test1", "auto": true, "undesired": undesired, "by-gadget": byGadget}, + }) + s.state.Unlock() + + _ = s.manager(c) + + // Run the setup-profiles task for the new revision and let it finish. + change := s.addSetupSnapSecurityChange(c, &snapstate.SnapSetup{ + SideInfo: &snap.SideInfo{ + RealName: newSnapInfo.SnapName(), + Revision: newSnapInfo.Revision, + }, + }) + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + + // Ensure that the task succeeded. + c.Assert(change.Err(), IsNil) + c.Check(change.Status(), Equals, state.DoneStatus) + + // Verify that old connection is gone and new one got connected + var conns map[string]interface{} + c.Assert(s.state.Get("conns", &conns), IsNil) + c.Check(conns, DeepEquals, expectedConns) +} + +func (s *interfaceManagerSuite) TestSetupProfilesRemovesMissingAutoconnectedSlots(c *C) { + s.testAutoconnectionsRemovedForMissingSlots(c, map[string]interface{}{ + "snap:test2 snap2:test2": map[string]interface{}{"interface": "test2", "auto": true}, + }) +} + +func (s *interfaceManagerSuite) testAutoconnectionsRemovedForMissingSlots(c *C, expectedConns map[string]interface{}) { + s.MockModel(c, nil) + + // Mock the interface that will be used by the test + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "test1"}, &ifacetest.TestInterface{InterfaceName: "test2"}) + + // Put sample snaps in place. + newSnapInfo1 := s.mockSnap(c, refreshedSnapYaml2) + _ = s.mockSnap(c, slotSnapYaml) + + s.state.Lock() + s.state.Set("conns", map[string]interface{}{ + "snap:test1 snap2:test1": map[string]interface{}{"interface": "test1", "auto": true}, + }) + s.state.Unlock() + + _ = s.manager(c) + + // Run the setup-profiles task for the new revision and let it finish. + change := s.addSetupSnapSecurityChange(c, &snapstate.SnapSetup{ + SideInfo: &snap.SideInfo{ + RealName: newSnapInfo1.SnapName(), + Revision: newSnapInfo1.Revision, + }, + }) + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + + // Ensure that the task succeeded. + c.Assert(change.Err(), IsNil) + c.Check(change.Status(), Equals, state.DoneStatus) + + // Verify that old connection is gone and new one got connected + var conns map[string]interface{} + c.Assert(s.state.Get("conns", &conns), IsNil) + c.Check(conns, DeepEquals, expectedConns) +} + // auto-connect needs to setup security for connected slots after autoconnection func (s *interfaceManagerSuite) TestAutoConnectSetupSecurityForConnectedSlots(c *C) { s.MockModel(c, nil) @@ -4423,6 +4584,8 @@ func (s *interfaceManagerSuite) TestManagerTransitionConnectionsCoreUndo(c *C) { // Test "core-support" connections that loop back to core is // renamed to match the rename of the plug. func (s *interfaceManagerSuite) TestCoreConnectionsRenamed(c *C) { + s.mockIfaces(c, &ifacetest.TestInterface{InterfaceName: "unrelated"}) + // Put state with old connection data. s.state.Lock() s.state.Set("conns", map[string]interface{}{ @@ -7061,3 +7224,195 @@ func (s *interfaceManagerSuite) TestTransitionConnectionsCoreMigration(c *C) { c.Assert(err, IsNil) c.Assert(repoConns, HasLen, 0) } + +func (s *interfaceManagerSuite) TestDoSetupSnapSecurityAutoConnectsDeclBasedAnySlotsPerPlugPlugSide(c *C) { + s.MockModel(c, nil) + + // the producer snap + s.MockSnapDecl(c, "theme1", "one-publisher", nil) + + // 2nd producer snap + s.MockSnapDecl(c, "theme2", "one-publisher", nil) + + // the consumer + s.MockSnapDecl(c, "theme-consumer", "one-publisher", map[string]interface{}{ + "format": "1", + "plugs": map[string]interface{}{ + "content": map[string]interface{}{ + "allow-auto-connection": map[string]interface{}{ + "slots-per-plug": "*", + }, + }, + }, + }) + + check := func(conns map[string]interface{}, repoConns []*interfaces.ConnRef) { + c.Check(repoConns, HasLen, 2) + + c.Check(conns, DeepEquals, map[string]interface{}{ + "theme-consumer:plug theme1:slot": map[string]interface{}{ + "auto": true, + "interface": "content", + "plug-static": map[string]interface{}{"content": "themes"}, + "slot-static": map[string]interface{}{"content": "themes"}, + }, + "theme-consumer:plug theme2:slot": map[string]interface{}{ + "auto": true, + "interface": "content", + "plug-static": map[string]interface{}{"content": "themes"}, + "slot-static": map[string]interface{}{"content": "themes"}, + }, + }) + } + + s.testDoSetupSnapSecurityAutoConnectsDeclBasedAnySlotsPerPlug(c, check) +} + +func (s *interfaceManagerSuite) testDoSetupSnapSecurityAutoConnectsDeclBasedAnySlotsPerPlug(c *C, check func(map[string]interface{}, []*interfaces.ConnRef)) { + const theme1Yaml = ` +name: theme1 +version: 1 +slots: + slot: + interface: content + content: themes +` + s.mockSnap(c, theme1Yaml) + const theme2Yaml = ` +name: theme2 +version: 1 +slots: + slot: + interface: content + content: themes +` + s.mockSnap(c, theme2Yaml) + + mgr := s.manager(c) + + const themeConsumerYaml = ` +name: theme-consumer +version: 1 +plugs: + plug: + interface: content + content: themes +` + snapInfo := s.mockSnap(c, themeConsumerYaml) + + // Run the setup-snap-security task and let it finish. + change := s.addSetupSnapSecurityChange(c, &snapstate.SnapSetup{ + SideInfo: &snap.SideInfo{ + RealName: snapInfo.SnapName(), + SnapID: snapInfo.SnapID, + Revision: snapInfo.Revision, + }, + }) + s.settle(c) + + s.state.Lock() + defer s.state.Unlock() + + // Ensure that the task succeeded. + c.Assert(change.Status(), Equals, state.DoneStatus) + + var conns map[string]interface{} + _ = s.state.Get("conns", &conns) + + repo := mgr.Repository() + plug := repo.Plug("theme-consumer", "plug") + c.Assert(plug, Not(IsNil)) + + check(conns, repo.Interfaces().Connections) +} + +func (s *interfaceManagerSuite) TestDoSetupSnapSecurityAutoConnectsDeclBasedAnySlotsPerPlugSlotSide(c *C) { + s.MockModel(c, nil) + + // the producer snap + s.MockSnapDecl(c, "theme1", "one-publisher", map[string]interface{}{ + "format": "1", + "slots": map[string]interface{}{ + "content": map[string]interface{}{ + "allow-auto-connection": map[string]interface{}{ + "slots-per-plug": "*", + }, + }, + }, + }) + + // 2nd producer snap + s.MockSnapDecl(c, "theme2", "one-publisher", map[string]interface{}{ + "format": "1", + "slots": map[string]interface{}{ + "content": map[string]interface{}{ + "allow-auto-connection": map[string]interface{}{ + "slots-per-plug": "*", + }, + }, + }, + }) + + // the consumer + s.MockSnapDecl(c, "theme-consumer", "one-publisher", nil) + + check := func(conns map[string]interface{}, repoConns []*interfaces.ConnRef) { + c.Check(repoConns, HasLen, 2) + + c.Check(conns, DeepEquals, map[string]interface{}{ + "theme-consumer:plug theme1:slot": map[string]interface{}{ + "auto": true, + "interface": "content", + "plug-static": map[string]interface{}{"content": "themes"}, + "slot-static": map[string]interface{}{"content": "themes"}, + }, + "theme-consumer:plug theme2:slot": map[string]interface{}{ + "auto": true, + "interface": "content", + "plug-static": map[string]interface{}{"content": "themes"}, + "slot-static": map[string]interface{}{"content": "themes"}, + }, + }) + } + + s.testDoSetupSnapSecurityAutoConnectsDeclBasedAnySlotsPerPlug(c, check) +} + +func (s *interfaceManagerSuite) TestDoSetupSnapSecurityAutoConnectsDeclBasedAnySlotsPerPlugAmbiguity(c *C) { + s.MockModel(c, nil) + + // the producer snap + s.MockSnapDecl(c, "theme1", "one-publisher", map[string]interface{}{ + "format": "1", + "slots": map[string]interface{}{ + "content": map[string]interface{}{ + "allow-auto-connection": map[string]interface{}{ + "slots-per-plug": "*", + }, + }, + }, + }) + + // 2nd producer snap + s.MockSnapDecl(c, "theme2", "one-publisher", map[string]interface{}{ + "format": "1", + "slots": map[string]interface{}{ + "content": map[string]interface{}{ + "allow-auto-connection": map[string]interface{}{ + "slots-per-plug": "1", + }, + }, + }, + }) + + // the consumer + s.MockSnapDecl(c, "theme-consumer", "one-publisher", nil) + + check := func(conns map[string]interface{}, repoConns []*interfaces.ConnRef) { + // slots-per-plug were ambigous, nothing was connected + c.Check(repoConns, HasLen, 0) + c.Check(conns, HasLen, 0) + } + + s.testDoSetupSnapSecurityAutoConnectsDeclBasedAnySlotsPerPlug(c, check) +} diff --git a/overlord/managers_test.go b/overlord/managers_test.go index 820fb57446..d2771d46fc 100644 --- a/overlord/managers_test.go +++ b/overlord/managers_test.go @@ -25,6 +25,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -38,6 +39,7 @@ import ( "time" . "gopkg.in/check.v1" + "gopkg.in/tomb.v2" "gopkg.in/yaml.v2" "github.com/snapcore/snapd/asserts" @@ -47,6 +49,7 @@ import ( "github.com/snapcore/snapd/bootloader/bootloadertest" "github.com/snapcore/snapd/client" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/gadget" "github.com/snapcore/snapd/interfaces" "github.com/snapcore/snapd/osutil" "github.com/snapcore/snapd/overlord" @@ -77,7 +80,7 @@ type automaticSnapshotCall struct { Flags *snapshotbackend.Flags } -type mgrsSuite struct { +type baseMgrsSuite struct { testutil.BaseTest tempdir string @@ -131,7 +134,7 @@ func verifyLastTasksetIsRerefresh(c *C, tts []*state.TaskSet) { c.Check(ts.Tasks()[0].Kind(), Equals, "check-rerefresh") } -func (s *mgrsSuite) SetUpTest(c *C) { +func (s *baseMgrsSuite) SetUpTest(c *C) { s.BaseTest.SetUpTest(c) s.tempdir = c.MkDir() @@ -303,6 +306,19 @@ func (s *mgrsSuite) SetUpTest(c *C) { c.Assert(assertstate.Add(st, a6), IsNil) c.Assert(s.storeSigning.Add(a6), IsNil) + // add pc snap declaration + headers = map[string]interface{}{ + "series": "16", + "snap-name": "pc", + "publisher-id": "can0nical", + "timestamp": time.Now().Format(time.RFC3339), + } + headers["snap-id"] = fakeSnapID(headers["snap-name"].(string)) + a7, err := s.storeSigning.Sign(asserts.SnapDeclarationType, headers, nil, "") + c.Assert(err, IsNil) + c.Assert(assertstate.Add(st, a7), IsNil) + c.Assert(s.storeSigning.Add(a7), IsNil) + // add core itself snapstate.Set(st, "core", &snapstate.SnapState{ Active: true, @@ -315,20 +331,30 @@ func (s *mgrsSuite) SetUpTest(c *C) { Required: true, }, }) + // don't actually try to talk to the store on snapstate.Ensure // needs doing after the call to devicestate.Manager (which happens in overlord.New) snapstate.CanAutoRefresh = nil st.Set("refresh-privacy-key", "privacy-key") + + // For triggering errors + erroringHandler := func(task *state.Task, _ *tomb.Tomb) error { + return errors.New("error out") + } + s.o.TaskRunner().AddHandler("error-trigger", erroringHandler, nil) +} + +type mgrsSuite struct { + baseMgrsSuite } var settleTimeout = 15 * time.Second -func makeTestSnap(c *C, snapYamlContent string) string { +func makeTestSnapWithFiles(c *C, snapYamlContent string, files [][]string) string { info, err := snap.InfoFromSnapYaml([]byte(snapYamlContent)) c.Assert(err, IsNil) - var files [][]string for _, app := range info.Apps { // files is a list of (filename, content) files = append(files, []string{app.Command, ""}) @@ -337,6 +363,10 @@ func makeTestSnap(c *C, snapYamlContent string) string { return snaptest.MakeTestSnapWithFiles(c, snapYamlContent, files) } +func makeTestSnap(c *C, snapYamlContent string) string { + return makeTestSnapWithFiles(c, snapYamlContent, nil) +} + func (s *mgrsSuite) TestHappyLocalInstall(c *C) { snapYamlContent := `name: foo apps: @@ -463,7 +493,7 @@ const ( var fooSnapID = fakeSnapID("foo") -func (s *mgrsSuite) prereqSnapAssertions(c *C, extraHeaders ...map[string]interface{}) *asserts.SnapDeclaration { +func (s *baseMgrsSuite) prereqSnapAssertions(c *C, extraHeaders ...map[string]interface{}) *asserts.SnapDeclaration { if len(extraHeaders) == 0 { extraHeaders = []map[string]interface{}{{}} } @@ -488,11 +518,11 @@ func (s *mgrsSuite) prereqSnapAssertions(c *C, extraHeaders ...map[string]interf return snapDecl } -func (s *mgrsSuite) makeStoreTestSnap(c *C, snapYaml string, revno string) (path, digest string) { +func (s *baseMgrsSuite) makeStoreTestSnapWithFiles(c *C, snapYaml string, revno string, files [][]string) (path, digest string) { info, err := snap.InfoFromSnapYaml([]byte(snapYaml)) c.Assert(err, IsNil) - snapPath := makeTestSnap(c, snapYaml) + snapPath := makeTestSnapWithFiles(c, snapYaml, files) snapDigest, size, err := asserts.SnapFileSHA3_384(snapPath) c.Assert(err, IsNil) @@ -513,7 +543,11 @@ func (s *mgrsSuite) makeStoreTestSnap(c *C, snapYaml string, revno string) (path return snapPath, snapDigest } -func (s *mgrsSuite) pathFor(name, revno string) string { +func (s *baseMgrsSuite) makeStoreTestSnap(c *C, snapYaml string, revno string) (path, digest string) { + return s.makeStoreTestSnapWithFiles(c, snapYaml, revno, nil) +} + +func (s *baseMgrsSuite) pathFor(name, revno string) string { if revno == s.serveRevision[name] { return s.serveSnapPath[name] } @@ -525,7 +559,7 @@ func (s *mgrsSuite) pathFor(name, revno string) string { return "/not/found" } -func (s *mgrsSuite) newestThatCanRead(name string, epoch snap.Epoch) (info *snap.Info, rev string) { +func (s *baseMgrsSuite) newestThatCanRead(name string, epoch snap.Epoch) (info *snap.Info, rev string) { if s.serveSnapPath[name] == "" { return nil, "" } @@ -553,7 +587,7 @@ func (s *mgrsSuite) newestThatCanRead(name string, epoch snap.Epoch) (info *snap } } -func (s *mgrsSuite) mockStore(c *C) *httptest.Server { +func (s *baseMgrsSuite) mockStore(c *C) *httptest.Server { var baseURL *url.URL fillHit := func(hitTemplate, revno string, info *snap.Info) string { epochBuf, err := json.Marshal(info.Epoch) @@ -747,7 +781,7 @@ func (s *mgrsSuite) mockStore(c *C) *httptest.Server { // serveSnap starts serving the snap at snapPath, moving the current // one onto the list of previous ones if already set. -func (s *mgrsSuite) serveSnap(snapPath, revno string) { +func (s *baseMgrsSuite) serveSnap(snapPath, revno string) { snapf, err := snap.Open(snapPath) if err != nil { panic(err) @@ -1595,10 +1629,9 @@ type: os c.Assert(err, IsNil) c.Assert(chg.Status(), Equals, state.DoneStatus, Commentf("install-snap change failed with: %v", chg.Err())) - } -func (s *mgrsSuite) mockSuccessfulReboot(c *C, bloader *bootloadertest.MockBootloader) { +func (s *baseMgrsSuite) mockSuccessfulReboot(c *C, bloader *bootloadertest.MockBootloader) { st := s.o.State() restarting, restartType := st.Restarting() c.Assert(restarting, Equals, true, Commentf("mockSuccessfulReboot called when there was no pending restart")) @@ -1613,7 +1646,7 @@ func (s *mgrsSuite) mockSuccessfulReboot(c *C, bloader *bootloadertest.MockBootl c.Assert(err, IsNil) } -func (s *mgrsSuite) mockRollbackAcrossReboot(c *C, bloader *bootloadertest.MockBootloader) { +func (s *baseMgrsSuite) mockRollbackAcrossReboot(c *C, bloader *bootloadertest.MockBootloader) { st := s.o.State() restarting, restartType := st.Restarting() c.Assert(restarting, Equals, true, Commentf("mockRollbackAcrossReboot called when there was no pending restart")) @@ -1719,6 +1752,114 @@ type: kernel` c.Assert(chg.Status(), Equals, state.DoneStatus, Commentf("install-snap change failed with: %v", chg.Err())) } +func (s *mgrsSuite) TestInstallKernelSnapUndoUpdatesBootloaderEnv(c *C) { + bloader := bootloadertest.Mock("mock", c.MkDir()) + bootloader.Force(bloader) + defer bootloader.Force(nil) + + restore := release.MockOnClassic(false) + defer restore() + + model := s.brands.Model("my-brand", "my-model", modelDefaults) + + const packageKernel = ` +name: pc-kernel +version: 4.0-1 +type: kernel` + + files := [][]string{ + {"kernel.img", "I'm a kernel"}, + {"initrd.img", "...and I'm an initrd"}, + {"meta/kernel.yaml", "version: 4.2"}, + } + snapPath := snaptest.MakeTestSnapWithFiles(c, packageKernel, files) + + st := s.o.State() + st.Lock() + defer st.Unlock() + + // pretend we have core18/pc-kernel + bloader.BootVars = map[string]string{ + "snap_core": "core18_2.snap", + "snap_kernel": "pc-kernel_123.snap", + "snap_mode": "", + } + si1 := &snap.SideInfo{RealName: "pc-kernel", Revision: snap.R(123)} + snapstate.Set(st, "pc-kernel", &snapstate.SnapState{ + SnapType: "kernel", + Active: true, + Sequence: []*snap.SideInfo{si1}, + Current: si1.Revision, + }) + snaptest.MockSnapWithFiles(c, packageKernel, si1, [][]string{ + {"meta/kernel.yaml", ""}, + }) + si2 := &snap.SideInfo{RealName: "core18", Revision: snap.R(2)} + snapstate.Set(st, "core18", &snapstate.SnapState{ + SnapType: "base", + Active: true, + Sequence: []*snap.SideInfo{si2}, + Current: si2.Revision, + }) + + // setup model assertion + assertstatetest.AddMany(st, s.brands.AccountsAndKeys("my-brand")...) + devicestatetest.SetDevice(st, &auth.DeviceState{ + Brand: "my-brand", + Model: "my-model", + Serial: "serialserialserial", + }) + err := assertstate.Add(st, model) + c.Assert(err, IsNil) + + ts, _, err := snapstate.InstallPath(st, &snap.SideInfo{RealName: "pc-kernel"}, snapPath, "", "", snapstate.Flags{}) + c.Assert(err, IsNil) + + terr := st.NewTask("error-trigger", "provoking total undo") + terr.WaitFor(ts.Tasks()[len(ts.Tasks())-1]) + ts.AddTask(terr) + chg := st.NewChange("install-snap", "...") + chg.AddAll(ts) + + // run, this will trigger a wait for the restart + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_core": "core18_2.snap", + "snap_kernel": "pc-kernel_123.snap", + "snap_try_kernel": "pc-kernel_x1.snap", + "snap_mode": "try", + }) + + // we are in restarting state and the change is not done yet + restarting, _ := st.Restarting() + c.Check(restarting, Equals, true) + c.Check(chg.Status(), Equals, state.DoingStatus) + // pretend we restarted + s.mockSuccessfulReboot(c, bloader) + + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + c.Assert(chg.Status(), Equals, state.ErrorStatus) + + // and we undo the bootvars and trigger a reboot + c.Check(bloader.BootVars, DeepEquals, map[string]string{ + "snap_core": "core18_2.snap", + "snap_try_core": "", + "snap_try_kernel": "pc-kernel_123.snap", + "snap_kernel": "pc-kernel_x1.snap", + "snap_mode": "try", + }) + restarting, _ = st.Restarting() + c.Check(restarting, Equals, true) +} + func (s *mgrsSuite) installLocalTestSnap(c *C, snapYamlContent string) *snap.Info { st := s.o.State() @@ -3244,10 +3385,19 @@ func validateDownloadCheckTasks(c *C, tasks []*state.Task, name, revno, channel return i } -func validateInstallTasks(c *C, tasks []*state.Task, name, revno string) int { +const ( + noConfigure = 1 << iota + isGadget +) + +func validateInstallTasks(c *C, tasks []*state.Task, name, revno string, flags int) int { var i int c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Mount snap "%s" (%s)`, name, revno)) i++ + if flags&isGadget != 0 { + c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Update assets from gadget "%s" (%s)`, name, revno)) + i++ + } c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Copy snap "%s" data`, name)) i++ c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Setup snap "%s" (%s) security profiles`, name, revno)) @@ -3264,14 +3414,16 @@ func validateInstallTasks(c *C, tasks []*state.Task, name, revno string) int { i++ c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Start snap "%s" (%s) services`, name, revno)) i++ - c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Run configure hook of "%s" snap if present`, name)) - i++ + if flags&noConfigure == 0 { + c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Run configure hook of "%s" snap if present`, name)) + i++ + } c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Run health check of "%s" snap`, name)) i++ return i } -func validateRefreshTasks(c *C, tasks []*state.Task, name, revno string) int { +func validateRefreshTasks(c *C, tasks []*state.Task, name, revno string, flags int) int { var i int c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Mount snap "%s" (%s)`, name, revno)) i++ @@ -3283,6 +3435,11 @@ func validateRefreshTasks(c *C, tasks []*state.Task, name, revno string) int { i++ c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Make current revision for snap "%s" unavailable`, name)) i++ + if flags&isGadget != 0 { + c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Update assets from gadget %q (%s)`, name, revno)) + i++ + + } c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Copy snap "%s" data`, name)) i++ c.Assert(tasks[i].Summary(), Equals, fmt.Sprintf(`Setup snap "%s" (%s) security profiles`, name, revno)) @@ -3408,7 +3565,7 @@ func (s *mgrsSuite) TestRemodelRequiredSnapsAdded(c *C) { } // then all installs in sequential order for _, name := range []string{"foo", "bar", "baz"} { - i += validateInstallTasks(c, tasks[i:], name, "1") + i += validateInstallTasks(c, tasks[i:], name, "1", 0) } // ensure that we only have the tasks we checked (plus the one // extra "set-model" task) @@ -3533,49 +3690,77 @@ type: base` }) chg, err := devicestate.Remodel(st, newModel) - c.Assert(err, ErrorMatches, "cannot remodel to different bases yet") + c.Assert(err, ErrorMatches, "cannot remodel from core to bases yet") c.Assert(chg, IsNil) } -func (s *mgrsSuite) TestRemodelSwitchKernelTrack(c *C) { +func (ms *mgrsSuite) TestRemodelSwitchToDifferentBase(c *C) { bloader := bootloadertest.Mock("mock", c.MkDir()) - bloader.SetBootKernel("pc-kernel_1.snap") - bloader.SetBootBase("core_1.snap") bootloader.Force(bloader) defer bootloader.Force(nil) + bloader.SetBootVars(map[string]string{ + "snap_mode": "", + "snap_core": "core18_1.snap", + "snap_kernel": "pc-kernel_1.snap", + }) restore := release.MockOnClassic(false) defer restore() - mockServer := s.mockStore(c) + mockServer := ms.mockStore(c) defer mockServer.Close() - st := s.o.State() + st := ms.o.State() st.Lock() defer st.Unlock() - si := &snap.SideInfo{RealName: "pc-kernel", SnapID: fakeSnapID("pc-kernel"), Revision: snap.R(1)} - snapstate.Set(st, "pc-kernel", &snapstate.SnapState{ + si := &snap.SideInfo{RealName: "core18", SnapID: fakeSnapID("core18"), Revision: snap.R(1)} + snapstate.Set(st, "core18", &snapstate.SnapState{ Active: true, Sequence: []*snap.SideInfo{si}, Current: snap.R(1), - SnapType: "kernel", + SnapType: "base", + }) + si2 := &snap.SideInfo{RealName: "pc", SnapID: fakeSnapID("pc"), Revision: snap.R(1)} + gadgetSnapYaml := "name: pc\nversion: 1.0\ntype: gadget" + snapstate.Set(st, "pc", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si2}, + Current: snap.R(1), + SnapType: "gadget", + }) + gadgetYaml := ` +volumes: + volume-id: + bootloader: grub +` + snaptest.MockSnapWithFiles(c, gadgetSnapYaml, si2, [][]string{ + {"meta/gadget.yaml", gadgetYaml}, }) - const kernelYaml = `name: pc-kernel -type: kernel -version: 2.0` - snapPath, _ := s.makeStoreTestSnap(c, kernelYaml, "2") - s.serveSnap(snapPath, "2") + // add "core20" snap to fake store + const core20Yaml = `name: core20 +type: base +version: 20.04` + ms.prereqSnapAssertions(c, map[string]interface{}{ + "snap-name": "core20", + "publisher-id": "can0nical", + }) + snapPath, _ := ms.makeStoreTestSnap(c, core20Yaml, "2") + ms.serveSnap(snapPath, "2") - s.prereqSnapAssertions(c, map[string]interface{}{ + // add "foo" snap to fake store + ms.prereqSnapAssertions(c, map[string]interface{}{ "snap-name": "foo", }) - snapPath, _ = s.makeStoreTestSnap(c, `{name: "foo", version: 1.0}`, "1") - s.serveSnap(snapPath, "1") + snapPath, _ = ms.makeStoreTestSnap(c, `{name: "foo", version: 1.0}`, "1") + ms.serveSnap(snapPath, "1") // create/set custom model assertion - model := s.brands.Model("can0nical", "my-model", modelDefaults) + model := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "base": "core18", + }) + // setup model assertion devicestatetest.SetDevice(st, &auth.DeviceState{ Brand: "can0nical", @@ -3586,8 +3771,8 @@ version: 2.0` c.Assert(err, IsNil) // create a new model - newModel := s.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ - "kernel": "pc-kernel=18", + newModel := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "base": "core20", "revision": "1", "required-snaps": []interface{}{"foo"}, }) @@ -3596,21 +3781,39 @@ version: 2.0` c.Assert(err, IsNil) st.Unlock() - err = s.o.Settle(settleTimeout) + err = ms.o.Settle(settleTimeout) st.Lock() c.Assert(err, IsNil) + c.Assert(chg.Err(), IsNil) - // system waits for a restart because of the new kernel + // system waits for a restart because of the new base t := findKind(chg, "auto-connect") c.Assert(t, NotNil) c.Assert(t.Status(), Equals, state.DoingStatus) - // simulate successful restart happened - s.mockSuccessfulReboot(c, bloader) + // check that the boot vars got updated as expected + bvars, err := bloader.GetBootVars("snap_mode", "snap_core", "snap_try_core", "snap_kernel", "snap_try_kernel") + c.Assert(err, IsNil) + c.Assert(bvars, DeepEquals, map[string]string{ + "snap_mode": "try", + "snap_core": "core18_1.snap", + "snap_try_core": "core20_2.snap", + "snap_kernel": "pc-kernel_1.snap", + "snap_try_kernel": "", + }) + + // simulate successful restart happened and that the bootvars + // got updated + state.MockRestarting(st, state.RestartUnset) + bloader.SetBootVars(map[string]string{ + "snap_mode": "", + "snap_core": "core20_2.snap", + "snap_kernel": "pc-kernel_1.snap", + }) // continue st.Unlock() - err = s.o.Settle(settleTimeout) + err = ms.o.Settle(settleTimeout) st.Lock() c.Assert(err, IsNil) @@ -3622,22 +3825,27 @@ version: 2.0` // first all downloads/checks in sequential order var i int - i += validateDownloadCheckTasks(c, tasks[i:], "pc-kernel", "2", "18") + i += validateDownloadCheckTasks(c, tasks[i:], "core20", "2", "stable") i += validateDownloadCheckTasks(c, tasks[i:], "foo", "1", "stable") // then all installs in sequential order - i += validateRefreshTasks(c, tasks[i:], "pc-kernel", "2") - i += validateInstallTasks(c, tasks[i:], "foo", "1") + i += validateInstallTasks(c, tasks[i:], "core20", "2", noConfigure) + i += validateInstallTasks(c, tasks[i:], "foo", "1", 0) // ensure that we only have the tasks we checked (plus the one // extra "set-model" task) c.Assert(tasks, HasLen, i+1) } -func (ms *mgrsSuite) TestRemodelSwitchToDifferentKernel(c *C) { +func (ms *mgrsSuite) TestRemodelSwitchToDifferentBaseUndo(c *C) { bloader := bootloadertest.Mock("mock", c.MkDir()) bootloader.Force(bloader) defer bootloader.Force(nil) + bloader.SetBootVars(map[string]string{ + "snap_mode": "", + "snap_core": "core18_1.snap", + "snap_kernel": "pc-kernel_1.snap", + }) restore := release.MockOnClassic(false) defer restore() @@ -3649,18 +3857,157 @@ func (ms *mgrsSuite) TestRemodelSwitchToDifferentKernel(c *C) { st.Lock() defer st.Unlock() - si := &snap.SideInfo{RealName: "pc-kernel", SnapID: fakeSnapID("pc-kernel"), Revision: snap.R(1)} - snapstate.Set(st, "pc-kernel", &snapstate.SnapState{ + si := &snap.SideInfo{RealName: "core18", SnapID: fakeSnapID("core18"), Revision: snap.R(1)} + snapstate.Set(st, "core18", &snapstate.SnapState{ Active: true, Sequence: []*snap.SideInfo{si}, Current: snap.R(1), - SnapType: "kernel", + SnapType: "base", }) + snaptest.MockSnapWithFiles(c, "name: core18\ntype: base\nversion: 1.0", si, nil) + + si2 := &snap.SideInfo{RealName: "pc", SnapID: fakeSnapID("pc"), Revision: snap.R(1)} + gadgetSnapYaml := "name: pc\nversion: 1.0\ntype: gadget" + snapstate.Set(st, "pc", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si2}, + Current: snap.R(1), + SnapType: "gadget", + }) + gadgetYaml := ` +volumes: + volume-id: + bootloader: grub +` + snaptest.MockSnapWithFiles(c, gadgetSnapYaml, si2, [][]string{ + {"meta/gadget.yaml", gadgetYaml}, + }) + + // add "core20" snap to fake store + const core20Yaml = `name: core20 +type: base +version: 20.04` + ms.prereqSnapAssertions(c, map[string]interface{}{ + "snap-name": "core20", + "publisher-id": "can0nical", + }) + snapPath, _ := ms.makeStoreTestSnap(c, core20Yaml, "2") + ms.serveSnap(snapPath, "2") + + // add "foo" snap to fake store + ms.prereqSnapAssertions(c, map[string]interface{}{ + "snap-name": "foo", + }) + snapPath, _ = ms.makeStoreTestSnap(c, `{name: "foo", version: 1.0}`, "1") + ms.serveSnap(snapPath, "1") + + // create/set custom model assertion + model := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "base": "core18", + }) + + // setup model assertion + devicestatetest.SetDevice(st, &auth.DeviceState{ + Brand: "can0nical", + Model: "my-model", + Serial: "serialserialserial", + }) + err := assertstate.Add(st, model) + c.Assert(err, IsNil) + + // create a new model + newModel := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "base": "core20", + "revision": "1", + "required-snaps": []interface{}{"foo"}, + }) + + devicestate.InjectSetModelError(fmt.Errorf("boom")) + defer devicestate.InjectSetModelError(nil) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + err = ms.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Assert(chg.Err(), IsNil) + + // system waits for a restart because of the new base + t := findKind(chg, "auto-connect") + c.Assert(t, NotNil) + c.Assert(t.Status(), Equals, state.DoingStatus) + + // check that the boot vars got updated as expected + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_mode": "try", + "snap_core": "core18_1.snap", + "snap_try_core": "core20_2.snap", + "snap_kernel": "pc-kernel_1.snap", + }) + // simulate successful restart happened + ms.mockSuccessfulReboot(c, bloader) + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_mode": "", + "snap_core": "core20_2.snap", + "snap_try_core": "", + "snap_kernel": "pc-kernel_1.snap", + "snap_try_kernel": "", + }) + + // continue + st.Unlock() + err = ms.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + c.Assert(chg.Status(), Equals, state.ErrorStatus) + + // and we are in restarting state + restarting, restartType := st.Restarting() + c.Check(restarting, Equals, true) + c.Check(restartType, Equals, state.RestartSystem) + + // and the undo gave us our old kernel back + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_core": "core20_2.snap", + "snap_try_core": "core18_1.snap", + "snap_kernel": "pc-kernel_1.snap", + "snap_try_kernel": "", + "snap_mode": "try", + }) +} + +func (ms *mgrsSuite) TestRemodelSwitchToDifferentBaseUndoOnRollback(c *C) { + bloader := bootloadertest.Mock("mock", c.MkDir()) + bootloader.Force(bloader) + defer bootloader.Force(nil) bloader.SetBootVars(map[string]string{ "snap_mode": "", - "snap_core": "core_1.snap", + "snap_core": "core18_1.snap", "snap_kernel": "pc-kernel_1.snap", }) + + restore := release.MockOnClassic(false) + defer restore() + + mockServer := ms.mockStore(c) + defer mockServer.Close() + + st := ms.o.State() + st.Lock() + defer st.Unlock() + + si := &snap.SideInfo{RealName: "core18", SnapID: fakeSnapID("core18"), Revision: snap.R(1)} + snapstate.Set(st, "core18", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si}, + Current: snap.R(1), + SnapType: "base", + }) + snaptest.MockSnapWithFiles(c, "name: core18\ntype: base\nversion: 1.0", si, nil) + si2 := &snap.SideInfo{RealName: "pc", SnapID: fakeSnapID("pc"), Revision: snap.R(1)} gadgetSnapYaml := "name: pc\nversion: 1.0\ntype: gadget" snapstate.Set(st, "pc", &snapstate.SnapState{ @@ -3678,15 +4025,15 @@ volumes: {"meta/gadget.yaml", gadgetYaml}, }) - // add "brand-kernel" snap to fake store - const brandKernelYaml = `name: brand-kernel -type: kernel -version: 1.0` + // add "core20" snap to fake store + const core20Yaml = `name: core20 +type: base +version: 20.04` ms.prereqSnapAssertions(c, map[string]interface{}{ - "snap-name": "brand-kernel", + "snap-name": "core20", "publisher-id": "can0nical", }) - snapPath, _ := ms.makeStoreTestSnap(c, brandKernelYaml, "2") + snapPath, _ := ms.makeStoreTestSnap(c, core20Yaml, "2") ms.serveSnap(snapPath, "2") // add "foo" snap to fake store @@ -3697,7 +4044,9 @@ version: 1.0` ms.serveSnap(snapPath, "1") // create/set custom model assertion - model := ms.brands.Model("can0nical", "my-model", modelDefaults) + model := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "base": "core18", + }) // setup model assertion devicestatetest.SetDevice(st, &auth.DeviceState{ @@ -3710,6 +4059,212 @@ version: 1.0` // create a new model newModel := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "base": "core20", + "revision": "1", + "required-snaps": []interface{}{"foo"}, + }) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + err = ms.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Assert(chg.Err(), IsNil) + + // system waits for a restart because of the new base + t := findKind(chg, "auto-connect") + c.Assert(t, NotNil) + c.Assert(t.Status(), Equals, state.DoingStatus) + + // check that the boot vars got updated as expected + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_mode": "try", + "snap_core": "core18_1.snap", + "snap_try_core": "core20_2.snap", + "snap_kernel": "pc-kernel_1.snap", + }) + // simulate successful restart happened + ms.mockRollbackAcrossReboot(c, bloader) + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_mode": "", + "snap_core": "core18_1.snap", + "snap_try_core": "", + "snap_kernel": "pc-kernel_1.snap", + "snap_try_kernel": "", + }) + + // continue + st.Unlock() + err = ms.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + c.Assert(chg.Status(), Equals, state.ErrorStatus) + + // and we are *not* in restarting state + restarting, _ := st.Restarting() + c.Check(restarting, Equals, false) + // bootvars unchanged + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_mode": "", + "snap_core": "core18_1.snap", + "snap_try_core": "", + "snap_kernel": "pc-kernel_1.snap", + "snap_try_kernel": "", + }) +} + +type kernelSuite struct { + baseMgrsSuite + + bloader *bootloadertest.MockBootloader +} + +var _ = Suite(&kernelSuite{}) + +func (s *kernelSuite) SetUpTest(c *C) { + s.baseMgrsSuite.SetUpTest(c) + + s.bloader = bootloadertest.Mock("mock", c.MkDir()) + s.bloader.SetBootKernel("pc-kernel_1.snap") + s.bloader.SetBootBase("core_1.snap") + bootloader.Force(s.bloader) + s.AddCleanup(func() { bootloader.Force(nil) }) + + restore := release.MockOnClassic(false) + s.AddCleanup(restore) + mockServer := s.mockStore(c) + s.AddCleanup(mockServer.Close) + + st := s.o.State() + st.Lock() + defer st.Unlock() + + // create/set custom model assertion + model := s.brands.Model("can0nical", "my-model", modelDefaults) + devicestatetest.SetDevice(st, &auth.DeviceState{ + Brand: "can0nical", + Model: "my-model", + Serial: "serialserialserial", + }) + err := assertstate.Add(st, model) + c.Assert(err, IsNil) + + // make a mock "pc-kernel" kernel + si := &snap.SideInfo{RealName: "pc-kernel", SnapID: fakeSnapID("pc-kernel"), Revision: snap.R(1)} + snapstate.Set(st, "pc-kernel", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si}, + Current: snap.R(1), + SnapType: "kernel", + }) + snaptest.MockSnapWithFiles(c, "name: pc-kernel\ntype: kernel\nversion: 1.0", si, nil) + + // make a mock "pc" gadget + si2 := &snap.SideInfo{RealName: "pc", SnapID: fakeSnapID("pc"), Revision: snap.R(1)} + gadgetSnapYaml := "name: pc\nversion: 1.0\ntype: gadget" + snapstate.Set(st, "pc", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si2}, + Current: snap.R(1), + SnapType: "gadget", + }) + gadgetYaml := ` +volumes: + volume-id: + bootloader: grub +` + snaptest.MockSnapWithFiles(c, gadgetSnapYaml, si2, [][]string{ + {"meta/gadget.yaml", gadgetYaml}, + }) + + // add some store snaps + const kernelYaml = `name: pc-kernel +type: kernel +version: 2.0` + snapPath, _ := s.makeStoreTestSnap(c, kernelYaml, "2") + s.serveSnap(snapPath, "2") + + const brandKernelYaml = `name: brand-kernel +type: kernel +version: 1.0` + s.prereqSnapAssertions(c, map[string]interface{}{ + "snap-name": "brand-kernel", + "publisher-id": "can0nical", + }) + snapPath, _ = s.makeStoreTestSnap(c, brandKernelYaml, "2") + s.serveSnap(snapPath, "2") + + s.prereqSnapAssertions(c, map[string]interface{}{ + "snap-name": "foo", + }) + snapPath, _ = s.makeStoreTestSnap(c, `{name: "foo", version: 1.0}`, "1") + s.serveSnap(snapPath, "1") +} + +func (s *kernelSuite) TestRemodelSwitchKernelTrack(c *C) { + st := s.o.State() + st.Lock() + defer st.Unlock() + + // create a new model + newModel := s.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "kernel": "pc-kernel=18", + "revision": "1", + "required-snaps": []interface{}{"foo"}, + }) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + // system waits for a restart because of the new kernel + t := findKind(chg, "auto-connect") + c.Assert(t, NotNil) + c.Assert(t.Status(), Equals, state.DoingStatus) + + // simulate successful restart happened + s.mockSuccessfulReboot(c, s.bloader) + + // continue + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + c.Assert(chg.Status(), Equals, state.DoneStatus, Commentf("upgrade-snap change failed with: %v", chg.Err())) + + // ensure tasks were run in the right order + tasks := chg.Tasks() + sort.Sort(byReadyTime(tasks)) + + // first all downloads/checks in sequential order + var i int + i += validateDownloadCheckTasks(c, tasks[i:], "pc-kernel", "2", "18") + i += validateDownloadCheckTasks(c, tasks[i:], "foo", "1", "stable") + + // then all installs in sequential order + i += validateRefreshTasks(c, tasks[i:], "pc-kernel", "2", 0) + i += validateInstallTasks(c, tasks[i:], "foo", "1", 0) + + // ensure that we only have the tasks we checked (plus the one + // extra "set-model" task) + c.Assert(tasks, HasLen, i+1) +} + +func (ms *kernelSuite) TestRemodelSwitchToDifferentKernel(c *C) { + st := ms.o.State() + st.Lock() + defer st.Unlock() + + // create a new model + newModel := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ "kernel": "brand-kernel", "revision": "1", "required-snaps": []interface{}{"foo"}, @@ -3719,7 +4274,7 @@ version: 1.0` c.Assert(err, IsNil) st.Unlock() - // regular settleTimeout is not enough on arm buildds :/ + // regular settleTimeout is not enough on arm builds :/ err = ms.o.Settle(4 * settleTimeout) st.Lock() c.Assert(err, IsNil) @@ -3731,7 +4286,7 @@ version: 1.0` c.Assert(t.Status(), Equals, state.DoingStatus) // check that the system tries to boot the new brand kernel - c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + c.Assert(ms.bloader.BootVars, DeepEquals, map[string]string{ "snap_core": "core_1.snap", "snap_kernel": "pc-kernel_1.snap", "snap_try_kernel": "brand-kernel_2.snap", @@ -3739,9 +4294,9 @@ version: 1.0` }) // simulate successful system-restart bootenv updates (those // vars will be cleared by snapd on a restart) - ms.mockSuccessfulReboot(c, bloader) + ms.mockSuccessfulReboot(c, ms.bloader) // bootvars are as expected - c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + c.Assert(ms.bloader.BootVars, DeepEquals, map[string]string{ "snap_core": "core_1.snap", "snap_kernel": "brand-kernel_2.snap", "snap_try_core": "", @@ -3751,7 +4306,7 @@ version: 1.0` // continue st.Unlock() - // regular settleTimeout is not enough on arm buildds :/ + // regular settleTimeout is not enough on arm builds :/ err = ms.o.Settle(4 * settleTimeout) st.Lock() c.Assert(err, IsNil) @@ -3760,7 +4315,7 @@ version: 1.0` // bootvars are as expected (i.e. nothing has changed since this // test simulated that we booted successfully) - c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + c.Assert(ms.bloader.BootVars, DeepEquals, map[string]string{ "snap_core": "core_1.snap", "snap_kernel": "brand-kernel_2.snap", "snap_try_kernel": "", @@ -3778,8 +4333,8 @@ version: 1.0` i += validateDownloadCheckTasks(c, tasks[i:], "foo", "1", "stable") // then all installs in sequential order - i += validateInstallTasks(c, tasks[i:], "brand-kernel", "2") - i += validateInstallTasks(c, tasks[i:], "foo", "1") + i += validateInstallTasks(c, tasks[i:], "brand-kernel", "2", 0) + i += validateInstallTasks(c, tasks[i:], "foo", "1", 0) // ensure that we only have the tasks we checked (plus the one // extra "set-model" task) @@ -3793,6 +4348,121 @@ version: 1.0` } } +func (ms *kernelSuite) TestRemodelSwitchToDifferentKernelUndo(c *C) { + st := ms.o.State() + st.Lock() + defer st.Unlock() + + // create a new model + newModel := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "kernel": "brand-kernel", + "revision": "1", + "required-snaps": []interface{}{"foo"}, + }) + + devicestate.InjectSetModelError(fmt.Errorf("boom")) + defer devicestate.InjectSetModelError(nil) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + // regular settleTimeout is not enough on arm builds :/ + err = ms.o.Settle(4 * settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Assert(chg.Err(), IsNil) + + // system waits for a restart because of the new kernel + t := findKind(chg, "auto-connect") + c.Assert(t, NotNil) + c.Assert(t.Status(), Equals, state.DoingStatus) + + // simulate successful restart happened + ms.mockSuccessfulReboot(c, ms.bloader) + + // continue + st.Unlock() + // regular settleTimeout is not enough on arm builds :/ + err = ms.o.Settle(4 * settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + // the change was not successful + c.Assert(chg.Status(), Equals, state.ErrorStatus) + + // and we are in restarting state + restarting, restartType := st.Restarting() + c.Check(restarting, Equals, true) + c.Check(restartType, Equals, state.RestartSystem) + + // and the undo gave us our old kernel back + c.Assert(ms.bloader.BootVars, DeepEquals, map[string]string{ + "snap_core": "core_1.snap", + "snap_try_core": "", + "snap_try_kernel": "pc-kernel_1.snap", + "snap_kernel": "brand-kernel_2.snap", + "snap_mode": "try", + }) +} + +func (ms *kernelSuite) TestRemodelSwitchToDifferentKernelUndoOnRollback(c *C) { + st := ms.o.State() + st.Lock() + defer st.Unlock() + + // create a new model + newModel := ms.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "kernel": "brand-kernel", + "revision": "1", + "required-snaps": []interface{}{"foo"}, + }) + + devicestate.InjectSetModelError(fmt.Errorf("boom")) + defer devicestate.InjectSetModelError(nil) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + // regular settleTimeout is not enough on arm builds :/ + err = ms.o.Settle(4 * settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Assert(chg.Err(), IsNil) + + // system waits for a restart because of the new kernel + t := findKind(chg, "auto-connect") + c.Assert(t, NotNil) + c.Assert(t.Status(), Equals, state.DoingStatus) + + // simulate rollback of the kernel during reboot + ms.mockRollbackAcrossReboot(c, ms.bloader) + + // continue + st.Unlock() + // regular settleTimeout is not enough on arm builds :/ + err = ms.o.Settle(4 * settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + // the change was not successful + c.Assert(chg.Status(), Equals, state.ErrorStatus) + + // and we are *not* in restarting state + restarting, _ := st.Restarting() + c.Check(restarting, Equals, false) + + // and the undo gave us our old kernel back + c.Assert(ms.bloader.BootVars, DeepEquals, map[string]string{ + "snap_core": "core_1.snap", + "snap_try_core": "", + "snap_kernel": "pc-kernel_1.snap", + "snap_try_kernel": "", + "snap_mode": "", + }) +} + func (s *mgrsSuite) TestRemodelStoreSwitch(c *C) { s.prereqSnapAssertions(c, map[string]interface{}{ "snap-name": "foo", @@ -3897,6 +4567,325 @@ func (s *mgrsSuite) TestRemodelStoreSwitch(c *C) { c.Check(device.SessionMacaroon, Equals, "switched-store-session") } +func (s *mgrsSuite) TestRemodelSwitchGadgetTrack(c *C) { + bloader := bootloadertest.Mock("mock", c.MkDir()) + bootloader.Force(bloader) + defer bootloader.Force(nil) + + restore := release.MockOnClassic(false) + defer restore() + + mockServer := s.mockStore(c) + defer mockServer.Close() + + st := s.o.State() + st.Lock() + defer st.Unlock() + + si := &snap.SideInfo{RealName: "pc", SnapID: fakeSnapID("pc"), Revision: snap.R(1)} + snapstate.Set(st, "pc", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si}, + Current: snap.R(1), + SnapType: "gadget", + }) + gadgetSnapYaml := "name: pc\nversion: 2.0\ntype: gadget" + gadgetYaml := ` +volumes: + volume-id: + bootloader: grub +` + snaptest.MockSnapWithFiles(c, gadgetSnapYaml, si, [][]string{ + {"meta/gadget.yaml", gadgetYaml}, + }) + snapPath, _ := s.makeStoreTestSnapWithFiles(c, gadgetSnapYaml, "2", [][]string{ + {"meta/gadget.yaml", gadgetYaml}, + }) + s.serveSnap(snapPath, "2") + + // create/set custom model assertion + model := s.brands.Model("can0nical", "my-model", modelDefaults) + + // setup model assertion + devicestatetest.SetDevice(st, &auth.DeviceState{ + Brand: "can0nical", + Model: "my-model", + }) + err := assertstate.Add(st, model) + c.Assert(err, IsNil) + + // create a new model + newModel := s.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "gadget": "pc=18", + "revision": "1", + }) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Assert(chg.Err(), IsNil) + + // ensure tasks were run in the right order + tasks := chg.Tasks() + sort.Sort(byReadyTime(tasks)) + + // first all downloads/checks in sequential order + var i int + i += validateDownloadCheckTasks(c, tasks[i:], "pc", "2", "18") + + // then all installs in sequential order + i += validateRefreshTasks(c, tasks[i:], "pc", "2", isGadget) + + // ensure that we only have the tasks we checked (plus the one + // extra "set-model" task) + c.Assert(tasks, HasLen, i+1) +} + +type mockUpdater struct{} + +func (m *mockUpdater) Backup() error { return nil } + +func (m *mockUpdater) Rollback() error { return nil } + +func (m *mockUpdater) Update() error { return nil } + +func (s *mgrsSuite) TestRemodelSwitchToDifferentGadget(c *C) { + bloader := bootloadertest.Mock("mock", c.MkDir()) + bootloader.Force(bloader) + defer bootloader.Force(nil) + restore := release.MockOnClassic(false) + defer restore() + + mockServer := s.mockStore(c) + defer mockServer.Close() + + st := s.o.State() + st.Lock() + defer st.Unlock() + + si := &snap.SideInfo{RealName: "core18", SnapID: fakeSnapID("core18"), Revision: snap.R(1)} + snapstate.Set(st, "core18", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si}, + Current: snap.R(1), + SnapType: "base", + }) + si2 := &snap.SideInfo{RealName: "pc", SnapID: fakeSnapID("pc"), Revision: snap.R(1)} + gadgetSnapYaml := "name: pc\nversion: 1.0\ntype: gadget" + snapstate.Set(st, "pc", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si2}, + Current: snap.R(1), + SnapType: "gadget", + }) + gadgetYaml := ` +volumes: + volume-id: + bootloader: grub + structure: + - name: foo + type: bare + size: 1M + content: + - image: foo.img +` + snaptest.MockSnapWithFiles(c, gadgetSnapYaml, si2, [][]string{ + {"meta/gadget.yaml", gadgetYaml}, + {"foo.img", "foo"}, + }) + + // add new gadget "other-pc" snap to fake store + const otherPcYaml = `name: other-pc +type: gadget +version: 2` + s.prereqSnapAssertions(c, map[string]interface{}{ + "snap-name": "other-pc", + "publisher-id": "can0nical", + }) + otherGadgetYaml := ` +volumes: + volume-id: + bootloader: grub + structure: + - name: foo + type: bare + size: 1M + content: + - image: new-foo.img +` + snapPath, _ := s.makeStoreTestSnapWithFiles(c, otherPcYaml, "2", [][]string{ + // use a compatible gadget YAML + {"meta/gadget.yaml", otherGadgetYaml}, + {"new-foo.img", "new foo"}, + }) + s.serveSnap(snapPath, "2") + + updaterForStructureCalls := 0 + restore = gadget.MockUpdaterForStructure(func(ps *gadget.LaidOutStructure, rootDir, rollbackDir string) (gadget.Updater, error) { + updaterForStructureCalls++ + c.Assert(ps.Name, Equals, "foo") + return &mockUpdater{}, nil + }) + defer restore() + + // create/set custom model assertion + model := s.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "gadget": "pc", + }) + + // setup model assertion + devicestatetest.SetDevice(st, &auth.DeviceState{ + Brand: "can0nical", + Model: "my-model", + }) + err := assertstate.Add(st, model) + c.Assert(err, IsNil) + + // create a new model + newModel := s.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "gadget": "other-pc=18", + "revision": "1", + }) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Assert(chg.Err(), IsNil) + + // gadget updater was set up + c.Check(updaterForStructureCalls, Equals, 1) + + // gadget update requests a restart + restarting, _ := st.Restarting() + c.Check(restarting, Equals, true) + + // simulate successful restart happened + state.MockRestarting(st, state.RestartUnset) + + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + + // ensure tasks were run in the right order + tasks := chg.Tasks() + sort.Sort(byReadyTime(tasks)) + + // first all downloads/checks + var i int + i += validateDownloadCheckTasks(c, tasks[i:], "other-pc", "2", "18") + + // then all installs + i += validateInstallTasks(c, tasks[i:], "other-pc", "2", isGadget) + + // ensure that we only have the tasks we checked (plus the one + // extra "set-model" task) + c.Assert(tasks, HasLen, i+1) +} + +func (s *mgrsSuite) TestRemodelSwitchToIncompatibleGadget(c *C) { + bloader := bootloadertest.Mock("mock", c.MkDir()) + bootloader.Force(bloader) + defer bootloader.Force(nil) + restore := release.MockOnClassic(false) + defer restore() + + mockServer := s.mockStore(c) + defer mockServer.Close() + + st := s.o.State() + st.Lock() + defer st.Unlock() + + si := &snap.SideInfo{RealName: "core18", SnapID: fakeSnapID("core18"), Revision: snap.R(1)} + snapstate.Set(st, "core18", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si}, + Current: snap.R(1), + SnapType: "base", + }) + si2 := &snap.SideInfo{RealName: "pc", SnapID: fakeSnapID("pc"), Revision: snap.R(1)} + gadgetSnapYaml := "name: pc\nversion: 1.0\ntype: gadget" + snapstate.Set(st, "pc", &snapstate.SnapState{ + Active: true, + Sequence: []*snap.SideInfo{si2}, + Current: snap.R(1), + SnapType: "gadget", + }) + gadgetYaml := ` +volumes: + volume-id: + bootloader: grub + structure: + - name: foo + type: 00000000-0000-0000-0000-0000deadcafe + size: 10M +` + snaptest.MockSnapWithFiles(c, gadgetSnapYaml, si2, [][]string{ + {"meta/gadget.yaml", gadgetYaml}, + }) + + // add new gadget "other-pc" snap to fake store + const otherPcYaml = `name: other-pc +type: gadget +version: 2` + // new gadget layout is incompatible, a structure that exited before has + // a different size now + otherGadgetYaml := ` +volumes: + volume-id: + bootloader: grub + structure: + - name: foo + type: 00000000-0000-0000-0000-0000deadcafe + size: 20M +` + s.prereqSnapAssertions(c, map[string]interface{}{ + "snap-name": "other-pc", + "publisher-id": "can0nical", + }) + snapPath, _ := s.makeStoreTestSnapWithFiles(c, otherPcYaml, "2", [][]string{ + {"meta/gadget.yaml", otherGadgetYaml}, + }) + s.serveSnap(snapPath, "2") + + // create/set custom model assertion + model := s.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "gadget": "pc", + }) + + // setup model assertion + devicestatetest.SetDevice(st, &auth.DeviceState{ + Brand: "can0nical", + Model: "my-model", + }) + err := assertstate.Add(st, model) + c.Assert(err, IsNil) + + // create a new model + newModel := s.brands.Model("can0nical", "my-model", modelDefaults, map[string]interface{}{ + "gadget": "other-pc=18", + "revision": "1", + }) + + chg, err := devicestate.Remodel(st, newModel) + c.Assert(err, IsNil) + + st.Unlock() + err = s.o.Settle(settleTimeout) + st.Lock() + c.Assert(err, IsNil) + c.Assert(chg.Err(), ErrorMatches, `cannot perform the following tasks:\n.*cannot remodel to an incompatible gadget: .*cannot change structure size.*`) +} + func (s *mgrsSuite) TestHappyDeviceRegistrationWithPrepareDeviceHook(c *C) { // just to 404 locally eager account-key requests mockStoreServer := s.mockStore(c) diff --git a/overlord/snapstate/check_snap.go b/overlord/snapstate/check_snap.go index 11bd1d7d6d..89ab6600e0 100644 --- a/overlord/snapstate/check_snap.go +++ b/overlord/snapstate/check_snap.go @@ -482,13 +482,11 @@ func checkGadgetOrKernel(st *state.State, snapInfo, curInfo *snap.Info, _ snap.C if err == state.ErrNoState { // check if we are in the remodel case if deviceCtx != nil && deviceCtx.ForRemodeling() { - model := deviceCtx.Model() - if model.Kernel() == snapInfo.InstanceName() { + if whichName(deviceCtx.Model()) == snapInfo.InstanceName() { return nil } } - - return fmt.Errorf("internal error: cannot remodel gadget yet") + return fmt.Errorf("internal error: no state for %s snap %q", kind, snapInfo.InstanceName()) } if err != nil { return fmt.Errorf("cannot find original %s snap: %v", kind, err) diff --git a/overlord/snapstate/check_snap_test.go b/overlord/snapstate/check_snap_test.go index e475566ced..cf04569ab3 100644 --- a/overlord/snapstate/check_snap_test.go +++ b/overlord/snapstate/check_snap_test.go @@ -665,6 +665,50 @@ version: 2 c.Check(err, ErrorMatches, "cannot replace kernel snap with a different one") } +func (s *checkSnapSuite) TestCheckSnapNoStateInfoInternalError(c *C) { + reset := release.MockOnClassic(false) + defer reset() + + st := state.New(nil) + st.Lock() + defer st.Unlock() + + si := &snap.SideInfo{RealName: "other-kernel", Revision: snap.R(2), SnapID: "kernel-id"} + snaptest.MockSnap(c, ` +name: other-kernel +type: kernel +version: 1 +`, si) + // we have a state information for snap of type kernel, but it's a + // different snap + snapstate.Set(st, "other-kernel", &snapstate.SnapState{ + SnapType: "kernel", + Active: true, + Sequence: []*snap.SideInfo{si}, + Current: si.Revision, + }) + + const yaml = `name: kernel +type: kernel +version: 2 +` + + info, err := snap.InfoFromSnapYaml([]byte(yaml)) + info.SnapID = "kernel-id" + c.Assert(err, IsNil) + + var openSnapFile = func(path string, si *snap.SideInfo) (*snap.Info, snap.Container, error) { + return info, emptyContainer(c), nil + } + restore := snapstate.MockOpenSnapFile(openSnapFile) + defer restore() + + st.Unlock() + err = snapstate.CheckSnap(st, "snap-path", "kernel", nil, nil, snapstate.Flags{}, s.deviceCtx) + st.Lock() + c.Check(err, ErrorMatches, "internal error: no state for kernel snap \"kernel\"") +} + func (s *checkSnapSuite) TestCheckSnapBasesErrorsIfMissing(c *C) { st := state.New(nil) st.Lock() @@ -1206,7 +1250,7 @@ version: 2 c.Check(err, IsNil) } -func (s *checkSnapSuite) TestCheckSnapRemodelGadgetDoesNotWorkYet(c *C) { +func (s *checkSnapSuite) TestCheckSnapRemodelGadget(c *C) { reset := release.MockOnClassic(false) defer reset() @@ -1253,7 +1297,7 @@ version: 2 } st.Unlock() - err = snapstate.CheckSnap(st, "snap-path", "new-kernel", nil, nil, snapstate.Flags{}, deviceCtx) + err = snapstate.CheckSnap(st, "snap-path", "new-gadget", nil, nil, snapstate.Flags{}, deviceCtx) st.Lock() - c.Check(err, ErrorMatches, "internal error: cannot remodel gadget yet") + c.Check(err, IsNil) } diff --git a/overlord/snapstate/export_test.go b/overlord/snapstate/export_test.go index 639f151af9..3b23ddb2d8 100644 --- a/overlord/snapstate/export_test.go +++ b/overlord/snapstate/export_test.go @@ -219,4 +219,6 @@ func MockPidsCgroupDir(dir string) (restore func()) { // link, misc handlers var ( MissingDisabledServices = missingDisabledServices + + MaybeUndoRemodelBootChanges = maybeUndoRemodelBootChanges ) diff --git a/overlord/snapstate/handlers.go b/overlord/snapstate/handlers.go index 276a9e12e7..3d08c2e501 100644 --- a/overlord/snapstate/handlers.go +++ b/overlord/snapstate/handlers.go @@ -31,6 +31,7 @@ import ( "gopkg.in/tomb.v2" + "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/boot" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/features" @@ -864,7 +865,7 @@ func (m *SnapManager) undoUnlinkCurrentSnap(t *state.Task, _ *tomb.Tomb) error { // if we just put back a previous a core snap, request a restart // so that we switch executing its snapd - maybeRestart(t, oldInfo) + maybeRestart(t, oldInfo, model) return nil } @@ -1221,22 +1222,16 @@ func (m *SnapManager) doLinkSnap(t *state.Task, _ *tomb.Tomb) (err error) { // if we just installed a core snap, request a restart // so that we switch executing its snapd - maybeRestart(t, newInfo) + maybeRestart(t, newInfo, model) return nil } // maybeRestart will schedule a reboot or restart as needed for the // just linked snap with info if it's a core or snapd or kernel snap. -func maybeRestart(t *state.Task, info *snap.Info) { +func maybeRestart(t *state.Task, info *snap.Info, model *asserts.Model) { st := t.State() - model, err := ModelFromTask(t) - if err != nil { - logger.Noticef("cannot get model assertion: %v", model) - return - } - typ := info.GetType() bp := boot.Participant(info, typ, model, release.OnClassic) if bp.ChangeRequiresReboot() { @@ -1280,6 +1275,71 @@ func daemonRestartReason(st *state.State, typ snap.Type) string { return "Requested daemon restart (snapd snap)." } +// maybeUndoRemodelBootChanges will check if an undo needs to update the +// bootloader. This can happen if e.g. a new kernel gets installed. This +// will switch the bootloader to the new kernel but if the change is later +// undone we need to switch back to the kernel of the old model. +func maybeUndoRemodelBootChanges(t *state.Task) error { + // get the new and the old model + deviceCtx, err := DeviceCtx(t.State(), t, nil) + if err != nil { + return err + } + // we only have an old model if we are in a remodel situation + oldModel := deviceCtx.OldModel() + if oldModel == nil { + return nil + } + newModel := deviceCtx.Model() + + // check type of the snap we are undoing, only kernel/base/core are + // relevant + snapsup, _, err := snapSetupAndState(t) + if err != nil { + return err + } + var newSnapName, snapName string + switch snapsup.Type { + case snap.TypeKernel: + snapName = oldModel.Kernel() + newSnapName = newModel.Kernel() + case snap.TypeOS, snap.TypeBase: + // XXX: add support for "core" + snapName = oldModel.Base() + newSnapName = newModel.Base() + default: + return nil + } + // we can stop if the kernel/base has not changed + if snapName == newSnapName { + return nil + } + // we can stop if the snap we are looking at is not a kernel/base + // of the new model + if snapsup.InstanceName() != newSnapName { + return nil + } + // get info for *old* kernel/base/core and see if we need to reboot + // TODO: we may need something like infoForDeviceSnap here + var snapst SnapState + if err = Get(t.State(), snapName, &snapst); err != nil { + return err + } + info, err := snapst.CurrentInfo() + if err != nil && err != ErrNoCurrent { + return err + } + bp := boot.Participant(info, info.GetType(), oldModel, release.OnClassic) + if err := bp.SetNextBoot(); err != nil { + return err + } + // we may just have switch back to the old kernel/base/core so + // we may need to restart + maybeRestart(t, info, oldModel) + + return nil +} + func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error { st := t.State() st.Lock() @@ -1403,6 +1463,10 @@ func (m *SnapManager) undoLinkSnap(t *state.Task, _ *tomb.Tomb) error { return err } + if err := maybeUndoRemodelBootChanges(t); err != nil { + return err + } + // mark as inactive Set(st, snapsup.InstanceName(), snapst) // write sequence file for failover helpers diff --git a/overlord/snapstate/handlers_link_test.go b/overlord/snapstate/handlers_link_test.go index 6d4872ef40..4478e8a6c4 100644 --- a/overlord/snapstate/handlers_link_test.go +++ b/overlord/snapstate/handlers_link_test.go @@ -28,6 +28,8 @@ import ( . "gopkg.in/check.v1" + "github.com/snapcore/snapd/bootloader" + "github.com/snapcore/snapd/bootloader/bootloadertest" "github.com/snapcore/snapd/dirs" "github.com/snapcore/snapd/overlord/auth" "github.com/snapcore/snapd/overlord/configstate/config" @@ -36,6 +38,7 @@ import ( "github.com/snapcore/snapd/overlord/state" "github.com/snapcore/snapd/release" "github.com/snapcore/snapd/snap" + "github.com/snapcore/snapd/snap/snaptest" "github.com/snapcore/snapd/testutil" ) @@ -1000,3 +1003,117 @@ func (s *linkSnapSuite) testDoUnlinkSnapRefreshAwareness(c *C) *state.Change { return chg } + +func (s *linkSnapSuite) setMockKernelRemodelCtx(c *C, oldKernel, newKernel string) { + newModel := MakeModel(map[string]interface{}{"kernel": newKernel}) + oldModel := MakeModel(map[string]interface{}{"kernel": oldKernel}) + mockRemodelCtx := &snapstatetest.TrivialDeviceContext{ + DeviceModel: newModel, + OldDeviceModel: oldModel, + } + restore := snapstatetest.MockDeviceContext(mockRemodelCtx) + s.AddCleanup(restore) +} + +func (s *linkSnapSuite) TestMaybeUndoRemodelBootChangesUnrelatedAppDoesNothing(c *C) { + s.state.Lock() + defer s.state.Unlock() + + s.setMockKernelRemodelCtx(c, "kernel", "new-kernel") + t := s.state.NewTask("link-snap", "...") + t.Set("snap-setup", &snapstate.SnapSetup{ + SideInfo: &snap.SideInfo{ + RealName: "some-app", + Revision: snap.R(1), + }, + }) + + err := snapstate.MaybeUndoRemodelBootChanges(t) + c.Assert(err, IsNil) + c.Check(s.stateBackend.restartRequested, HasLen, 0) +} + +func (s *linkSnapSuite) TestMaybeUndoRemodelBootChangesSameKernel(c *C) { + s.state.Lock() + defer s.state.Unlock() + + s.setMockKernelRemodelCtx(c, "kernel", "kernel") + t := s.state.NewTask("link-snap", "...") + t.Set("snap-setup", &snapstate.SnapSetup{ + SideInfo: &snap.SideInfo{ + RealName: "kernel", + Revision: snap.R(1), + }, + Type: "kernel", + }) + + err := snapstate.MaybeUndoRemodelBootChanges(t) + c.Assert(err, IsNil) + c.Check(s.stateBackend.restartRequested, HasLen, 0) +} + +func (s *linkSnapSuite) TestMaybeUndoRemodelBootChangesNeedsUndo(c *C) { + s.state.Lock() + defer s.state.Unlock() + + // undoing remodel bootenv changes is only relevant on !classic + restore := release.MockOnClassic(false) + defer restore() + + // using "snaptest.MockSnap()" is more convenient here so we avoid + // the (default) mocking of snapReadInfo() + restore = snapstate.MockSnapReadInfo(snap.ReadInfo) + defer restore() + + // we need to init the boot-id + err := s.state.VerifyReboot("some-boot-id") + c.Assert(err, IsNil) + + // we pretend we do a remodel from kernel -> new-kernel + s.setMockKernelRemodelCtx(c, "kernel", "new-kernel") + + // and we pretend that we booted into the "new-kernel" already + // and now that needs to be undone + bloader := bootloadertest.Mock("mock", c.MkDir()) + bootloader.Force(bloader) + bloader.SetBootKernel("new-kernel_1.snap") + + // both kernel and new-kernel are instaleld at this point + si := &snap.SideInfo{RealName: "kernel", Revision: snap.R(1)} + snapstate.Set(s.state, "kernel", &snapstate.SnapState{ + Sequence: []*snap.SideInfo{si}, + SnapType: "kernel", + Current: snap.R(1), + }) + snaptest.MockSnap(c, "name: kernel\ntype: kernel\nversion: 1.0", si) + si2 := &snap.SideInfo{RealName: "new-kernel", Revision: snap.R(1)} + snapstate.Set(s.state, "new-kernel", &snapstate.SnapState{ + Sequence: []*snap.SideInfo{si2}, + SnapType: "kernel", + Current: snap.R(1), + }) + snaptest.MockSnap(c, "name: new-kernel\ntype: kernel\nversion: 1.0", si) + + t := s.state.NewTask("link-snap", "...") + t.Set("snap-setup", &snapstate.SnapSetup{ + SideInfo: &snap.SideInfo{ + RealName: "new-kernel", + Revision: snap.R(1), + SnapID: "new-kernel-id", + }, + Type: "kernel", + }) + + // now we simulate that the new kernel is getting undone + err = snapstate.MaybeUndoRemodelBootChanges(t) + c.Assert(err, IsNil) + + // that will schedule a boot into the previous kernel + c.Assert(bloader.BootVars, DeepEquals, map[string]string{ + "snap_mode": "try", + "snap_kernel": "new-kernel_1.snap", + "snap_try_kernel": "kernel_1.snap", + }) + c.Check(s.stateBackend.restartRequested, HasLen, 1) + c.Check(s.stateBackend.restartRequested[0], Equals, state.RestartSystem) +} diff --git a/overlord/snapstate/snapstate.go b/overlord/snapstate/snapstate.go index a65e215243..e3e047d758 100644 --- a/overlord/snapstate/snapstate.go +++ b/overlord/snapstate/snapstate.go @@ -1561,6 +1561,55 @@ func AutoRefresh(ctx context.Context, st *state.State) ([]string, []*state.TaskS return UpdateMany(ctx, st, nil, userID, &Flags{IsAutoRefresh: true}) } +// LinkNewBaseOrKernel will create prepare/link-snap tasks for a remodel +func LinkNewBaseOrKernel(st *state.State, name string) (*state.TaskSet, error) { + var snapst SnapState + err := Get(st, name, &snapst) + if err == state.ErrNoState { + return nil, &snap.NotInstalledError{Snap: name} + } + if err != nil { + return nil, err + } + + if err := CheckChangeConflict(st, name, nil); err != nil { + return nil, err + } + + info, err := snapst.CurrentInfo() + if err != nil { + return nil, err + } + + switch info.GetType() { + case snap.TypeOS, snap.TypeBase, snap.TypeKernel: + // good + default: + // bad + return nil, fmt.Errorf("cannot link type %v", info.GetType()) + } + + snapsup := &SnapSetup{ + SideInfo: snapst.CurrentSideInfo(), + Flags: snapst.Flags.ForSnapSetup(), + Type: info.GetType(), + PlugsOnly: len(info.Slots) == 0, + InstanceKey: snapst.InstanceKey, + } + + prepareSnap := st.NewTask("prepare-snap", fmt.Sprintf(i18n.G("Prepare snap %q (%s) for remodel"), snapsup.InstanceName(), snapst.Current)) + prepareSnap.Set("snap-setup", &snapsup) + + linkSnap := st.NewTask("link-snap", fmt.Sprintf(i18n.G("Make snap %q (%s) available to the system during remodel"), snapsup.InstanceName(), snapst.Current)) + linkSnap.Set("snap-setup-task", prepareSnap.ID()) + linkSnap.WaitFor(prepareSnap) + + // we need this for remodel + ts := state.NewTaskSet(prepareSnap, linkSnap) + ts.MarkEdge(prepareSnap, DownloadAndChecksDoneEdge) + return ts, nil +} + // Enable sets a snap to the active state func Enable(st *state.State, name string) (*state.TaskSet, error) { var snapst SnapState diff --git a/sandbox/seccomp/compiler.go b/sandbox/seccomp/compiler.go index 9edc66cfde..db36ab66b7 100644 --- a/sandbox/seccomp/compiler.go +++ b/sandbox/seccomp/compiler.go @@ -31,14 +31,19 @@ import ( ) var ( - // version-info format: <build-id> <libseccomp-version> <hash> <features> - // Where, the hash is calculated over all syscall names supported by the - // libseccomp library. The build-id is a 160-bit SHA-1 (40 char) string - // and the hash is a 256-bit SHA-256 (64 char) string. Allow libseccomp - // version to be 1-5 chars per field (eg, 1.2.3 or 12345.23456.34567) - // and 1-30 chars of colon-separated features. - // Ex: 7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c bpf-actlog - validVersionInfo = regexp.MustCompile(`^[0-9a-f]{1,40} [0-9]{1,5}\.[0-9]{1,5}\.[0-9]{1,5} [0-9a-f]{1,64} [-a-z0-9:]{1,30}$`) + // version-info format: <build-id> <libseccomp-version> <hash> + // <features> Where, the hash is calculated over all syscall names + // supported by the libseccomp library. The build-id is a string of up + // to 166 chars, accommodates 128-bit MD5 (32 chars), 160-bit SHA-1 (40 + // chars) generated by GNU ld, and 83-byte (166 chars) build ID + // generated by Go toolchain, also provides an upper limit of the + // user-settable build ID. The hash is a 256-bit SHA-256 (64 char) + // string. Allow libseccomp version to be 1-5 chars per field (eg, 1.2.3 + // or 12345.23456.34567) and 1-30 chars of colon-separated features. Ex: + // 7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 + // 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c + // bpf-actlog + validVersionInfo = regexp.MustCompile(`^[0-9a-f]{1,166} [0-9]{1,5}\.[0-9]{1,5}\.[0-9]{1,5} [0-9a-f]{1,64} [-a-z0-9:]{1,30}$`) ) type Compiler struct { diff --git a/sandbox/seccomp/compiler_test.go b/sandbox/seccomp/compiler_test.go index 16b97588f2..12a378b4ff 100644 --- a/sandbox/seccomp/compiler_test.go +++ b/sandbox/seccomp/compiler_test.go @@ -49,16 +49,22 @@ func (s *compilerSuite) TestVersionInfoValidate(c *C) { exp string err string }{ - // valid + // all valid + // 20-byte sha1 build ID added by GNU ld {"7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c bpf-actlog", "7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c bpf-actlog", ""}, {"7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c foo:bar", "7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c foo:bar", ""}, {"7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c -", "7ac348ac9c934269214b00d1692dfa50d5d4a157 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c -", ""}, + // 16-byte md5/uuid build ID added by GNU ld + {"3817b197e7abe71a952c1245e8bdf8d9 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c -", "3817b197e7abe71a952c1245e8bdf8d9 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c -", ""}, + // 83-byte Go build ID + {"4e444571495f482d30796b5f57307065544e47692f594c61795f384b7a5258362d6a6f4272736e38302f773374475869496e433176527749797a457a4b532f3967324d4f76556f3130323644572d56326e6248 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c -", "4e444571495f482d30796b5f57307065544e47692f594c61795f384b7a5258362d6a6f4272736e38302f773374475869496e433176527749797a457a4b532f3967324d4f76556f3130323644572d56326e6248 2.3.3 03e996919907bc7163bc83b95bca0ecab31300f20dfa365ea14047c698340e7c -", ""}, + // sanity {"abcdef 0.0.0 abcd bpf-actlog", "abcdef 0.0.0 abcd bpf-actlog", ""}, {"abcdef 0.0.0 abcd -", "abcdef 0.0.0 abcd -", ""}, // invalid all the way down from here // this is over/under the sane length limit for the fields - {"00000000000000000000000000000000000000001 2.4.1 0000000000000000000000000000000000000000000000000000000000000000 -", "", "invalid format of version-info: .*"}, + {"00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001 2.4.1 0000000000000000000000000000000000000000000000000000000000000000 -", "", "invalid format of version-info: .*"}, {"0000000000000000000000000000000000000000 123456.0.0 0000000000000000000000000000000000000000000000000000000000000000 -", "", "invalid format of version-info: .*"}, {"0000000000000000000000000000000000000000 0.123456.0 0000000000000000000000000000000000000000000000000000000000000000 -", "", "invalid format of version-info: .*"}, {"0000000000000000000000000000000000000000 0.0.123456 0000000000000000000000000000000000000000000000000000000000000000 -", "", "invalid format of version-info: .*"}, diff --git a/seed/seed20.go b/seed/seed20.go index 30ad574775..cf2a183d1c 100644 --- a/seed/seed20.go +++ b/seed/seed20.go @@ -102,6 +102,10 @@ func (s *seed20) LoadAssertions(db asserts.RODatabase, commitTo func(*asserts.Ba } modelRef := refs[0] + if len(declRefs) != len(revRefs) { + return fmt.Errorf("system unexpectedly holds a different number of snap-declaration than snap-revision assertions") + } + // this also verifies the consistency of all of them if err := commitTo(batch); err != nil { return err @@ -217,7 +221,7 @@ func (s *seed20) lookupVerifiedRevision(snapRef naming.SnapRef) (snapPath string snapRev = s.snapRevsByID[snapID] if snapRev == nil { - return "", nil, nil, fmt.Errorf("cannot find snap-revision for snap-id: %s", snapID) + return "", nil, nil, fmt.Errorf("internal error: cannot find snap-revision for snap-id: %s", snapID) } snapName := snapDecl.SnapName() @@ -225,11 +229,11 @@ func (s *seed20) lookupVerifiedRevision(snapRef naming.SnapRef) (snapPath string fi, err := os.Stat(snapPath) if err != nil { - return "", nil, nil, fmt.Errorf("cannot stat snap %q: %v", snapPath, err) + return "", nil, nil, fmt.Errorf("cannot stat snap: %v", err) } if fi.Size() != int64(snapRev.SnapSize()) { - return "", nil, nil, fmt.Errorf("cannot validate snap %q for snap %q (snap-id %q), wrong size", snapPath, snapName, snapID) + return "", nil, nil, fmt.Errorf("cannot validate %q for snap %q (snap-id %q), wrong size", snapPath, snapName, snapID) } snapSHA3_384, _, err := asserts.SnapFileSHA3_384(snapPath) @@ -238,7 +242,7 @@ func (s *seed20) lookupVerifiedRevision(snapRef naming.SnapRef) (snapPath string } if snapSHA3_384 != snapRev.SnapSHA3_384() { - return "", nil, nil, fmt.Errorf("cannot validate snap %q for snap %q (snap-id %q), hash mismatch with snap-revision", snapPath, snapName, snapID) + return "", nil, nil, fmt.Errorf("cannot validate %q for snap %q (snap-id %q), hash mismatch with snap-revision", snapPath, snapName, snapID) } diff --git a/seed/seed20_test.go b/seed/seed20_test.go index 59a78478bd..70f52f2243 100644 --- a/seed/seed20_test.go +++ b/seed/seed20_test.go @@ -20,12 +20,16 @@ package seed_test import ( + "os" "path/filepath" + "strings" + "time" . "gopkg.in/check.v1" "github.com/snapcore/snapd/asserts" "github.com/snapcore/snapd/asserts/assertstest" + "github.com/snapcore/snapd/osutil" "github.com/snapcore/snapd/seed" "github.com/snapcore/snapd/seed/seedtest" "github.com/snapcore/snapd/snap" @@ -162,6 +166,366 @@ func (s *seed20Suite) TestLoadMetaCore20Minimal(c *C) { c.Check(runSnaps, HasLen, 0) } +func (s *seed20Suite) makeCore20MinimalSeed(c *C, sysLabel string) string { + s.makeSnap(c, "snapd", "") + s.makeSnap(c, "core20", "") + s.makeSnap(c, "pc-kernel=20", "") + s.makeSnap(c, "pc=20", "") + + s.MakeSeed(c, sysLabel, "my-brand", "my-model", map[string]interface{}{ + "display-name": "my model", + "architecture": "amd64", + "base": "core20", + "snaps": []interface{}{ + map[string]interface{}{ + "name": "pc-kernel", + "id": s.AssertedSnapID("pc-kernel"), + "type": "kernel", + "default-channel": "20", + }, + map[string]interface{}{ + "name": "pc", + "id": s.AssertedSnapID("pc"), + "type": "gadget", + "default-channel": "20", + }}, + }) + + return filepath.Join(s.SeedDir, "systems", sysLabel) +} + +func (s *seed20Suite) TestLoadAssertionsModelTempDBHappy(c *C) { + r := seed.MockTrusted(s.StoreSigning.Trusted) + defer r() + + sysLabel := "20191031" + s.makeCore20MinimalSeed(c, sysLabel) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(nil, nil) + c.Assert(err, IsNil) + + model, err := seed20.Model() + c.Assert(err, IsNil) + c.Check(model.Model(), Equals, "my-model") + c.Check(model.Base(), Equals, "core20") +} + +func (s *seed20Suite) TestLoadAssertionsMultiModels(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + err := osutil.CopyFile(filepath.Join(sysDir, "model"), filepath.Join(sysDir, "assertions", "model2"), 0) + c.Assert(err, IsNil) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Check(err, ErrorMatches, `system cannot have any model assertion but the one in the system model assertion file`) +} + +func (s *seed20Suite) TestLoadAssertionsInvalidModelAssertFile(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + modelAssertFn := filepath.Join(sysDir, "model") + + // copy over multiple assertions + err := osutil.CopyFile(filepath.Join(sysDir, "assertions", "model-etc"), modelAssertFn, osutil.CopyFlagOverwrite) + c.Assert(err, IsNil) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Check(err, ErrorMatches, `system model assertion file must contain exactly the model assertion`) + + // write whatever single non model assertion + seedtest.WriteAssertions(modelAssertFn, s.AssertedSnapRevision("snapd")) + + seed20, err = seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Check(err, ErrorMatches, `system model assertion file must contain exactly the model assertion`) +} + +func (s *seed20Suite) massageAssertions(c *C, fn string, filter func(asserts.Assertion) asserts.Assertion) { + assertions := seedtest.ReadAssertions(c, fn) + filtered := make([]asserts.Assertion, 0, len(assertions)) + for _, a := range assertions { + a1 := filter(a) + if a1 != nil { + filtered = append(filtered, a1) + } + } + seedtest.WriteAssertions(fn, filtered...) +} + +func (s *seed20Suite) TestLoadAssertionsUnbalancedDeclsAndRevs(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + s.massageAssertions(c, filepath.Join(sysDir, "assertions", "snaps"), func(a asserts.Assertion) asserts.Assertion { + if a.Type() == asserts.SnapRevisionType && a.HeaderString("snap-id") == s.AssertedSnapID("core20") { + return nil + } + return a + }) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Check(err, ErrorMatches, `system unexpectedly holds a different number of snap-declaration than snap-revision assertions`) +} + +func (s *seed20Suite) TestLoadAssertionsMultiSnapRev(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + spuriousRev, err := s.StoreSigning.Sign(asserts.SnapRevisionType, map[string]interface{}{ + "snap-sha3-384": strings.Repeat("B", 64), + "snap-size": "1000", + "snap-id": s.AssertedSnapID("core20"), + "developer-id": "canonical", + "snap-revision": "99", + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + s.massageAssertions(c, filepath.Join(sysDir, "assertions", "snaps"), func(a asserts.Assertion) asserts.Assertion { + if a.Type() == asserts.SnapRevisionType && a.HeaderString("snap-id") == s.AssertedSnapID("snapd") { + return spuriousRev + } + return a + }) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Check(err, ErrorMatches, `cannot have multiple snap-revisions for the same snap-id: core20ididididididididididididid`) +} + +func (s *seed20Suite) TestLoadAssertionsMultiSnapDecl(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + spuriousDecl, err := s.StoreSigning.Sign(asserts.SnapDeclarationType, map[string]interface{}{ + "series": "16", + "snap-id": "idididididididididididididididid", + "publisher-id": "canonical", + "snap-name": "core20", + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + spuriousRev, err := s.StoreSigning.Sign(asserts.SnapRevisionType, map[string]interface{}{ + "snap-sha3-384": strings.Repeat("B", 64), + "snap-size": "1000", + "snap-id": s.AssertedSnapID("core20"), + "developer-id": "canonical", + "snap-revision": "99", + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + s.massageAssertions(c, filepath.Join(sysDir, "assertions", "snaps"), func(a asserts.Assertion) asserts.Assertion { + if a.Type() == asserts.SnapDeclarationType && a.HeaderString("snap-name") == "snapd" { + return spuriousDecl + } + if a.Type() == asserts.SnapRevisionType && a.HeaderString("snap-id") == s.AssertedSnapID("snapd") { + return spuriousRev + } + return a + }) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Check(err, ErrorMatches, `cannot have multiple snap-declarations for the same snap-name: core20`) +} + +func (s *seed20Suite) TestLoadMetaMissingSnapDeclByName(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + wrongDecl, err := s.StoreSigning.Sign(asserts.SnapDeclarationType, map[string]interface{}{ + "series": "16", + "snap-id": "idididididididididididididididid", + "publisher-id": "canonical", + "snap-name": "core20X", + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + wrongRev, err := s.StoreSigning.Sign(asserts.SnapRevisionType, map[string]interface{}{ + "snap-sha3-384": strings.Repeat("B", 64), + "snap-size": "1000", + "snap-id": "idididididididididididididididid", + "developer-id": "canonical", + "snap-revision": "99", + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + s.massageAssertions(c, filepath.Join(sysDir, "assertions", "snaps"), func(a asserts.Assertion) asserts.Assertion { + if a.Type() == asserts.SnapDeclarationType && a.HeaderString("snap-name") == "core20" { + return wrongDecl + } + if a.Type() == asserts.SnapRevisionType && a.HeaderString("snap-id") == s.AssertedSnapID("core20") { + return wrongRev + } + return a + }) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Assert(err, IsNil) + + err = seed20.LoadMeta(s.perfTimings) + c.Check(err, ErrorMatches, `cannot find snap-declaration for snap name: core20`) +} + +func (s *seed20Suite) TestLoadMetaMissingSnapDeclByID(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + wrongDecl, err := s.StoreSigning.Sign(asserts.SnapDeclarationType, map[string]interface{}{ + "series": "16", + "snap-id": "idididididididididididididididid", + "publisher-id": "canonical", + "snap-name": "pc", + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + wrongRev, err := s.StoreSigning.Sign(asserts.SnapRevisionType, map[string]interface{}{ + "snap-sha3-384": strings.Repeat("B", 64), + "snap-size": "1000", + "snap-id": "idididididididididididididididid", + "developer-id": "canonical", + "snap-revision": "99", + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + s.massageAssertions(c, filepath.Join(sysDir, "assertions", "snaps"), func(a asserts.Assertion) asserts.Assertion { + if a.Type() == asserts.SnapDeclarationType && a.HeaderString("snap-name") == "pc" { + return wrongDecl + } + if a.Type() == asserts.SnapRevisionType && a.HeaderString("snap-id") == s.AssertedSnapID("pc") { + return wrongRev + } + return a + }) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Assert(err, IsNil) + + err = seed20.LoadMeta(s.perfTimings) + c.Check(err, ErrorMatches, `cannot find snap-declaration for snap-id: pcididididididididididididididid`) +} + +func (s *seed20Suite) TestLoadMetaMissingSnap(c *C) { + sysLabel := "20191031" + s.makeCore20MinimalSeed(c, sysLabel) + + err := os.Remove(filepath.Join(s.SeedDir, "snaps", "pc_1.snap")) + c.Assert(err, IsNil) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Assert(err, IsNil) + + err = seed20.LoadMeta(s.perfTimings) + c.Check(err, ErrorMatches, `cannot stat snap:.*pc_1\.snap.*`) +} + +func (s *seed20Suite) TestLoadMetaWrongSizeSnap(c *C) { + sysLabel := "20191031" + s.makeCore20MinimalSeed(c, sysLabel) + + err := os.Truncate(filepath.Join(s.SeedDir, "snaps", "pc_1.snap"), 5) + c.Assert(err, IsNil) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Assert(err, IsNil) + + err = seed20.LoadMeta(s.perfTimings) + c.Check(err, ErrorMatches, `cannot validate ".*pc_1\.snap" for snap "pc" \(snap-id "pc.*"\), wrong size`) +} + +func (s *seed20Suite) TestLoadMetaWrongHashSnap(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + pcRev := s.AssertedSnapRevision("pc") + wrongRev, err := s.StoreSigning.Sign(asserts.SnapRevisionType, map[string]interface{}{ + "snap-sha3-384": strings.Repeat("B", 64), + "snap-size": pcRev.HeaderString("snap-size"), + "snap-id": s.AssertedSnapID("pc"), + "developer-id": "canonical", + "snap-revision": pcRev.HeaderString("snap-revision"), + "timestamp": time.Now().UTC().Format(time.RFC3339), + }, nil, "") + c.Assert(err, IsNil) + + s.massageAssertions(c, filepath.Join(sysDir, "assertions", "snaps"), func(a asserts.Assertion) asserts.Assertion { + if a.Type() == asserts.SnapRevisionType && a.HeaderString("snap-id") == s.AssertedSnapID("pc") { + return wrongRev + } + return a + }) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Assert(err, IsNil) + + err = seed20.LoadMeta(s.perfTimings) + c.Check(err, ErrorMatches, `cannot validate ".*pc_1\.snap" for snap "pc" \(snap-id "pc.*"\), hash mismatch with snap-revision`) +} + +func (s *seed20Suite) TestLoadMetaWrongGadgetBase(c *C) { + sysLabel := "20191031" + sysDir := s.makeCore20MinimalSeed(c, sysLabel) + + // pc with base: core18 + pc18Decl, pc18Rev := s.MakeAssertedSnap(c, snapYaml["pc=18"], nil, snap.R(2), "canonical") + err := os.Rename(s.AssertedSnap("pc"), filepath.Join(s.SeedDir, "snaps", "pc_2.snap")) + c.Assert(err, IsNil) + s.massageAssertions(c, filepath.Join(sysDir, "assertions", "snaps"), func(a asserts.Assertion) asserts.Assertion { + if a.Type() == asserts.SnapDeclarationType && a.HeaderString("snap-name") == "pc" { + return pc18Decl + } + if a.Type() == asserts.SnapRevisionType && a.HeaderString("snap-id") == s.AssertedSnapID("pc") { + return pc18Rev + } + return a + }) + + seed20, err := seed.Open(s.SeedDir, sysLabel) + c.Assert(err, IsNil) + + err = seed20.LoadAssertions(s.db, s.commitTo) + c.Assert(err, IsNil) + + err = seed20.LoadMeta(s.perfTimings) + c.Check(err, ErrorMatches, `cannot use gadget snap because its base "core18" is different from model base "core20"`) +} + func (s *seed20Suite) TestLoadMetaCore20(c *C) { s.makeSnap(c, "snapd", "") s.makeSnap(c, "core20", "") diff --git a/seed/seedtest/seedtest.go b/seed/seedtest/seedtest.go index dd3da3a7c5..8c5d17e86f 100644 --- a/seed/seedtest/seedtest.go +++ b/seed/seedtest/seedtest.go @@ -21,6 +21,7 @@ package seedtest import ( "fmt" + "io" "os" "path/filepath" "strings" @@ -188,6 +189,24 @@ func WriteAssertions(fn string, assertions ...asserts.Assertion) { } } +func ReadAssertions(c *C, fn string) []asserts.Assertion { + f, err := os.Open(fn) + c.Assert(err, IsNil) + + var as []asserts.Assertion + dec := asserts.NewDecoder(f) + for { + a, err := dec.Decode() + if err == io.EOF { + break + } + c.Assert(err, IsNil) + as = append(as, a) + } + + return as +} + // TestingSeed20 helps setting up a populated Core 20 testing seed directory. type TestingSeed20 struct { SeedSnaps diff --git a/seed/seedwriter/writer_test.go b/seed/seedwriter/writer_test.go index 39f2f58ba6..bbe0231286 100644 --- a/seed/seedwriter/writer_test.go +++ b/seed/seedwriter/writer_test.go @@ -22,7 +22,6 @@ package seedwriter_test import ( "encoding/json" "fmt" - "io" "io/ioutil" "os" "path/filepath" @@ -755,24 +754,6 @@ func (s *writerSuite) TestDownloadedCheckTypeCore(c *C) { c.Check(err, ErrorMatches, `core snap has unexpected type: base`) } -func readAssertions(c *C, fn string) []asserts.Assertion { - f, err := os.Open(fn) - c.Assert(err, IsNil) - - var as []asserts.Assertion - dec := asserts.NewDecoder(f) - for { - a, err := dec.Decode() - if err == io.EOF { - break - } - c.Assert(err, IsNil) - as = append(as, a) - } - - return as -} - func (s *writerSuite) TestSeedSnapsWriteMetaCore18(c *C) { model := s.Brands.Model("my-brand", "my-model", map[string]interface{}{ "display-name": "my model", @@ -865,7 +846,7 @@ func (s *writerSuite) TestSeedSnapsWriteMetaCore18(c *C) { c.Check(filepath.Join(seedAssertsDir, "model"), testutil.FileEquals, asserts.Encode(model)) - acct := readAssertions(c, filepath.Join(seedAssertsDir, "my-brand.account")) + acct := seedtest.ReadAssertions(c, filepath.Join(seedAssertsDir, "my-brand.account")) c.Assert(acct, HasLen, 1) c.Check(acct[0].Type(), Equals, asserts.AccountType) c.Check(acct[0].HeaderString("account-id"), Equals, "my-brand") @@ -873,12 +854,12 @@ func (s *writerSuite) TestSeedSnapsWriteMetaCore18(c *C) { // check the snap assertions are also in place for _, snapName := range []string{"snapd", "pc-kernel", "core18", "pc", "cont-consumer", "cont-producer"} { p := filepath.Join(seedAssertsDir, fmt.Sprintf("16,%s.snap-declaration", s.AssertedSnapID(snapName))) - decl := readAssertions(c, p) + decl := seedtest.ReadAssertions(c, p) c.Assert(decl, HasLen, 1) c.Check(decl[0].Type(), Equals, asserts.SnapDeclarationType) c.Check(decl[0].HeaderString("snap-name"), Equals, snapName) p = filepath.Join(seedAssertsDir, fmt.Sprintf("%s.snap-revision", s.AssertedSnapRevision(snapName).SnapSHA3_384())) - rev := readAssertions(c, p) + rev := seedtest.ReadAssertions(c, p) c.Assert(rev, HasLen, 1) c.Check(rev[0].Type(), Equals, asserts.SnapRevisionType) c.Check(rev[0].HeaderString("snap-id"), Equals, s.AssertedSnapID(snapName)) @@ -1090,12 +1071,12 @@ func (s *writerSuite) TestLocalSnapsCore18FullUse(c *C) { seedAssertsDir := filepath.Join(s.opts.SeedDir, "assertions") for _, snapName := range []string{"snapd", "cont-producer"} { p := filepath.Join(seedAssertsDir, fmt.Sprintf("16,%s.snap-declaration", s.AssertedSnapID(snapName))) - decl := readAssertions(c, p) + decl := seedtest.ReadAssertions(c, p) c.Assert(decl, HasLen, 1) c.Check(decl[0].Type(), Equals, asserts.SnapDeclarationType) c.Check(decl[0].HeaderString("snap-name"), Equals, snapName) p = filepath.Join(seedAssertsDir, fmt.Sprintf("%s.snap-revision", s.AssertedSnapRevision(snapName).SnapSHA3_384())) - rev := readAssertions(c, p) + rev := seedtest.ReadAssertions(c, p) c.Assert(rev, HasLen, 1) c.Check(rev[0].Type(), Equals, asserts.SnapRevisionType) c.Check(rev[0].HeaderString("snap-id"), Equals, s.AssertedSnapID(snapName)) @@ -1471,12 +1452,12 @@ func (s *writerSuite) TestSeedSnapsWriteMetaExtraSnaps(c *C) { seedAssertsDir := filepath.Join(s.opts.SeedDir, "assertions") for _, snapName := range []string{"snapd", "core", "pc-kernel", "core18", "pc", "cont-consumer", "cont-producer", "required"} { p := filepath.Join(seedAssertsDir, fmt.Sprintf("16,%s.snap-declaration", s.AssertedSnapID(snapName))) - decl := readAssertions(c, p) + decl := seedtest.ReadAssertions(c, p) c.Assert(decl, HasLen, 1) c.Check(decl[0].Type(), Equals, asserts.SnapDeclarationType) c.Check(decl[0].HeaderString("snap-name"), Equals, snapName) p = filepath.Join(seedAssertsDir, fmt.Sprintf("%s.snap-revision", s.AssertedSnapRevision(snapName).SnapSHA3_384())) - rev := readAssertions(c, p) + rev := seedtest.ReadAssertions(c, p) c.Assert(rev, HasLen, 1) c.Check(rev[0].Type(), Equals, asserts.SnapRevisionType) c.Check(rev[0].HeaderString("snap-id"), Equals, s.AssertedSnapID(snapName)) @@ -1741,7 +1722,7 @@ func (s *writerSuite) TestSeedSnapsWriteMetaCore20(c *C) { c.Check(filepath.Join(systemDir, "model"), testutil.FileEquals, asserts.Encode(model)) assertsDir := filepath.Join(systemDir, "assertions") - modelEtc := readAssertions(c, filepath.Join(assertsDir, "model-etc")) + modelEtc := seedtest.ReadAssertions(c, filepath.Join(assertsDir, "model-etc")) c.Check(modelEtc, HasLen, 4) keyPKs := make(map[string]bool) @@ -1763,7 +1744,7 @@ func (s *writerSuite) TestSeedSnapsWriteMetaCore20(c *C) { }) // check snap assertions - snapAsserts := readAssertions(c, filepath.Join(assertsDir, "snaps")) + snapAsserts := seedtest.ReadAssertions(c, filepath.Join(assertsDir, "snaps")) seen := make(map[string]bool) for _, a := range snapAsserts { diff --git a/snap/channel/channel_test.go b/snap/channel/channel_test.go index 058d4e2778..915863d5de 100644 --- a/snap/channel/channel_test.go +++ b/snap/channel/channel_test.go @@ -189,20 +189,27 @@ func (s storeChannelSuite) TestParseErrors(c *C) { for _, tc := range []struct { channel string err string + full string }{ - {"", "channel name cannot be empty"}, - {"1.0////", "channel name has too many components: 1.0////"}, - {"1.0/cand", "invalid risk in channel name: 1.0/cand"}, - {"fix//hotfix", "invalid risk in channel name: fix//hotfix"}, - {"/stable/", "invalid track in channel name: /stable/"}, - {"//stable", "invalid risk in channel name: //stable"}, - {"stable/", "invalid branch in channel name: stable/"}, - {"/stable", "invalid track in channel name: /stable"}, + {"", "channel name cannot be empty", ""}, + {"1.0////", "channel name has too many components: 1.0////", "1.0/stable"}, + {"1.0/cand", "invalid risk in channel name: 1.0/cand", ""}, + {"fix//hotfix", "invalid risk in channel name: fix//hotfix", ""}, + {"/stable/", "invalid track in channel name: /stable/", "latest/stable"}, + {"//stable", "invalid risk in channel name: //stable", "latest/stable"}, + {"stable/", "invalid branch in channel name: stable/", "latest/stable"}, + {"/stable", "invalid track in channel name: /stable", "latest/stable"}, } { _, err := channel.Parse(tc.channel, "") c.Check(err, ErrorMatches, tc.err) _, err = channel.ParseVerbatim(tc.channel, "") c.Check(err, ErrorMatches, tc.err) + if tc.full != "" { + // testing Full behavior on the malformed channel + full, err := channel.Full(tc.channel) + c.Check(err, IsNil) + c.Check(full, Equals, tc.full) + } } } diff --git a/snap/snapenv/snapenv.go b/snap/snapenv/snapenv.go index 03ae4490bc..63f6c2319d 100644 --- a/snap/snapenv/snapenv.go +++ b/snap/snapenv/snapenv.go @@ -28,6 +28,7 @@ import ( "github.com/snapcore/snapd/arch" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/features" "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/snap" ) @@ -129,11 +130,18 @@ func userEnv(info *snap.Info, home string) map[string]string { result := map[string]string{ "SNAP_USER_COMMON": info.UserCommonDataDir(home), "SNAP_USER_DATA": info.UserDataDir(home), - "XDG_RUNTIME_DIR": info.UserXdgRuntimeDir(sys.Geteuid()), } - // For non-classic snaps, we set HOME but on classic allow snaps to see real HOME - if !info.NeedsClassic() { + if info.NeedsClassic() { + // Snaps using classic confinement don't have an override for + // HOME but may have an override for XDG_RUNTIME_DIR. + if !features.ClassicPreservesXdgRuntimeDir.IsEnabled() { + result["XDG_RUNTIME_DIR"] = info.UserXdgRuntimeDir(sys.Geteuid()) + } + } else { + // Snaps using strict or devmode confinement get an override for both + // HOME and XDG_RUNTIME_DIR. result["HOME"] = info.UserDataDir(home) + result["XDG_RUNTIME_DIR"] = info.UserXdgRuntimeDir(sys.Geteuid()) } return result } diff --git a/snap/snapenv/snapenv_test.go b/snap/snapenv/snapenv_test.go index 030f35ede6..376e07ee9d 100644 --- a/snap/snapenv/snapenv_test.go +++ b/snap/snapenv/snapenv_test.go @@ -21,6 +21,7 @@ package snapenv import ( "fmt" + "io/ioutil" "os" "os/user" "strings" @@ -30,6 +31,7 @@ import ( "github.com/snapcore/snapd/arch" "github.com/snapcore/snapd/dirs" + "github.com/snapcore/snapd/features" "github.com/snapcore/snapd/osutil/sys" "github.com/snapcore/snapd/snap" "github.com/snapcore/snapd/testutil" @@ -108,14 +110,31 @@ func (ts *HTestSuite) TestUser(c *C) { } func (ts *HTestSuite) TestUserForClassicConfinement(c *C) { + dirs.SetRootDir(c.MkDir()) + defer dirs.SetRootDir("/") + c.Assert(os.MkdirAll(dirs.FeaturesDir, 0755), IsNil) + + // With the classic-preserves-xdg-runtime-dir feature disabled the snap + // per-user environment contains an override for XDG_RUNTIME_DIR. env := userEnv(mockClassicSnapInfo, "/root") + c.Assert(env, DeepEquals, map[string]string{ + // NOTE: Both HOME and XDG_RUNTIME_DIR are not defined here. + "SNAP_USER_COMMON": "/root/snap/foo/common", + "SNAP_USER_DATA": "/root/snap/foo/17", + "XDG_RUNTIME_DIR": fmt.Sprintf(dirs.GlobalRootDir+"/run/user/%d/snap.foo", sys.Geteuid()), + }) + // With the classic-preserves-xdg-runtime-dir feature enabled the snap + // per-user environment contains no overrides for XDG_RUNTIME_DIR. + f := features.ClassicPreservesXdgRuntimeDir + c.Assert(ioutil.WriteFile(f.ControlFile(), nil, 0644), IsNil) + env = userEnv(mockClassicSnapInfo, "/root") c.Assert(env, DeepEquals, map[string]string{ - // NOTE HOME Is absent! we no longer override it + // NOTE: Both HOME and XDG_RUNTIME_DIR are not defined here. "SNAP_USER_COMMON": "/root/snap/foo/common", "SNAP_USER_DATA": "/root/snap/foo/17", - "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.foo", sys.Geteuid()), }) + } func (s *HTestSuite) TestSnapRunSnapExecEnv(c *C) { @@ -217,15 +236,31 @@ func (ts *HTestSuite) TestParallelInstallUser(c *C) { } func (ts *HTestSuite) TestParallelInstallUserForClassicConfinement(c *C) { + dirs.SetRootDir(c.MkDir()) + defer dirs.SetRootDir("/") + c.Assert(os.MkdirAll(dirs.FeaturesDir, 0755), IsNil) + info := *mockClassicSnapInfo info.InstanceKey = "bar" + + // With the classic-preserves-xdg-runtime-dir feature disabled the snap + // per-user environment contains an override for XDG_RUNTIME_DIR. env := userEnv(&info, "/root") + c.Assert(env, DeepEquals, map[string]string{ + "SNAP_USER_COMMON": "/root/snap/foo_bar/common", + "SNAP_USER_DATA": "/root/snap/foo_bar/17", + "XDG_RUNTIME_DIR": fmt.Sprintf(dirs.GlobalRootDir+"/run/user/%d/snap.foo_bar", sys.Geteuid()), + }) + // With the classic-preserves-xdg-runtime-dir feature enabled the snap + // per-user environment contains no overrides for XDG_RUNTIME_DIR. + f := features.ClassicPreservesXdgRuntimeDir + c.Assert(ioutil.WriteFile(f.ControlFile(), nil, 0644), IsNil) + env = userEnv(&info, "/root") c.Assert(env, DeepEquals, map[string]string{ - // NOTE HOME Is absent! we no longer override it + // NOTE: Both HOME and XDG_RUNTIME_DIR are not defined here. "SNAP_USER_COMMON": "/root/snap/foo_bar/common", "SNAP_USER_DATA": "/root/snap/foo_bar/17", - "XDG_RUNTIME_DIR": fmt.Sprintf("/run/user/%d/snap.foo_bar", sys.Geteuid()), }) } diff --git a/spread.yaml b/spread.yaml index aabdff5179..185098d5b1 100644 --- a/spread.yaml +++ b/spread.yaml @@ -29,6 +29,7 @@ environment: TRUST_TEST_KEYS: '$(HOST: echo "${SPREAD_TRUST_TEST_KEYS:-true}")' MANAGED_DEVICE: "false" CORE_CHANNEL: '$(HOST: echo "${SPREAD_CORE_CHANNEL:-edge}")' + BASE_CHANNEL: '$(HOST: echo "${SPREAD_BASE_CHANNEL:-edge}")' KERNEL_CHANNEL: '$(HOST: echo "${SPREAD_KERNEL_CHANNEL:-edge}")' GADGET_CHANNEL: '$(HOST: echo "${SPREAD_GADGET_CHANNEL:-edge}")' SNAPD_CHANNEL: '$(HOST: echo "${SPREAD_SNAPD_CHANNEL:-edge}")' diff --git a/tests/lib/assertions/developer1-pc-18-new-base.model b/tests/lib/assertions/developer1-pc-18-new-base.model new file mode 100644 index 0000000000..7cd5d58035 --- /dev/null +++ b/tests/lib/assertions/developer1-pc-18-new-base.model @@ -0,0 +1,23 @@ +type: model +authority-id: developer1 +revision: 2 +series: 16 +brand-id: developer1 +model: my-model +architecture: amd64 +base: test-snapd-core18 +gadget: pc=18 +kernel: pc-kernel=18 +timestamp: 2017-05-28T19:40:00+00:00 +sign-key-sha3-384: EAD4DbLxK_kn0gzNCXOs3kd6DeMU3f-L6BEsSEuJGBqCORR0gXkdDxMbOm11mRFu + +AcLBUgQAAQoABgUCXXkbOAAACgsQAF7CSfowmuUnv5R1QMrGtzFGJurjQvgLVHc9olSygOZl2H/m +Gmb8whFKbUsWjQFO6S2BMUQ+L3C5BUsVLnHclNTq9RrkaLDsPMNuso7uqCcVSk83r+N/ptXXzZhm +wEL2WLu6F5WUBxdwIljlorwuw9V4uP+1gdQ+jEEIJ9Hl0RRdAt1Pm39/5+nWRMKSy6TBH3E8y70I +B5Qu0K4HCG1Tz11XZGIf541rIGa7mBzeuJ7FWcCsdvcKcvFsQXYq6FV1qzBwT0gM+Aqg3rgdVE7y +V4b8UUv1CBDNn5lvkXGDV3Kbl5odX4sbmCqTTT5jFoWyhDXISpg31f+UIMXcg5EUjSfBP22F3NjI +fdpMY9CBYpecHmVhsBpbtDjI2bfDuuAFMKWkeKFySjbhRrcpTm4iEfq8ezrpRJrusUzWPb6jTB7S +cb7Dp9Hq8FzFi2GrV2w4NVQCGihwS3HuQAvQ5ZxDtgS1Nv+5oTRSiqkZRGr8h9LA8PPUDvIrBAg0 +TrZgbql7Qwjo4PkjtZPQBNzyOcHFC38mKVq9r1b5ZGkF6yGvPK76a2Oe7vmWPw+EyQKDc+5+UYiU +vYXFbqWwtAkF0fkMis2TiSqd6FfBWRM64OE+8/n/UrEgbS69i4/XXjm34iscc8umbmZ1w5Z/hsC0 +sDZUfqBALcc31dvgi1ClJUTEnotw diff --git a/tests/lib/assertions/developer1-pc-18-new-gadget.model b/tests/lib/assertions/developer1-pc-18-new-gadget.model new file mode 100644 index 0000000000..3f7cf77ff5 --- /dev/null +++ b/tests/lib/assertions/developer1-pc-18-new-gadget.model @@ -0,0 +1,23 @@ +type: model +authority-id: developer1 +revision: 2 +series: 16 +brand-id: developer1 +model: my-model +architecture: amd64 +base: core18 +gadget: test-snapd-pc +kernel: pc-kernel=18 +timestamp: 2018-09-11T22:00:00+00:00 +sign-key-sha3-384: EAD4DbLxK_kn0gzNCXOs3kd6DeMU3f-L6BEsSEuJGBqCORR0gXkdDxMbOm11mRFu + +AcLBUgQAAQoABgUCXbBcrAAAEnsQABIv573DoVzl30EAjAXW56l3RbG8Jd8nNu4sBjelngE/PiWt +zZqL7pNheCHCIFNWBPULq++e3jppvB+fKSmTq9xRYF+l/+Vqv9xzHJAJYSH+TWJV1xSvg98MyPBh +jxLKrwL7JZc0KKUH+mQ2mNdoCo567qysJzkqFZIBN/IN68juWZwCcSoSSTIc0O6vLo5Md6SdHfPU +Ndmex/vt8WQYVXo7CGonBlho8LowKSfV0R/5d2Yu/FgePUgUvowLo8dmh+mVToZohNC1fYYYnHNB +lpf2yMlWwrTPVnHSfjQLHAQ9DaJ+VivbYMzOIfnHXrMheSdr0oMTvduL5IJk/3vY/wT4ec8Z8q8g +aUH3g5MqTvfYDQttiiHoFy7IyIzfoNDuBeaDAmNH4wB0Nfq0HdEiGUGyvSuc1mNpFkPVzcsSzWT6 +0wfcJ0nIFGIgnJcnt/NYAo+Q/iUXJgY5HfmudvH+/HzO7iIczp+oal3cqCvlKHDecSscH+6rSOJW +5H8h3kp4FIxZ8I53kHPWFRFO1L/9T/B153tWyr4uPQGjj2v5RTQAqMgJUSjmp1OJ5w4QyvwUqyM4 +bCoYm5Btuyj/W7ckuAI6Nwr1j6VL78/OCPH6ZLgPPYEWqX643nmOL7cnMOskqwo4YogssNbNSOXv +qLqsd5En1ICT3ATUluYYKml4qqsi diff --git a/tests/lib/assertions/developer1-pc-18-new-gadget.model.json b/tests/lib/assertions/developer1-pc-18-new-gadget.model.json new file mode 100644 index 0000000000..b0fa463cf2 --- /dev/null +++ b/tests/lib/assertions/developer1-pc-18-new-gadget.model.json @@ -0,0 +1,12 @@ +{ + "type": "model", + "series": "16", + "brand-id": "developer1", + "model": "my-model", + "architecture": "amd64", + "gadget": "test-snapd-pc", + "base": "core18", + "kernel": "pc-kernel=18", + "revision": "2", + "timestamp": "2018-09-11T22:00:00+00:00" +} diff --git a/tests/lib/state.sh b/tests/lib/state.sh index 297592c654..5690af700a 100755 --- a/tests/lib/state.sh +++ b/tests/lib/state.sh @@ -47,6 +47,8 @@ save_snapd_state() { cp -rf /var/cache/snapd "$SNAPD_STATE_PATH"/snapd-cache cp -rf "$boot_path" "$SNAPD_STATE_PATH"/boot cp -f /etc/systemd/system/snap-*core*.mount "$SNAPD_STATE_PATH"/system-units + mkdir -p "$SNAPD_STATE_PATH"/var-snap + cp -a /var/snap/* "$SNAPD_STATE_PATH"/var-snap/ else systemctl daemon-reload escaped_snap_mount_dir="$(systemd-escape --path "$SNAP_MOUNT_DIR")" @@ -94,11 +96,14 @@ restore_snapd_state() { cp -rf "$SNAPD_STATE_PATH"/snapd-cache/* /var/cache/snapd cp -rf "$SNAPD_STATE_PATH"/boot/* "$boot_path" cp -f "$SNAPD_STATE_PATH"/system-units/* /etc/systemd/system + rm -rf /var/snap/* + cp -a "$SNAPD_STATE_PATH"/var-snap/* /var/snap/ else # Purge all the systemd service units config rm -rf /etc/systemd/system/snapd.service.d rm -rf /etc/systemd/system/snapd.socket.d + # TODO: remove files created by the test tar -C/ -xf "$SNAPD_STATE_FILE" fi diff --git a/tests/main/gadget-update-pc/task.yaml b/tests/main/gadget-update-pc/task.yaml index 0168c57bb4..d57bed7a13 100644 --- a/tests/main/gadget-update-pc/task.yaml +++ b/tests/main/gadget-update-pc/task.yaml @@ -91,6 +91,13 @@ restore: | # restore the original gadget snap snap install gadget.snap + # restoring the snapshot of / does not clean up /var/snap/pc/<rev> + # directories that were created during the test, given that the state is + # restored these directories will not be accounted for + for i in $(seq 0 2); do + rm -rf "/var/snap/pc/$((START_REVISION+i))" + done + execute: | # external backends do not enable test keys if [ "$TRUST_TEST_KEYS" = "false" ]; then diff --git a/tests/main/refresh-app-awareness/task.yaml b/tests/main/refresh-app-awareness/task.yaml index 64aea91290..f6e1d7c6e3 100644 --- a/tests/main/refresh-app-awareness/task.yaml +++ b/tests/main/refresh-app-awareness/task.yaml @@ -5,11 +5,17 @@ details: | prepare: | # This feature depends on the release-app-awareness feature snap set core experimental.refresh-app-awareness=true + sed -e "s/@CONFINEMENT@/$CONFINEMENT/g" <test-snapd-refresh.v1/meta/snap.yaml.in >test-snapd-refresh.v1/meta/snap.yaml + sed -e "s/@CONFINEMENT@/$CONFINEMENT/g" <test-snapd-refresh.v2/meta/snap.yaml.in >test-snapd-refresh.v2/meta/snap.yaml snap pack test-snapd-refresh.v1 snap pack test-snapd-refresh.v2 +environment: + CONFINEMENT/classic: classic + CONFINEMENT/strict: strict restore: | snap remove test-snapd-refresh rm -f test-snapd-refresh-{1,2}_all.snap + rm -f test-snapd-refresh.*/meta/snap.yaml rmdir /sys/fs/cgroup/pids/snap.test-snapd-refresh.sh || true rmdir /sys/fs/cgroup/pids/snap.test-snapd-refresh.version || true # TODO: There is currently no way to unset configuration options. @@ -17,8 +23,19 @@ restore: | # snap unset core experimental.refresh-app-awareness rm -f install.log execute: | + if ! snap debug sandbox-features --required "confinement-options:$CONFINEMENT"; then + echo "SKIP: unsupported confinement variant" + exit 0 + fi # Install v1 and see that it runs as expected. - snap install --dangerous test-snapd-refresh_1_all.snap + case "$CONFINEMENT" in + classic) + snap install --dangerous --classic test-snapd-refresh_1_all.snap + ;; + strict) + snap install --dangerous test-snapd-refresh_1_all.snap + ;; + esac test-snapd-refresh.version | MATCH v1 # Run a sleeper app to keep the snap busy. The purpose of the stamp file is @@ -30,14 +47,7 @@ execute: | # Ensure that snap-confine has finished its task and that the snap process # is active. Note that we don't want to wait forever either. - wait_for_stamp() { - for _ in $(seq 30); do - test -e "$1" && break - sleep 0.1 - done - } - - wait_for_stamp /var/snap/test-snapd-refresh/current/stamp + retry-tool -n 30 --wait 0.1 test -e /var/snap/test-snapd-refresh/current/stamp # Try to install v2, it should fail because v1 is running. Snapd is kind # enough to tell us what is preventing the install from working. @@ -45,7 +55,14 @@ execute: | unwrap_msg() { tr '\n' ' ' | sed -e 's/ \+/ /g' } - not snap install --dangerous test-snapd-refresh_2_all.snap >install.log 2>&1 + case "$CONFINEMENT" in + classic) + not snap install --dangerous --classic test-snapd-refresh_2_all.snap >install.log 2>&1 + ;; + strict) + not snap install --dangerous test-snapd-refresh_2_all.snap >install.log 2>&1 + ;; + esac unwrap_msg < install.log | MATCH 'error: cannot install snap file: snap "test-snapd-refresh" has running apps +\(sh\)' test-snapd-refresh.version | MATCH v1 @@ -53,5 +70,12 @@ execute: | kill "$pid" wait "$pid" || true # wait returns the exit code and we kill the process # Try to install v2 again, it should now work. - snap install --dangerous test-snapd-refresh_2_all.snap + case "$CONFINEMENT" in + classic) + snap install --dangerous --classic test-snapd-refresh_2_all.snap + ;; + strict) + snap install --dangerous test-snapd-refresh_2_all.snap + ;; + esac test-snapd-refresh.version | MATCH v2 diff --git a/tests/main/refresh-app-awareness/test-snapd-refresh.v1/meta/snap.yaml b/tests/main/refresh-app-awareness/test-snapd-refresh.v1/meta/snap.yaml.in index 6aff277eb2..4c61de9ace 100644 --- a/tests/main/refresh-app-awareness/test-snapd-refresh.v1/meta/snap.yaml +++ b/tests/main/refresh-app-awareness/test-snapd-refresh.v1/meta/snap.yaml.in @@ -1,5 +1,6 @@ name: test-snapd-refresh version: 1 +confinement: @CONFINEMENT@ apps: sh: command: bin/sh diff --git a/tests/main/refresh-app-awareness/test-snapd-refresh.v2/meta/snap.yaml b/tests/main/refresh-app-awareness/test-snapd-refresh.v2/meta/snap.yaml.in index c783c339c6..762101db43 100644 --- a/tests/main/refresh-app-awareness/test-snapd-refresh.v2/meta/snap.yaml +++ b/tests/main/refresh-app-awareness/test-snapd-refresh.v2/meta/snap.yaml.in @@ -1,5 +1,6 @@ name: test-snapd-refresh version: 2 +confinement: @CONFINEMENT@ apps: # NOTE: the v2 version of the application does not have the "sh" # application anymore. This is so that we are correctly looking at the diff --git a/tests/main/remodel-base/task.yaml b/tests/main/remodel-base/task.yaml new file mode 100644 index 0000000000..1520f0fd8a --- /dev/null +++ b/tests/main/remodel-base/task.yaml @@ -0,0 +1,122 @@ +summary: Test a remodel that switches to a new base +environment: + OLD_BASE: core18 + NEW_BASE: test-snapd-core18 + +systems: [ubuntu-core-18-64] + +prepare: | + if [ "$TRUST_TEST_KEYS" = "false" ]; then + echo "This test needs test keys to be trusted" + exit + fi + #shellcheck source=tests/lib/systemd.sh + . "$TESTSLIB"/systemd.sh + systemctl stop snapd.service snapd.socket + rm -rf /var/lib/snapd/assertions/* + rm -rf /var/lib/snapd/device + rm -rf /var/lib/snapd/state.json + mv /var/lib/snapd/seed/assertions/model model.bak + cp "$TESTSLIB"/assertions/developer1.account /var/lib/snapd/seed/assertions + cp "$TESTSLIB"/assertions/developer1.account-key /var/lib/snapd/seed/assertions + cp "$TESTSLIB"/assertions/developer1-pc-18.model /var/lib/snapd/seed/assertions + cp "$TESTSLIB"/assertions/testrootorg-store.account-key /var/lib/snapd/seed/assertions + # kick first boot again + systemctl start snapd.service snapd.socket + retry-tool -n 60 --wait 5 sh -c 'snap changes | grep -q "Done.*Initialize system state"' + +restore: | + if [ "$TRUST_TEST_KEYS" = "false" ]; then + echo "This test needs test keys to be trusted" + exit + fi + #shellcheck source=tests/lib/systemd.sh + . "$TESTSLIB"/systemd.sh + systemctl stop snapd.service snapd.socket + rm -rf /var/lib/snapd/assertions/* + rm -rf /var/lib/snapd/device + rm -rf /var/lib/snapd/state.json + rm -f /var/lib/snapd/seed/assertions/developer1.account + rm -f /var/lib/snapd/seed/assertions/developer1.account-key + rm -f /var/lib/snapd/seed/assertions/developer1-pc-18.model + rm -f /var/lib/snapd/seed/assertions/testrootorg-store.account-key + mv model.bak /var/lib/snapd/seed/assertions/model + rm -f ./*.bak + # kick first boot again + systemctl start snapd.service snapd.socket + # wait for first boot to be done + snap wait system seed.loaded + retry-tool -n 60 --wait 5 sh -c 'snap changes | grep -q "Done.*Initialize system state"' + # extra paranoia because failure to cleanup earlier took us a long time + # to find + if [ -e /var/snap/$NEW_BASE/current ]; then + echo "Leftover $NEW_BASE data dir found, test does not " + echo "properly cleanup" + echo "see https://github.com/snapcore/snapd/pull/6620" + echo + find /var/snap + exit 1 + fi +execute: | + if [ "$TRUST_TEST_KEYS" = "false" ]; then + echo "This test needs test keys to be trusted" + exit + fi + #shellcheck source=tests/lib/boot.sh + . "$TESTSLIB"/boot.sh + wait_change_done() { + chg_summary="$1" + for _ in $(seq 10); do + if snap changes | MATCH "[0-9]+\\ +Done\\ +.* $chg_summary"; then + break + fi + # some debug output + snap changes + # wait a bit + sleep 5 + done + snap changes | MATCH "[0-9]+\\ +Done\\ +.* $chg_summary" + } + # initial boot with the current model + if [ "$SPREAD_REBOOT" = 0 ]; then + # sanity check + snap list "$OLD_BASE" + + echo "We have the right model assertion" + snap debug model|MATCH "model: my-model" + echo "Now we remodel" + snap remodel "$TESTSLIB"/assertions/developer1-pc-18-new-base.model + echo "Double check that we boot into the right base" + MATCH "snap_try_core=$NEW_BASE" < /boot/grub/grubenv + echo "reboot to finish the change" + REBOOT + fi + # first boot with the new model base + if [ "$SPREAD_REBOOT" = 1 ]; then + echo "and we have the new base snap installed" + snap list "$NEW_BASE" + echo "And are using it" + wait_core_post_boot + MATCH "snap_core=$NEW_BASE" < /boot/grub/grubenv + echo "and we got the new model assertion" + wait_change_done "Refresh model assertion from revision 0 to 2" + snap debug model|MATCH "revision: 2" + echo "and we cannot remove the base snap" + not snap remove "$NEW_BASE" + # TODO: test when keeping the old base, test removing the old base + # (not possible here as the pc gadget uses core18 as its base) + echo "And we can remodel again and remove the new base" + snap remodel "$TESTSLIB"/assertions/developer1-pc-18-revno3.model + REBOOT + fi + # reboot from new model to undo the new model again (to not pollute tests) + if [ "$SPREAD_REBOOT" = 2 ]; then + wait_core_post_boot + MATCH "snap_core=$OLD_BASE" < /boot/grub/grubenv + wait_change_done "Refresh model assertion from revision 2 to 3" + snap debug model|MATCH "revision: 3" + echo "cleanup" + snap remove "$NEW_BASE" + snap refresh --channel="$BASE_CHANNEL" "$OLD_BASE" + REBOOT + fi diff --git a/tests/main/remodel-gadget/task.yaml b/tests/main/remodel-gadget/task.yaml new file mode 100644 index 0000000000..b7e5a6a042 --- /dev/null +++ b/tests/main/remodel-gadget/task.yaml @@ -0,0 +1,124 @@ +summary: Test a remodel that switches to a new gadget +environment: + OLD_GADGET: pc + NEW_GADGET: test-snapd-pc + +systems: [ubuntu-core-18-64] + +prepare: | + if [ "$TRUST_TEST_KEYS" = "false" ]; then + echo "This test needs test keys to be trusted" + exit + fi + #shellcheck source=tests/lib/systemd.sh + . "$TESTSLIB"/systemd.sh + systemctl stop snapd.service snapd.socket + rm -rf /var/lib/snapd/assertions/* + rm -rf /var/lib/snapd/device + rm -rf /var/lib/snapd/state.json + mv /var/lib/snapd/seed/assertions/model model.bak + cp "$TESTSLIB"/assertions/developer1.account /var/lib/snapd/seed/assertions + cp "$TESTSLIB"/assertions/developer1.account-key /var/lib/snapd/seed/assertions + cp "$TESTSLIB"/assertions/developer1-pc-18.model /var/lib/snapd/seed/assertions + cp "$TESTSLIB"/assertions/testrootorg-store.account-key /var/lib/snapd/seed/assertions + # kick first boot again + systemctl start snapd.service snapd.socket + retry-tool -n 60 --wait 5 sh -c 'snap changes | grep -q "Done.*Initialize system state"' + +restore: | + if [ "$TRUST_TEST_KEYS" = "false" ]; then + echo "This test needs test keys to be trusted" + exit + fi + #shellcheck source=tests/lib/systemd.sh + . "$TESTSLIB"/systemd.sh + systemctl stop snapd.service snapd.socket + rm -rf /var/lib/snapd/assertions/* + rm -rf /var/lib/snapd/device + rm -rf /var/lib/snapd/state.json + rm -f /var/lib/snapd/seed/assertions/developer1.account + rm -f /var/lib/snapd/seed/assertions/developer1.account-key + rm -f /var/lib/snapd/seed/assertions/developer1-pc-18.model + rm -f /var/lib/snapd/seed/assertions/testrootorg-store.account-key + mv model.bak /var/lib/snapd/seed/assertions/model + rm -f ./*.bak + # kick first boot again + systemctl start snapd.service snapd.socket + # wait for first boot to be done + snap wait system seed.loaded + # cleanup the canary file + rm -f /boot/efi/canary.txt + retry-tool -n 60 --wait 5 sh -c 'snap changes | grep -q "Done.*Initialize system state"' + # extra paranoia because failure to cleanup earlier took us a long time + # to find + if [ -e /var/snap/$NEW_GADGET/current ]; then + echo "Leftover $NEW_GADGET data dir found, test does not " + echo "properly cleanup" + echo "see https://github.com/snapcore/snapd/pull/6620" + echo + find /var/snap + exit 1 + fi +execute: | + if [ "$TRUST_TEST_KEYS" = "false" ]; then + echo "This test needs test keys to be trusted" + exit + fi + #shellcheck source=tests/lib/boot.sh + . "$TESTSLIB"/boot.sh + wait_change_done() { + chg_summary="$1" + for _ in $(seq 10); do + if snap changes | MATCH "[0-9]+\\ +Done\\ +.* $chg_summary"; then + break + fi + # some debug output + snap changes + # wait a bit + sleep 5 + done + snap changes | MATCH "[0-9]+\\ +Done\\ +.* $chg_summary" + } + # initial boot with the current model + if [ "$SPREAD_REBOOT" = 0 ]; then + # sanity check + snap list "$OLD_GADGET" + + echo "We have the right model assertion" + snap debug model|MATCH "model: my-model" + echo "Now we remodel" + snap remodel "$TESTSLIB"/assertions/developer1-pc-18-new-gadget.model + echo "Double check that new gadget was installed" + test "$(cat /boot/efi/canary.txt)" = "this is test-snapd-pc gadget" + echo "reboot to finish the change" + REBOOT + fi + # first boot with the new gadget + if [ "$SPREAD_REBOOT" = 1 ]; then + echo "and we have the new gadget snap installed" + snap list "$NEW_GADGET" + echo "And are using it" + wait_core_post_boot + # gadget data is still there + test "$(cat /boot/efi/canary.txt)" = "this is test-snapd-pc gadget" + echo "and we got the new model assertion" + wait_change_done "Refresh model assertion from revision 0 to 2" + snap debug model|MATCH "revision: 2" + echo "and we cannot remove the gadget snap" + not snap remove "$NEW_GADGET" + echo "but we can remove the old gadget" + snap remove "$OLD_GADGET" + echo "And we can remodel again and remove the new gadget" + snap remodel "$TESTSLIB"/assertions/developer1-pc-18-revno3.model + REBOOT + fi + # reboot from new model to undo the new model again (to not pollute tests) + if [ "$SPREAD_REBOOT" = 2 ]; then + wait_core_post_boot + wait_change_done "Refresh model assertion from revision 2 to 3" + snap debug model|MATCH "revision: 3" + echo "cleanup" + snap remove "$NEW_GADGET" + snap refresh --channel="$GADGET_CHANNEL" "$OLD_GADGET" + REBOOT + fi diff --git a/tests/main/remove-auto-connections/simplesnap.v1/meta/snap.yaml b/tests/main/remove-auto-connections/simplesnap.v1/meta/snap.yaml new file mode 100644 index 0000000000..07d561b8db --- /dev/null +++ b/tests/main/remove-auto-connections/simplesnap.v1/meta/snap.yaml @@ -0,0 +1,5 @@ +name: simplesnap +version: 1 +summary: Simple snap +plugs: + home: diff --git a/tests/main/remove-auto-connections/simplesnap.v2/meta/snap.yaml b/tests/main/remove-auto-connections/simplesnap.v2/meta/snap.yaml new file mode 100644 index 0000000000..162f5328a0 --- /dev/null +++ b/tests/main/remove-auto-connections/simplesnap.v2/meta/snap.yaml @@ -0,0 +1,4 @@ +name: simplesnap +version: 2 +summary: Simple snap + diff --git a/tests/main/remove-auto-connections/task.yaml b/tests/main/remove-auto-connections/task.yaml new file mode 100644 index 0000000000..67a444e915 --- /dev/null +++ b/tests/main/remove-auto-connections/task.yaml @@ -0,0 +1,50 @@ +summary: Automatic connection gets dropped on refresh + +details: | + Automatic connection gets removed and not reported by connections if respective plug + is no longer present in the refreshed snap. + +# home is not auto-connected on core, the test relies on this +systems: [-ubuntu-core-*] + +prepare: | + snap install jq + snap pack simplesnap.v1 + snap pack simplesnap.v2 + +execute: | + snap install --dangerous simplesnap_1_all.snap + snap connections simplesnap | MATCH "home *simplesnap:home *:home *- *" + + inspect_connection() { + # shellcheck disable=SC2002 + cat /var/lib/snapd/state.json | jq -r '.data["conns"] | has("simplesnap:home core:home")' + } + + # sanity + inspect_connection | MATCH "true" + + echo "Checking that 'home' connection is not reported after removing the test snap" + snap remove simplesnap + if snap connections simplesnap | MATCH "home"; then + echo "Expected simplesnap:home to be gone after snap got removed" + exit 1 + fi + + inspect_connection | MATCH "false" + + echo "Checking that 'home' connection is not reported after refresh to a revision that doesn't have home plug" + + snap install --dangerous simplesnap_1_all.snap + snap connections simplesnap | MATCH "home *simplesnap:home *:home *- *" + snap install --dangerous simplesnap_2_all.snap + if snap connections simplesnap | MATCH "home"; then + echo "Expected simplesnap:home to be gone" + exit 1 + fi + + inspect_connection | MATCH "false" + +restore: | + snap remove simplesnap + rm -f simplesnap_*_all.snap diff --git a/tests/main/uc20-snap-recovery/task.yaml b/tests/main/uc20-snap-recovery/task.yaml index d73c9dd03c..444e85ed93 100644 --- a/tests/main/uc20-snap-recovery/task.yaml +++ b/tests/main/uc20-snap-recovery/task.yaml @@ -24,6 +24,7 @@ prepare: | echo "Create an empty partition header" echo "label: gpt" | sfdisk "$LOOP" + # XXX: update to the UC20 gadget when it's available echo "Get the UC16 gadget" snap download pc unsquashfs -d gadget-dir pc_*.snap |
