enforce section marshal ordering

internals/plan/extensions_test.go

@@ -414,6 +414,85 @@ nexttest:
 	}
 }
 
+// TestSectionOrderExt ensures built-in and extension section ordering
+// does not change. Extensions are ordered according to the order of
+// registration.
+func (s *S) TestSectionOrderExt(c *C) {
+	plan.RegisterExtension("x-field", &xExtension{})
+	plan.RegisterExtension("y-field", &yExtension{})
+	defer func() {
+		plan.UnregisterExtension("x-field")
+		plan.UnregisterExtension("y-field")
+	}()
+
+	layer, err := plan.ParseLayer(1, "label", reindent(`
+	y-field:
+		y1:
+			override: replace
+			a: a
+			b: b
+	checks:
+		chk1:
+			override: replace
+			exec:
+				command: ping 8.8.8.8
+	x-field:
+		x1:
+			override: replace
+			a: a
+			b: b
+			y-field:
+				- y1
+	log-targets:
+		lt1:
+			override: replace
+			type: loki
+			location: http://192.168.1.2:3100/loki/api/v1/push
+	services:
+		srv1:
+			override: replace
+			command: cmd`))
+	c.Assert(err, IsNil)
+	combined, err := plan.CombineLayers(layer)
+	c.Assert(err, IsNil)
+	plan := plan.Plan{
+		Services:   combined.Services,
+		Checks:     combined.Checks,
+		LogTargets: combined.LogTargets,
+		Sections:   combined.Sections,
+	}
+	data, err := yaml.Marshal(plan)
+	c.Assert(string(data), Equals, string(reindent(`
+	services:
+		srv1:
+			override: replace
+			command: cmd
+	checks:
+		chk1:
+			override: replace
+			threshold: 3
+			exec:
+				command: ping 8.8.8.8
+	log-targets:
+		lt1:
+			type: loki
+			location: http://192.168.1.2:3100/loki/api/v1/push
+			services: []
+			override: replace
+	x-field:
+		x1:
+			override: replace
+			a: a
+			b: b
+			y-field:
+				- y1
+	y-field:
+		y1:
+			override: replace
+			a: a
+			b: b`)))
+}
+
 // writeLayerFiles writes layer files of a test to disk.
 func (s *S) writeLayerFiles(c *C, layersDir string, inputs []*inputLayer) {
 	err := os.MkdirAll(layersDir, 0755)

internals/plan/plan.go

@@ -68,16 +68,23 @@ const (
 	defaultCheckThreshold = 3
 )
 
-// layerExtensions keeps a map of registered extensions.
-var layerExtensions = map[string]LayerSectionExtension{}
+var (
+	// layerExtensions keeps a map of registered extensions.
+	layerExtensions = map[string]LayerSectionExtension{}
+
+	// layerExtensionsOrder records the order in which the extensions were registered.
+	layerExtensionsOrder = []string{}
+)
 
 // layerBuiltins represents all the built-in layer sections. This list is used
 // for identifying built-in fields in this package. It is unit tested to match
 // the YAML fields exposed in the Layer type, to catch inconsistencies.
 var layerBuiltins = []string{"summary", "description", "services", "checks", "log-targets"}
 
 // RegisterExtension adds a plan schema extension. All registrations must be
-// done before the plan library is used.
+// done before the plan library is used. The order in which extensions are
+// registered determines the order in which the sections are marshalled.
+// Extension sections are marshalled after the built-in sections.
 func RegisterExtension(field string, ext LayerSectionExtension) {
 	if slices.Contains(layerBuiltins, field) {
 		panic(fmt.Sprintf("internal error: extension %q already used as built-in field", field))
@@ -86,12 +93,16 @@ func RegisterExtension(field string, ext LayerSectionExtension) {
 		panic(fmt.Sprintf("internal error: extension %q already registered", field))
 	}
 	layerExtensions[field] = ext
+	layerExtensionsOrder = append(layerExtensionsOrder, field)
 }
 
 // UnregisterExtension removes a plan schema extension. This is only
 // intended for use by tests during cleanup.
 func UnregisterExtension(field string) {
 	delete(layerExtensions, field)
+	layerExtensionsOrder = slices.DeleteFunc(layerExtensionsOrder, func(n string) bool {
+		return n == field
+	})
 }
 
 type Plan struct {
@@ -114,22 +125,40 @@ func (p *Plan) Section(field string) LayerSection {
 // This is required since Sections are based on an inlined map, for which
 // omitempty and inline together is not currently supported.
 func (p *Plan) MarshalYAML() (interface{}, error) {
-	data := make(map[string]interface{})
-	if len(p.Services) != 0 {
-		data["services"] = p.Services
-	}
-	if len(p.LogTargets) != 0 {
-		data["log-targets"] = p.LogTargets
-	}
-	if len(p.Checks) != 0 {
-		data["checks"] = p.Checks
-	}
-	for field, section := range p.Sections {
-		if !section.IsZero() {
-			data[field] = section
-		}
-	}
-	return data, nil
+	// Define the content inside a structure so we can control the ordering
+	// of top level sections.
+	ordered := []reflect.StructField{{
+		Name: "Services",
+		Type: reflect.TypeOf(p.Services),
+		Tag:  `yaml:"services,omitempty"`,
+	}, {
+		Name: "Checks",
+		Type: reflect.TypeOf(p.Checks),
+		Tag:  `yaml:"checks,omitempty"`,
+	}, {
+		Name: "LogTargets",
+		Type: reflect.TypeOf(p.LogTargets),
+		Tag:  `yaml:"log-targets,omitempty"`,
+	}}
+	for i, field := range layerExtensionsOrder {
+		section := p.Sections[field]
+		ordered = append(ordered, reflect.StructField{
+			Name: fmt.Sprintf("Dummy%v", i),
+			Type: reflect.TypeOf(section),
+			Tag:  reflect.StructTag(fmt.Sprintf("yaml:\"%s,omitempty\"", field)),
+		})
+	}
+	typ := reflect.StructOf(ordered)
+	// Assign the plan data to the structure layout we created.
+	v := reflect.New(typ).Elem()
+	v.Field(0).Set(reflect.ValueOf(p.Services))
+	v.Field(1).Set(reflect.ValueOf(p.Checks))
+	v.Field(2).Set(reflect.ValueOf(p.LogTargets))
+	for i, field := range layerExtensionsOrder {
+		v.Field(3 + i).Set(reflect.ValueOf(p.Sections[field]))
+	}
+	plan := v.Addr().Interface()
+	return plan, nil
 }
 
 type Layer struct {

internals/plan/plan_test.go

@@ -2109,3 +2109,48 @@ func firstLetterToLower(s string) string {
 	r[0] = unicode.ToLower(r[0])
 	return string(r)
 }
+
+// TestSectionOrder ensures built-in section order is maintained.
+func (s *S) TestSectionOrder(c *C) {
+	layer, err := plan.ParseLayer(1, "label", reindent(`
+	checks:
+		chk1:
+			override: replace
+			exec:
+				command: ping 8.8.8.8
+	log-targets:
+		lt1:
+			override: replace
+			type: loki
+			location: http://192.168.1.2:3100/loki/api/v1/push
+	services:
+		srv1:
+			override: replace
+			command: cmd`))
+	c.Assert(err, IsNil)
+	combined, err := plan.CombineLayers(layer)
+	c.Assert(err, IsNil)
+	plan := plan.Plan{
+		Services:   combined.Services,
+		Checks:     combined.Checks,
+		LogTargets: combined.LogTargets,
+	}
+	data, err := yaml.Marshal(plan)
+	c.Assert(string(data), Equals, string(reindent(`
+	services:
+		srv1:
+			override: replace
+			command: cmd
+	checks:
+		chk1:
+			override: replace
+			threshold: 3
+			exec:
+				command: ping 8.8.8.8
+	log-targets:
+		lt1:
+			type: loki
+			location: http://192.168.1.2:3100/loki/api/v1/push
+			services: []
+			override: replace`)))
+}