Description
By my evaluation of the code, the plan library is implemented with very specific handling of the layers. The content of layers come from the original layers directory load, and can then be further modified through HTTP API append and combine calls.
However, when performing any combine of layers, deep copies of layer contents must be used to prevent reference / pointer types backed by memory inside the layer struct from getting mutated during the lifetime of the process. Allowing this will corrupt future combine attempts of the original layers stack.
At the top level we seem to also copy the contents that may originate from the combined layer, which already can only contain copied content, as it always starts empty.
Although this does not hurt, with plan extensions support, it may be a good idea to have the code as accurate as possible, as this will typically be copy/pasted as reference code when implementing an extension.
In the code snippet below, do we really want to copy the already combined service, which itself is a copy?
combined := &Layer{
Services: make(map[string]*Service),
Checks: make(map[string]*Check),
LogTargets: make(map[string]*LogTarget),
Sections: make(map[string]Section),
}
:
combined.Summary = last.Summary
combined.Description = last.Description
for _, layer := range layers {
for name, service := range layer.Services {
switch service.Override {
case MergeOverride:
if old, ok := combined.Services[name]; ok {
copied := old.Copy() <<<============================== ?
copied.Merge(service)
combined.Services[name] = copied
break
}
fallthrough
case ReplaceOverride:
combined.Services[name] = service.Copy()
case UnknownOverride:
return nil, &FormatError{
Message: fmt.Sprintf(`layer %q must define "override" for service %q`,
layer.Label, service.Name),
}
default:
return nil, &FormatError{
Message: fmt.Sprintf(`layer %q has invalid "override" value for service %q`,
layer.Label, service.Name),
}
}
}
:
Activity