From 874e350b2f76f9d3d364a8ba57c23206c953e342 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Frank=20Schr=C3=B6der?= Date: Tue, 31 Oct 2017 23:30:01 +0100 Subject: [PATCH] config: add -config-format option (#3626) * config: refactor ReadPath(s) methods without side-effects Return the sources instead of modifying the state. * config: clean data dir before every test * config: add tests for config-file and config-dir * config: add -config-format option Starting with Consul 1.0 all config files must have a '.json' or '.hcl' extension to make it unambigous how the data should be parsed. Some automation tools generate temporary files by appending a random string to the generated file which obfuscates the extension and prevents the file type detection. This patch adds a -config-format option which can be used to override the auto-detection behavior by forcing all config files or all files within a config directory independent of their extension to be interpreted as of this format. Fixes #3620 --- agent/config/builder.go | 65 ++++++----- agent/config/config.go | 12 +-- agent/config/flags.go | 7 +- agent/config/runtime_test.go | 126 +++++++++++++++++++--- website/source/docs/agent/options.html.md | 6 ++ 5 files changed, 165 insertions(+), 51 deletions(-) diff --git a/agent/config/builder.go b/agent/config/builder.go index 4458f0fe71..869be6b660 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -110,14 +110,16 @@ func NewBuilder(flags Flags) (*Builder, error) { slices, values := b.splitSlicesAndValues(b.Flags.Config) b.Head = append(b.Head, newSource("flags.slices", slices)) for _, path := range b.Flags.ConfigFiles { - if err := b.ReadPath(path); err != nil { + sources, err := b.ReadPath(path) + if err != nil { return nil, err } + b.Sources = append(b.Sources, sources...) } b.Tail = append(b.Tail, newSource("flags.values", values)) for i, s := range b.Flags.HCL { b.Tail = append(b.Tail, Source{ - Name: fmt.Sprintf("flags.hcl.%d", i), + Name: fmt.Sprintf("flags-%d.hcl", i), Format: "hcl", Data: s, }) @@ -131,64 +133,59 @@ func NewBuilder(flags Flags) (*Builder, error) { // ReadPath reads a single config file or all files in a directory (but // not its sub-directories) and appends them to the list of config -// sources. If path refers to a file then the format is assumed to be -// JSON unless the file has a '.hcl' suffix. If path refers to a -// directory then the format is determined by the suffix and only files -// with a '.json' or '.hcl' suffix are processed. -func (b *Builder) ReadPath(path string) error { +// sources. +func (b *Builder) ReadPath(path string) ([]Source, error) { f, err := os.Open(path) if err != nil { - return fmt.Errorf("config: Open failed on %s. %s", path, err) + return nil, fmt.Errorf("config: Open failed on %s. %s", path, err) } defer f.Close() fi, err := f.Stat() if err != nil { - return fmt.Errorf("config: Stat failed on %s. %s", path, err) + return nil, fmt.Errorf("config: Stat failed on %s. %s", path, err) } if !fi.IsDir() { - return b.ReadFile(path) + src, err := b.ReadFile(path) + if err != nil { + return nil, err + } + return []Source{src}, nil } fis, err := f.Readdir(-1) if err != nil { - return fmt.Errorf("config: Readdir failed on %s. %s", path, err) + return nil, fmt.Errorf("config: Readdir failed on %s. %s", path, err) } // sort files by name sort.Sort(byName(fis)) + var sources []Source for _, fi := range fis { // do not recurse into sub dirs if fi.IsDir() { continue } - // skip files without json or hcl extension - if !strings.HasSuffix(fi.Name(), ".json") && !strings.HasSuffix(fi.Name(), ".hcl") { - continue - } - - if err := b.ReadFile(filepath.Join(path, fi.Name())); err != nil { - return err + src, err := b.ReadFile(filepath.Join(path, fi.Name())) + if err != nil { + return nil, err } + sources = append(sources, src) } - return nil + return sources, nil } // ReadFile parses a JSON or HCL config file and appends it to the list of // config sources. -func (b *Builder) ReadFile(path string) error { - if !strings.HasSuffix(path, ".json") && !strings.HasSuffix(path, ".hcl") { - return fmt.Errorf(`Missing or invalid file extension for %q. Please use ".json" or ".hcl".`, path) - } +func (b *Builder) ReadFile(path string) (Source, error) { data, err := ioutil.ReadFile(path) if err != nil { - return fmt.Errorf("config: ReadFile failed on %s: %s", path, err) + return Source{}, fmt.Errorf("config: ReadFile failed on %s: %s", path, err) } - b.Sources = append(b.Sources, NewSource(path, string(data))) - return nil + return Source{Name: path, Data: string(data)}, nil } type byName []os.FileInfo @@ -222,10 +219,24 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { // merge config sources as follows // + configFormat := b.stringVal(b.Flags.ConfigFormat) + if configFormat != "" && configFormat != "json" && configFormat != "hcl" { + return RuntimeConfig{}, fmt.Errorf("config: -config-format must be either 'hcl' or 'json'") + } + // build the list of config sources var srcs []Source srcs = append(srcs, b.Head...) - srcs = append(srcs, b.Sources...) + for _, src := range b.Sources { + src.Format = FormatFrom(src.Name) + if configFormat != "" { + src.Format = configFormat + } + if src.Format == "" { + return RuntimeConfig{}, fmt.Errorf(`config: Missing or invalid file extension for %q. Please use ".json" or ".hcl".`, src.Name) + } + srcs = append(srcs, src) + } srcs = append(srcs, b.Tail...) // parse the config sources into a configuration diff --git a/agent/config/config.go b/agent/config/config.go index 03c0d6994c..d79773fd55 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -21,15 +21,15 @@ type Source struct { Data string } -func NewSource(name, data string) Source { - return Source{Name: name, Format: FormatFrom(name), Data: data} -} - func FormatFrom(name string) string { - if strings.HasSuffix(name, ".hcl") { + switch { + case strings.HasSuffix(name, ".json"): + return "json" + case strings.HasSuffix(name, ".hcl"): return "hcl" + default: + return "" } - return "json" } // Parse parses a config fragment in either JSON or HCL format. diff --git a/agent/config/flags.go b/agent/config/flags.go index 6e0d7e31cf..3bad0eaef0 100644 --- a/agent/config/flags.go +++ b/agent/config/flags.go @@ -16,11 +16,15 @@ type Flags struct { // that should be read. ConfigFiles []string + // ConfigFormat forces all config files to be interpreted as this + // format independent of their extension. + ConfigFormat *string + + // HCL contains an arbitrary config in hcl format. // DevMode indicates whether the agent should be started in development // mode. This cannot be configured in a config file. DevMode *bool - // HCL contains an arbitrary config in hcl format. HCL []string // Args contains the remaining unparsed flags. @@ -57,6 +61,7 @@ func AddFlags(fs *flag.FlagSet, f *Flags) { add(&f.Config.ClientAddr, "client", "Sets the address to bind for client access. This includes RPC, DNS, HTTP and HTTPS (if configured).") add(&f.ConfigFiles, "config-dir", "Path to a directory to read configuration files from. This will read every file ending in '.json' as configuration in this directory in alphabetical order. Can be specified multiple times.") add(&f.ConfigFiles, "config-file", "Path to a JSON file to read configuration from. Can be specified multiple times.") + add(&f.ConfigFormat, "config-format", "Config files are in this format irrespective of their extension. Must be 'hcl' or 'json'") add(&f.Config.DataDir, "data-dir", "Path to a data directory to store agent state.") add(&f.Config.Datacenter, "datacenter", "Datacenter of the agent.") add(&f.DevMode, "dev", "Starts the agent in development mode.") diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 9fcd608c8d..e14b8ade07 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -177,6 +177,50 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { rt.DataDir = dataDir }, }, + { + desc: "-config-dir", + args: []string{ + `-data-dir=` + dataDir, + `-config-dir`, filepath.Join(dataDir, "conf.d"), + }, + patch: func(rt *RuntimeConfig) { + rt.Datacenter = "a" + rt.DataDir = dataDir + }, + pre: func() { + writeFile(filepath.Join(dataDir, "conf.d/conf.json"), []byte(`{"datacenter":"a"}`)) + }, + }, + { + desc: "-config-file json", + args: []string{ + `-data-dir=` + dataDir, + `-config-file`, filepath.Join(dataDir, "conf.json"), + }, + patch: func(rt *RuntimeConfig) { + rt.Datacenter = "a" + rt.DataDir = dataDir + }, + pre: func() { + writeFile(filepath.Join(dataDir, "conf.json"), []byte(`{"datacenter":"a"}`)) + }, + }, + { + desc: "-config-file hcl and json", + args: []string{ + `-data-dir=` + dataDir, + `-config-file`, filepath.Join(dataDir, "conf.hcl"), + `-config-file`, filepath.Join(dataDir, "conf.json"), + }, + patch: func(rt *RuntimeConfig) { + rt.Datacenter = "b" + rt.DataDir = dataDir + }, + pre: func() { + writeFile(filepath.Join(dataDir, "conf.hcl"), []byte(`datacenter = "a"`)) + writeFile(filepath.Join(dataDir, "conf.json"), []byte(`{"datacenter":"b"}`)) + }, + }, { desc: "-data-dir empty", args: []string{ @@ -317,6 +361,43 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { rt.DataDir = dataDir }, }, + { + desc: "-config-format=json", + args: []string{ + `-data-dir=` + dataDir, + `-config-format=json`, + `-config-file`, filepath.Join(dataDir, "conf"), + }, + patch: func(rt *RuntimeConfig) { + rt.Datacenter = "a" + rt.DataDir = dataDir + }, + pre: func() { + writeFile(filepath.Join(dataDir, "conf"), []byte(`{"datacenter":"a"}`)) + }, + }, + { + desc: "-config-format=hcl", + args: []string{ + `-data-dir=` + dataDir, + `-config-format=hcl`, + `-config-file`, filepath.Join(dataDir, "conf"), + }, + patch: func(rt *RuntimeConfig) { + rt.Datacenter = "a" + rt.DataDir = dataDir + }, + pre: func() { + writeFile(filepath.Join(dataDir, "conf"), []byte(`datacenter = "a"`)) + }, + }, + { + desc: "-config-format invalid", + args: []string{ + `-config-format=foobar`, + }, + err: "-config-format must be either 'hcl' or 'json'", + }, { desc: "-http-port", args: []string{ @@ -1723,9 +1804,6 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { pre: func() { writeFile(filepath.Join(dataDir, SerfLANKeyring), []byte("i0P+gFTkLPg0h53eNYjydg==")) }, - post: func() { - os.Remove(filepath.Join(filepath.Join(dataDir, SerfLANKeyring))) - }, warns: []string{`WARNING: LAN keyring exists but -encrypt given, using keyring`}, }, { @@ -1745,9 +1823,6 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { pre: func() { writeFile(filepath.Join(dataDir, SerfWANKeyring), []byte("i0P+gFTkLPg0h53eNYjydg==")) }, - post: func() { - os.Remove(filepath.Join(filepath.Join(dataDir, SerfWANKeyring))) - }, warns: []string{`WARNING: WAN keyring exists but -encrypt given, using keyring`}, }, { @@ -1855,6 +1930,9 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { func testConfig(t *testing.T, tests []configTest, dataDir string) { for _, tt := range tests { for pass, format := range []string{"json", "hcl"} { + // clean data dir before every test + cleanDir(dataDir) + // when we test only flags then there are no JSON or HCL // sources and we need to make only one pass over the // tests. @@ -1895,6 +1973,15 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) { } flags.Args = fs.Args() + if tt.pre != nil { + tt.pre() + } + defer func() { + if tt.post != nil { + tt.post() + } + }() + // Then create a builder with the flags. b, err := NewBuilder(flags) if err != nil { @@ -1926,28 +2013,20 @@ func testConfig(t *testing.T, tests []configTest, dataDir string) { // read the source fragements for i, data := range srcs { b.Sources = append(b.Sources, Source{ - Name: fmt.Sprintf("%s-%d", format, i), + Name: fmt.Sprintf("src-%d.%s", i, format), Format: format, Data: data, }) } for i, data := range tails { b.Tail = append(b.Tail, Source{ - Name: fmt.Sprintf("%s-%d", format, i), + Name: fmt.Sprintf("tail-%d.%s", i, format), Format: format, Data: data, }) } // build/merge the config fragments - if tt.pre != nil { - tt.pre() - } - defer func() { - if tt.post != nil { - tt.post() - } - }() rt, err := b.BuildAndValidate() if err == nil && tt.err != "" { t.Fatalf("got no error want %q", tt.err) @@ -3484,7 +3563,7 @@ func TestFullConfig(t *testing.T) { if err != nil { t.Fatalf("NewBuilder: %s", err) } - b.Sources = append(b.Sources, Source{Name: "full", Format: format, Data: data}) + b.Sources = append(b.Sources, Source{Name: "full." + format, Data: data}) b.Tail = append(b.Tail, tail[format]...) b.Tail = append(b.Tail, VersionSource("JNtPSav3", "R909Hblt", "ZT1JOQLn")) @@ -4027,6 +4106,19 @@ func writeFile(path string, data []byte) { } } +func cleanDir(path string) { + root := path + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if path == root { + return nil + } + return os.RemoveAll(path) + }) + if err != nil { + panic(err) + } +} + func randomString(n int) string { s := "" for ; n > 0; n-- { diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index 32e0ec6812..59a81da2c9 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -125,6 +125,12 @@ will exit with an error at startup. For more information on the format of the configuration files, see the [Configuration Files](#configuration_files) section. +* `-config-format` - The format + of the configuration files to load. Normally, Consul detects the format of the + config files from the ".json" or ".hcl" extension. Setting this option to + either "json" or "hcl" forces Consul to interpret any file with or without + extension to be interpreted in that format. + * `-data-dir` - This flag provides a data directory for the agent to store state. This is required for all agents. The directory should be durable across reboots.