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.
This commit is contained in:
Daniel Nephin 2020-05-01 20:17:27 -04:00
parent 2c2da41b3d
commit cb736b6947
7 changed files with 23 additions and 14 deletions

View File

@ -108,7 +108,7 @@ func NewBuilder(opts BuilderOpts) (*Builder, error) {
slices, values := b.splitSlicesAndValues(opts.Config) slices, values := b.splitSlicesAndValues(opts.Config)
b.Head = append(b.Head, newSource("flags.slices", slices)) b.Head = append(b.Head, newSource("flags.slices", slices))
for _, path := range opts.ConfigFiles { for _, path := range opts.ConfigFiles {
sources, err := sourcesFromPath(path, opts) sources, err := b.sourcesFromPath(path, opts.ConfigFormat)
if err != nil { if err != nil {
return nil, err 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 // sourcesFromPath reads a single config file or all files in a directory (but
// not its sub-directories) and returns Sources created from the // not its sub-directories) and returns Sources created from the
// files. // files.
func sourcesFromPath(path string, options BuilderOpts) ([]Source, error) { func (b *Builder) sourcesFromPath(path string, format string) ([]Source, error) {
f, err := os.Open(path) f, err := os.Open(path)
if err != nil { if err != nil {
return nil, fmt.Errorf("config: Open failed on %s. %s", path, err) 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 !fi.IsDir() {
if !shouldParseFile(path, options.ConfigFormat) { if !shouldParseFile(path, format) {
// TODO: log warning b.warn("skipping file %v, extension must be .hcl or .json, or config format must be set", path)
return nil, nil return nil, nil
} }
src, err := newSourceFromFile(path, options.ConfigFormat) src, err := newSourceFromFile(path, format)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -185,11 +185,11 @@ func sourcesFromPath(path string, options BuilderOpts) ([]Source, error) {
continue continue
} }
if !shouldParseFile(fp, options.ConfigFormat) { if !shouldParseFile(fp, format) {
// TODO: log warning b.warn("skipping file %v, extension must be .hcl or .json, or config format must be set", fp)
continue continue
} }
src, err := newSourceFromFile(fp, options.ConfigFormat) src, err := newSourceFromFile(fp, format)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -251,9 +251,6 @@ func (b *Builder) BuildAndValidate() (RuntimeConfig, error) {
// warnings can still contain deprecation or format warnings that should // warnings can still contain deprecation or format warnings that should
// be presented to the user. // be presented to the user.
func (b *Builder) Build() (rt RuntimeConfig, err error) { func (b *Builder) Build() (rt RuntimeConfig, err error) {
b.err = nil
b.Warnings = nil
// TODO: move to NewBuilder to remove Builder.options field // TODO: move to NewBuilder to remove Builder.options field
configFormat := b.options.ConfigFormat configFormat := b.options.ConfigFormat
if configFormat != "" && configFormat != "json" && configFormat != "hcl" { if configFormat != "" && configFormat != "json" && configFormat != "hcl" {

View File

@ -44,6 +44,7 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) {
{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"}, {Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"},
} }
require.Equal(t, expected, b.Sources) require.Equal(t, expected, b.Sources)
require.Len(t, b.Warnings, 2)
} }
func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.T) { func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.T) {

View File

@ -451,6 +451,9 @@ func TestConfigFlagsAndEdgecases(t *testing.T) {
writeFile(filepath.Join(dataDir, "conf", "valid.json"), []byte(`{"datacenter":"a"}`)) writeFile(filepath.Join(dataDir, "conf", "valid.json"), []byte(`{"datacenter":"a"}`))
writeFile(filepath.Join(dataDir, "conf", "invalid.skip"), []byte(`NOPE`)) 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", desc: "-config-format=json",
@ -6002,6 +6005,7 @@ func TestFullConfig(t *testing.T) {
if err := fs.Parse(flagSrc); err != nil { if err := fs.Parse(flagSrc); err != nil {
t.Fatalf("ParseFlags: %s", err) t.Fatalf("ParseFlags: %s", err)
} }
require.Len(t, fs.Args(), 0)
b, err := NewBuilder(flags) b, err := NewBuilder(flags)
if err != nil { if err != nil {

View File

@ -7,12 +7,13 @@ import (
"github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/config"
"github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api" "github.com/hashicorp/consul/api"
"github.com/mitchellh/cli"
"github.com/mitchellh/mapstructure" "github.com/mitchellh/mapstructure"
) )
// ServicesFromFiles returns the list of agent service registration structs // ServicesFromFiles returns the list of agent service registration structs
// from a set of file arguments. // 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 // We set devMode to true so we can get the basic valid default
// configuration. devMode doesn't set any services by default so this // configuration. devMode doesn't set any services by default so this
// is okay since we only look at services. // is okay since we only look at services.
@ -29,6 +30,9 @@ func ServicesFromFiles(files []string) ([]*api.AgentServiceRegistration, error)
if err != nil { if err != nil {
return nil, err return nil, err
} }
for _, w := range b.Warnings {
ui.Warn(w)
}
// The services are now in "structs.ServiceDefinition" form and we need // The services are now in "structs.ServiceDefinition" form and we need
// them in "api.AgentServiceRegistration" form so do the conversion. // them in "api.AgentServiceRegistration" form so do the conversion.

View File

@ -55,7 +55,7 @@ func (c *cmd) Run(args []string) int {
ID: c.flagId}} ID: c.flagId}}
if len(args) > 0 { if len(args) > 0 {
var err error var err error
svcs, err = services.ServicesFromFiles(args) svcs, err = services.ServicesFromFiles(c.UI, args)
if err != nil { if err != nil {
c.UI.Error(fmt.Sprintf("Error: %s", err)) c.UI.Error(fmt.Sprintf("Error: %s", err))
return 1 return 1

View File

@ -103,7 +103,7 @@ func (c *cmd) Run(args []string) int {
if len(args) > 0 { if len(args) > 0 {
var err error var err error
svcs, err = services.ServicesFromFiles(args) svcs, err = services.ServicesFromFiles(c.UI, args)
if err != nil { if err != nil {
c.UI.Error(fmt.Sprintf("Error: %s", err)) c.UI.Error(fmt.Sprintf("Error: %s", err))
return 1 return 1

View File

@ -61,6 +61,9 @@ func (c *cmd) Run(args []string) int {
return 1 return 1
} }
if !c.quiet { if !c.quiet {
for _, w := range b.Warnings {
c.UI.Warn(w)
}
c.UI.Output("Configuration is valid!") c.UI.Output("Configuration is valid!")
} }
return 0 return 0