Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate configtest and add validate command #2732

Merged
merged 3 commits into from
Feb 11, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions command/base/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
4 changes: 3 additions & 1 deletion command/configtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ func (c *ConfigTestCommand) Help() string {
helpText := `
Usage: consul configtest [options]

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
contents just as the "consul agent" command would, and catch any errors.
Expand Down Expand Up @@ -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"
}
75 changes: 75 additions & 0 deletions command/validate.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
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
var quiet bool

f := c.Command.NewFlagSet(c)
f.Var((*agent.AppendSliceValue)(&configFiles), "config-file",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyhavlov I still think it'd be a better UX to just accept one option here and determine if it's a file or folder appropriately. I don't see a reason why we need to treat them separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left those in as hidden flags to stay compatible with configtest; everything (args, -config-file, config-dir) gets added to configFiles which works with files or directories. Do you think we should just remove the flags from validate altogether?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah sorry. I didn't see the "hidden" call below. +1 to BC! Great work on this and sorry for the confusion

"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.")
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 {
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
}

if !quiet {
c.Ui.Output("Configuration is valid!")
}
return 0
}

func (c *ValidateCommand) Synopsis() string {
return "Validate config files/directories"
}
140 changes: 140 additions & 0 deletions command/validate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
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)
}
}

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() != "<nil>" {
t.Fatalf("bad: %v", ui.OutputWriter.String())
}
}
9 changes: 9 additions & 0 deletions commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
10 changes: 9 additions & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
34 changes: 0 additions & 34 deletions website/source/docs/commands/configtest.html.markdown

This file was deleted.

39 changes: 39 additions & 0 deletions website/source/docs/commands/validate.html.markdown
Original file line number Diff line number Diff line change
@@ -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!
```

Loading