From be29d6bf75aa10681518272dcabb653b18d8453a Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 May 2020 20:17:27 -0400 Subject: [PATCH] config: warn when a config file is skipped All commands which read config (agent, services, and validate) will now print warnings when one of the config files is skipped because it did not match an expected format. Also ensures that config validate prints all warnings. --- agent/config/builder.go | 19 ++++++++----------- agent/config/builder_test.go | 1 + agent/config/runtime_test.go | 4 ++++ command/services/config.go | 6 +++++- command/services/deregister/deregister.go | 2 +- command/services/register/register.go | 2 +- command/validate/validate.go | 3 +++ 7 files changed, 23 insertions(+), 14 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index f1d787ebc7..919f2fdbb2 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -108,7 +108,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) { slices, values := b.splitSlicesAndValues(opts.Config) b.Head = append(b.Head, newSource("flags.slices", slices)) for _, path := range opts.ConfigFiles { - sources, err := sourcesFromPath(path, opts) + sources, err := b.sourcesFromPath(path, opts.ConfigFormat) if err != nil { return nil, err } @@ -132,7 +132,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) { // sourcesFromPath reads a single config file or all files in a directory (but // not its sub-directories) and returns Sources created from the // files. -func sourcesFromPath(path string, options BuilderOpts) ([]Source, error) { +func (b *Builder) sourcesFromPath(path string, format string) ([]Source, error) { f, err := os.Open(path) if err != nil { return nil, fmt.Errorf("config: Open failed on %s. %s", path, err) @@ -145,12 +145,12 @@ func sourcesFromPath(path string, options BuilderOpts) ([]Source, error) { } if !fi.IsDir() { - if !shouldParseFile(path, options.ConfigFormat) { - // TODO: log warning + if !shouldParseFile(path, format) { + b.warn("skipping file %v, extension must be .hcl or .json, or config format must be set", path) return nil, nil } - src, err := newSourceFromFile(path, options.ConfigFormat) + src, err := newSourceFromFile(path, format) if err != nil { return nil, err } @@ -185,11 +185,11 @@ func sourcesFromPath(path string, options BuilderOpts) ([]Source, error) { continue } - if !shouldParseFile(fp, options.ConfigFormat) { - // TODO: log warning + if !shouldParseFile(fp, format) { + b.warn("skipping file %v, extension must be .hcl or .json, or config format must be set", fp) continue } - src, err := newSourceFromFile(fp, options.ConfigFormat) + src, err := newSourceFromFile(fp, format) if err != nil { return nil, err } @@ -251,9 +251,6 @@ func (b *Builder) BuildAndValidate() (RuntimeConfig, error) { // warnings can still contain deprecation or format warnings that should // be presented to the user. func (b *Builder) Build() (rt RuntimeConfig, err error) { - b.err = nil - b.Warnings = nil - // TODO: move to NewBuilder to remove Builder.options field configFormat := b.options.ConfigFormat if configFormat != "" && configFormat != "json" && configFormat != "hcl" { diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index aa6cec17f4..a2eddaf75d 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -44,6 +44,7 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) { {Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"}, } require.Equal(t, expected, b.Sources) + require.Len(t, b.Warnings, 2) } func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.T) { diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 2598946bb3..424f87afce 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -451,6 +451,9 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { writeFile(filepath.Join(dataDir, "conf", "valid.json"), []byte(`{"datacenter":"a"}`)) writeFile(filepath.Join(dataDir, "conf", "invalid.skip"), []byte(`NOPE`)) }, + warns: []string{ + "skipping file " + filepath.Join(dataDir, "conf", "invalid.skip") + ", extension must be .hcl or .json, or config format must be set", + }, }, { desc: "-config-format=json", @@ -6002,6 +6005,7 @@ func TestFullConfig(t *testing.T) { if err := fs.Parse(flagSrc); err != nil { t.Fatalf("ParseFlags: %s", err) } + require.Len(t, fs.Args(), 0) b, err := NewBuilder(flags) if err != nil { diff --git a/command/services/config.go b/command/services/config.go index 94b026ff5b..677757da39 100644 --- a/command/services/config.go +++ b/command/services/config.go @@ -7,12 +7,13 @@ import ( "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" + "github.com/mitchellh/cli" "github.com/mitchellh/mapstructure" ) // ServicesFromFiles returns the list of agent service registration structs // from a set of file arguments. -func ServicesFromFiles(files []string) ([]*api.AgentServiceRegistration, error) { +func ServicesFromFiles(ui cli.Ui, files []string) ([]*api.AgentServiceRegistration, error) { // We set devMode to true so we can get the basic valid default // configuration. devMode doesn't set any services by default so this // is okay since we only look at services. @@ -29,6 +30,9 @@ func ServicesFromFiles(files []string) ([]*api.AgentServiceRegistration, error) if err != nil { return nil, err } + for _, w := range b.Warnings { + ui.Warn(w) + } // The services are now in "structs.ServiceDefinition" form and we need // them in "api.AgentServiceRegistration" form so do the conversion. diff --git a/command/services/deregister/deregister.go b/command/services/deregister/deregister.go index fc8efedd09..2cae8172a6 100644 --- a/command/services/deregister/deregister.go +++ b/command/services/deregister/deregister.go @@ -55,7 +55,7 @@ func (c *cmd) Run(args []string) int { ID: c.flagId}} if len(args) > 0 { var err error - svcs, err = services.ServicesFromFiles(args) + svcs, err = services.ServicesFromFiles(c.UI, args) if err != nil { c.UI.Error(fmt.Sprintf("Error: %s", err)) return 1 diff --git a/command/services/register/register.go b/command/services/register/register.go index 96835de09a..52f030edb2 100644 --- a/command/services/register/register.go +++ b/command/services/register/register.go @@ -103,7 +103,7 @@ func (c *cmd) Run(args []string) int { if len(args) > 0 { var err error - svcs, err = services.ServicesFromFiles(args) + svcs, err = services.ServicesFromFiles(c.UI, args) if err != nil { c.UI.Error(fmt.Sprintf("Error: %s", err)) return 1 diff --git a/command/validate/validate.go b/command/validate/validate.go index fb3d55f475..754f0e5ded 100644 --- a/command/validate/validate.go +++ b/command/validate/validate.go @@ -61,6 +61,9 @@ func (c *cmd) Run(args []string) int { return 1 } if !c.quiet { + for _, w := range b.Warnings { + c.UI.Warn(w) + } c.UI.Output("Configuration is valid!") } return 0