From fe7b26a996ca019f4e25ac1076f4d47b1a76771c Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 10 Feb 2017 19:21:51 -0500 Subject: [PATCH 1/3] Deprecate the configtest command and add the validate command --- command/base/command.go | 13 ++++- command/configtest.go | 6 +- command/validate.go | 70 ++++++++++++++++++++++ command/validate_test.go | 121 +++++++++++++++++++++++++++++++++++++++ commands.go | 9 +++ 5 files changed, 215 insertions(+), 4 deletions(-) create mode 100644 command/validate.go create mode 100644 command/validate_test.go diff --git a/command/base/command.go b/command/base/command.go index a9fd6db661c5..048f31f0aac5 100644 --- a/command/base/command.go +++ b/command/base/command.go @@ -33,6 +33,7 @@ type Command struct { Flags FlagSetFlags flagSet *flag.FlagSet + hidden *flag.FlagSet // These are the options which correspond to the HTTP API options httpAddr stringValue @@ -137,10 +138,18 @@ func (c *Command) NewFlagSet(command cli.Command) *flag.FlagSet { f.SetOutput(errW) c.flagSet = f + c.hidden = flag.NewFlagSet("", flag.ContinueOnError) return f } +// HideFlags is used to set hidden flags that will not be shown in help text +func (c *Command) HideFlags(flags ...string) { + for _, f := range flags { + c.hidden.String(f, "", "") + } +} + // Parse is used to parse the underlying flag set. func (c *Command) Parse(args []string) error { return c.flagSet.Parse(args) @@ -199,7 +208,7 @@ func (c *Command) helpFlagsFor(f *flag.FlagSet) string { firstCommand := true f.VisitAll(func(f *flag.Flag) { // Skip HTTP flags as they will be grouped separately - if flagContains(httpFlagsClient, f) || flagContains(httpFlagsServer, f) { + if flagContains(httpFlagsClient, f) || flagContains(httpFlagsServer, f) || flagContains(c.hidden, f) { return } if firstCommand { @@ -240,7 +249,7 @@ func flagContains(fs *flag.FlagSet, f *flag.Flag) bool { return } - if f.Name == hf.Name && f.Usage == hf.Usage { + if f.Name == hf.Name { skip = true return } diff --git a/command/configtest.go b/command/configtest.go index 81eafef512ea..48f95419648c 100644 --- a/command/configtest.go +++ b/command/configtest.go @@ -16,7 +16,9 @@ type ConfigTestCommand struct { func (c *ConfigTestCommand) Help() string { helpText := ` -Usage: consul configtest [options] +Usage: consul configtest [options] FILE_OR_DIRECTORY + + DEPRECATED. Use the 'consul validate' command instead. Performs a basic sanity test on Consul configuration files. For each file or directory given, the configtest command will attempt to parse the @@ -59,5 +61,5 @@ func (c *ConfigTestCommand) Run(args []string) int { } func (c *ConfigTestCommand) Synopsis() string { - return "Validate config file" + return "DEPRECATED. Use the validate command instead" } diff --git a/command/validate.go b/command/validate.go new file mode 100644 index 000000000000..55f7c76992c9 --- /dev/null +++ b/command/validate.go @@ -0,0 +1,70 @@ +package command + +import ( + "fmt" + "strings" + + "github.com/hashicorp/consul/command/agent" + "github.com/hashicorp/consul/command/base" +) + +// ValidateCommand is a Command implementation that is used to +// verify config files +type ValidateCommand struct { + base.Command +} + +func (c *ValidateCommand) Help() string { + helpText := ` +Usage: consul validate [options] FILE_OR_DIRECTORY + + Performs a basic sanity test on Consul configuration files. For each file + or directory given, the validate command will attempt to parse the + contents just as the "consul agent" command would, and catch any errors. + This is useful to do a test of the configuration only, without actually + starting the agent. + + Returns 0 if the configuration is valid, or 1 if there are problems. + +` + c.Command.Help() + + return strings.TrimSpace(helpText) +} + +func (c *ValidateCommand) Run(args []string) int { + var configFiles []string + + f := c.Command.NewFlagSet(c) + f.Var((*agent.AppendSliceValue)(&configFiles), "config-file", + "Path to a JSON file to read configuration from. This can be specified multiple times.") + f.Var((*agent.AppendSliceValue)(&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.") + c.Command.HideFlags("config-file", "config-dir") + + if err := c.Command.Parse(args); err != nil { + return 1 + } + + if len(f.Args()) > 0 { + configFiles = append(configFiles, f.Args()...) + } + + if len(configFiles) < 1 { + c.Ui.Error("Must specify at least one config file or directory") + return 1 + } + + _, err := agent.ReadConfigPaths(configFiles) + if err != nil { + c.Ui.Error(fmt.Sprintf("Config validation failed: %v", err.Error())) + return 1 + } + + c.Ui.Output("Configuration is valid!") + return 0 +} + +func (c *ValidateCommand) Synopsis() string { + return "Validate config files/directories" +} diff --git a/command/validate_test.go b/command/validate_test.go new file mode 100644 index 000000000000..0476efef8c03 --- /dev/null +++ b/command/validate_test.go @@ -0,0 +1,121 @@ +package command + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/hashicorp/consul/command/base" + "github.com/mitchellh/cli" +) + +func testValidateCommand(t *testing.T) (*cli.MockUi, *ValidateCommand) { + ui := new(cli.MockUi) + return ui, &ValidateCommand{ + Command: base.Command{ + Ui: ui, + Flags: base.FlagSetNone, + }, + } +} + +func TestValidateCommand_implements(t *testing.T) { + var _ cli.Command = &ValidateCommand{} +} + +func TestValidateCommandFailOnEmptyFile(t *testing.T) { + tmpFile, err := ioutil.TempFile("", "consul") + if err != nil { + t.Fatalf("err: %s", err) + } + defer os.RemoveAll(tmpFile.Name()) + + _, cmd := testValidateCommand(t) + + args := []string{tmpFile.Name()} + + if code := cmd.Run(args); code == 0 { + t.Fatalf("bad: %d", code) + } +} + +func TestValidateCommandSucceedOnEmptyDir(t *testing.T) { + td, err := ioutil.TempDir("", "consul") + if err != nil { + t.Fatalf("err: %s", err) + } + defer os.RemoveAll(td) + + ui, cmd := testValidateCommand(t) + + args := []string{td} + + if code := cmd.Run(args); code != 0 { + t.Fatalf("bad: %d, %s", code, ui.ErrorWriter.String()) + } +} + +func TestValidateCommandSucceedOnMinimalConfigFile(t *testing.T) { + td, err := ioutil.TempDir("", "consul") + if err != nil { + t.Fatalf("err: %s", err) + } + defer os.RemoveAll(td) + + fp := filepath.Join(td, "config.json") + err = ioutil.WriteFile(fp, []byte(`{}`), 0644) + if err != nil { + t.Fatalf("err: %s", err) + } + + _, cmd := testValidateCommand(t) + + args := []string{fp} + + if code := cmd.Run(args); code != 0 { + t.Fatalf("bad: %d", code) + } +} + +func TestValidateCommandSucceedOnMinimalConfigDir(t *testing.T) { + td, err := ioutil.TempDir("", "consul") + if err != nil { + t.Fatalf("err: %s", err) + } + defer os.RemoveAll(td) + + err = ioutil.WriteFile(filepath.Join(td, "config.json"), []byte(`{}`), 0644) + if err != nil { + t.Fatalf("err: %s", err) + } + + _, cmd := testValidateCommand(t) + + args := []string{td} + + if code := cmd.Run(args); code != 0 { + t.Fatalf("bad: %d", code) + } +} + +func TestValidateCommandSucceedOnConfigDirWithEmptyFile(t *testing.T) { + td, err := ioutil.TempDir("", "consul") + if err != nil { + t.Fatalf("err: %s", err) + } + defer os.RemoveAll(td) + + err = ioutil.WriteFile(filepath.Join(td, "config.json"), []byte{}, 0644) + if err != nil { + t.Fatalf("err: %s", err) + } + + _, cmd := testValidateCommand(t) + + args := []string{td} + + if code := cmd.Run(args); code != 0 { + t.Fatalf("bad: %d", code) + } +} diff --git a/commands.go b/commands.go index 6ead48b9546f..f79c025f99d3 100644 --- a/commands.go +++ b/commands.go @@ -264,6 +264,15 @@ func init() { }, nil }, + "validate": func() (cli.Command, error) { + return &command.ValidateCommand{ + Command: base.Command{ + Flags: base.FlagSetNone, + Ui: ui, + }, + }, nil + }, + "version": func() (cli.Command, error) { return &command.VersionCommand{ HumanVersion: version.GetHumanVersion(), From eee5eb3fb8ec07d8abbc05cba798d025c8db2b5e Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 10 Feb 2017 19:29:00 -0500 Subject: [PATCH 2/3] Update website docs for validate command --- command/configtest.go | 2 +- command/validate.go | 2 +- main.go | 10 ++++- .../docs/commands/configtest.html.markdown | 34 ---------------- .../docs/commands/validate.html.markdown | 39 +++++++++++++++++++ website/source/layouts/docs.erb | 9 +++-- 6 files changed, 55 insertions(+), 41 deletions(-) delete mode 100644 website/source/docs/commands/configtest.html.markdown create mode 100644 website/source/docs/commands/validate.html.markdown diff --git a/command/configtest.go b/command/configtest.go index 48f95419648c..e945d9d8a2c1 100644 --- a/command/configtest.go +++ b/command/configtest.go @@ -16,7 +16,7 @@ type ConfigTestCommand struct { func (c *ConfigTestCommand) Help() string { helpText := ` -Usage: consul configtest [options] FILE_OR_DIRECTORY +Usage: consul configtest [options] DEPRECATED. Use the 'consul validate' command instead. diff --git a/command/validate.go b/command/validate.go index 55f7c76992c9..c37d97139b69 100644 --- a/command/validate.go +++ b/command/validate.go @@ -16,7 +16,7 @@ type ValidateCommand struct { func (c *ValidateCommand) Help() string { helpText := ` -Usage: consul validate [options] FILE_OR_DIRECTORY +Usage: consul validate [options] FILE_OR_DIRECTORY... Performs a basic sanity test on Consul configuration files. For each file or directory given, the validate command will attempt to parse the diff --git a/main.go b/main.go index 249e6e25387b..1867a73f4cf5 100644 --- a/main.go +++ b/main.go @@ -37,10 +37,18 @@ func realMain() int { } } + // Filter out the configtest command from the help display + var included []string + for command := range Commands { + if command != "configtest" { + included = append(included, command) + } + } + cli := &cli.CLI{ Args: args, Commands: Commands, - HelpFunc: cli.BasicHelpFunc("consul"), + HelpFunc: cli.FilteredHelpFunc(included, cli.BasicHelpFunc("consul")), } exitCode, err := cli.Run() diff --git a/website/source/docs/commands/configtest.html.markdown b/website/source/docs/commands/configtest.html.markdown deleted file mode 100644 index 78a629271c49..000000000000 --- a/website/source/docs/commands/configtest.html.markdown +++ /dev/null @@ -1,34 +0,0 @@ ---- -layout: "docs" -page_title: "Commands: ConfigTest" -sidebar_current: "docs-commands-configtest" -description: > - The `consul configtest` command tests that config files are valid by - attempting to parse them. Useful to ensure a configuration change will - not cause consul to fail after a restart. ---- - -# Consul ConfigTest - -The `consul configtest` command performs a basic sanity test on Consul -configuration files. For each file or directory given, the configtest command -will attempt to parse the contents just as the "consul agent" command would, -and catch any errors. This is useful to do a test of the configuration only, -without actually starting the agent. - -For more information on the format of Consul's configuration files, read the -consul agent [Configuration Files](/docs/agent/options.html#configuration_files) -section. - -## Usage - -Usage: `consul configtest [options]` - -At least one `-config-file` or `-config-dir` parameter must be given. Returns 0 -if the configuration is valid, or 1 if there are problems. The list of -available flags are: - -* `-config-file` - Path to a config file. May be specified multiple times. - -* `-config-dir` - Path to a directory of config files. All files ending in - `.json` in the directory will be included. May be specified multiple times. diff --git a/website/source/docs/commands/validate.html.markdown b/website/source/docs/commands/validate.html.markdown new file mode 100644 index 000000000000..d71339b67334 --- /dev/null +++ b/website/source/docs/commands/validate.html.markdown @@ -0,0 +1,39 @@ +--- +layout: "docs" +page_title: "Commands: Validate" +sidebar_current: "docs-commands-validate" +description: > + The `consul validate` command tests that config files are valid by + attempting to parse them. Useful to ensure a configuration change will + not cause consul to fail after a restart. +--- + +# Consul Validate + +The `consul validate` command performs a basic sanity test on Consul +configuration files. For each file or directory given, the validate command +will attempt to parse the contents just as the "consul agent" command would, +and catch any errors. This is useful to do a test of the configuration only, +without actually starting the agent. + +For more information on the format of Consul's configuration files, read the +consul agent [Configuration Files](/docs/agent/options.html#configuration_files) +section. + +## Usage + +Usage: `consul validate [options] FILE_OR_DIRECTORY...` + +Performs a basic sanity test on Consul configuration files. For each file +or directory given, the validate command will attempt to parse the +contents just as the "consul agent" command would, and catch any errors. +This is useful to do a test of the configuration only, without actually +starting the agent. + +Returns 0 if the configuration is valid, or 1 if there are problems. + +```text +$ consul validate /etc/consul.d +Configuration is valid! +``` + diff --git a/website/source/layouts/docs.erb b/website/source/layouts/docs.erb index b36421f9c38f..9d37c0615d3d 100644 --- a/website/source/layouts/docs.erb +++ b/website/source/layouts/docs.erb @@ -71,10 +71,6 @@ agent - > - configtest - - > event @@ -173,6 +169,11 @@ + > + validate + + + > version From 2aebff3bd32f31bc71a9e5027822612e8065e084 Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 10 Feb 2017 20:14:22 -0500 Subject: [PATCH 3/3] Add -quiet flag to validate --- command/validate.go | 7 ++++++- command/validate_test.go | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/command/validate.go b/command/validate.go index c37d97139b69..f536cfe6b131 100644 --- a/command/validate.go +++ b/command/validate.go @@ -33,6 +33,7 @@ Usage: consul validate [options] FILE_OR_DIRECTORY... func (c *ValidateCommand) Run(args []string) int { var configFiles []string + var quiet bool f := c.Command.NewFlagSet(c) f.Var((*agent.AppendSliceValue)(&configFiles), "config-file", @@ -40,6 +41,8 @@ func (c *ValidateCommand) Run(args []string) int { f.Var((*agent.AppendSliceValue)(&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.") + f.BoolVar(&quiet, "quiet", false, + "When given, a successful run will produce no output.") c.Command.HideFlags("config-file", "config-dir") if err := c.Command.Parse(args); err != nil { @@ -61,7 +64,9 @@ func (c *ValidateCommand) Run(args []string) int { return 1 } - c.Ui.Output("Configuration is valid!") + if !quiet { + c.Ui.Output("Configuration is valid!") + } return 0 } diff --git a/command/validate_test.go b/command/validate_test.go index 0476efef8c03..30abdf164e3b 100644 --- a/command/validate_test.go +++ b/command/validate_test.go @@ -119,3 +119,22 @@ func TestValidateCommandSucceedOnConfigDirWithEmptyFile(t *testing.T) { t.Fatalf("bad: %d", code) } } + +func TestValidateCommandQuiet(t *testing.T) { + td, err := ioutil.TempDir("", "consul") + if err != nil { + t.Fatalf("err: %s", err) + } + defer os.RemoveAll(td) + + ui, cmd := testValidateCommand(t) + + args := []string{"-quiet", td} + + if code := cmd.Run(args); code != 0 { + t.Fatalf("bad: %d, %s", code, ui.ErrorWriter.String()) + } + if ui.OutputWriter.String() != "" { + t.Fatalf("bad: %v", ui.OutputWriter.String()) + } +}