From f6c5c970d312374fd73c6512ef01766e9a10d466 Mon Sep 17 00:00:00 2001 From: Wasif Malik Date: Thu, 1 Feb 2024 11:02:21 +0100 Subject: [PATCH 1/5] chore: Use printer.Stderr.Fatal for all fatal messages There were four kinds of fatal messages in the codebase: * `fatal(err, msg)` * `c.fatal(msg, err)` * `errlog.fatal` * `logger.Fatal` All fatal messages have been standardized by using `fatal(err, msg)` which uses `printer.Stderr.Fatal. --- cmd/terramate/cli/cli.go | 228 ++++++++---------- cmd/terramate/cli/cloud.go | 24 +- cmd/terramate/cli/cloud_credential_google.go | 2 +- cmd/terramate/cli/project.go | 20 +- cmd/terramate/cli/run.go | 28 +-- cmd/terramate/cli/script_info.go | 2 +- cmd/terramate/cli/script_run.go | 24 +- .../e2etests/cloud/run_cloud_config_test.go | 2 +- errors/errlog/errlog.go | 10 - printer/printer.go | 9 +- 10 files changed, 151 insertions(+), 198 deletions(-) diff --git a/cmd/terramate/cli/cli.go b/cmd/terramate/cli/cli.go index c31334002d..e17ca4ffb2 100644 --- a/cmd/terramate/cli/cli.go +++ b/cmd/terramate/cli/cli.go @@ -26,7 +26,6 @@ import ( "github.com/terramate-io/terramate/config/filter" "github.com/terramate-io/terramate/config/tag" "github.com/terramate-io/terramate/errors" - "github.com/terramate-io/terramate/errors/errlog" "github.com/terramate-io/terramate/event" "github.com/terramate-io/terramate/generate" "github.com/terramate-io/terramate/globals" @@ -358,7 +357,7 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr kong.Writers(stdout, stderr), ) if err != nil { - fatal(err, "creating cli parser") + fatal("creating cli parser", err) } kongplete.Complete(parser, @@ -380,7 +379,7 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr } if err != nil { - fatal(err, "parsing cli args %v", args) + fatal(sprintf("parsing cli args %v", args), err) } configureLogging(parsedArgs.LogLevel, parsedArgs.LogFmt, @@ -401,7 +400,7 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr clicfg, err := cliconfig.Load() if err != nil { - fatal(err, "failed to load cli configuration file") + fatal("failed to load cli configuration file", err) } // cmdline flags override configuration file. @@ -417,12 +416,13 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr if clicfg.UserTerramateDir == "" { homeTmDir, err := userTerramateDir() if err != nil { - output.MsgStdErr("Please either export the %s environment variable or "+ + title := sprintf("Please either export the %s environment variable or "+ "set the homeTerramateDir option in the %s configuration file", cliconfig.DirEnv, - cliconfig.Filename) + cliconfig.Filename, + ) - fatal(err) + fatal(title, err) } clicfg.UserTerramateDir = homeTmDir } @@ -474,7 +474,7 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr err := parsedArgs.InstallCompletions.Run(ctx) if err != nil { - fatal(err, "installing shell completions") + fatal("installing shell completions", err) } return &cli{exit: true} case "experimental cloud login": // Deprecated: use cloud login @@ -482,7 +482,7 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr case "cloud login": err := googleLogin(output, idpkey(), clicfg) if err != nil { - fatal(err, "authentication failed") + fatal("authentication failed", err) } output.MsgStdOut("authenticated successfully") return &cli{exit: true} @@ -490,7 +490,7 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr wd, err := os.Getwd() if err != nil { - fatal(err, "getting workdir") + fatal("getting workdir", err) } logger = logger.With(). @@ -503,23 +503,23 @@ func newCLI(version string, args []string, stdin io.Reader, stdout, stderr io.Wr Msg("Changing working directory") err = os.Chdir(parsedArgs.Chdir) if err != nil { - fatal(err, "changing working dir to %s", parsedArgs.Chdir) + fatal(sprintf("changing working dir to %s", parsedArgs.Chdir), err) } wd, err = os.Getwd() if err != nil { - fatal(err, "getting workdir: %s") + fatal("getting workdir: %s", err) } } wd, err = filepath.EvalSymlinks(wd) if err != nil { - log.Fatal().Msgf("evaluating symlinks on working dir: %s", wd) + fatal(sprintf("evaluating symlinks on working dir: %s", wd), err) } prj, foundRoot, err := lookupProject(wd) if err != nil { - fatal(err, "looking up project root") + fatal("unable to parse configuration", err) } if !foundRoot { @@ -538,15 +538,15 @@ Please see https://terramate.io/docs/cli/configuration/project-setup for details err = prj.setDefaults() if err != nil { - fatal(err, "setting configuration") + fatal("setting configuration", err) } if parsedArgs.Changed && !prj.isRepo { - log.Fatal().Msg("flag --changed provided but no git repository found") + fatal("flag --changed provided but no git repository found", nil) } if parsedArgs.Changed && !prj.hasCommits() { - log.Fatal().Msg("flag --changed requires a repository with at least two commits") + fatal("flag --changed requires a repository with at least two commits", nil) } uimode := HumanMode @@ -602,7 +602,7 @@ func (c *cli) run() { c.setupGit() c.printStacks() case "run": - log.Fatal().Msg("no command specified") + fatal("no command specified", nil) case "run ": c.setupGit() c.setupSafeguards(c.parsedArgs.Run.runSafeguardsCliSpec) @@ -636,15 +636,15 @@ func (c *cli) run() { c.setupGit() c.printRuntimeEnv() case "experimental eval": - log.Fatal().Msg("no expression specified") + fatal("no expression specified", nil) case "experimental eval ": c.eval() case "experimental partial-eval": - log.Fatal().Msg("no expression specified") + fatal("no expression specified", nil) case "experimental partial-eval ": c.partialEval() case "experimental get-config-value": - log.Fatal().Msg("no variable specified") + fatal("no variable specified", nil) case "experimental get-config-value ": c.getConfigValue() case "experimental cloud info": // Deprecated @@ -663,20 +663,20 @@ func (c *cli) run() { c.printScriptTree() case "script info": c.checkScriptEnabled() - log.Fatal().Msg("no script specified") + fatal("no script specified", nil) case "script info ": c.checkScriptEnabled() c.printScriptInfo() case "script run": c.checkScriptEnabled() - log.Fatal().Msg("no script specified") + fatal("no script specified", nil) case "script run ": c.checkScriptEnabled() c.setupGit() c.setupSafeguards(c.parsedArgs.Script.Run.runSafeguardsCliSpec) c.runScript() default: - log.Fatal().Msg("unexpected command sequence") + fatal("unexpected command sequence", nil) } } @@ -701,7 +701,7 @@ func (c *cli) setupSafeguards(run runSafeguardsCliSpec) { } if run.DisableSafeguards.Has(safeguard.All) && run.DisableSafeguards.Has(safeguard.None) { - c.fatal("Disabling safeguards", + fatal("Disabling safeguards", errors.E(clitest.ErrSafeguardKeywordValidation, `the safeguards keywords "all" and "none" are incompatible`), ) @@ -726,7 +726,7 @@ func (c *cli) setupGit() { if err := c.prj.checkDefaultRemote(); err != nil { if c.prj.git.remoteConfigured { - fatal(err, "checking git default remote") + fatal("checking git default remote", err) } else { remoteCheckFailed = true } @@ -755,10 +755,10 @@ func (c *cli) vendorDownload() { parsedSource, err := tf.ParseSource(source) if err != nil { - log.Fatal().Msgf("parsing module source %s: %s", source, err) + fatal(sprintf("parsing module source %s: %s", source, err), nil) } if parsedSource.Ref != "" { - log.Fatal().Msgf("module source %s should not contain a reference", source) + fatal(sprintf("module source %s should not contain a reference", source), nil) } parsedSource.Ref = ref @@ -819,7 +819,7 @@ func (c *cli) vendorDir() prj.Path { checkVendorDir := func(dir string) prj.Path { if !path.IsAbs(dir) { - log.Fatal().Msgf("vendorDir %s defined is not an absolute path", dir) + fatal(sprintf("vendorDir %s defined is not an absolute path", dir), nil) } return prj.NewPath(dir) } @@ -831,7 +831,7 @@ func (c *cli) vendorDir() prj.Path { cfg, err := hcl.ParseDir(c.rootdir(), filepath.Join(c.rootdir(), ".terramate")) if err != nil { - fatal(err, "parsing vendor dir configuration on .terramate") + fatal("parsing vendor dir configuration on .terramate", err) } if hasVendorDirConfig(cfg) { @@ -857,7 +857,7 @@ func (c *cli) triggerStackByFilter() { expStatus := c.parsedArgs.Experimental.Trigger.ExperimentalStatus cloudStatus := c.parsedArgs.Experimental.Trigger.CloudStatus if expStatus != "" && cloudStatus != "" { - fatal(errors.E("--experimental-status and --cloud-status cannot be used together")) + fatal("--experimental-status and --cloud-status cannot be used together", nil) } statusStr := expStatus @@ -866,13 +866,13 @@ func (c *cli) triggerStackByFilter() { } if statusStr == "" { - fatal(errors.E("trigger command expects either a stack path or the --cloud-status flag")) + fatal("trigger command expects either a stack path or the --cloud-status flag", nil) } status := parseStatusFilter(statusStr) stacksReport, err := c.listStacks(false, status) if err != nil { - fatal(err) + fatal("unable to list stacks", err) } for _, st := range stacksReport.Stacks { @@ -900,16 +900,16 @@ func (c *cli) triggerStack(stack string) { stack = filepath.Clean(stack) if tmp, err := filepath.EvalSymlinks(stack); err != nil || tmp != stack { - errlog.Fatal(logger, errors.E("symlinks are disallowed in the stack path")) + fatal("symlinks are disallowed in the stack path", nil) } if !strings.HasPrefix(stack, c.rootdir()) { - errlog.Fatal(logger, errors.E("stack %s is outside project", stack)) + fatal(sprintf("stack %s is outside project", stack), nil) } stackPath := prj.PrjAbsPath(c.rootdir(), stack) if err := trigger.Create(c.cfg(), stackPath, reason); err != nil { - errlog.Fatal(logger, err) + fatal("unable to create trigger", err) } c.output.MsgStdOut("Created trigger for stack %q", stackPath) @@ -926,7 +926,7 @@ func (c *cli) cloneStack() { n, err := stack.Clone(c.cfg(), absDestdir, absSrcdir, skipChildStacks) if err != nil { - fatal(err, "cloning %s to %s", srcdir, destdir) + fatal(sprintf("cloning %s to %s", srcdir, destdir), err) } c.output.MsgStdOut("Cloned %d stack(s) from %s to %s with success", n, srcdir, destdir) @@ -1050,7 +1050,7 @@ func (c *cli) gitFileSafeguards(shouldAbort bool) { if c.checkGitUntracked() && len(c.prj.git.repoChecks.UntrackedFiles) > 0 { const msg = "repository has untracked files" if shouldAbort { - log.Fatal().Msg(msg) + fatal(msg, nil) } else { log.Warn().Msg(msg) } @@ -1059,7 +1059,7 @@ func (c *cli) gitFileSafeguards(shouldAbort bool) { if c.checkGitUncommited() && len(c.prj.git.repoChecks.UncommittedFiles) > 0 { const msg = "repository has uncommitted files" if shouldAbort { - log.Fatal().Msg(msg) + fatal(msg, nil) } else { log.Warn().Msg(msg) } @@ -1078,7 +1078,7 @@ func (c *cli) gitSafeguardDefaultBranchIsReachable() { } if err := c.prj.checkRemoteDefaultBranchIsReachable(); err != nil { - fatal(err) + fatal("unable to reach remote default branch", err) } } @@ -1147,11 +1147,11 @@ func (c *cli) listStacks(isChanged bool, status cloudstack.FilterStatus) (*stack func (c *cli) scanCreate() { if c.parsedArgs.Create.EnsureStackIds && c.parsedArgs.Create.AllTerraform { - c.fatal("Invalid args", errors.E("--all-terraform conflicts with --ensure-stack-ids")) + fatal("Invalid args", errors.E("--all-terraform conflicts with --ensure-stack-ids")) } if !c.parsedArgs.Create.AllTerraform && !c.parsedArgs.Create.EnsureStackIds { - c.fatal( + fatal( "Invalid args", errors.E("terramate create requires a path or --all-terraform or --ensure-stack-ids"), ) @@ -1173,7 +1173,7 @@ func (c *cli) scanCreate() { len(c.parsedArgs.Create.Before) != 0 || len(c.parsedArgs.Create.Import) != 0 { - c.fatal( + fatal( "Invalid args", errors.E( "%s is incompatible with path and the flags: "+ @@ -1200,7 +1200,7 @@ func (c *cli) scanCreate() { func (c *cli) initTerraform() { err := c.initDir(c.wd()) if err != nil { - fatal(err, "failed to initialize some directories") + fatal("failed to initialize some directories", err) } if c.parsedArgs.Create.NoGenerate { @@ -1210,7 +1210,7 @@ func (c *cli) initTerraform() { root, err := config.LoadRoot(c.rootdir()) if err != nil { - fatal(err, "reloading the configuration") + fatal("reloading the configuration", err) } c.prj.root = *root @@ -1243,7 +1243,7 @@ func (c *cli) initDir(baseDir string) error { dirs, err := os.ReadDir(baseDir) if err != nil { - fatal(errors.E(err, "listing directory entries")) + fatal("unable to read directory while listing directory entries", err) } errs := errors.L() @@ -1268,7 +1268,7 @@ func (c *cli) initDir(baseDir string) error { found, err := tf.IsStack(path) if err != nil { - fatal(errors.E(err, "parsing terraform")) + fatal("parsing terraform", err) } if !found { @@ -1279,7 +1279,7 @@ func (c *cli) initDir(baseDir string) error { stackID, err := uuid.NewRandom() dirBasename := filepath.Base(stackDir) if err != nil { - fatal(err, "creating stack UUID") + fatal("creating stack UUID", err) } stackSpec := config.Stack{ Dir: prj.PrjAbsPath(c.rootdir(), stackDir), @@ -1316,7 +1316,7 @@ func (c *cli) createStack() { id, err := uuid.NewRandom() if err != nil { - fatal(err, "creating stack UUID") + fatal("creating stack UUID", err) } stackID = id.String() } @@ -1365,7 +1365,7 @@ func (c *cli) createStack() { Logger() } - c.fatal("Cannot create stack", err) + fatal("Cannot create stack", err) } printer.Stdout.Success("Created stack " + stackSpec.Dir.String()) @@ -1377,7 +1377,7 @@ func (c *cli) createStack() { err = c.prj.root.LoadSubTree(stackSpec.Dir) if err != nil { - c.fatal("Unable to load new stack", err) + fatal("Unable to load new stack", err) } report, vendorReport := c.gencodeWithVendor() @@ -1399,12 +1399,12 @@ func (c *cli) createStack() { func (c *cli) format() { if c.parsedArgs.Fmt.Check && c.parsedArgs.Fmt.DetailedExitCode { - c.fatal("Invalid args", errors.E("--check conflicts with --detailed-exit-code")) + fatal("Invalid args", errors.E("--check conflicts with --detailed-exit-code")) } results, err := fmt.FormatTree(c.wd()) if err != nil { - c.fatal("formatting files", err) + fatal("formatting files", err) } for _, res := range results { @@ -1428,19 +1428,19 @@ func (c *cli) format() { } if err := errs.AsError(); err != nil { - fatal(err, "saving files formatted files") + fatal("saving files formatted files", err) } } func (c *cli) printStacks() { if c.parsedArgs.List.Why && !c.parsedArgs.Changed { - c.fatal("Invalid args", errors.E("the --why flag must be used together with --changed")) + fatal("Invalid args", errors.E("the --why flag must be used together with --changed")) } expStatus := c.parsedArgs.List.ExperimentalStatus cloudStatus := c.parsedArgs.List.CloudStatus if expStatus != "" && cloudStatus != "" { - c.fatal("Invalid args", errors.E("--experimental-status and --cloud-status cannot be used together")) + fatal("Invalid args", errors.E("--experimental-status and --cloud-status cannot be used together")) } statusStr := expStatus @@ -1451,7 +1451,7 @@ func (c *cli) printStacks() { status := parseStatusFilter(statusStr) report, err := c.listStacks(c.parsedArgs.Changed, status) if err != nil { - c.fatal("Unable to list stacks", err) + fatal("Unable to list stacks", err) } c.gitFileSafeguards(false) @@ -1473,7 +1473,7 @@ func (c *cli) printStacksList(allStacks []stack.Entry, why bool, runOrder bool) var err error stacks, failReason, err = run.Sort(c.cfg(), stacks) if err != nil { - c.fatal("Invalid stack configuration", errors.E(err, failReason)) + fatal("Invalid stack configuration", errors.E(err, failReason)) } } @@ -1499,7 +1499,7 @@ func parseStatusFilter(strStatus string) cloudstack.FilterStatus { if strStatus != "" { status = cloudstack.NewStatusFilter(strStatus) if status.Is(cloudstack.Unrecognized) { - fatal(errors.E("unrecognized stack filter: %s", strStatus)) + fatal(sprintf("unrecognized stack filter: %s", strStatus), nil) } } return status @@ -1508,13 +1508,13 @@ func parseStatusFilter(strStatus string) cloudstack.FilterStatus { func (c *cli) printRuntimeEnv() { report, err := c.listStacks(c.parsedArgs.Changed, cloudstack.NoFilter) if err != nil { - fatal(err, "listing stacks") + fatal("listing stacks", err) } for _, stackEntry := range c.filterStacks(report.Stacks) { envVars, err := run.LoadEnv(c.cfg(), stackEntry.Stack) if err != nil { - fatal(err, "loading stack run environment") + fatal("loading stack run environment", err) } c.output.MsgStdOut("\nstack %q:", stackEntry.Stack.Dir) @@ -1543,13 +1543,12 @@ func (c *cli) generateGraph() { getLabel = func(s *config.Stack) string { return s.Dir.String() } default: - logger.Fatal(). - Msg("-label expects the values \"stack.name\" or \"stack.dir\"") + fatal(`-label expects the values "stack.name" or "stack.dir"`, nil) } entries, err := stack.List(c.cfg().Tree()) if err != nil { - fatal(err, "listing stacks to build graph") + fatal("listing stacks to build graph", err) } logger.Debug().Msg("Create new graph.") @@ -1573,16 +1572,14 @@ func (c *cli) generateGraph() { func(s config.Stack) []string { return s.After }, visited, ); err != nil { - fatal(err, "building order tree") + fatal("building order tree", err) } } for _, id := range graph.IDs() { val, err := graph.Node(id) if err != nil { - log.Fatal(). - Err(err). - Msg("generating graph") + fatal("generating graph", err) } generateDot(dotGraph, graph, id, val.(*config.Stack), getLabel) @@ -1599,15 +1596,12 @@ func (c *cli) generateGraph() { f, err := os.Create(outFile) if err != nil { - logger := log.With(). - Str("path", outFile). - Logger() - errlog.Fatal(logger, err, "opening file") + fatal(sprintf("opening file %s", outFile), err) } defer func() { if err := f.Close(); err != nil { - fatal(err, "closing output graph file") + fatal("closing output graph file", err) } }() @@ -1618,11 +1612,7 @@ func (c *cli) generateGraph() { Msg("Write graph to output.") _, err = out.Write([]byte(dotGraph.String())) if err != nil { - logger := log.With(). - Str("path", outFile). - Logger() - - errlog.Fatal(logger, err, "writing output") + fatal(sprintf("writing output %s", outFile), err) } } @@ -1637,7 +1627,7 @@ func generateDot( for _, childid := range graph.AncestorsOf(id) { val, err := graph.Node(childid) if err != nil { - fatal(err, "generating dot file") + fatal("generating dot file", err) } s := val.(*config.Stack) n := dotGraph.Node(getLabel(s)) @@ -1667,16 +1657,16 @@ func (c *cli) printRunOrder(friendlyFmt bool) { stacks, err := c.computeSelectedStacks(false) if err != nil { - fatal(err, "computing selected stacks") + fatal("computing selected stacks", err) } logger.Debug().Msg("Get run order.") orderedStacks, reason, err := run.Sort(c.cfg(), stacks) if err != nil { if errors.IsKind(err, dag.ErrCycleDetected) { - c.fatal("Invalid stack configuration", errors.E(err, reason)) + fatal("Invalid stack configuration", errors.E(err, reason)) } else { - c.fatal("Failed to plan execution", err) + fatal("Failed to plan execution", err) } } @@ -1703,7 +1693,7 @@ func (c *cli) generateDebug() { // not be filtered by stack selection. stacks, err := c.computeSelectedStacks(false) if err != nil { - fatal(err, "generate debug: selecting stacks") + fatal("generate debug: selecting stacks", err) } selectedStacks := map[prj.Path]struct{}{} @@ -1715,7 +1705,7 @@ func (c *cli) generateDebug() { results, err := generate.Load(c.cfg(), c.vendorDir()) if err != nil { - fatal(err, "generate debug: loading generated code") + fatal("generate debug: loading generated code", err) } for _, res := range results { @@ -1747,18 +1737,14 @@ func (c *cli) generateDebug() { func (c *cli) printStacksGlobals() { report, err := c.listStacks(c.parsedArgs.Changed, cloudstack.NoFilter) if err != nil { - fatal(err, "listing stacks globals: listing stacks") + fatal("listing stacks globals: listing stacks", err) } for _, stackEntry := range c.filterStacks(report.Stacks) { stack := stackEntry.Stack report := globals.ForStack(c.cfg(), stack) if err := report.AsError(); err != nil { - logger := log.With(). - Stringer("stack", stack.Dir). - Logger() - - errlog.Fatal(logger, err, "listing stacks globals: loading stack") + fatal(sprintf("listing stacks globals: loading stack at %s", stack.Dir), err) } globalsStrRepr := report.Globals.String() @@ -1780,7 +1766,7 @@ func (c *cli) printMetadata() { report, err := c.listStacks(c.parsedArgs.Changed, cloudstack.NoFilter) if err != nil { - fatal(err, "loading metadata: listing stacks") + fatal("loading metadata: listing stacks", err) } stackEntries := c.filterStacks(report.Stacks) @@ -1839,7 +1825,7 @@ func (c *cli) checkGenCode() bool { func (c *cli) ensureStackID() { report, err := c.listStacks(false, cloudstack.NoFilter) if err != nil { - fatal(err, "listing stacks") + fatal("listing stacks", err) } for _, entry := range report.Stacks { @@ -1849,7 +1835,7 @@ func (c *cli) ensureStackID() { id, err := stack.UpdateStackID(entry.Stack.HostDir(c.cfg())) if err != nil { - fatal(err, "failed to update stack.id of stack %s", entry.Stack.Dir) + fatal(sprintf("failed to update stack.id of stack %s", entry.Stack.Dir), err) } c.output.MsgStdOut("Generated ID %s for stack %s", id, entry.Stack.Dir) @@ -1861,11 +1847,11 @@ func (c *cli) eval() { for _, exprStr := range c.parsedArgs.Experimental.Eval.Exprs { expr, err := ast.ParseExpression(exprStr, "") if err != nil { - fatal(err) + fatal("unable to parse expression", err) } val, err := ctx.Eval(expr) if err != nil { - fatal(err, "eval %q", exprStr) + fatal(sprintf("eval %q", exprStr), err) } c.outputEvalResult(val, c.parsedArgs.Experimental.Eval.AsJSON) } @@ -1876,11 +1862,11 @@ func (c *cli) partialEval() { for _, exprStr := range c.parsedArgs.Experimental.PartialEval.Exprs { expr, err := ast.ParseExpression(exprStr, "") if err != nil { - fatal(err) + fatal("unable to parse expression", err) } newexpr, err := ctx.PartialEval(expr) if err != nil { - fatal(err, "partial eval %q", exprStr) + fatal(sprintf("partial eval %q", exprStr), err) } c.output.MsgStdOut("%s", string(hclwrite.Format(ast.TokensForExpression(newexpr).Bytes()))) } @@ -1909,30 +1895,26 @@ func (c *cli) evalRunArgs(st *config.Stack, cmd []string) ([]string, error) { } func (c *cli) getConfigValue() { - logger := log.With(). - Str("action", "cli.getConfigValue()"). - Logger() - ctx := c.detectEvalContext(c.parsedArgs.Experimental.GetConfigValue.Global) for _, exprStr := range c.parsedArgs.Experimental.GetConfigValue.Vars { expr, err := ast.ParseExpression(exprStr, "") if err != nil { - fatal(err) + fatal("unable to parse expression", err) } iteratorTraversal, diags := hhcl.AbsTraversalForExpr(expr) if diags.HasErrors() { - fatal(errors.E(diags), "expected a variable accessor") + fatal("expected a variable accessor", errors.E(diags)) } varns := iteratorTraversal.RootName() if varns != "terramate" && varns != "global" { - logger.Fatal().Msg("only terramate and global variables are supported") + fatal("only terramate and global variables are supported", nil) } val, err := ctx.Eval(expr) if err != nil { - fatal(err, "evaluating expression: %s", exprStr) + fatal(sprintf("evaluating expression: %s", exprStr), err) } c.outputEvalResult(val, c.parsedArgs.Experimental.GetConfigValue.AsJSON) @@ -1945,7 +1927,7 @@ func (c *cli) outputEvalResult(val cty.Value, asJSON bool) { var err error data, err = json.Marshal(val, val.Type()) if err != nil { - fatal(err, "converting value %s to json", val.GoString()) + fatal(sprintf("converting value %s to json", val.GoString()), err) } } else { if val.Type() == cty.String { @@ -1965,7 +1947,7 @@ func (c *cli) detectEvalContext(overrideGlobals map[string]string) *eval.Context var err error st, err = config.LoadStack(c.cfg(), prj.PrjAbsPath(c.rootdir(), c.wd())) if err != nil { - fatal(err, "setup eval context: loading stack config") + fatal("setup eval context: loading stack config", err) } } return c.setupEvalContext(st, overrideGlobals) @@ -1988,17 +1970,17 @@ func (c *cli) setupEvalContext(st *config.Stack, overrideGlobals map[string]stri wdPath := prj.PrjAbsPath(c.rootdir(), tdir) tree, ok := c.cfg().Lookup(wdPath) if !ok { - fatal(errors.E("configuration at %s not found", wdPath)) + fatal("Missing configuration", errors.E("configuration at %s not found", wdPath)) } exprs, err := globals.LoadExprs(tree) if err != nil { - fatal(err, "loading globals expressions") + fatal("loading globals expressions", err) } for name, exprStr := range overrideGlobals { expr, err := ast.ParseExpression(exprStr, "") if err != nil { - fatal(errors.E(err, "--global %s=%s is an invalid expresssion", name, exprStr)) + fatal("unable to parse expression", errors.E(err, "--global %s=%s is an invalid expresssion", name, exprStr)) } parts := strings.Split(name, ".") length := len(parts) @@ -2033,7 +2015,7 @@ func (c *cli) checkOutdatedGeneratedCode() { outdatedFiles, err := generate.DetectOutdated(c.cfg(), c.vendorDir()) if err != nil { - fatal(err, "failed to check outdated code on project") + fatal("failed to check outdated code on project", err) } for _, outdated := range outdatedFiles { @@ -2043,9 +2025,8 @@ func (c *cli) checkOutdatedGeneratedCode() { } if len(outdatedFiles) > 0 { - logger.Fatal(). - Err(errors.E(ErrOutdatedGenCodeDetected)). - Msg("please run: 'terramate generate' to update generated code") + fatal(errors.E(ErrOutdatedGenCodeDetected).Error(), + errors.E("please run: 'terramate generate' to update generated code")) } } @@ -2167,7 +2148,7 @@ func (c cli) checkVersion() { rootcfg.Terramate.RequiredVersion, rootcfg.Terramate.RequiredVersionAllowPreReleases, ); err != nil { - fatal(err) + fatal("version check failed", err) } } @@ -2207,7 +2188,7 @@ func runCheckpoint(version string, clicfg cliconfig.Config, result chan *checkpo func (c *cli) setupFilterTags() { clauses, found, err := filter.ParseTagClauses(c.parsedArgs.Tags...) if err != nil { - fatal(err) + fatal("unable to parse tag clauses", err) } if found { c.tags = clauses @@ -2216,7 +2197,7 @@ func (c *cli) setupFilterTags() { for _, val := range c.parsedArgs.NoTags { err := tag.Validate(val) if err != nil { - fatal(err) + fatal("unable validate tag", err) } } var noClauses filter.TagClause @@ -2339,7 +2320,7 @@ func configureLogging(logLevel, logFmt, logdest string, stdout, stderr io.Writer case "stderr": output = stderr default: - log.Fatal().Msgf("unknown log destination %q", logdest) + fatal(sprintf("unknown log destination %q", logdest), nil) } zloglevel, err := zerolog.ParseLevel(logLevel) @@ -2360,12 +2341,11 @@ func configureLogging(logLevel, logFmt, logdest string, stdout, stderr io.Writer } } -// Deprecated: use c.fatal -func fatal(err error, args ...any) { - errlog.Fatal(log.Logger, err, args...) +func fatal(title string, err error) { + printer.Stderr.Fatal(title, err) } -func (c *cli) fatal(title string, err error) { - printer.Stderr.ErrorWithDetails(title, err) - os.Exit(1) +// sprintf is an alias for fmt.Sprintf +func sprintf(format string, a ...interface{}) string { + return stdfmt.Sprintf(format, a...) } diff --git a/cmd/terramate/cli/cloud.go b/cmd/terramate/cli/cloud.go index 63db7be273..dd8f6a35e5 100644 --- a/cmd/terramate/cli/cloud.go +++ b/cmd/terramate/cli/cloud.go @@ -88,7 +88,7 @@ func (c *cli) disableCloudFeatures(err error) { func (c *cli) handleCriticalError(err error) { if err != nil { if c.uimode == HumanMode { - fatal(err) + fatal("aborting", err) } c.disableCloudFeatures(err) @@ -251,7 +251,7 @@ func (c *cli) cloudSyncCancelStacks(runs []runContext) { func (c *cli) cloudInfo() { err := c.loadCredential() if err != nil { - c.fatal("failed to load credentials", err) + fatal("failed to load credentials", err) } c.cred().info(c.cloudOrgName()) @@ -262,27 +262,27 @@ func (c *cli) cloudInfo() { func (c *cli) cloudDriftShow() { err := c.setupCloudConfig() if err != nil { - fatal(err) + fatal("unable to setup cloud configuration", err) } st, found, err := config.TryLoadStack(c.cfg(), prj.PrjAbsPath(c.rootdir(), c.wd())) if err != nil { - fatal(err, "loading stack in current directory") + fatal("loading stack in current directory", err) } if !found { - fatal(errors.E("No stack selected. Please enter a stack to show a potential drift.")) + fatal("No stack selected. Please enter a stack to show a potential drift.", nil) } if st.ID == "" { - fatal(errors.E("The stack must have an ID for using TMC features")) + fatal("The stack must have an ID for using TMC features", nil) } ctx, cancel := context.WithTimeout(context.Background(), defaultCloudTimeout) defer cancel() stackResp, found, err := c.cloud.client.GetStack(ctx, c.cloud.run.orgUUID, c.prj.prettyRepo(), st.ID) if err != nil { - fatal(err) + fatal("unable to fetch stack", err) } if !found { - fatal(errors.E("Stack %s was not yet synced with the Terramate Cloud.", st.Dir.String())) + fatal(sprintf("Stack %s was not yet synced with the Terramate Cloud.", st.Dir.String()), nil) } if stackResp.Status != stack.Drifted && stackResp.DriftStatus != drift.Drifted { @@ -296,10 +296,10 @@ func (c *cli) cloudDriftShow() { // stack is drifted driftsResp, err := c.cloud.client.StackLastDrift(ctx, c.cloud.run.orgUUID, stackResp.ID) if err != nil { - fatal(err) + fatal("unable to fetch drift", err) } if len(driftsResp.Drifts) == 0 { - fatal(errors.E("Stack %s is drifted, but no details are available.", st.Dir.String())) + fatal(sprintf("Stack %s is drifted, but no details are available.", st.Dir.String()), nil) } driftData := driftsResp.Drifts[0] @@ -307,10 +307,10 @@ func (c *cli) cloudDriftShow() { defer cancel() driftData, err = c.cloud.client.DriftDetails(ctx, c.cloud.run.orgUUID, stackResp.ID, driftData.ID) if err != nil { - fatal(err) + fatal("unable to fetch drift details", err) } if driftData.Status != drift.Drifted || driftData.Details == nil || driftData.Details.Provisioner == "" { - fatal(errors.E("Stack %s is drifted, but no details are available.", st.Dir.String())) + fatal(sprintf("Stack %s is drifted, but no details are available.", st.Dir.String()), nil) } c.output.MsgStdOutV("drift provisioner: %s", driftData.Details.Provisioner) c.output.MsgStdOut(driftData.Details.ChangesetASCII) diff --git a/cmd/terramate/cli/cloud_credential_google.go b/cmd/terramate/cli/cloud_credential_google.go index 2a79dc3348..a0b2ed1000 100644 --- a/cmd/terramate/cli/cloud_credential_google.go +++ b/cmd/terramate/cli/cloud_credential_google.go @@ -405,7 +405,7 @@ func loadCredential(output out.O, clicfg cliconfig.Config) (cachedCredential, bo func endpointURL(endpoint string, idpKey string) *url.URL { u, err := url.Parse(endpoint) if err != nil { - fatal(err, "failed to parse endpoint URL for createAuthURI") + fatal("failed to parse endpoint URL for createAuthURI", err) } q := u.Query() diff --git a/cmd/terramate/cli/project.go b/cmd/terramate/cli/project.go index 159e58f1dc..40067d0404 100644 --- a/cmd/terramate/cli/project.go +++ b/cmd/terramate/cli/project.go @@ -68,15 +68,11 @@ func (p *project) localDefaultBranchCommit() string { if p.git.localDefaultBranchCommit != "" { return p.git.localDefaultBranchCommit } - logger := log.With(). - Str("action", "localDefaultBranchCommit()"). - Logger() - gitcfg := p.gitcfg() refName := gitcfg.DefaultRemote + "/" + gitcfg.DefaultBranch val, err := p.git.wrapper.RevParse(refName) if err != nil { - logger.Fatal().Err(err).Send() + fatal("unable to git rev-parse", err) } p.git.localDefaultBranchCommit = val @@ -102,13 +98,9 @@ func (p *project) headCommit() string { return p.git.headCommit } - logger := log.With(). - Str("action", "headCommit()"). - Logger() - val, err := p.git.wrapper.RevParse("HEAD") if err != nil { - logger.Fatal().Err(err).Send() + fatal("unable to git rev-parse", err) } p.git.headCommit = val @@ -120,18 +112,14 @@ func (p *project) remoteDefaultCommit() string { return p.git.remoteDefaultBranchCommit } - logger := log.With(). - Str("action", "remoteDefaultCommit()"). - Logger() - gitcfg := p.gitcfg() remoteRef, err := p.git.wrapper.FetchRemoteRev(gitcfg.DefaultRemote, gitcfg.DefaultBranch) if err != nil { - logger.Fatal().Err( + fatal("unable to fetch remote commit", fmt.Errorf("fetching remote commit of %s/%s: %v", gitcfg.DefaultRemote, gitcfg.DefaultBranch, err, - )).Send() + )) } p.git.remoteDefaultBranchCommit = remoteRef.CommitID diff --git a/cmd/terramate/cli/run.go b/cmd/terramate/cli/run.go index abe5e0adb5..a10c9b7642 100644 --- a/cmd/terramate/cli/run.go +++ b/cmd/terramate/cli/run.go @@ -59,15 +59,10 @@ type runResult struct { } func (c *cli) runOnStacks() { - logger := log.With(). - Str("action", "cli.runOnStacks()"). - Str("workingDir", c.wd()). - Logger() - c.gitSafeguardDefaultBranchIsReachable() if len(c.parsedArgs.Run.Command) == 0 { - logger.Fatal().Msgf("run expects a cmd") + fatal("run expects a cmd", nil) } c.checkOutdatedGeneratedCode() @@ -77,12 +72,11 @@ func (c *cli) runOnStacks() { if c.parsedArgs.Run.NoRecursive { st, found, err := config.TryLoadStack(c.cfg(), prj.PrjAbsPath(c.rootdir(), c.wd())) if err != nil { - fatal(err, "loading stack in current directory") + fatal("loading stack in current directory", err) } if !found { - logger.Fatal(). - Msg("--no-recursive provided but no stack found in the current directory") + fatal("--no-recursive provided but no stack found in the current directory", nil) } stacks = append(stacks, st.Sortable()) @@ -90,16 +84,16 @@ func (c *cli) runOnStacks() { var err error stacks, err = c.computeSelectedStacks(true) if err != nil { - fatal(err, "computing selected stacks") + fatal("computing selected stacks", err) } } orderedStacks, reason, err := runutil.Sort(c.cfg(), stacks) if err != nil { if errors.IsKind(err, dag.ErrCycleDetected) { - fatal(err, "cycle detected: %s", reason) + fatal(sprintf("cycle detected: %s", reason), err) } else { - fatal(err, "failed to plan execution") + fatal("failed to plan execution", err) } } @@ -108,18 +102,18 @@ func (c *cli) runOnStacks() { } if c.parsedArgs.Run.CloudSyncDeployment && c.parsedArgs.Run.CloudSyncDriftStatus { - fatal(errors.E("--cloud-sync-deployment conflicts with --cloud-sync-drift-status")) + fatal(sprintf("--cloud-sync-deployment conflicts with --cloud-sync-drift-status"), nil) } cloudSyncEnabled := c.parsedArgs.Run.CloudSyncDeployment || c.parsedArgs.Run.CloudSyncDriftStatus if c.parsedArgs.Run.CloudSyncTerraformPlanFile != "" && !cloudSyncEnabled { - fatal(errors.E("--cloud-sync-terraform-plan-file requires flags --cloud-sync-deployment or --cloud-sync-drift-status")) + fatal(sprintf("--cloud-sync-terraform-plan-file requires flags --cloud-sync-deployment or --cloud-sync-drift-status"), nil) } if c.parsedArgs.Run.CloudSyncDeployment || c.parsedArgs.Run.CloudSyncDriftStatus { if !c.prj.isRepo { - fatal(errors.E("cloud features requires a git repository")) + fatal("cloud features requires a git repository", nil) } c.ensureAllStackHaveIDs(orderedStacks) c.detectCloudMetadata() @@ -141,7 +135,7 @@ func (c *cli) runOnStacks() { if c.parsedArgs.Run.Eval { run.Cmd, err = c.evalRunArgs(run.Stack, run.Cmd) if err != nil { - c.fatal("unable to evaluate command", err) + fatal("unable to evaluate command", err) } } runs = append(runs, run) @@ -164,7 +158,7 @@ func (c *cli) runOnStacks() { ContinueOnError: c.parsedArgs.Run.ContinueOnError, }) if err != nil { - c.fatal("one or more commands failed", err) + fatal("one or more commands failed", err) } } diff --git a/cmd/terramate/cli/script_info.go b/cmd/terramate/cli/script_info.go index 724aa598d6..20e4eb6e43 100644 --- a/cmd/terramate/cli/script_info.go +++ b/cmd/terramate/cli/script_info.go @@ -22,7 +22,7 @@ func (c *cli) printScriptInfo() { stacks, err := c.computeSelectedStacks(false) if err != nil { - fatal(err, "computing selected stacks") + fatal("computing selected stacks", err) } m := newScriptsMatcher(labels) diff --git a/cmd/terramate/cli/script_run.go b/cmd/terramate/cli/script_run.go index 9a9a34f3dc..945fe16eee 100644 --- a/cmd/terramate/cli/script_run.go +++ b/cmd/terramate/cli/script_run.go @@ -10,7 +10,6 @@ import ( "strings" "github.com/fatih/color" - "github.com/rs/zerolog/log" "github.com/terramate-io/terramate/cloud" "github.com/terramate-io/terramate/config" "github.com/terramate-io/terramate/errors" @@ -24,11 +23,6 @@ import ( ) func (c *cli) runScript() { - logger := log.With(). - Str("action", "cli.runScript()"). - Str("workingDir", c.wd()). - Logger() - c.gitSafeguardDefaultBranchIsReachable() c.checkOutdatedGeneratedCode() @@ -36,11 +30,11 @@ func (c *cli) runScript() { if c.parsedArgs.Script.Run.NoRecursive { st, found, err := config.TryLoadStack(c.cfg(), prj.PrjAbsPath(c.rootdir(), c.wd())) if err != nil { - logger.Fatal().Err(err).Msg("failed to load stack in current directory") + fatal("failed to load stack in current directory", err) } if !found { - logger.Fatal().Msg("--no-recursive provided but no stack found in the current directory") + fatal("--no-recursive provided but no stack found in the current directory", nil) } stacks = append(stacks, st.Sortable()) @@ -48,7 +42,7 @@ func (c *cli) runScript() { var err error stacks, err = c.computeSelectedStacks(true) if err != nil { - logger.Fatal().Err(err).Msg("failed to compute selected stacks") + fatal("failed to compute selected stacks", err) } } @@ -82,12 +76,12 @@ func (c *cli) runScript() { for _, st := range result.Stacks { ectx, err := scriptEvalContext(c.cfg(), st.Stack) if err != nil { - logger.Fatal().Err(err).Msg("failed to get context") + fatal("failed to get context", err) } evalScript, err := config.EvalScript(ectx, *result.ScriptCfg) if err != nil { - logger.Fatal().Err(err).Msg("failed to eval script") + fatal("failed to eval script", err) } for jobIdx, job := range evalScript.Jobs { @@ -114,9 +108,9 @@ func (c *cli) runScript() { orderedStacks, reason, err := runutil.Sort(c.cfg(), stacks) if err != nil { if errors.IsKind(err, dag.ErrCycleDetected) { - fatal(err, "cycle detected: %s", reason) + fatal(sprintf("cycle detected: %s", reason), err) } else { - fatal(err, "failed to plan execution") + fatal("failed to plan execution", err) } } @@ -142,7 +136,7 @@ func (c *cli) runScript() { ContinueOnError: false, }) if err != nil { - c.fatal("one or more commands failed", err) + fatal("one or more commands failed", err) } } @@ -163,7 +157,7 @@ func (c *cli) prepareScriptCloudDeploymentSync(runStacks []runContext) { } if !c.prj.isRepo { - fatal(errors.E("cloud features require a git repository")) + fatal("cloud features require a git repository", nil) } err := c.setupCloudConfig() diff --git a/cmd/terramate/e2etests/cloud/run_cloud_config_test.go b/cmd/terramate/e2etests/cloud/run_cloud_config_test.go index 4672403dc4..9ed85d46be 100644 --- a/cmd/terramate/e2etests/cloud/run_cloud_config_test.go +++ b/cmd/terramate/e2etests/cloud/run_cloud_config_test.go @@ -29,7 +29,7 @@ func TestCloudConfig(t *testing.T) { customEnv map[string]string } - const fatalErr = `FTL ` + string(clitest.ErrCloud) + const fatalErr = string(clitest.ErrCloud) for _, tc := range []testcase{ { diff --git a/errors/errlog/errlog.go b/errors/errlog/errlog.go index d9c1909ba5..0df6214a5a 100644 --- a/errors/errlog/errlog.go +++ b/errors/errlog/errlog.go @@ -7,22 +7,12 @@ package errlog import ( "fmt" - "os" "strings" "github.com/rs/zerolog" "github.com/terramate-io/terramate/errors" ) -// Fatal logs the error as a Fatal if the error is not nil. -func Fatal(logger zerolog.Logger, err error, args ...any) { - logerrs(logger, zerolog.FatalLevel, zerolog.ErrorLevel, err, args) - // zerolog does not call os.Exit if you pass the fatal level - // with WithLevel, it only aborts when calling the Fatal() method. - // So we need to ensure the exit 1 here. - os.Exit(1) -} - // Warn logs the error as a warning if the error is not nil. func Warn(logger zerolog.Logger, err error, args ...any) { logerrs(logger, zerolog.WarnLevel, zerolog.WarnLevel, err, args) diff --git a/printer/printer.go b/printer/printer.go index e3920fd734..809a95524a 100644 --- a/printer/printer.go +++ b/printer/printer.go @@ -63,6 +63,13 @@ func (p *Printer) ErrorWithDetails(title string, err error) { } } +// Fatal prints an error with a title and the underlying error and calls +// os.Exit(1). +func (p *Printer) Fatal(title string, err error) { + p.ErrorWithDetails(title, err) + os.Exit(1) +} + // WarnWithDetails is similar to ErrorWithDetailsln but prints a warning // instead func (p *Printer) WarnWithDetails(title string, err error) { @@ -73,7 +80,7 @@ func (p *Printer) WarnWithDetails(title string, err error) { } } -// Error prints a message with a "Error:" prefix. The prefix is prinited in +// Error prints a message with a "Error:" prefix. The prefix is printed in // the boldRed style. func (p *Printer) Error(title string) { fmt.Fprintln(p.w, boldRed("Error:"), bold(title)) From 7b272d7f161fcada0dba06ac40adf41f951bea29 Mon Sep 17 00:00:00 2001 From: Tiago Natel Date: Thu, 8 Feb 2024 23:20:09 +0000 Subject: [PATCH 2/5] feat: add terramate.config.generate.hcl_magic_header_comment_style. Signed-off-by: Tiago Natel --- CHANGELOG.md | 4 ++ docs/cli/configuration/index.md | 8 +++ docs/cli/projects/configuration.md | 23 +++++- generate/generate.go | 22 +++--- generate/generate_hcl_test.go | 6 +- generate/generate_list_test.go | 30 +++++++- generate/generate_test.go | 2 +- generate/genhcl/genhcl.go | 111 +++++++++++++++++++++++------ hcl/hcl.go | 62 +++++++++++++++- hcl/hcl_test.go | 103 ++++++++++++++++++++++++++ 10 files changed, 329 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4f9995668..bf0871258d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ Given a version number `MAJOR.MINOR.PATCH`, we increment the: ## Unreleased +### Added + +- Add `terramate.config.generate.hcl_magic_header_comment_style` option for setting the generated comment style. + ## 0.4.5 ### Added diff --git a/docs/cli/configuration/index.md b/docs/cli/configuration/index.md index 178e157abc..11264e16df 100644 --- a/docs/cli/configuration/index.md +++ b/docs/cli/configuration/index.md @@ -176,6 +176,14 @@ The `terramate.config.git` block has no labels and has the following schema: | check\_uncommitted | boolean | (DEPRECATED) Enable check of uncommitted files | true | check\_remote | boolean | (DEPRECATED) Enable checking if local main is updated with remote | true +## terramate.config.generate block schema + +The `terramate.config.generate` block has no labels and has the following schema: + +| name | type | description | default | +|------------------|----------------|-------------|---------| +| hcl\_magic\_header\_comment\_style | string | The comment style used in `generate_hcl`` blocks | "//" + ## terramate.config.run block schema The `terramate.config.run` block has no labels and has the following schema: diff --git a/docs/cli/projects/configuration.md b/docs/cli/projects/configuration.md index 88f0fccc2a..5e596212fc 100644 --- a/docs/cli/projects/configuration.md +++ b/docs/cli/projects/configuration.md @@ -93,10 +93,29 @@ terramate { } ``` +### The `terramate.config.generate` block + +The `terramate.config.generate` block can be used to configure the code generate feature. +For now, only the `hcl_magic_header_comment_style` attribute is supported and it can be used to +define which HCL comment style must be used by Terramate when generating HCL files. +Example below: + +``` +terramate { + config { + generate { + hcl_magic_header_comment_style = "#" + } + } +} +``` + +The config above will make Terramate generate files using `#` as comment style. +The only valid options are `//` and `#`. + ### The `terramate.config.run` Block -Configuration for the `terramate run` command can be set in the -`terramate.config.run` block. +Configuration for the `terramate run` command can be set in the `terramate.config.run` block. #### Disable code generation check diff --git a/generate/generate.go b/generate/generate.go index c01b86447a..7d75aa53e3 100644 --- a/generate/generate.go +++ b/generate/generate.go @@ -249,7 +249,7 @@ func doStackGeneration( oldFileBody, oldExists := allFiles[filename] if !oldExists || oldFileBody != body { - err := writeGeneratedCode(path, file) + err := writeGeneratedCode(root, path, file) if err != nil { report.err = errors.E(err, "saving file %q", filename) return report @@ -476,7 +476,7 @@ processSubdirs: return nil, errors.E(err, "checking if file is generated %q", file) } - if hasGenHCLHeader(string(data)) { + if hasGenHCLHeader(genhcl.CommentStyleFromConfig(root.Tree()), string(data)) { genfiles = append(genfiles, filepath.ToSlash( filepath.Join(relSubdir, entry.Name()))) } @@ -666,14 +666,14 @@ func updateOutdatedFiles( return nil } -func writeGeneratedCode(target string, genfile GenFile) error { +func writeGeneratedCode(root *config.Root, target string, genfile GenFile) error { body := genfile.Header() + genfile.Body() if genfile.Header() != "" { // WHY: some file generation strategies don't provide // headers, like generate_file, so we can't detect // if we are overwriting a Terramate generated file. - if err := checkFileCanBeOverwritten(target); err != nil { + if err := checkFileCanBeOverwritten(root, target); err != nil { return err } } @@ -685,8 +685,8 @@ func writeGeneratedCode(target string, genfile GenFile) error { return os.WriteFile(target, []byte(body), 0666) } -func checkFileCanBeOverwritten(path string) error { - _, _, err := readGeneratedFile(path) +func checkFileCanBeOverwritten(root *config.Root, path string) error { + _, _, err := readGeneratedFile(root, path) return err } @@ -697,7 +697,7 @@ func checkFileCanBeOverwritten(path string) error { // The returned boolean indicates if the file exists, so the contents of // the file + true is returned if a file is found, but if no file is found // it will return an empty string and false indicating that the file doesn't exist. -func readGeneratedFile(path string) (string, bool, error) { +func readGeneratedFile(root *config.Root, path string) (string, bool, error) { data, found, err := readFile(path) if err != nil { return "", false, err @@ -707,7 +707,7 @@ func readGeneratedFile(path string) (string, bool, error) { return "", false, nil } - if hasGenHCLHeader(data) { + if hasGenHCLHeader(genhcl.CommentStyleFromConfig(root.Tree()), data) { return data, true, nil } @@ -911,7 +911,7 @@ func generateRootFiles(root *config.Root, genfiles []GenFile, report *Report) { Bool("fileChanged", body != diskContent). Msg("writing file") - err := writeGeneratedCode(abspath, genfile) + err := writeGeneratedCode(root, abspath, genfile) if err != nil { dirReport.err = errors.E(err, "saving file %s", label) report.addDirReport(dir, dirReport) @@ -948,10 +948,10 @@ func genBlockLogger(logger zerolog.Logger, blockname, label, context string) zer Logger() } -func hasGenHCLHeader(code string) bool { +func hasGenHCLHeader(commentStyle genhcl.CommentStyle, code string) bool { // When changing headers we need to support old ones (or break). // For now keeping them here, to avoid breaks. - for _, header := range []string{genhcl.Header, genhcl.HeaderV0} { + for _, header := range []string{genhcl.Header(commentStyle), genhcl.HeaderV0} { if strings.HasPrefix(code, header) { return true } diff --git a/generate/generate_hcl_test.go b/generate/generate_hcl_test.go index c8ed79f975..0199097e55 100644 --- a/generate/generate_hcl_test.go +++ b/generate/generate_hcl_test.go @@ -2230,7 +2230,7 @@ func TestGenerateHCLCleanupOldFilesIgnoreSymlinks(t *testing.T) { // Creates a file with a generated header inside the symlinked directory. // It should never return in the report. - test.WriteFile(t, targEntry.Path(), "test.tf", genhcl.Header) + test.WriteFile(t, targEntry.Path(), "test.tf", genhcl.DefaultHeader()) root, err := config.LoadRoot(rootEntry.Path()) assert.NoError(t, err) @@ -2251,8 +2251,8 @@ func TestGenerateHCLCleanupOldFilesIgnoreDotDirs(t *testing.T) { s := sandbox.NoGit(t, true) // Creates a file with a generated header inside dot dirs. - test.WriteFile(t, filepath.Join(s.RootDir(), ".terramate"), "test.tf", genhcl.Header) - test.WriteFile(t, filepath.Join(s.RootDir(), ".another"), "test.tf", genhcl.Header) + test.WriteFile(t, filepath.Join(s.RootDir(), ".terramate"), "test.tf", genhcl.DefaultHeader()) + test.WriteFile(t, filepath.Join(s.RootDir(), ".another"), "test.tf", genhcl.DefaultHeader()) assertEqualReports(t, s.Generate(), generate.Report{}) } diff --git a/generate/generate_list_test.go b/generate/generate_list_test.go index 00350e7142..f9b5ff4542 100644 --- a/generate/generate_list_test.go +++ b/generate/generate_list_test.go @@ -13,6 +13,7 @@ import ( "github.com/terramate-io/terramate/config" "github.com/terramate-io/terramate/generate" "github.com/terramate-io/terramate/generate/genhcl" + . "github.com/terramate-io/terramate/test/hclwrite/hclutils" "github.com/terramate-io/terramate/test/sandbox" ) @@ -50,6 +51,33 @@ func TestGeneratedFilesListing(t *testing.T) { "f:another.tm.hcl:terramate {}", }, }, + { + name: "single file generated but configured to a different comment style", + layout: []string{ + "f:somefile.tf:" + genhcl.Header(genhcl.SlashComment) + "test", + "f:terramate.tm:" + Doc(Terramate( + Config( + Block("generate", Doc( + Str("hcl_magic_header_comment_style", "#"), + )), + ), + )).String(), + }, + }, + { + name: "single file generated and properly configured with same comment style", + layout: []string{ + "f:somefile.tf:" + genhcl.Header(genhcl.HashComment) + "test", + "f:terramate.tm:" + Doc(Terramate( + Config( + Block("generate", Doc( + Str("hcl_magic_header_comment_style", "#"), + )), + ), + )).String(), + }, + want: []string{"somefile.tf"}, + }, { name: "single generated file on root", layout: []string{ @@ -201,5 +229,5 @@ func TestGeneratedFilesListing(t *testing.T) { } func genfile(path string, body ...string) string { - return fmt.Sprintf("f:%s:%s\n%s", path, genhcl.Header, strings.Join(body, "")) + return fmt.Sprintf("f:%s:%s\n%s", path, genhcl.DefaultHeader(), strings.Join(body, "")) } diff --git a/generate/generate_test.go b/generate/generate_test.go index 4718e7aa66..aeed81cfd4 100644 --- a/generate/generate_test.go +++ b/generate/generate_test.go @@ -571,7 +571,7 @@ func TestGenerateCleanupFailsToReadFiles(t *testing.T) { s := sandbox.NoGit(t, true) dir := s.RootEntry().CreateDir("dir") - file := dir.CreateFile("file.hcl", genhcl.Header) + file := dir.CreateFile("file.hcl", genhcl.DefaultHeader()) file.Chmod(0) defer file.Chmod(0755) diff --git a/generate/genhcl/genhcl.go b/generate/genhcl/genhcl.go index 51173acf71..046839dc72 100644 --- a/generate/genhcl/genhcl.go +++ b/generate/genhcl/genhcl.go @@ -34,16 +34,29 @@ import ( // Is contains parsed and evaluated code on it and information // about the origin of the generated code. type HCL struct { - label string - origin info.Range - body string - condition bool - asserts []config.Assert + magicCommentStyle CommentStyle + label string + origin info.Range + body string + condition bool + asserts []config.Assert } +// CommentStyle is the configured comment style that must be generated. +type CommentStyle int + +// Comment styles supported. +const ( + SlashComment CommentStyle = iota + HashComment + invalid + + DefaultComment = SlashComment +) + const ( - // Header is the current header string used by generate_hcl code generation. - Header = "// TERRAMATE: GENERATED AUTOMATICALLY DO NOT EDIT" + // HeaderMagic is the current header magic string used by generate_hcl code generation. + HeaderMagic = "TERRAMATE: GENERATED AUTOMATICALLY DO NOT EDIT" // HeaderV0 is the deprecated header string used by generate_hcl code generation. HeaderV0 = "// GENERATED BY TERRAMATE: DO NOT EDIT" @@ -94,7 +107,7 @@ func (h HCL) Asserts() []config.Assert { // Header returns the header of the generated HCL file. func (h HCL) Header() string { - return Header + "\n\n" + return Header(h.magicCommentStyle) } // Body returns a string representation of the HCL code @@ -123,6 +136,41 @@ func (h HCL) String() string { h.Label(), h.Condition(), h.Body(), h.Range().HostPath()) } +// Header returns the HCL header based on the comment style. +func Header(comment CommentStyle) string { + return stdfmt.Sprintf("%s "+HeaderMagic+"\n\n", comment) +} + +// DefaultHeader returns the header for the default comment style. +func DefaultHeader() string { + return Header(DefaultComment) +} + +// commentStyleFromString returns the comment style given an string. +func commentStyleFromString(str string) CommentStyle { + switch str { + case "//": + return SlashComment + case "#": + return HashComment + default: + panic(errors.E(errors.ErrInternal, "invalid comment style")) + } +} + +// CommentStyleFromConfig returns the CommentStyle from the configuration or the +// default if not defined. +func CommentStyleFromConfig(tree *config.Tree) CommentStyle { + tmConfig := tree.Node.Terramate + if tmConfig == nil || + tmConfig.Config == nil || + tmConfig.Config.Generate == nil || + tmConfig.Config.Generate.HCLMagicHeaderCommentStyle == nil { + return DefaultComment + } + return commentStyleFromString(*tmConfig.Config.Generate.HCLMagicHeaderCommentStyle) +} + // Load loads from the file system all generate_hcl for // a given stack. It will navigate the file system from the stack dir until // it reaches rootdir, loading generate_hcl and merging them appropriately. @@ -146,6 +194,8 @@ func Load( return nil, errors.E("loading generate_hcl", err) } + commentStyle := CommentStyleFromConfig(root.Tree()) + var hcls []HCL for _, hclBlock := range hclBlocks { name := hclBlock.Label @@ -170,9 +220,10 @@ func Load( if !matchedAnyStackFilter { hcls = append(hcls, HCL{ - label: name, - origin: hclBlock.Range, - condition: false, + magicCommentStyle: commentStyle, + label: name, + origin: hclBlock.Range, + condition: false, }) continue } @@ -211,9 +262,10 @@ func Load( if !condition { hcls = append(hcls, HCL{ - label: name, - origin: hclBlock.Range, - condition: condition, + magicCommentStyle: commentStyle, + label: name, + origin: hclBlock.Range, + condition: condition, }) continue } @@ -240,10 +292,11 @@ func Load( if assertFailed { hcls = append(hcls, HCL{ - label: name, - origin: hclBlock.Range, - condition: condition, - asserts: asserts, + magicCommentStyle: commentStyle, + label: name, + origin: hclBlock.Range, + condition: condition, + asserts: asserts, }) continue } @@ -262,11 +315,12 @@ func Load( )) } hcls = append(hcls, HCL{ - label: name, - origin: hclBlock.Range, - body: formatted, - condition: condition, - asserts: asserts, + magicCommentStyle: commentStyle, + label: name, + origin: hclBlock.Range, + body: formatted, + condition: condition, + asserts: asserts, }) } @@ -667,6 +721,17 @@ func getContentBlock(blocks hclsyntax.Blocks) (*hclsyntax.Block, error) { return contentBlock, nil } +func (c CommentStyle) String() string { + switch c { + case SlashComment: + return "//" + case HashComment: + return "#" + default: + panic(errors.E(errors.ErrInternal, "magic comment misconfigured")) + } +} + func attrErr(attr *hclsyntax.Attribute, msg string, args ...interface{}) error { return errors.E(ErrParsing, attr.Expr.Range(), stdfmt.Sprintf(msg, args...)) } diff --git a/hcl/hcl.go b/hcl/hcl.go index 0aa9ad837a..ea7e63c75c 100644 --- a/hcl/hcl.go +++ b/hcl/hcl.go @@ -151,6 +151,11 @@ type GitConfig struct { CheckRemote OptionalCheck } +// GenerateRootConfig represents the AST node for the `terramate.config.generate` block. +type GenerateRootConfig struct { + HCLMagicHeaderCommentStyle *string +} + // CloudConfig represents Terramate cloud configuration. type CloudConfig struct { // Organization is the name of the cloud organization @@ -160,6 +165,7 @@ type CloudConfig struct { // RootConfig represents the root config block of a Terramate configuration. type RootConfig struct { Git *GitConfig + Generate *GenerateRootConfig Run *RunConfig Cloud *CloudConfig Experiments []string @@ -1619,7 +1625,7 @@ func (p *TerramateParser) parseRootConfig(cfg *RootConfig, block *ast.MergedBloc } } - errs.AppendWrap(ErrTerramateSchema, block.ValidateSubBlocks("git", "run", "cloud")) + errs.AppendWrap(ErrTerramateSchema, block.ValidateSubBlocks("git", "generate", "run", "cloud")) gitBlock, ok := block.Blocks[ast.NewEmptyLabelBlockType("git")] if ok { @@ -1638,6 +1644,13 @@ func (p *TerramateParser) parseRootConfig(cfg *RootConfig, block *ast.MergedBloc errs.Append(parseCloudConfig(cfg.Cloud, cloudBlock)) } + generateBlock, ok := block.Blocks[ast.NewEmptyLabelBlockType("generate")] + if ok { + cfg.Generate = &GenerateRootConfig{} + + errs.Append(parseGenerateRootConfig(cfg.Generate, generateBlock)) + } + return errs.AsError() } @@ -1691,6 +1704,53 @@ func parseRunConfig(cfg *RootConfig, runBlock *ast.MergedBlock) error { return errs.AsError() } +func parseGenerateRootConfig(cfg *GenerateRootConfig, generateBlock *ast.MergedBlock) error { + errs := errors.L() + + errs.AppendWrap(ErrTerramateSchema, generateBlock.ValidateSubBlocks()) + + for _, attr := range generateBlock.Attributes.SortedList() { + value, diags := attr.Expr.Value(nil) + if diags.HasErrors() { + errs.Append(errors.E(diags, + "failed to evaluate terramate.config.generate.%s attribute", attr.Name, + )) + continue + } + + switch attr.Name { + case "hcl_magic_header_comment_style": + if value.Type() != cty.String { + errs.Append(attrErr(attr, + "terramate.config.generate.hcl_magic_header_comment_style is not a string but %q", + value.Type().FriendlyName(), + )) + + continue + } + + str := value.AsString() + if str != "//" && str != "#" { + errs.Append(attrErr(attr, + "terramate.config.generate.hcl_magic_header_comment_style must be either `//` or `#` but %q was given", + str, + )) + continue + } + + cfg.HCLMagicHeaderCommentStyle = &str + + default: + errs.Append(errors.E( + attr.NameRange, + "unrecognized attribute terramate.config.generate.%s", + attr.Name, + )) + } + } + return errs.AsError() +} + func parseRunEnv(runEnv *RunEnv, envBlock *ast.MergedBlock) error { if len(envBlock.Attributes) > 0 { runEnv.Attributes = envBlock.Attributes diff --git a/hcl/hcl_test.go b/hcl/hcl_test.go index 20945dbe1c..0e90eab738 100644 --- a/hcl/hcl_test.go +++ b/hcl/hcl_test.go @@ -600,6 +600,7 @@ func TestHCLParserTerramateBlock(t *testing.T) { } func TestHCLParserRootConfig(t *testing.T) { + ptr := func(s string) *string { return &s } for _, tc := range []testcase{ { name: "no config returns empty config", @@ -907,6 +908,62 @@ func TestHCLParserRootConfig(t *testing.T) { }, }, }, + { + name: "terramate.config.generate.hcl_magic_header_comment_style = //", + input: []cfgfile{ + { + filename: "cfg.tm", + body: ` + terramate { + config { + generate { + hcl_magic_header_comment_style = "//" + } + } + } + `, + }, + }, + want: want{ + config: hcl.Config{ + Terramate: &hcl.Terramate{ + Config: &hcl.RootConfig{ + Generate: &hcl.GenerateRootConfig{ + HCLMagicHeaderCommentStyle: ptr("//"), + }, + }, + }, + }, + }, + }, + { + name: "terramate.config.generate.hcl_magic_header_comment_style = #", + input: []cfgfile{ + { + filename: "cfg.tm", + body: ` + terramate { + config { + generate { + hcl_magic_header_comment_style = "#" + } + } + } + `, + }, + }, + want: want{ + config: hcl.Config{ + Terramate: &hcl.Terramate{ + Config: &hcl.RootConfig{ + Generate: &hcl.GenerateRootConfig{ + HCLMagicHeaderCommentStyle: ptr("#"), + }, + }, + }, + }, + }, + }, } { testParser(t, tc) } @@ -1080,6 +1137,52 @@ func TestHCLParserMultipleErrors(t *testing.T) { }, }, }, + { + name: "terramate.config.generate..hcl_magic_header_comment_style is not string -- fail", + input: []cfgfile{ + { + filename: "tm.tm", + body: ` + terramate { + config { + generate { + hcl_magic_header_comment_style = 1 + } + } + } + `, + }, + }, + want: want{ + errs: []error{ + errors.E(hcl.ErrTerramateSchema, + Mkrange("tm.tm", Start(5, 42, 92), End(5, 43, 93))), + }, + }, + }, + { + name: "terramate.config.generate..hcl_magic_header_comment_style with unknown value", + input: []cfgfile{ + { + filename: "tm.tm", + body: ` + terramate { + config { + generate { + hcl_magic_header_comment_style = "/*" + } + } + } + `, + }, + }, + want: want{ + errs: []error{ + errors.E(hcl.ErrTerramateSchema, + Mkrange("tm.tm", Start(5, 42, 92), End(5, 46, 96))), + }, + }, + }, } { testParser(t, tc) } From dd802737c5ec0de22a7e454c2a2525e1f42c5e2b Mon Sep 17 00:00:00 2001 From: Tiago Natel Date: Fri, 9 Feb 2024 02:41:20 +0000 Subject: [PATCH 3/5] chore: bump v0.5.0-dev version. Signed-off-by: Tiago Natel --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 4d08f8553c..4259c642a3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.4.6-dev +0.5.0-dev From ad731d8f591fb1a2b3587258b096d957b3c2f6d7 Mon Sep 17 00:00:00 2001 From: Wasif Malik Date: Fri, 9 Feb 2024 14:32:40 +0100 Subject: [PATCH 4/5] fix: use --quiet flag in interop tests --- .../e2etests/cloud/interop/interoperability_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/terramate/e2etests/cloud/interop/interoperability_test.go b/cmd/terramate/e2etests/cloud/interop/interoperability_test.go index fb466c1d74..b592f3522a 100644 --- a/cmd/terramate/e2etests/cloud/interop/interoperability_test.go +++ b/cmd/terramate/e2etests/cloud/interop/interoperability_test.go @@ -21,7 +21,7 @@ func TestInteropSyncDeployment(t *testing.T) { Stdout: nljoin("."), }) AssertRunResult(t, - tmcli.Run("run", "--cloud-sync-deployment", "--", HelperPath, "false"), + tmcli.Run("run", "--quiet", "--cloud-sync-deployment", "--", HelperPath, "false"), RunExpected{ IgnoreStderr: true, Status: 1, @@ -32,7 +32,7 @@ func TestInteropSyncDeployment(t *testing.T) { Stdout: nljoin("."), }, ) - AssertRun(t, tmcli.Run("run", "--cloud-sync-deployment", "--", HelperPath, "true")) + AssertRun(t, tmcli.Run("run", "--quiet", "--cloud-sync-deployment", "--", HelperPath, "true")) AssertRun(t, tmcli.Run("list", "--cloud-status=unhealthy")) } @@ -43,7 +43,7 @@ func TestInteropDrift(t *testing.T) { }) // initialize the providers AssertRunResult(t, - tmcli.Run("run", "--", TerraformTestPath, "init"), + tmcli.Run("run", "--quiet", "--", TerraformTestPath, "init"), RunExpected{ Status: 0, IgnoreStdout: true, @@ -53,7 +53,7 @@ func TestInteropDrift(t *testing.T) { // basic drift, without details AssertRunResult(t, - tmcli.Run("run", "--cloud-sync-drift-status", "--", TerraformTestPath, "plan", "-detailed-exitcode"), + tmcli.Run("run", "--quiet", "--cloud-sync-drift-status", "--", TerraformTestPath, "plan", "-detailed-exitcode"), RunExpected{ Status: 0, IgnoreStdout: true, @@ -102,7 +102,7 @@ func TestInteropDrift(t *testing.T) { ) // check reseting the drift status to OK - AssertRun(t, tmcli.Run("run", "--cloud-sync-drift-status", "--", HelperPath, "exit", "0")) + AssertRun(t, tmcli.Run("run", "--quiet", "--cloud-sync-drift-status", "--", HelperPath, "exit", "0")) AssertRun(t, tmcli.Run("list", "--cloud-status=unhealthy")) AssertRunResult(t, tmcli.Run("cloud", "drift", "show"), From 924fce3d030e2794ab8619db0cf058f866fe94a4 Mon Sep 17 00:00:00 2001 From: Tiago Natel Date: Fri, 9 Feb 2024 14:18:55 +0000 Subject: [PATCH 5/5] feat: fmt of individual files and stdin. Signed-off-by: Tiago Natel --- CHANGELOG.md | 1 + cmd/terramate/cli/cli.go | 49 +++- cmd/terramate/e2etests/core/fmt_test.go | 213 ++++++++++++++++++ .../e2etests/internal/runner/runner.go | 17 ++ hcl/fmt/fmt.go | 79 +++++-- hcl/fmt/fmt_test.go | 96 ++++---- 6 files changed, 386 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf0871258d..0b0eedea15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ Given a version number `MAJOR.MINOR.PATCH`, we increment the: ### Added - Add `terramate.config.generate.hcl_magic_header_comment_style` option for setting the generated comment style. +- Add support for formatting specific files and stdin (`terramate fmt [file...]` or `terramate fmt -`). ## 0.4.5 diff --git a/cmd/terramate/cli/cli.go b/cmd/terramate/cli/cli.go index 72a41fd3b8..99d7a30631 100644 --- a/cmd/terramate/cli/cli.go +++ b/cmd/terramate/cli/cli.go @@ -133,8 +133,9 @@ type cliSpec struct { } `cmd:"" help:"Creates a stack on the project"` Fmt struct { - Check bool `hidden:"" help:"Lists unformatted files, exit with 0 if all is formatted, 1 otherwise"` - DetailedExitCode bool `help:"Return an appropriate exit code (0 = ok, 1 = error, 2 = no error but changes were made)"` + Files []string `arg:"" optional:"true" predictor:"file" help:"files to be formatted"` + Check bool `hidden:"" help:"Lists unformatted files, exit with 0 if all is formatted, 1 otherwise"` + DetailedExitCode bool `help:"Return an appropriate exit code (0 = ok, 1 = error, 2 = no error but changes were made)"` } `cmd:"" help:"Format all files inside dir recursively"` List struct { @@ -595,6 +596,8 @@ func (c *cli) run() { switch c.ctx.Command() { case "fmt": c.format() + case "fmt ": + c.format() case "create ": c.createStack() case "create": @@ -1403,9 +1406,45 @@ func (c *cli) format() { fatal("Invalid args", errors.E("--check conflicts with --detailed-exit-code")) } - results, err := fmt.FormatTree(c.wd()) - if err != nil { - fatal("formatting files", err) + var results []fmt.FormatResult + switch len(c.parsedArgs.Fmt.Files) { + case 0: + var err error + results, err = fmt.FormatTree(c.wd()) + if err != nil { + fatal(sprintf("formatting directory %s", c.wd()), err) + } + case 1: + if c.parsedArgs.Fmt.Files[0] == "-" { + content, err := io.ReadAll(os.Stdin) + if err != nil { + fatal("reading stdin", err) + } + original := string(content) + formatted, err := fmt.Format(original, "") + if err != nil { + fatal("formatting stdin", err) + } + + if c.parsedArgs.Fmt.Check { + var status int + if formatted != original { + status = 1 + } + os.Exit(status) + } + + stdfmt.Print(formatted) + return + } + + fallthrough + default: + var err error + results, err = fmt.FormatFiles(c.wd(), c.parsedArgs.Fmt.Files) + if err != nil { + fatal("formatting files", err) + } } for _, res := range results { diff --git a/cmd/terramate/e2etests/core/fmt_test.go b/cmd/terramate/e2etests/core/fmt_test.go index 7b89cef456..839a8e55bb 100644 --- a/cmd/terramate/e2etests/core/fmt_test.go +++ b/cmd/terramate/e2etests/core/fmt_test.go @@ -194,3 +194,216 @@ name = "name" assertWantedFilesContents(t, formattedHCL) }) } + +func TestFmtFiles(t *testing.T) { + type want struct { + layout []string + res RunExpected + } + type testcase struct { + name string + layout []string + files []string + check bool + stdin string + absPaths bool + want want + } + + for _, tc := range []testcase{ + { + name: "non-existent file", + files: []string{"non-existent.tm"}, + want: want{ + res: RunExpected{ + StderrRegex: string(fmt.ErrReadFile), + Status: 1, + }, + }, + }, + { + name: "single file", + layout: []string{ + `f:example.tm:terramate { + config{} + }`, + }, + files: []string{"example.tm"}, + want: want{ + res: RunExpected{ + Stdout: nljoin("example.tm"), + }, + layout: []string{ + `f:example.tm:terramate { + config {} +}`, + }, + }, + }, + { + name: "multiple files", + layout: []string{ + `f:example1.tm:terramate { + config{} + }`, + `f:example2.tm:terramate { + config{} + }`, + }, + files: []string{"example1.tm", "example2.tm"}, + want: want{ + res: RunExpected{ + Stdout: nljoin("example1.tm", "example2.tm"), + }, + layout: []string{ + `f:example1.tm:terramate { + config {} +}`, + `f:example2.tm:terramate { + config {} +}`, + }, + }, + }, + { + name: "multiple files with --check", + layout: []string{ + `f:example1.tm:terramate { + config{} + }`, + `f:example2.tm:terramate { + config{} + }`, + }, + files: []string{"example1.tm", "example2.tm"}, + check: true, + want: want{ + res: RunExpected{ + Stdout: nljoin("example1.tm", "example2.tm"), + Status: 1, + }, + }, + }, + { + name: "multiple files with absolute path", + layout: []string{ + `f:example1.tm:terramate { + config{} + }`, + `f:example2.tm:terramate { + config{} + }`, + }, + absPaths: true, + files: []string{"example1.tm", "example2.tm"}, + want: want{ + res: RunExpected{ + Stdout: nljoin("example1.tm", "example2.tm"), + }, + layout: []string{ + `f:example1.tm:terramate { + config {} +}`, + `f:example2.tm:terramate { + config {} +}`, + }, + }, + }, + { + name: "format stdin", + files: []string{"-"}, + stdin: `stack { +name="name" + description = "desc" + }`, + want: want{ + res: RunExpected{ + Stdout: `stack { + name = "name" + description = "desc" +}`, + }, + }, + }, + { + name: "format stdin with multiple blocks", + files: []string{"-"}, + stdin: `stack { +name="name" + description = "desc" + } + + + + generate_file "a.txt" { + content = "a" + } + + +`, + want: want{ + res: RunExpected{ + Stdout: `stack { + name = "name" + description = "desc" +} + + + +generate_file "a.txt" { + content = "a" +} + + +`, + }, + }, + }, + { + name: "format stdin without content", + files: []string{"-"}, + }, + { + name: "format stdin with --check", + files: []string{"-"}, + stdin: `stack { +name="name" + description = "desc" + }`, + check: true, + want: want{ + res: RunExpected{ + Status: 1, + }, + }, + }, + } { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + s := sandbox.NoGit(t, true) + s.BuildTree(tc.layout) + cli := NewCLI(t, s.RootDir()) + files := tc.files + if tc.absPaths { + for i, f := range files { + files[i] = filepath.Join(s.RootDir(), f) + } + } + args := []string{"fmt"} + if tc.check { + args = append(args, "--check") + } + args = append(args, files...) + var result RunResult + if len(files) == 1 && files[0] == "-" { + result = cli.RunWithStdin(tc.stdin, args...) + } else { + result = cli.Run(args...) + } + AssertRunResult(t, result, tc.want.res) + s.AssertTree(tc.want.layout) + }) + } +} diff --git a/cmd/terramate/e2etests/internal/runner/runner.go b/cmd/terramate/e2etests/internal/runner/runner.go index bfdfffd2b5..c3c4e76900 100644 --- a/cmd/terramate/e2etests/internal/runner/runner.go +++ b/cmd/terramate/e2etests/internal/runner/runner.go @@ -244,6 +244,23 @@ func (tm CLI) Run(args ...string) RunResult { } } +// RunWithStdin runs the CLI but uses the provided string as stdin. +func (tm CLI) RunWithStdin(stdin string, args ...string) RunResult { + t := tm.t + t.Helper() + + cmd := tm.NewCmd(args...) + cmd.Stdin.b.WriteString(stdin) + _ = cmd.Run() + + return RunResult{ + Cmd: strings.Join(args, " "), + Stdout: cmd.Stdout.String(), + Stderr: cmd.Stderr.String(), + Status: cmd.ExitCode(), + } +} + // RunScript is a helper for executing `terramate run-script`. func (tm CLI) RunScript(args ...string) RunResult { return tm.Run(append([]string{"script", "run"}, args...)...) diff --git a/hcl/fmt/fmt.go b/hcl/fmt/fmt.go index e9e4102342..aa58867ae2 100644 --- a/hcl/fmt/fmt.go +++ b/hcl/fmt/fmt.go @@ -20,6 +20,9 @@ import ( // ErrHCLSyntax is the error kind for syntax errors. const ErrHCLSyntax errors.Kind = "HCL syntax error" +// ErrReadFile is the error kind for any error related to reading the file content. +const ErrReadFile errors.Kind = "failed to read file" + // FormatResult represents the result of a formatting operation. type FormatResult struct { path string @@ -68,39 +71,17 @@ func FormatTree(dir string) ([]FormatResult, error) { } sort.Strings(files) - results := []FormatResult{} errs := errors.L() + results, err := FormatFiles(dir, files) - for _, f := range files { - path := filepath.Join(dir, f) - fileContents, err := os.ReadFile(path) - if err != nil { - errs.Append(err) - continue - } - - currentCode := string(fileContents) - formatted, err := Format(currentCode, path) - if err != nil { - errs.Append(err) - continue - } - - if currentCode == formatted { - continue - } - - results = append(results, FormatResult{ - path: path, - formatted: formatted, - }) - } + errs.Append(err) dirs, err := fs.ListTerramateDirs(dir) if err != nil { errs.Append(err) return nil, errors.E(errFormatTree, errs) } + sort.Strings(dirs) for _, d := range dirs { @@ -118,6 +99,54 @@ func FormatTree(dir string) ([]FormatResult, error) { return results, nil } +// FormatFiles will format all the provided Terramate paths. +// Only Terramate configuration files can be reliably formatted with this function. +// If HCL files for a different tool is provided, the result is unpredictable. +// +// Note: The provided file paths can be absolute or relative. If relative, ensure +// working directory is corrected adjusted. The special `-` filename is treated as a +// normal filename, then if it needs to be interpreted as `stdin` this needs to be +// handled separately by the caller. +// +// Files that are already formatted are ignored. If all files are formatted +// this function returns an empty result. +// +// All files will be left untouched. To save the formatted result on disk you +// can use FormatResult.Save for each FormatResult. +func FormatFiles(basedir string, files []string) ([]FormatResult, error) { + results := []FormatResult{} + errs := errors.L() + + for _, file := range files { + fname := file + if !filepath.IsAbs(file) { + fname = filepath.Join(basedir, file) + } + fileContents, err := os.ReadFile(fname) + if err != nil { + errs.Append(errors.E(ErrReadFile, err)) + continue + } + currentCode := string(fileContents) + formatted, err := Format(currentCode, fname) + if err != nil { + errs.Append(err) + continue + } + if currentCode == formatted { + continue + } + results = append(results, FormatResult{ + path: fname, + formatted: formatted, + }) + } + if err := errs.AsError(); err != nil { + return nil, err + } + return results, nil +} + // Save will save the formatted result on the original file, replacing // its original contents. func (f FormatResult) Save() error { diff --git a/hcl/fmt/fmt_test.go b/hcl/fmt/fmt_test.go index 4c2117510b..0f668427be 100644 --- a/hcl/fmt/fmt_test.go +++ b/hcl/fmt/fmt_test.go @@ -1353,30 +1353,10 @@ d = [] continue } - // piggyback on the overall formatting scenarios to check - // for hcl.FormatTree behavior. - t.Run("Tree/"+tcase.name, func(t *testing.T) { - const ( - filename = "file.tm" - subdirName = "subdir" - ) - - rootdir := test.TempDir(t) - test.Mkdir(t, rootdir, subdirName) - subdir := filepath.Join(rootdir, subdirName) - - test.WriteFile(t, rootdir, filename, tcase.input) - test.WriteFile(t, subdir, filename, tcase.input) - - got, err := fmt.FormatTree(rootdir) - - // Since we have identical files we expect the same - // set of errors for each filepath to be present. - wantFilepath := filepath.Join(rootdir, filename) - wantSubdirFilepath := filepath.Join(subdir, filename) + checkResults := func(t *testing.T, res []fmt.FormatResult, wantFiles []string, tcase testcase, gotErr error) { wantErrs := []error{} - for _, path := range []string{wantFilepath, wantSubdirFilepath} { + for _, path := range wantFiles { for _, wantErr := range tcase.wantErrs { if e, ok := wantErr.(*errors.Error); ok { err := *e @@ -1389,33 +1369,71 @@ d = [] } } - errtest.AssertErrorList(t, err, wantErrs) - if err != nil { + errtest.AssertErrorList(t, gotErr, wantErrs) + if gotErr != nil { return } - assert.EqualInts(t, 2, len(got), "want 2 formatted files, got: %v", got) + assert.EqualInts(t, 2, len(res), "want %d formatted files, got: %v", len(wantFiles), res) - for _, res := range got { + for _, res := range res { assert.EqualStrings(t, tcase.want, res.Formatted()) assertFileContains(t, res.Path(), tcase.input) } - assert.EqualStrings(t, wantFilepath, got[0].Path()) - assert.EqualStrings(t, wantSubdirFilepath, got[1].Path()) + for i, wantFile := range wantFiles { + assert.EqualStrings(t, wantFile, res[i].Path()) + } + } - t.Run("saving format results", func(t *testing.T) { - for _, res := range got { - assert.NoError(t, res.Save()) - assertFileContains(t, res.Path(), res.Formatted()) - } + saveFiles := func(t *testing.T, rootdir string, res []fmt.FormatResult) { + for _, r := range res { + assert.NoError(t, r.Save()) + assertFileContains(t, r.Path(), r.Formatted()) + } - got, err := fmt.FormatTree(rootdir) - assert.NoError(t, err) + got, err := fmt.FormatTree(rootdir) + assert.NoError(t, err) - if len(got) > 0 { - t.Fatalf("after formatting want 0 fmt results, got: %v", got) - } - }) + if len(got) > 0 { + t.Fatalf("after formatting want 0 fmt results, got: %v", got) + } + } + + sandbox := func(t *testing.T) (string, []string) { + const ( + filename = "file.tm" + subdirName = "subdir" + ) + + rootdir := test.TempDir(t) + test.Mkdir(t, rootdir, subdirName) + subdir := filepath.Join(rootdir, subdirName) + + wantFilepath := test.WriteFile(t, rootdir, filename, tcase.input) + wantSubdirFilepath := test.WriteFile(t, subdir, filename, tcase.input) + return rootdir, []string{wantFilepath, wantSubdirFilepath} + } + + // piggyback on the overall formatting scenarios to check + // for hcl.FormatTree behavior. + t.Run("Tree/"+tcase.name, func(t *testing.T) { + rootdir, files := sandbox(t) + got, err := fmt.FormatTree(rootdir) + checkResults(t, got, files, tcase, err) + if err == nil { + saveFiles(t, rootdir, got) + } + }) + + // piggyback on the overall formatting scenarios to check + // for hcl.FormatFiles behavior. + t.Run("Files/"+tcase.name, func(t *testing.T) { + rootdir, files := sandbox(t) + got, err := fmt.FormatFiles(rootdir, files) + checkResults(t, got, files, tcase, err) + if err == nil { + saveFiles(t, rootdir, got) + } }) } }