Skip to content

Commit

Permalink
o/devicestate: skip optional snaps in model when creating recovery sy…
Browse files Browse the repository at this point in the history 
…stem

Snaps that are optional in the model should only be a part of the
recovery system if they are required by one of the validation sets or if
they are already installed on the system.
  • Loading branch information
@andrewphelpsj
andrewphelpsj committed Feb 27, 2024
1 parent 069b7f6 commit 72b0aef
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 81 deletions.
18 changes: 13 additions & 5 deletions overlord/devicestate/devicestate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1718,11 +1718,6 @@ func CreateRecoverySystem(st *state.State, label string, opts CreateRecoverySyst
return nil, err
}

// snaps required by validation sets must also be required by the model
if err := checkForRequiredSnapsNotRequiredInModel(model, valsets); err != nil {
return nil, err
}

tracker := snap.NewSelfContainedSetPrereqTracker()
offline := len(opts.LocalSnaps) > 0

Expand All @@ -1744,6 +1739,19 @@ func CreateRecoverySystem(st *state.State, label string, opts CreateRecoverySyst
continue
}

if sn.Presence != "required" {
sets, _, err := valsets.CheckPresenceRequired(sn)
if err != nil {
return nil, err
}

// snap isn't already installed, and it isn't required by model or
// any validation sets, so we should skip it
if len(sets) == 0 {
continue
}
}

