From 9b7661a2a4e7eccf30ac243ca1d27ad580427340 Mon Sep 17 00:00:00 2001 From: Frederik Lotter Date: Fri, 30 Aug 2024 08:54:58 +0200 Subject: [PATCH] feat(plan): add plan section support (#488) The plan library was originally written as a configuration schema for the services manager (servstate). Over time the need arose to support configurations for other managers such as checks (checkstate) and log-targets (logstate). The configuration schema for these managers, closely related to the services manager, has since also been built in to the plan library. The services manager and its related checks and log-targets functionality will always be part of the Pebble core. However, as Pebble is getting more functionality (additional managers) and also used as a core in derivative projects, a more modular and dynamic approach to extending the schema is needed. Add an the ```SectionExtension``` interface for use by managers who wishes to register a schema extension during Pebble startup. Inside each layer, top level entries are now referred to as ```sections``` (built-in sections includes ```summary```, ```description```, ```services```, ```log-targets``` and ```checks```). Each section has an associated ```field``` that is the top level key, and if supplied by an extension, an opaque backing type ```Section```. **SectionExtension interface:** ``` // SectionExtension allows the plan layer schema to be extended without // adding centralised schema knowledge to the plan library. type SectionExtension interface { // ParseSection returns a newly allocated concrete type containing the // unmarshalled section content. ParseSection(data yaml.Node) (LayerSection, error) // CombineSections returns a newly allocated concrete type containing the // result of combining the supplied sections in order. CombineSections(sections ...LayerSection) (LayerSection, error) // ValidatePlan takes the complete plan as input, and allows the // extension to validate the plan. This can be used for cross section // dependency validation. ValidatePlan(plan *Plan) error } type Section interface { // Validate checks whether the section is valid, returning an error if not. Validate() error // IsZero reports whether the section is empty. IsZero() bool } ``` **Example usage:** ``` // New SectionExtension type type fooExtension struct{} func (f *fooExtension) ParseSection(data yaml.Node) (LayerSection, error) {...} func (f *fooExtension) CombineSections(sections ...LayerSection) (LayerSection, error) {...} func (f *fooExtension) ValidatePlan(plan *Plan) error {...} // New Section type type FooSection struct { Entries map[string]Bar `yaml:",inline,omitempty"` } type Bar struct { Name string `yaml:"name,omitempty"` } func (s *FooSection) Validate() error {...} func (s *FooSection) IsZero() bool {...} ``` ``` // Early startup plan.RegisterExtension("foo", &fooExtension{}) : // Load layers containing new section newPlan := plan.ReadDir(layersDir) : // Show plan yaml.Marshal(newPlan) ``` Example YAML output: ``` foo: bar1: name: test1 bar2: name: test2 ``` --- internals/plan/export_test.go | 17 + internals/plan/extensions_test.go | 778 ++++++++++++++++++++++++++++++ internals/plan/plan.go | 230 ++++++++- internals/plan/plan_test.go | 115 ++++- 4 files changed, 1127 insertions(+), 13 deletions(-) create mode 100644 internals/plan/export_test.go create mode 100644 internals/plan/extensions_test.go diff --git a/internals/plan/export_test.go b/internals/plan/export_test.go new file mode 100644 index 00000000..1078d199 --- /dev/null +++ b/internals/plan/export_test.go @@ -0,0 +1,17 @@ +// Copyright (c) 2024 Canonical Ltd +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License version 3 as +// published by the Free Software Foundation. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package plan + +var BuiltinSections = builtinSections diff --git a/internals/plan/extensions_test.go b/internals/plan/extensions_test.go new file mode 100644 index 00000000..0be9d051 --- /dev/null +++ b/internals/plan/extensions_test.go @@ -0,0 +1,778 @@ +// Copyright (c) 2024 Canonical Ltd +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plan_test + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "slices" + "strings" + + . "gopkg.in/check.v1" + "gopkg.in/yaml.v3" + + "github.com/canonical/pebble/internals/plan" +) + +type inputLayer struct { + order int + label string + yaml string +} + +// PlanResult represents the final content of a combined plan. Since this +// test file exclusively focuses on extensions, all built-in sections are +// empty and ignored in the test results. +type planResult struct { + x *xSection + y *ySection +} + +type extension struct { + field string + ext plan.SectionExtension +} + +var extensionTests = []struct { + summary string + extensions []extension + layers []*inputLayer + error string + result *planResult + resultYaml string +}{{ + summary: "No Sections", + resultYaml: "{}\n", +}, { + summary: "Section using built-in name", + extensions: []extension{{ + field: "summary", + ext: &xExtension{}, + }}, + error: ".*already used as built-in field.*", +}, { + summary: "Sections with empty YAML", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + result: &planResult{ + x: &xSection{}, + y: &ySection{}, + }, + resultYaml: "{}\n", +}, { + summary: "Load file layers invalid section", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-xy", + yaml: ` + summary: xy + description: desc + invalid:`, + }}, + error: "cannot parse layer .*: unknown section .*", +}, { + summary: "Load file layers not unique order", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-1", + yaml: ` + summary: xy + description: desc`, + }, { + order: 1, + label: "layer-2", + yaml: ` + summary: xy + description: desc`, + }}, + error: "invalid layer filename: .* not unique .*", +}, { + summary: "Load file layers not unique label", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-xy", + yaml: ` + summary: xy + description: desc`, + }, { + order: 2, + label: "layer-xy", + yaml: ` + summary: xy + description: desc`, + }}, + error: "invalid layer filename: .* not unique .*", +}, { + summary: "Load file layers with empty section", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x`, + }, { + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y`, + }}, + result: &planResult{ + x: &xSection{}, + y: &ySection{}, + }, + resultYaml: "{}\n", +}, { + summary: "Load file layers with section validation failure #1", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + z1: + override: replace + a: a + b: b`, + }}, + error: ".*cannot accept entry not starting.*", +}, { + summary: "Load file layers with section validation failure #2", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + x1:`, + }}, + error: ".*cannot have nil entry.*", +}, { + summary: "Load file layers failed plan validation", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + x1: + override: replace + a: a + b: b + y-field: + - y2`, + }, { + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y + y-field: + y1: + override: replace + a: a + b: b`, + }}, + error: ".*cannot find.*", +}, { + summary: "Check empty section omits entry", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field:`, + }, { + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y + y-field:`, + }}, + result: &planResult{ + x: &xSection{}, + y: &ySection{}, + }, + resultYaml: "{}\n", +}, { + summary: "Load file layers", + extensions: []extension{{ + field: "x-field", + ext: &xExtension{}, + }, { + field: "y-field", + ext: &yExtension{}, + }}, + layers: []*inputLayer{{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + x1: + override: replace + a: a + b: b + y-field: + - y1`, + }, { + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y + y-field: + y1: + override: replace + a: a + b: b`, + }}, + result: &planResult{ + x: &xSection{ + Entries: map[string]*X{ + "x1": &X{ + Name: "x1", + Override: plan.ReplaceOverride, + A: "a", + B: "b", + Y: []string{ + "y1", + }, + }, + }, + }, + y: &ySection{ + Entries: map[string]*Y{ + "y1": &Y{ + Name: "y1", + Override: plan.ReplaceOverride, + A: "a", + B: "b", + }, + }, + }, + }, + resultYaml: string(reindent(` + x-field: + x1: + override: replace + a: a + b: b + y-field: + - y1 + y-field: + y1: + override: replace + a: a + b: b`)), +}} + +func (s *S) TestPlanExtensions(c *C) { + registeredExtensions := []string{} + defer func() { + // Remove remaining registered extensions. + for _, field := range registeredExtensions { + plan.UnregisterSectionExtension(field) + } + }() + +nexttest: + for testIndex, testData := range extensionTests { + c.Logf("TestPlanExtensions :: %s (data index %v)", testData.summary, testIndex) + + // Unregister extensions from previous test iteraton. + for _, field := range registeredExtensions { + plan.UnregisterSectionExtension(field) + } + registeredExtensions = []string{} + + // Write layers to test directory. + layersDir := filepath.Join(c.MkDir(), "layers") + s.writeLayerFiles(c, layersDir, testData.layers) + var p *plan.Plan + + // Register extensions for this test iteration. + for _, e := range testData.extensions { + err := func() (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("%v", r) + } + }() + plan.RegisterSectionExtension(e.field, e.ext) + registeredExtensions = append(registeredExtensions, e.field) + return nil + }() + if err != nil { + c.Assert(err, ErrorMatches, testData.error) + continue nexttest + } + } + + // Load the plan layer from disk (parse, combine and validate). + p, err := plan.ReadDir(layersDir) + if testData.error != "" || err != nil { + // Expected error. + c.Assert(err, ErrorMatches, testData.error) + continue nexttest + } + + if slices.ContainsFunc(testData.extensions, func(n extension) bool { + return n.field == xField + }) { + // Verify "x-field" data. + var x *xSection + x = p.Sections[xField].(*xSection) + c.Assert(err, IsNil) + c.Assert(x.Entries, DeepEquals, testData.result.x.Entries) + } + + if slices.ContainsFunc(testData.extensions, func(n extension) bool { + return n.field == yField + }) { + // Verify "y-field" data. + var y *ySection + y = p.Sections[yField].(*ySection) + c.Assert(err, IsNil) + c.Assert(y.Entries, DeepEquals, testData.result.y.Entries) + } + + // Verify combined plan YAML. + planYAML, err := yaml.Marshal(p) + c.Assert(err, IsNil) + c.Assert(string(planYAML), Equals, testData.resultYaml) + } +} + +// TestSectionOrderExt ensures built-in and extension section ordering +// rules are maintained. Extensions are ordered according to the order of +// registration and follows the built-in sections which are ordered +// the same way they are defined in the Plan struct. +func (s *S) TestSectionOrderExt(c *C) { + plan.RegisterSectionExtension("x-field", &xExtension{}) + plan.RegisterSectionExtension("y-field", &yExtension{}) + defer func() { + plan.UnregisterSectionExtension("x-field") + plan.UnregisterSectionExtension("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) + c.Assert(err, IsNil) + + for _, input := range inputs { + err := ioutil.WriteFile(filepath.Join(layersDir, fmt.Sprintf("%03d-%s.yaml", input.order, input.label)), reindent(input.yaml), 0644) + c.Assert(err, IsNil) + } +} + +const xField string = "x-field" + +// xExtension implements the SectionExtension interface. +type xExtension struct{} + +func (x xExtension) ParseSection(data yaml.Node) (plan.Section, error) { + xs := &xSection{} + err := data.Decode(xs) + if err != nil { + return nil, err + } + // Propagate the name. + for name, entry := range xs.Entries { + if entry != nil { + xs.Entries[name].Name = name + } + } + return xs, nil +} + +func (x xExtension) CombineSections(sections ...plan.Section) (plan.Section, error) { + xs := &xSection{} + for _, section := range sections { + err := xs.Combine(section) + if err != nil { + return nil, err + } + } + return xs, nil +} + +func (x xExtension) ValidatePlan(p *plan.Plan) error { + var xs *xSection + xs = p.Sections[xField].(*xSection) + if xs != nil { + var ys *ySection + ys = p.Sections[yField].(*ySection) + + // Test dependency: Make sure every Y field in X refer to an existing Y entry. + for xEntryField, xEntryValue := range xs.Entries { + for _, yReference := range xEntryValue.Y { + found := false + for yEntryField, _ := range ys.Entries { + if yReference == yEntryField { + found = true + break + } + } + if !found { + return fmt.Errorf("cannot find ySection entry %v as required by xSection entry %v ", yReference, xEntryField) + } + } + } + } + return nil +} + +// xSection is the backing type for xExtension. +type xSection struct { + Entries map[string]*X `yaml:",inline,omitempty"` +} + +func (xs *xSection) Validate() error { + for field, entry := range xs.Entries { + if entry == nil { + return fmt.Errorf("cannot have nil entry for %q", field) + } + // Fictitious test requirement: entry names must start with x + if !strings.HasPrefix(field, "x") { + return fmt.Errorf("cannot accept entry not starting with letter 'x'") + } + } + return nil +} + +func (xs *xSection) IsZero() bool { + return xs.Entries == nil +} + +func (xs *xSection) Combine(other plan.Section) error { + otherxSection, ok := other.(*xSection) + if !ok { + return fmt.Errorf("cannot combine incompatible section type") + } + + for field, entry := range otherxSection.Entries { + xs.Entries = makeMapIfNil(xs.Entries) + switch entry.Override { + case plan.MergeOverride: + if old, ok := xs.Entries[field]; ok { + copied := old.Copy() + copied.Merge(entry) + xs.Entries[field] = copied + break + } + fallthrough + case plan.ReplaceOverride: + xs.Entries[field] = entry.Copy() + case plan.UnknownOverride: + return &plan.FormatError{ + Message: fmt.Sprintf(`invalid "override" value for entry %q`, field), + } + default: + return &plan.FormatError{ + Message: fmt.Sprintf(`unknown "override" value for entry %q`, field), + } + } + } + return nil +} + +type X struct { + Name string `yaml:"-"` + Override plan.Override `yaml:"override,omitempty"` + A string `yaml:"a,omitempty"` + B string `yaml:"b,omitempty"` + C string `yaml:"c,omitempty"` + Y []string `yaml:"y-field,omitempty"` +} + +func (x *X) Copy() *X { + copied := *x + copied.Y = append([]string(nil), x.Y...) + return &copied +} + +func (x *X) Merge(other *X) { + if other.A != "" { + x.A = other.A + } + if other.B != "" { + x.B = other.B + } + if other.C != "" { + x.C = other.C + } + x.Y = append(x.Y, other.Y...) +} + +const yField string = "y-field" + +// yExtension implements the SectionExtension interface. +type yExtension struct{} + +func (y yExtension) ParseSection(data yaml.Node) (plan.Section, error) { + ys := &ySection{} + err := data.Decode(ys) + if err != nil { + return nil, err + } + // Propagate the name. + for name, entry := range ys.Entries { + if entry != nil { + ys.Entries[name].Name = name + } + } + return ys, nil +} + +func (y yExtension) CombineSections(sections ...plan.Section) (plan.Section, error) { + ys := &ySection{} + for _, section := range sections { + err := ys.Combine(section) + if err != nil { + return nil, err + } + } + return ys, nil +} + +func (y yExtension) ValidatePlan(p *plan.Plan) error { + // This extension has no dependencies on the Plan to validate. + return nil +} + +// ySection is the backing type for yExtension. +type ySection struct { + Entries map[string]*Y `yaml:",inline,omitempty"` +} + +func (ys *ySection) Validate() error { + for field, entry := range ys.Entries { + if entry == nil { + return fmt.Errorf("cannot have nil entry for %q", field) + } + // Fictitious test requirement: entry names must start with y + if !strings.HasPrefix(field, "y") { + return fmt.Errorf("cannot accept entry not starting with letter 'y'") + } + } + return nil +} + +func (ys *ySection) IsZero() bool { + return ys.Entries == nil +} + +func (ys *ySection) Combine(other plan.Section) error { + otherySection, ok := other.(*ySection) + if !ok { + return fmt.Errorf("cannot combine incompatible section type") + } + + for field, entry := range otherySection.Entries { + ys.Entries = makeMapIfNil(ys.Entries) + switch entry.Override { + case plan.MergeOverride: + if old, ok := ys.Entries[field]; ok { + copied := old.Copy() + copied.Merge(entry) + ys.Entries[field] = copied + break + } + fallthrough + case plan.ReplaceOverride: + ys.Entries[field] = entry.Copy() + case plan.UnknownOverride: + return &plan.FormatError{ + Message: fmt.Sprintf(`invalid "override" value for entry %q`, field), + } + default: + return &plan.FormatError{ + Message: fmt.Sprintf(`unknown "override" value for entry %q`, field), + } + } + } + return nil +} + +type Y struct { + Name string `yaml:"-"` + Override plan.Override `yaml:"override,omitempty"` + A string `yaml:"a,omitempty"` + B string `yaml:"b,omitempty"` + C string `yaml:"c,omitempty"` +} + +func (y *Y) Copy() *Y { + copied := *y + return &copied +} + +func (y *Y) Merge(other *Y) { + if other.A != "" { + y.A = other.A + } + if other.B != "" { + y.B = other.B + } + if other.C != "" { + y.C = other.C + } +} + +func makeMapIfNil[K comparable, V any](m map[K]V) map[K]V { + if m == nil { + m = make(map[K]V) + } + return m +} diff --git a/internals/plan/plan.go b/internals/plan/plan.go index 2b669e63..d75146b7 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -21,6 +21,7 @@ import ( "path/filepath" "reflect" "regexp" + "slices" "strconv" "strings" "time" @@ -32,6 +33,31 @@ import ( "github.com/canonical/pebble/internals/osutil" ) +// SectionExtension allows the plan layer schema to be extended without +// adding centralised schema knowledge to the plan library. +type SectionExtension interface { + // ParseSection returns a newly allocated concrete type containing the + // unmarshalled section content. + ParseSection(data yaml.Node) (Section, error) + + // CombineSections returns a newly allocated concrete type containing the + // result of combining the supplied sections in order. + CombineSections(sections ...Section) (Section, error) + + // ValidatePlan takes the complete plan as input, and allows the + // extension to validate the plan. This can be used for cross section + // dependency validation. + ValidatePlan(plan *Plan) error +} + +type Section interface { + // Validate checks whether the section is valid, returning an error if not. + Validate() error + + // IsZero reports whether the section is empty. + IsZero() bool +} + const ( defaultBackoffDelay = 500 * time.Millisecond defaultBackoffFactor = 2.0 @@ -42,11 +68,90 @@ const ( defaultCheckThreshold = 3 ) +var ( + // sectionExtensions keeps a map of registered extensions. + sectionExtensions = map[string]SectionExtension{} + + // sectionExtensionsOrder records the order in which the extensions were registered. + sectionExtensionsOrder = []string{} +) + +// builtinSections 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 builtinSections = []string{"summary", "description", "services", "checks", "log-targets"} + +// RegisterSectionExtension adds a plan schema extension. All registrations must be +// 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 RegisterSectionExtension(field string, ext SectionExtension) { + if slices.Contains(builtinSections, field) { + panic(fmt.Sprintf("internal error: extension %q already used as built-in field", field)) + } + if _, ok := sectionExtensions[field]; ok { + panic(fmt.Sprintf("internal error: extension %q already registered", field)) + } + sectionExtensions[field] = ext + sectionExtensionsOrder = append(sectionExtensionsOrder, field) +} + +// UnregisterSectionExtension removes a plan schema extension. This is only +// intended for use by tests during cleanup. +func UnregisterSectionExtension(field string) { + delete(sectionExtensions, field) + sectionExtensionsOrder = slices.DeleteFunc(sectionExtensionsOrder, func(n string) bool { + return n == field + }) +} + type Plan struct { Layers []*Layer `yaml:"-"` Services map[string]*Service `yaml:"services,omitempty"` Checks map[string]*Check `yaml:"checks,omitempty"` LogTargets map[string]*LogTarget `yaml:"log-targets,omitempty"` + + Sections map[string]Section `yaml:",inline"` +} + +// MarshalYAML implements an override for top level omitempty tags handling. +// 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) { + // 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 sectionExtensionsOrder { + 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 sectionExtensionsOrder { + v.Field(3 + i).Set(reflect.ValueOf(p.Sections[field])) + } + plan := v.Addr().Interface() + return plan, nil } type Layer struct { @@ -57,6 +162,8 @@ type Layer struct { Services map[string]*Service `yaml:"services,omitempty"` Checks map[string]*Check `yaml:"checks,omitempty"` LogTargets map[string]*LogTarget `yaml:"log-targets,omitempty"` + + Sections map[string]Section `yaml:",inline"` } type Service struct { @@ -559,7 +666,27 @@ func CombineLayers(layers ...*Layer) (*Layer, error) { Services: make(map[string]*Service), Checks: make(map[string]*Check), LogTargets: make(map[string]*LogTarget), + Sections: make(map[string]Section), + } + + // Combine the same sections from each layer. Note that we do this before + // the layers length check because we need the extension to provide us with + // a zero value section, even if no layers are supplied (similar to the + // allocations taking place above for the built-in types). + for field, extension := range sectionExtensions { + var sections []Section + for _, layer := range layers { + if section := layer.Sections[field]; section != nil { + sections = append(sections, section) + } + } + var err error + combined.Sections[field], err = extension.CombineSections(sections...) + if err != nil { + return nil, err + } } + if len(layers) == 0 { return combined, nil } @@ -825,11 +952,18 @@ func (layer *Layer) Validate() error { } } + for _, section := range layer.Sections { + err := section.Validate() + if err != nil { + return err + } + } + return nil } -// Validate checks that the combined layers form a valid plan. -// See also Layer.Validate, which checks that the individual layers are valid. +// Validate checks that the combined layers form a valid plan. See also +// Layer.Validate, which checks that the individual layers are valid. func (p *Plan) Validate() error { for name, service := range p.Services { if service.Command == "" { @@ -917,6 +1051,15 @@ func (p *Plan) Validate() error { if err != nil { return err } + + // Each section extension must validate the combined plan. + for _, extension := range sectionExtensions { + err = extension.ValidatePlan(p) + if err != nil { + return err + } + } + return nil } @@ -1085,19 +1228,83 @@ func (p *Plan) checkCycles() error { } func ParseLayer(order int, label string, data []byte) (*Layer, error) { - layer := Layer{ - Services: map[string]*Service{}, - Checks: map[string]*Check{}, - LogTargets: map[string]*LogTarget{}, - } - dec := yaml.NewDecoder(bytes.NewBuffer(data)) - dec.KnownFields(true) - err := dec.Decode(&layer) + layer := &Layer{ + Services: make(map[string]*Service), + Checks: make(map[string]*Check), + LogTargets: make(map[string]*LogTarget), + Sections: make(map[string]Section), + } + + // The following manual approach is required because: + // + // 1. Extended sections are YAML inlined, and also do not have a + // concrete type at this level, we cannot simply unmarshal the layer + // in one step. + // + // 2. We honor KnownFields = true behaviour for non extended schema + // sections, and at the top field level, which includes Section field + // names. + builtins := map[string]interface{}{ + "summary": &layer.Summary, + "description": &layer.Description, + "services": &layer.Services, + "checks": &layer.Checks, + "log-targets": &layer.LogTargets, + } + + sections := make(map[string]yaml.Node) + // Deliberately pre-allocate at least an empty yaml.Node for every + // extension section. Extension sections that have unmarshalled + // will update the respective node, while non-existing sections + // will at least have an empty node. This means we can consistently + // let the extension allocate and decode the yaml node for all sections, + // and in the case where it is zero, we get an empty backing type instance. + for field, _ := range sectionExtensions { + sections[field] = yaml.Node{} + } + err := yaml.Unmarshal(data, §ions) if err != nil { return nil, &FormatError{ Message: fmt.Sprintf("cannot parse layer %q: %v", label, err), } } + + for field, section := range sections { + if slices.Contains(builtinSections, field) { + // The following issue prevents us from using the yaml.Node decoder + // with KnownFields = true behaviour. Once one of the proposals get + // merged, we can remove the intermediate Marshal step. + // https://github.com/go-yaml/yaml/issues/460 + data, err := yaml.Marshal(§ion) + if err != nil { + return nil, fmt.Errorf("internal error: cannot marshal %v section: %w", field, err) + } + dec := yaml.NewDecoder(bytes.NewReader(data)) + dec.KnownFields(true) + if err = dec.Decode(builtins[field]); err != nil { + return nil, &FormatError{ + Message: fmt.Sprintf("cannot parse layer %q section %q: %v", label, field, err), + } + } + } else { + extension, ok := sectionExtensions[field] + if !ok { + // At the top level we do not ignore keys we do not understand. + // This preserves the current Pebble behaviour of decoding with + // KnownFields = true. + return nil, &FormatError{ + Message: fmt.Sprintf("cannot parse layer %q: unknown section %q", label, field), + } + } + + // Section unmarshal rules are defined by the extension itself. + layer.Sections[field], err = extension.ParseSection(section) + if err != nil { + return nil, err + } + } + } + layer.Order = order layer.Label = label @@ -1125,7 +1332,7 @@ func ParseLayer(order int, label string, data []byte) (*Layer, error) { return nil, err } - return &layer, err + return layer, err } func validServiceAction(action ServiceAction, additionalValid ...ServiceAction) bool { @@ -1228,6 +1435,7 @@ func ReadDir(layersDir string) (*Plan, error) { Services: combined.Services, Checks: combined.Checks, LogTargets: combined.LogTargets, + Sections: combined.Sections, } err = plan.Validate() if err != nil { diff --git a/internals/plan/plan_test.go b/internals/plan/plan_test.go index 4d740c28..99b617a8 100644 --- a/internals/plan/plan_test.go +++ b/internals/plan/plan_test.go @@ -19,13 +19,16 @@ import ( "fmt" "os" "path/filepath" + "reflect" "strings" "time" + "unicode" . "gopkg.in/check.v1" "gopkg.in/yaml.v3" "github.com/canonical/pebble/internals/plan" + "github.com/canonical/pebble/internals/testutil" ) const ( @@ -203,6 +206,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, { Order: 1, Label: "layer-1", @@ -253,6 +257,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }}, result: &plan.Layer{ Summary: "Simple override layer.", @@ -332,6 +337,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, start: map[string][]string{ "srv1": {"srv2", "srv1", "srv3"}, @@ -394,6 +400,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }}, }, { summary: "Unknown keys are not accepted", @@ -477,7 +484,7 @@ var planTests = []planTest{{ `}, }, { summary: `Invalid backoff-delay duration`, - error: `cannot parse layer "layer-0": invalid duration "foo"`, + error: `cannot parse layer "layer-0" section "services": invalid duration "foo"`, input: []string{` services: "svc1": @@ -507,7 +514,7 @@ var planTests = []planTest{{ `}, }, { summary: `Invalid backoff-factor`, - error: `cannot parse layer "layer-0": invalid floating-point number "foo"`, + error: `cannot parse layer "layer-0" section "services": invalid floating-point number "foo"`, input: []string{` services: "svc1": @@ -544,6 +551,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }}, }, { summary: `Invalid service command: cannot have any arguments after [ ... ] group`, @@ -652,6 +660,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, }, { summary: "Checks override replace works correctly", @@ -729,6 +738,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, }, { summary: "Checks override merge works correctly", @@ -812,6 +822,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, }, { summary: "Timeout is capped at period", @@ -841,6 +852,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, }, { summary: "Unset timeout is capped at period", @@ -869,6 +881,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, }, { summary: "One of http, tcp, or exec must be present for check", @@ -989,6 +1002,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.Section{}, }, }, { summary: "Overriding log targets", @@ -1085,6 +1099,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.Section{}, }, { Label: "layer-1", Order: 1, @@ -1123,6 +1138,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.Section{}, }}, result: &plan.Layer{ Services: map[string]*plan.Service{ @@ -1168,6 +1184,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.Section{}, }, }, { summary: "Log target requires type field", @@ -1277,6 +1294,7 @@ var planTests = []planTest{{ }, }, }, + Sections: map[string]plan.Section{}, }, { Order: 1, Label: "layer-1", @@ -1302,6 +1320,7 @@ var planTests = []planTest{{ }, }, }, + Sections: map[string]plan.Section{}, }}, result: &plan.Layer{ Services: map[string]*plan.Service{}, @@ -1329,6 +1348,7 @@ var planTests = []planTest{{ }, }, }, + Sections: map[string]plan.Section{}, }, }, { summary: "Reserved log target labels", @@ -1379,6 +1399,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.Section{}, }, }, { summary: "Three layers missing command", @@ -1452,6 +1473,7 @@ func (s *S) TestParseLayer(c *C) { Services: result.Services, Checks: result.Checks, LogTargets: result.LogTargets, + Sections: result.Sections, } err = p.Validate() } @@ -1494,6 +1516,7 @@ services: Services: combined.Services, Checks: combined.Checks, LogTargets: combined.LogTargets, + Sections: combined.Sections, } err = p.Validate() c.Assert(err, ErrorMatches, `services in before/after loop: .*`) @@ -1534,6 +1557,7 @@ services: Services: combined.Services, Checks: combined.Checks, LogTargets: combined.LogTargets, + Sections: combined.Sections, } err = p.Validate() c.Check(err, ErrorMatches, `plan must define "command" for service "srv1"`) @@ -1885,6 +1909,8 @@ func (s *S) TestMergeServiceContextNoContext(c *C) { Group: "grp", WorkingDir: "/working/dir", } + // This test ensures an empty service name results in no lookup, and + // simply leaves the provided context unchanged. merged, err := plan.MergeServiceContext(nil, "", overrides) c.Assert(err, IsNil) c.Check(merged, DeepEquals, overrides) @@ -2045,3 +2071,88 @@ func (s *S) TestStartStopOrderMultipleLanes(c *C) { c.Assert(lanes[1], DeepEquals, []string{"srv2"}) c.Assert(lanes[2], DeepEquals, []string{"srv3"}) } + +// TestSectionFieldStability detects changes in plan.Layer and plan.Plan +// YAML fields, and fails on any change that could break hardcoded section +// fields in the code. On failure, please carefully inspect and update +// the plan library where required. +func (s *S) TestSectionFieldStability(c *C) { + layerFields := structYamlFields(plan.Layer{}) + c.Assert(layerFields, testutil.DeepUnsortedMatches, []string{"summary", "description", "services", "checks", "log-targets", "sections"}) + planFields := structYamlFields(plan.Plan{}) + c.Assert(planFields, testutil.DeepUnsortedMatches, []string{"services", "checks", "log-targets", "sections"}) +} + +// structYamlFields extracts the YAML fields from a struct. If the YAML tag +// is omitted, the field name with the first letter lower case will be used. +func structYamlFields(inStruct any) []string { + var fields []string + inStructType := reflect.TypeOf(inStruct) + for i := range inStructType.NumField() { + fieldType := inStructType.Field(i) + yamlTag := fieldType.Tag.Get("yaml") + if fieldType.IsExported() && yamlTag != "-" { + tag, _, _ := strings.Cut(fieldType.Tag.Get("yaml"), ",") + if tag == "" { + tag = firstLetterToLower(fieldType.Name) + } + fields = append(fields, tag) + } + } + return fields +} + +func firstLetterToLower(s string) string { + if len(s) == 0 { + return s + } + r := []rune(s) + r[0] = unicode.ToLower(r[0]) + return string(r) +} + +// TestSectionOrder ensures built-in section order is maintained +// during Plan marshal operations. +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`))) +}