fix(planstate): leave the plan unchanged when an appended layer fails validation

internals/overlord/planstate/manager.go

@@ -116,7 +116,8 @@ func (m *PlanManager) Plan() *plan.Plan {
 // layer.Order field to the new order. If a layer with layer.Label already
 // exists, return an error of type *LabelExists. Inner must be set to true
 // if the append operation may be demoted to an insert due to the layer
-// configuration being located in a sub-directory.
+// configuration being located in a sub-directory. On any error the plan is
+// left exactly as it was, including its layer list.
 func (m *PlanManager) AppendLayer(layer *plan.Layer, inner bool) error {
 	var newPlan *plan.Plan
 	defer func() { m.callChangeListeners(newPlan) }()
@@ -240,7 +241,9 @@ func (m *PlanManager) appendLayer(newLayer *plan.Layer, inner bool) (*plan.Plan,
 		return nil, fmt.Errorf("cannot insert sub-directory layer without 'inner' attribute set")
 	}
 
-	newLayers := slices.Insert(m.plan.Layers, newIndex, newLayer)
+	// Insert into a copy: slices.Insert shifts in place when the slice has
+	// spare capacity, which would change the live plan before validation.
+	newLayers := slices.Insert(slices.Clone(m.plan.Layers), newIndex, newLayer)
 	newPlan, err := m.updatePlanLayers(newLayers)
 	if err != nil {
 		return nil, err

internals/overlord/planstate/manager_test.go

@@ -15,6 +15,9 @@
 package planstate_test
 
 import (
+	"fmt"
+	"sort"
+	"strings"
 	"sync/atomic"
 	"time"
 
@@ -674,3 +677,77 @@ workloads:
 	err = ps.planMgr.CombineLayer(layer, false)
 	c.Assert(err, IsNil)
 }
+
+// TestAppendLayerRejectedLeavesPlanUnchanged pins the rollback contract of
+// AppendLayer: a layer that fails plan validation must leave the plan's layer
+// list exactly as it was. The layout matters - the sub-directory layer is
+// first so the insert index is not the end of the list, and there are enough
+// layers for the slice to carry spare capacity, which is what once let
+// slices.Insert shift the live list in place before validation ran.
+func (ps *planSuite) TestAppendLayerRejectedLeavesPlanUnchanged(c *C) {
+	var err error
+	ps.planMgr, err = planstate.NewManager(ps.layersDir)
+	c.Assert(err, IsNil)
+
+	serviceLayer := string(reindent(`
+		services:
+			%s:
+				override: replace
+				command: sleep 1000
+	`))
+
+	appendLabels := []string{"foo/one", "bbb", "ccc", "ddd", "eee"}
+	for _, label := range appendLabels {
+		name := "svc-" + strings.ReplaceAll(label, "/", "-")
+		layer := ps.parseLayer(c, 0, label, fmt.Sprintf(serviceLayer, name))
+		err = ps.planMgr.AppendLayer(layer, true)
+		c.Assert(err, IsNil)
+	}
+
+	before := ps.planMgr.Plan()
+	labelsBefore := layerLabels(before)
+	servicesBefore := serviceNames(before)
+
+	// This layer parses, but fails plan validation because svc-missing is
+	// not defined anywhere.
+	rejected := ps.parseLayer(c, 0, "foo/two", string(reindent(`
+		services:
+			svc-injected:
+				override: replace
+				command: sleep 1000
+				requires: [svc-missing]
+	`)))
+	err = ps.planMgr.AppendLayer(rejected, true)
+	c.Assert(err, ErrorMatches, `service "svc-missing" does not exist`)
+
+	after := ps.planMgr.Plan()
+	c.Assert(layerLabels(after), DeepEquals, labelsBefore)
+	c.Assert(serviceNames(after), DeepEquals, servicesBefore)
+
+	// A later valid append must not resurrect the rejected layer, and must
+	// not have lost a layer either.
+	accepted := ps.parseLayer(c, 0, "later", fmt.Sprintf(serviceLayer, "svc-later"))
+	err = ps.planMgr.AppendLayer(accepted, false)
+	c.Assert(err, IsNil)
+
+	final := ps.planMgr.Plan()
+	c.Assert(layerLabels(final), DeepEquals, append(append([]string(nil), labelsBefore...), "later"))
+	c.Assert(serviceNames(final), DeepEquals, append(append([]string(nil), servicesBefore...), "svc-later"))
+}
+
+func layerLabels(p *plan.Plan) []string {
+	labels := make([]string, 0, len(p.Layers))
+	for _, layer := range p.Layers {
+		labels = append(labels, layer.Label)
+	}
+	return labels
+}
+
+func serviceNames(p *plan.Plan) []string {
+	names := make([]string, 0, len(p.Services))
+	for name := range p.Services {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	return names
+}