if offline {
info, err := offlineSnapInfo(sn, rev, opts)
if err != nil {
Expand Down
168 changes: 92 additions & 76 deletions overlord/devicestate/devicestate_systems_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2830,61 +2830,6 @@ func fakeSnapID(name string) string {
return snaptest.AssertedSnapID(name)
}

func (s *deviceMgrSystemsCreateSuite) TestDeviceManagerCreateRecoverySystemValidationSetsModelMissingRequired(c *C) {
devicestate.SetBootOkRan(s.mgr, true)

vset1, err := s.brands.Signing("canonical").Sign(asserts.ValidationSetType, map[string]interface{}{
"type": "validation-set",
"authority-id": "canonical",
"series": "16",
"account-id": "canonical",
"name": "vset-1",
"sequence": "1",
"snaps": []interface{}{
map[string]interface{}{
"name": "snapd",
"id": fakeSnapID("snapd"),
"revision": "12",
"presence": "required",
},
map[string]interface{}{
"name": "core20",
"id": fakeSnapID("core20"),
"revision": "12",
"presence": "required",
},
map[string]interface{}{
"name": "pc",
"id": fakeSnapID("pc"),
"revision": "12",
"presence": "required",
},
map[string]interface{}{
"name": "pc-kernel",
"id": fakeSnapID("pc-kernel"),
"revision": "12",
"presence": "required",
},
map[string]interface{}{
"name": "snap-1",
"id": fakeSnapID("snap-1"),
"revision": "12",
"presence": "required",
},
},
"timestamp": time.Now().UTC().Format(time.RFC3339),
}, nil, "")
c.Assert(err, IsNil)

s.state.Lock()
defer s.state.Unlock()

_, err = devicestate.CreateRecoverySystem(s.state, "1234", devicestate.CreateRecoverySystemOptions{
ValidationSets: []*asserts.ValidationSet{vset1.(*asserts.ValidationSet)},
})
c.Assert(err, ErrorMatches, "missing required snap in model: snap-1")
}

func (s *deviceMgrSystemsCreateSuite) TestDeviceManagerCreateRecoverySystemValidationSetsSnapInvalid(c *C) {
devicestate.SetBootOkRan(s.mgr, true)

Expand Down Expand Up @@ -3185,16 +3130,34 @@ func checkForSnapsInSeed(c *C, snaps ...string) {
}

func (s *deviceMgrSystemsCreateSuite) TestDeviceManagerCreateRecoverySystemValidationSetsMarkCurrent(c *C) {
const markCurrent = true
s.testDeviceManagerCreateRecoverySystemValidationSetsHappy(c, markCurrent)
s.testDeviceManagerCreateRecoverySystemValidationSetsHappy(c, testCreateRecoverySystemValidationSetsOptions{
MarkCurrent: true,
})
}

func (s *deviceMgrSystemsCreateSuite) TestDeviceManagerCreateRecoverySystemValidationSetsNoMarkCurrent(c *C) {
const markCurrent = false
s.testDeviceManagerCreateRecoverySystemValidationSetsHappy(c, markCurrent)
s.testDeviceManagerCreateRecoverySystemValidationSetsHappy(c, testCreateRecoverySystemValidationSetsOptions{})
}

func (s *deviceMgrSystemsCreateSuite) TestDeviceManagerCreateRecoverySystemValidationSetsOptionalSnap(c *C) {
s.testDeviceManagerCreateRecoverySystemValidationSetsHappy(c, testCreateRecoverySystemValidationSetsOptions{
RequireOptionalSnapInValidationSet: true,
})
}

func (s *deviceMgrSystemsCreateSuite) TestDeviceManagerCreateRecoverySystemValidationSetsPreinstallOptionalSnap(c *C) {
s.testDeviceManagerCreateRecoverySystemValidationSetsHappy(c, testCreateRecoverySystemValidationSetsOptions{
PreInstallOptionalSnap: true,
})
}

type testCreateRecoverySystemValidationSetsOptions struct {
MarkCurrent bool
RequireOptionalSnapInValidationSet bool
PreInstallOptionalSnap bool
}

func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValidationSetsHappy(c *C, markCurrent bool) {
func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValidationSetsHappy(c *C, opts testCreateRecoverySystemValidationSetsOptions) {
devicestate.SetBootOkRan(s.mgr, true)

s.state.Lock()
Expand Down Expand Up @@ -3228,6 +3191,12 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
"id": s.ss.AssertedSnapID("snapd"),
"type": "snapd",
},
map[string]interface{}{
"name": "other-required",
"id": s.ss.AssertedSnapID("other-required"),
"type": "app",
"presence": "optional",
},
},
"validation-sets": []interface{}{
map[string]interface{}{
Expand Down Expand Up @@ -3265,19 +3234,23 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
})

snapRevisions := map[string]snap.Revision{
"pc": snap.R(10),
"pc-kernel": snap.R(11),
"core20": snap.R(12),
"snapd": snap.R(13),
"pc": snap.R(10),
"pc-kernel": snap.R(11),
"core20": snap.R(12),
"snapd": snap.R(13),
"other-required": snap.R(14),
}

snapTypes := map[string]snap.Type{
"pc": snap.TypeGadget,
"pc-kernel": snap.TypeKernel,
"core20": snap.TypeBase,
"snapd": snap.TypeSnapd,
"pc": snap.TypeGadget,
"pc-kernel": snap.TypeKernel,
"core20": snap.TypeBase,
"snapd": snap.TypeSnapd,
"other-required": snap.TypeApp,
}

var validationSets []*asserts.ValidationSet

vsetAssert, err := s.brands.Signing("canonical").Sign(asserts.ValidationSetType, map[string]interface{}{
"type": "validation-set",
"authority-id": "canonical",
Expand Down Expand Up @@ -3315,7 +3288,34 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
}, nil, "")
c.Assert(err, IsNil)

vset := vsetAssert.(*asserts.ValidationSet)
validationSets = append(validationSets, vsetAssert.(*asserts.ValidationSet))

if opts.PreInstallOptionalSnap {
s.makeSnapInState(c, "other-required", snapRevisions["other-required"], nil)
}

if opts.RequireOptionalSnapInValidationSet {
vsetAssert, err := s.brands.Signing("canonical").Sign(asserts.ValidationSetType, map[string]interface{}{
"type": "validation-set",
"authority-id": "canonical",
"series": "16",
"account-id": "canonical",
"name": "vset-2",
"sequence": "1",
"snaps": []interface{}{
map[string]interface{}{
"name": "other-required",
"id": fakeSnapID("other-required"),
"revision": snapRevisions["other-required"].String(),
"presence": "required",
},
},
"timestamp": time.Now().UTC().Format(time.RFC3339),
}, nil, "")
c.Assert(err, IsNil)

validationSets = append(validationSets, vsetAssert.(*asserts.ValidationSet))
}

s.o.TaskRunner().AddHandler("mock-validate", func(task *state.Task, _ *tomb.Tomb) error {
st := task.State()
Expand Down Expand Up @@ -3412,15 +3412,21 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
s.mockStandardSnapsModeenvAndBootloaderState(c)

chg, err := devicestate.CreateRecoverySystem(s.state, "1234", devicestate.CreateRecoverySystemOptions{
ValidationSets: []*asserts.ValidationSet{vset},
ValidationSets: validationSets,
TestSystem: true,
MarkCurrent: markCurrent,
MarkCurrent: opts.MarkCurrent,
})
c.Assert(err, IsNil)
c.Assert(chg, NotNil)
tsks := chg.Tasks()
// 2*4 snaps (download + validate) + create system + finalize system
c.Check(tsks, HasLen, (2*4)+2)

snapCount := 4
if opts.RequireOptionalSnapInValidationSet {
snapCount++
}
// 2*snapCount snaps (download + validate) + create system + finalize system
c.Check(tsks, HasLen, (2*snapCount)+2)

tskCreate := tsks[0]
tskFinalize := tsks[1]
c.Assert(tskCreate.Summary(), Matches, `Create recovery system with label "1234"`)
Expand All @@ -3437,7 +3443,12 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
// a reboot is expected
c.Check(s.restartRequests, DeepEquals, []restart.RestartType{restart.RestartSystemNow})

validateCore20Seed(c, "1234", s.model, s.storeSigning.Trusted)
var runModeSnaps []string
if opts.RequireOptionalSnapInValidationSet || opts.PreInstallOptionalSnap {
runModeSnaps = []string{"other-required"}
}
validateCore20Seed(c, "1234", s.model, s.storeSigning.Trusted, runModeSnaps...)

m, err := s.bootloader.GetBootVars("try_recovery_system", "recovery_system_status")
c.Assert(err, IsNil)
c.Check(m, DeepEquals, map[string]string{
Expand All @@ -3460,8 +3471,13 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
})

// verify that new files are tracked correctly
expectedFiles := []string{"snapd_13.snap", "pc-kernel_11.snap", "core20_12.snap", "pc_10.snap"}
if opts.RequireOptionalSnapInValidationSet || opts.PreInstallOptionalSnap {
expectedFiles = append(expectedFiles, "other-required_14.snap")
}

expectedFilesLog := &bytes.Buffer{}
for _, fname := range []string{"snapd_13.snap", "pc-kernel_11.snap", "core20_12.snap", "pc_10.snap"} {
for _, fname := range expectedFiles {
fmt.Fprintln(expectedFilesLog, filepath.Join(boot.InitramfsUbuntuSeedDir, "snaps", fname))
}

Expand Down Expand Up @@ -3513,7 +3529,7 @@ func (s *deviceMgrSystemsCreateSuite) testDeviceManagerCreateRecoverySystemValid
c.Check(s.bootloader.SetBootVarsCalls, Equals, 1)
c.Check(filepath.Join(boot.InitramfsUbuntuSeedDir, "systems", "1234", "snapd-new-file-log"), testutil.FileAbsent)

if markCurrent {
if opts.MarkCurrent {
var systems []devicestate.SeededSystem
err := s.state.Get("seeded-systems", &systems)
c.Assert(err, IsNil)
Expand Down

0 comments on commit 72b0aef

Please sign in to comment.