Skip to content

Commit

Permalink
fix bug with 2 templates, same ID, different paths
Browse files Browse the repository at this point in the history
The way template configs were being tracked meant that if 2 templates
had the same content but different config otherwise, they would only use
the last config. Having more than 1 template with the same content is
rare but a user ran into it with 2 templates and the same path.

This eliminates the separate tracking of template configs and bundles
them with the templates themselves. Having a 1-1 template/config and
eliminating the separate data structure makes this area of code simpler
to understand and work with while fixing this issue.

This also fixes the TemplateConfigMapping() call which should have kept
backwards compatibility with the signature.
  • Loading branch information
eikenb committed Apr 11, 2022
1 parent 3de645b commit 56093c6
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 13 deletions.
23 changes: 10 additions & 13 deletions manager/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ type Runner struct {
outStream, errStream io.Writer
inStream io.Reader

// ctemplatesMap is a map of each template ID to the TemplateConfigs
// that made it.
ctemplatesMap map[string]*config.TemplateConfig

// templates is the list of calculated templates.
templates []*template.Template

Expand Down Expand Up @@ -919,7 +915,6 @@ func (r *Runner) init() error {

numTemplates := len(*r.config.Templates)
templates := make([]*template.Template, 0, numTemplates)
ctemplatesMap := make(map[string]*config.TemplateConfig)

// Iterate over each TemplateConfig, creating a new Template resource for each
// entry. Templates are parsed and saved, and a map of templates to their
Expand All @@ -944,14 +939,13 @@ func (r *Runner) init() error {
RightDelim: rightDelim,
FunctionDenylist: ctmpl.FunctionDenylist,
SandboxPath: config.StringVal(ctmpl.SandboxPath),
Config: ctmpl,
})
if err != nil {
return err
}

templates = append(templates, tmpl)

ctemplatesMap[tmpl.ID()] = ctmpl
}

// Convert the map of templates (which was only used to ensure uniqueness)
Expand All @@ -964,7 +958,6 @@ func (r *Runner) init() error {
r.renderedCh = make(chan struct{}, 1)
r.renderEventCh = make(chan struct{}, 1)

r.ctemplatesMap = ctemplatesMap
r.inStream = os.Stdin
r.outStream = os.Stdout
r.errStream = os.Stderr
Expand Down Expand Up @@ -1017,18 +1010,22 @@ func (r *Runner) diffAndUpdateDeps(depsMap map[string]dep.Dependency) {

// TemplateConfigFor returns the TemplateConfig for the given Template
func (r *Runner) templateConfigFor(tmpl *template.Template) *config.TemplateConfig {
return r.ctemplatesMap[tmpl.ID()]
return tmpl.Config()
}

// TemplateConfigMapping returns a mapping between the template ID and the set
// of TemplateConfig represented by the template ID
func (r *Runner) TemplateConfigMapping() map[string]*config.TemplateConfig {
func (r *Runner) TemplateConfigMapping() map[string][]*config.TemplateConfig {
// this method is primarily used to support embedding consul-template
// in other applications (ex. Nomad)
m := make(map[string]*config.TemplateConfig)
m := make(map[string][]*config.TemplateConfig)

for _, tmpl := range r.templates {
m[tmpl.ID()] = []*config.TemplateConfig{}
}

for id, set := range r.ctemplatesMap {
m[id] = set
for _, tmpl := range r.templates {
m[tmpl.ID()] = append(m[tmpl.ID()], tmpl.Config())
}

return m
Expand Down
32 changes: 32 additions & 0 deletions manager/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,38 @@ import (
"github.com/hashicorp/consul-template/template"
)

func TestRunner_initTemplates(t *testing.T) {

c := config.TestConfig(
&config.Config{
Templates: &config.TemplateConfigs{
&config.TemplateConfig{
Contents: config.String(`template`),
},
&config.TemplateConfig{
Contents: config.String(`template`),
},
},
})

r, err := NewRunner(c, true)
if err != nil {
t.Fatal(err)
}

confMap := r.TemplateConfigMapping()

for _, tmpl := range r.templates {
if _, ok := confMap[tmpl.ID()]; !ok {
t.Errorf("config map missing template entry")
}
if confs := confMap[tmpl.ID()]; len(confs) != len(r.templates) {
t.Errorf("should be %v templates, but there are %v",
len(r.templates), len(confs))
}
}
}

func TestRunner_Receive(t *testing.T) {

c := config.TestConfig(&config.Config{Once: true})
Expand Down
13 changes: 13 additions & 0 deletions template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"text/template"

"github.com/Masterminds/sprig"
"github.com/hashicorp/consul-template/config"
dep "github.com/hashicorp/consul-template/dependency"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -61,6 +62,9 @@ type Template struct {
// and causes an error if a relative path tries to traverse outside that
// prefix.
sandboxPath string

// local reference to configuration for this template
config *config.TemplateConfig
}

// NewTemplateInput is used as input when creating the template.
Expand Down Expand Up @@ -94,6 +98,9 @@ type NewTemplateInput struct {
// and causes an error if a relative path tries to traverse outside that
// prefix.
SandboxPath string

// Config keeps local reference to config struct
Config *config.TemplateConfig
}

// NewTemplate creates and parses a new Consul Template template at the given
Expand Down Expand Up @@ -122,6 +129,7 @@ func NewTemplate(i *NewTemplateInput) (*Template, error) {
t.functionDenylist = i.FunctionDenylist
t.sandboxPath = i.SandboxPath
t.destination = i.Destination
t.config = i.Config

if i.Source != "" {
contents, err := ioutil.ReadFile(i.Source)
Expand All @@ -148,6 +156,11 @@ func (t *Template) Contents() string {
return t.contents
}

// Config returns the template's config
func (t *Template) Config() *config.TemplateConfig {
return t.config
}

// Source returns the filepath source of this template.
func (t *Template) Source() string {
if t.source == "" {
Expand Down

0 comments on commit 56093c6

Please sign in to comment.