From 5ff2951ae349a83ddc53526ef0831467502cdcbc Mon Sep 17 00:00:00 2001 From: Fred Lotter Date: Wed, 24 Jul 2024 14:55:46 +0200 Subject: [PATCH] feat(plan): add plan section support --- internals/plan/extensions_test.go | 690 ++++++++++++++++++++++++++++++ internals/plan/plan.go | 222 +++++++++- internals/plan/plan_test.go | 27 +- 3 files changed, 926 insertions(+), 13 deletions(-) create mode 100644 internals/plan/extensions_test.go diff --git a/internals/plan/extensions_test.go b/internals/plan/extensions_test.go new file mode 100644 index 00000000..c7209a7c --- /dev/null +++ b/internals/plan/extensions_test.go @@ -0,0 +1,690 @@ +// 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" + "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 +// package exclusively focuses extensions, all built-in sections are empty +// and ignored in the test results. +type planResult struct { + x *xSection + y *ySection +} + +var extensionTests = []struct { + extensions map[string]plan.LayerSectionExtension + layers []*inputLayer + errorString string + combinedPlanResult *planResult + combinedPlanResultYaml string +}{ + // Index 0: No Sections + { + combinedPlanResultYaml: string(reindent(` + {}`)), + }, + // Index 1: Sections with empty YAML + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + combinedPlanResult: &planResult{ + x: &xSection{}, + y: &ySection{}, + }, + combinedPlanResultYaml: string(reindent(` + {}`)), + }, + // Index 2: Load file layers invalid section + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-xy", + yaml: ` + summary: xy + description: desc + invalid: + `, + }, + }, + errorString: "cannot parse layer .*: unknown section .*", + }, + // Index 3: Load file layers not unique order + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-1", + yaml: ` + summary: xy + description: desc + `, + }, + &inputLayer{ + order: 1, + label: "layer-2", + yaml: ` + summary: xy + description: desc + `, + }, + }, + errorString: "invalid layer filename: .* not unique .*", + }, + // Index 4: Load file layers not unique label + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-xy", + yaml: ` + summary: xy + description: desc + `, + }, + &inputLayer{ + order: 2, + label: "layer-xy", + yaml: ` + summary: xy + description: desc + `, + }, + }, + errorString: "invalid layer filename: .* not unique .*", + }, + // Index 5: Load file layers with empty section + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + `, + }, + &inputLayer{ + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y + `, + }, + }, + combinedPlanResult: &planResult{ + x: &xSection{}, + y: &ySection{}, + }, + combinedPlanResultYaml: string("{}\n"), + }, + // Index 6: Load file layers with section validation failure #1 + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + z1: + override: replace + a: a + b: b + `, + }, + }, + errorString: "cannot validate layer section .* cannot accept entry not starting .*", + }, + // Index 7: Load file layers with section validation failure #2 + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + x1: + `, + }, + }, + errorString: "cannot validate layer section .* cannot have nil entry .*", + }, + // Index 8: Load file layers failed plan validation + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + x1: + override: replace + a: a + b: b + y-field: + - y2 + `, + }, + &inputLayer{ + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y + y-field: + y1: + override: replace + a: a + b: b + `, + }, + }, + errorString: "cannot validate plan section .* cannot find .* as required by .*", + }, + // Index 9: Check empty section omits entry + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + `, + }, + &inputLayer{ + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y + y-field: + `, + }, + }, + combinedPlanResult: &planResult{ + x: &xSection{}, + y: &ySection{}, + }, + combinedPlanResultYaml: string(reindent(` + {}`)), + }, + // Index 10: Load file layers + { + extensions: map[string]plan.LayerSectionExtension{ + "x-field": &xExtension{}, + "y-field": &yExtension{}, + }, + layers: []*inputLayer{ + &inputLayer{ + order: 1, + label: "layer-x", + yaml: ` + summary: x + description: desc-x + x-field: + x1: + override: replace + a: a + b: b + y-field: + - y1 + `, + }, + &inputLayer{ + order: 2, + label: "layer-y", + yaml: ` + summary: y + description: desc-y + y-field: + y1: + override: replace + a: a + b: b + `, + }, + }, + combinedPlanResult: &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", + }, + }, + }, + }, + combinedPlanResultYaml: 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) { + for testIndex, planTest := range extensionTests { + c.Logf("Running TestPlanExtensions with test data index %v", testIndex) + + // Write layers to test directory. + baseDir := c.MkDir() + s.writeLayerFiles(c, baseDir, planTest.layers) + var p *plan.Plan + + // Register test extensions. + for field, extension := range planTest.extensions { + plan.RegisterExtension(field, extension) + } + + // Load the plan layer from disk (parse, combine and validate). + p, err := plan.ReadDir(baseDir) + if planTest.errorString != "" || err != nil { + // Expected error. + c.Assert(err, ErrorMatches, planTest.errorString) + } else { + if _, ok := planTest.extensions[xField]; ok { + // Verify "x-field" data. + var x *xSection + err = p.Section(xField, &x) + c.Assert(err, IsNil) + c.Assert(x.Entries, DeepEquals, planTest.combinedPlanResult.x.Entries) + } + + if _, ok := planTest.extensions[yField]; ok { + // Verify "y-field" data. + var y *ySection + err = p.Section(yField, &y) + c.Assert(err, IsNil) + c.Assert(y.Entries, DeepEquals, planTest.combinedPlanResult.y.Entries) + } + + // Verify combined plan YAML. + planYAML, err := yaml.Marshal(p) + c.Assert(err, IsNil) + c.Assert(string(planYAML), Equals, planTest.combinedPlanResultYaml) + } + + // Unregister test extensions. + for field, _ := range planTest.extensions { + plan.UnregisterExtension(field) + } + } +} + +// writeLayerFiles writes layer files of a test to disk. +func (s *S) writeLayerFiles(c *C, baseDir string, inputs []*inputLayer) { + layersDir := filepath.Join(baseDir, "layers") + 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 LayerSectionExtension interface. +type xExtension struct{} + +func (x xExtension) ParseSection(data yaml.Node) (plan.LayerSection, 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.LayerSection) (plan.LayerSection, 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 + err := p.Section(xField, &xs) + if err != nil { + return err + } + if xs != nil { + var ys *ySection + err = p.Section(yField, &ys) + if err != nil { + return err + } + if ys == nil { + return fmt.Errorf("cannot validate %v field without %v field", xField, yField) + } + + // 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.LayerSection) 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 LayerSectionExtension interface. +type yExtension struct{} + +func (y yExtension) ParseSection(data yaml.Node) (plan.LayerSection, 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.LayerSection) (plan.LayerSection, 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.LayerSection) 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..e35d88ea 100644 --- a/internals/plan/plan.go +++ b/internals/plan/plan.go @@ -32,6 +32,31 @@ import ( "github.com/canonical/pebble/internals/osutil" ) +// LayerSectionExtension allows the plan layer schema to be extended without +// adding centralised schema knowledge to the plan library. +type LayerSectionExtension 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 LayerSection interface { + // Ask the section to validate itself. + Validate() error + + // Returns true if the section is empty. + IsZero() bool +} + const ( defaultBackoffDelay = 500 * time.Millisecond defaultBackoffFactor = 2.0 @@ -42,11 +67,80 @@ const ( defaultCheckThreshold = 3 ) +// layerExtensions keeps a map of registered extensions. +var layerExtensions = map[string]LayerSectionExtension{} + +// RegisterExtension adds a plan schema extension. All registrations must be +// done before the plan library is used. +func RegisterExtension(field string, ext LayerSectionExtension) error { + if _, ok := layerExtensions[field]; ok { + return fmt.Errorf("internal error: extension %q already registered", field) + } + layerExtensions[field] = ext + return nil +} + +// UnregisterExtension removes a plan schema extension. This is only +// intended for use by tests during cleanup. +func UnregisterExtension(field string) { + delete(layerExtensions, 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]LayerSection `yaml:",inline"` +} + +// Section retrieves a section from the plan. +func (p *Plan) Section(field string, out interface{}) error { + if _, found := layerExtensions[field]; !found { + return fmt.Errorf("cannot find registered extension for field %q", field) + } + + outVal := reflect.ValueOf(out) + if outVal.Kind() != reflect.Ptr || outVal.IsNil() { + return fmt.Errorf("cannot read non pointer to section type %q", outVal.Kind()) + } + + section, exists := p.Sections[field] + if !exists { + return fmt.Errorf("internal error: section %q is nil", field) + } + + sectionVal := reflect.ValueOf(section) + sectionType := sectionVal.Type() + outValPtrType := outVal.Elem().Type() + if !sectionType.AssignableTo(outValPtrType) { + return fmt.Errorf("cannot assign value of type %s to out argument of type %s", sectionType, outValPtrType) + } + outVal.Elem().Set(sectionVal) + return nil +} + +// 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) { + marshalData := make(map[string]interface{}) + if len(p.Services) != 0 { + marshalData["services"] = p.Services + } + if len(p.LogTargets) != 0 { + marshalData["log-targets"] = p.LogTargets + } + if len(p.Checks) != 0 { + marshalData["checks"] = p.Checks + } + for field, section := range p.Sections { + if !section.IsZero() { + marshalData[field] = section + } + } + return marshalData, nil } type Layer struct { @@ -57,6 +151,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]LayerSection `yaml:",inline"` } type Service struct { @@ -559,7 +655,29 @@ 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]LayerSection), + } + + // 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 layerExtensions { + var sections []LayerSection + 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, &FormatError{ + Message: fmt.Sprintf(`cannot combine section %q: %v`, field, err), + } + } } + if len(layers) == 0 { return combined, nil } @@ -825,11 +943,18 @@ func (layer *Layer) Validate() error { } } + for field, section := range layer.Sections { + err := section.Validate() + if err != nil { + return fmt.Errorf("cannot validate layer section %q: %w", field, 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 +1042,15 @@ func (p *Plan) Validate() error { if err != nil { return err } + + // Each section extension must validate the combined plan. + for field, extension := range layerExtensions { + err = extension.ValidatePlan(p) + if err != nil { + return fmt.Errorf("cannot validate plan section %q: %w", field, err) + } + } + return nil } @@ -1085,19 +1219,84 @@ 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]LayerSection), + } + + // 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. + builtinSections := map[string]interface{}{ + "summary": &layer.Summary, + "description": &layer.Description, + "services": &layer.Services, + "checks": &layer.Checks, + "log-targets": &layer.LogTargets, + } + + layerSections := 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 layerExtensions { + layerSections[field] = yaml.Node{} + } + err := yaml.Unmarshal(data, &layerSections) if err != nil { return nil, &FormatError{ Message: fmt.Sprintf("cannot parse layer %q: %v", label, err), } } + + for field, section := range layerSections { + if _, builtin := builtinSections[field]; builtin { + // 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(builtinSections[field]); err != nil { + return nil, &FormatError{ + Message: fmt.Sprintf("cannot parse layer %q section %q: %v", label, field, err), + } + } + } else { + if extension, ok := layerExtensions[field]; ok { + // Section unmarshal rules are defined by the extension itself. + layer.Sections[field], err = extension.ParseSection(section) + if err != nil { + return nil, &FormatError{ + Message: fmt.Sprintf("cannot parse layer %q section %q: %v", label, field, err), + } + } + } else { + // 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), + } + } + } + } + layer.Order = order layer.Label = label @@ -1125,7 +1324,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 +1427,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..fbc1fb96 100644 --- a/internals/plan/plan_test.go +++ b/internals/plan/plan_test.go @@ -203,6 +203,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, { Order: 1, Label: "layer-1", @@ -253,6 +254,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }}, result: &plan.Layer{ Summary: "Simple override layer.", @@ -332,6 +334,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, start: map[string][]string{ "srv1": {"srv2", "srv1", "srv3"}, @@ -394,6 +397,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }}, }, { summary: "Unknown keys are not accepted", @@ -477,7 +481,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 +511,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 +548,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }}, }, { summary: `Invalid service command: cannot have any arguments after [ ... ] group`, @@ -652,6 +657,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Checks override replace works correctly", @@ -729,6 +735,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Checks override merge works correctly", @@ -812,6 +819,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Timeout is capped at period", @@ -841,6 +849,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Unset timeout is capped at period", @@ -869,6 +878,7 @@ var planTests = []planTest{{ }, }, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "One of http, tcp, or exec must be present for check", @@ -989,6 +999,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Overriding log targets", @@ -1085,6 +1096,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.LayerSection{}, }, { Label: "layer-1", Order: 1, @@ -1123,6 +1135,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.LayerSection{}, }}, result: &plan.Layer{ Services: map[string]*plan.Service{ @@ -1168,6 +1181,7 @@ var planTests = []planTest{{ Override: plan.MergeOverride, }, }, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Log target requires type field", @@ -1277,6 +1291,7 @@ var planTests = []planTest{{ }, }, }, + Sections: map[string]plan.LayerSection{}, }, { Order: 1, Label: "layer-1", @@ -1302,6 +1317,7 @@ var planTests = []planTest{{ }, }, }, + Sections: map[string]plan.LayerSection{}, }}, result: &plan.Layer{ Services: map[string]*plan.Service{}, @@ -1329,6 +1345,7 @@ var planTests = []planTest{{ }, }, }, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Reserved log target labels", @@ -1379,6 +1396,7 @@ var planTests = []planTest{{ }, Checks: map[string]*plan.Check{}, LogTargets: map[string]*plan.LogTarget{}, + Sections: map[string]plan.LayerSection{}, }, }, { summary: "Three layers missing command", @@ -1452,6 +1470,7 @@ func (s *S) TestParseLayer(c *C) { Services: result.Services, Checks: result.Checks, LogTargets: result.LogTargets, + Sections: result.Sections, } err = p.Validate() } @@ -1494,6 +1513,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 +1554,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 +1906,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)