overlord/snapstate: add migration function to fix invalid channel spec

overlord/snapstate/snapstate.go

@@ -1373,6 +1373,41 @@ func Update(st *state.State, name string, opts *RevisionOptions, userID int, fla
 	return UpdateWithDeviceContext(st, name, opts, userID, flags, nil, "")
 }
 
+// maybeMigrateSnapStateChannel checks if the stored Channel in the snap state is
+// of the form "/<some-risk>", and if so migrates to just be "<some-risk>"
+// this is needed because previously we didn't verify the snap channel when
+// installing / refreshing, but now we verify it on refresh so previous channel
+// specs that were saved like this need to migrated so that refreshes work
+func maybeMigrateSnapStateChannel(st *state.State, snapName string) (string, error) {
+	var snapst SnapState
+	err := Get(st, snapName, &snapst)
+	if err != nil && err != state.ErrNoState {
+		return "", err
+	}
+
+	if snapst.Channel != "" {
+		parts := strings.Split(snapst.Channel, "/")
+		if len(parts) > 1 && parts[0] == "" {
+			// migration needed, but note that there's option of any of the
+			// following no longer supported specifications:
+			// * /<risk>
+			// * /<track>
+			// * /<track>/<risk>
+			// * /<track>/<risk>/<branch>
+			// * /<risk>/<branch>
+			// but the migration for now is to just drop the leading "/", so we
+			// handle them all the same
+			// see https://bugs.launchpad.net/snapd/+bug/1841475 for more
+			// details
+			parts = parts[1:]
+			snapst.Channel = strings.Join(parts, "/")
+			Set(st, snapName, &snapst)
+		}
+	}
+
+	return snapst.Channel, nil
+}
+
 // UpdateWithDeviceContext initiates a change updating a snap.
 // It will query for the snap with the given deviceCtx.
 // Note that the state must be locked by the caller.
@@ -1403,6 +1438,16 @@ func UpdateWithDeviceContext(st *state.State, name string, opts *RevisionOptions
 		return nil, err
 	}
 
+	// check if we need to migrate the snap channel before in snap state before
+	// resolving the channel
+	newChannel, err := maybeMigrateSnapStateChannel(st, name)
+	if err != nil {
+		return nil, err
+	}
+
+	// update the channel with what we maybe got changed from
+	snapst.Channel = newChannel
+
 	opts.Channel, err = resolveChannel(st, name, opts.Channel, deviceCtx)
 	if err != nil {
 		return nil, err

overlord/snapstate/snapstate_test.go

@@ -2258,6 +2258,53 @@ func (s *snapmgrTestSuite) TestUpdateWithDeviceContext(c *C) {
 	c.Check(validateCalled, Equals, true)
 }
 
+func (s *snapmgrTestSuite) TestUpdateWithDeviceContextWithOldChannelSpecPatched(c *C) {
+	s.state.Lock()
+	defer s.state.Unlock()
+
+	// unset the global store, it will need to come via the device context
+	snapstate.ReplaceStore(s.state, nil)
+
+	deviceCtx := &snapstatetest.TrivialDeviceContext{
+		DeviceModel: DefaultModel(),
+		CtxStore:    s.fakeStore,
+	}
+
+	happyValidateRefreshes := func(st *state.State, refreshes []*snap.Info, ignoreValidation map[string]bool, userID int, deviceCtx1 snapstate.DeviceContext) ([]*snap.Info, error) {
+		return refreshes, nil
+	}
+	// hook it up
+	snapstate.ValidateRefreshes = happyValidateRefreshes
+
+	for _, tt := range []struct {
+		channelBefore string
+		channelAfter  string
+	}{
+		{"/edge", "edge"},
+		{"/beta", "beta"},
+		{"/candidate", "candidate"},
+		{"/stable", "stable"},
+	} {
+		snapstate.Set(s.state, "some-snap", &snapstate.SnapState{
+			Active:   true,
+			Channel:  tt.channelBefore,
+			Sequence: []*snap.SideInfo{{RealName: "some-snap", SnapID: "some-snap-id", Revision: snap.R(7)}},
+			Current:  snap.R(7),
+			SnapType: "app",
+		})
+
+		ts, err := snapstate.UpdateWithDeviceContext(s.state, "some-snap", &snapstate.RevisionOptions{Channel: "/edge"}, s.user.ID, snapstate.Flags{}, deviceCtx, "")
+		c.Assert(err, IsNil, Commentf("%+v", tt))
+		verifyUpdateTasks(c, unlinkBefore|cleanupAfter|doesReRefresh, 0, ts, s.state)
+
+		afterSnapState := snapstate.SnapState{}
+		snapstate.Get(s.state, "some-snap", &afterSnapState)
+		c.Check(afterSnapState.Channel, Equals, tt.channelAfter, Commentf("%+v", tt))
+
+	}
+
+}
+
 func (s *snapmgrTestSuite) TestUpdateWithDeviceContextToRevision(c *C) {
 	s.state.Lock()
 	defer s.state.Unlock()