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`))) +}