From dc7f793be9f263de29b5220d9d7664fdca734659 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 22 May 2018 21:32:43 -0700 Subject: [PATCH] command: terraform fmt to use new HCL formatter This doesn't yet include test updates, since there are problems in core currently blocking these tests from running. The tests will therefore be updated in a subsequent commit. --- command/fmt.go | 215 +++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 192 insertions(+), 23 deletions(-) diff --git a/command/fmt.go b/command/fmt.go index 9fefd2517..3e86a8c54 100644 --- a/command/fmt.go +++ b/command/fmt.go @@ -5,25 +5,35 @@ import ( "flag" "fmt" "io" + "io/ioutil" + "log" "os" + "os/exec" + "path/filepath" "strings" - "github.com/hashicorp/hcl/hcl/fmtcmd" + "github.com/hashicorp/terraform/configs" + "github.com/mitchellh/cli" + + "github.com/hashicorp/hcl2/hclwrite" + "github.com/hashicorp/terraform/tfdiags" ) const ( - stdinArg = "-" - fileExtension = "tf" + stdinArg = "-" ) // FmtCommand is a Command implementation that rewrites Terraform config // files to a canonical format and style. type FmtCommand struct { Meta - opts fmtcmd.Options - check bool - input io.Reader // STDIN if nil + list bool + write bool + diff bool + check bool + recursive bool + input io.Reader // STDIN if nil } func (c *FmtCommand) Run(args []string) int { @@ -37,10 +47,11 @@ func (c *FmtCommand) Run(args []string) int { } cmdFlags := flag.NewFlagSet("fmt", flag.ContinueOnError) - cmdFlags.BoolVar(&c.opts.List, "list", true, "list") - cmdFlags.BoolVar(&c.opts.Write, "write", true, "write") - cmdFlags.BoolVar(&c.opts.Diff, "diff", false, "diff") + cmdFlags.BoolVar(&c.list, "list", true, "list") + cmdFlags.BoolVar(&c.write, "write", true, "write") + cmdFlags.BoolVar(&c.diff, "diff", false, "diff") cmdFlags.BoolVar(&c.check, "check", false, "check") + cmdFlags.BoolVar(&c.recursive, "recursive", false, "recursive") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { @@ -58,27 +69,27 @@ func (c *FmtCommand) Run(args []string) int { if len(args) == 0 { dirs = []string{"."} } else if args[0] == stdinArg { - c.opts.List = false - c.opts.Write = false + c.list = false + c.write = false } else { dirs = []string{args[0]} } var output io.Writer - list := c.opts.List // preserve the original value of -list + list := c.list // preserve the original value of -list if c.check { // set to true so we can use the list output to check // if the input needs formatting - c.opts.List = true - c.opts.Write = false + c.list = true + c.write = false output = &bytes.Buffer{} } else { output = &cli.UiWriter{Ui: c.Ui} } - err = fmtcmd.Run(dirs, []string{fileExtension}, c.input, output, c.opts) - if err != nil { - c.Ui.Error(fmt.Sprintf("Error running fmt: %s", err)) + diags := c.fmt(dirs, c.input, output) + c.showDiagnostics(diags) + if diags.HasErrors() { return 2 } @@ -98,25 +109,156 @@ func (c *FmtCommand) Run(args []string) int { return 0 } +func (c *FmtCommand) fmt(paths []string, stdin io.Reader, stdout io.Writer) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + if len(paths) == 0 { // Assuming stdin, then. + if c.write { + diags = diags.Append(fmt.Errorf("Option -write cannot be used when reading from stdin")) + return diags + } + fileDiags := c.processFile("", stdin, stdout, true) + diags = diags.Append(fileDiags) + return diags + } + + for _, path := range paths { + path = c.normalizePath(path) + dirDiags := c.processDir(path, stdout) + diags = diags.Append(dirDiags) + } + + return diags +} + +func (c *FmtCommand) processFile(path string, r io.Reader, w io.Writer, isStdout bool) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + log.Printf("[TRACE] terraform fmt: Formatting %s", path) + + src, err := ioutil.ReadAll(r) + if err != nil { + diags = diags.Append(fmt.Errorf("Failed to read %s", path)) + return diags + } + + result := hclwrite.Format(src) + + if !bytes.Equal(src, result) { + // Something was changed + if c.list { + fmt.Fprintln(w, path) + } + if c.write { + err := ioutil.WriteFile(path, result, 0644) + if err != nil { + diags = diags.Append(fmt.Errorf("Failed to write %s", path)) + return diags + } + } + if c.diff { + diff, err := bytesDiff(src, result, path) + if err != nil { + diags = diags.Append(fmt.Errorf("Failed to generate diff for %s: %s", path, err)) + return diags + } + w.Write(diff) + } + } + + if !c.list && !c.write && !c.diff { + _, err = w.Write(result) + if err != nil { + diags = diags.Append(fmt.Errorf("Failed to write result")) + } + } + + return diags +} + +func (c *FmtCommand) processDir(path string, stdout io.Writer) tfdiags.Diagnostics { + var diags tfdiags.Diagnostics + + log.Printf("[TRACE] terraform fmt: looking for files in %s", path) + + entries, err := ioutil.ReadDir(path) + if err != nil { + switch { + case os.IsNotExist(err): + diags = diags.Append(fmt.Errorf("There is no configuration directory at %s", path)) + default: + // ReadDir does not produce error messages that are end-user-appropriate, + // so we'll need to simplify here. + diags = diags.Append(fmt.Errorf("Cannot read directory %s", path)) + } + return diags + } + + for _, info := range entries { + name := info.Name() + if configs.IsIgnoredFile(name) { + continue + } + subPath := filepath.Join(path, name) + if info.IsDir() { + if c.recursive { + subDiags := c.processDir(subPath, stdout) + diags = diags.Append(subDiags) + } + + // We do not recurse into child directories by default because we + // want to mimic the file-reading behavior of "terraform plan", etc, + // operating on one module at a time. + continue + } + + ext := filepath.Ext(name) + switch ext { + case ".tf", ".tfvars": + f, err := os.Open(subPath) + if err != nil { + // Open does not produce error messages that are end-user-appropriate, + // so we'll need to simplify here. + diags = diags.Append(fmt.Errorf("Failed to read file %s", subPath)) + continue + } + + fileDiags := c.processFile(c.normalizePath(subPath), f, stdout, false) + diags = diags.Append(fileDiags) + f.Close() + } + } + + return diags +} + func (c *FmtCommand) Help() string { helpText := ` Usage: terraform fmt [options] [DIR] - Rewrites all Terraform configuration files to a canonical format. + Rewrites all Terraform configuration files to a canonical format. Both + configuration files (.tf) and variables files (.tfvars) are updated. + JSON files (.tf.json or .tfvars.json) are not modified. If DIR is not specified then the current working directory will be used. - If DIR is "-" then content will be read from STDIN. + If DIR is "-" then content will be read from STDIN. The given content must + be in the Terraform language native syntax; JSON is not supported. Options: - -list=true List files whose formatting differs (always false if using STDIN) + -list=false Don't list files whose formatting differs + (always disabled if using STDIN) - -write=true Write result to source file instead of STDOUT (always false if using STDIN or -check) + -write=false Don't write to source files + (always disabled if using STDIN or -check) - -diff=false Display diffs of formatting changes + -diff Display diffs of formatting changes - -check=false Check if the input is formatted. Exit status will be 0 if all input is properly formatted and non-zero otherwise. + -check Check if the input is formatted. Exit status will be 0 if all + input is properly formatted and non-zero otherwise. + -recursive Also process files in subdirectories. By default, only the + given directory (or current directory) is processed. ` return strings.TrimSpace(helpText) } @@ -124,3 +266,30 @@ Options: func (c *FmtCommand) Synopsis() string { return "Rewrites config files to canonical format" } + +func bytesDiff(b1, b2 []byte, path string) (data []byte, err error) { + f1, err := ioutil.TempFile("", "") + if err != nil { + return + } + defer os.Remove(f1.Name()) + defer f1.Close() + + f2, err := ioutil.TempFile("", "") + if err != nil { + return + } + defer os.Remove(f2.Name()) + defer f2.Close() + + f1.Write(b1) + f2.Write(b2) + + data, err = exec.Command("diff", "--label=old/"+path, "--label=new/"+path, "-u", f1.Name(), f2.Name()).CombinedOutput() + if len(data) > 0 { + // diff exits with a non-zero status when the files don't match. + // Ignore that failure as long as we get output. + err = nil + } + return +}