From 2b40a24a776ab45dec8ce0484c72a2c3cc7e2f08 Mon Sep 17 00:00:00 2001 From: Levko Burburas <62853952+levkohimins@users.noreply.github.com> Date: Thu, 1 Aug 2024 12:06:44 +0200 Subject: [PATCH] Sorting output and fixing minor bugs in `hclvalidate` command (#3309) * feat: sorting hclvalidate output * chore: improving updating of unknown values * fix: go-fmt * fix: updating of unknown values * fix: tests * fix: tests * chore: rename env var --- cli/commands/hclvalidate/action.go | 12 ++++- cli/commands/hclvalidate/command.go | 10 ++-- cli/commands/hclvalidate/options.go | 4 +- config/config.go | 10 +--- config/config_helpers_test.go | 5 +- config/config_partial.go | 6 --- config/cty_helpers.go | 52 ++++++++++++++----- configstack/stack_test.go | 2 +- docs/_docs/04_reference/cli-options.md | 16 +++--- internal/view/human_render.go | 2 +- internal/view/json_render.go | 2 +- internal/view/writer.go | 8 +-- test/integration_test.go | 72 +++++++++++++------------- 13 files changed, 114 insertions(+), 87 deletions(-) diff --git a/cli/commands/hclvalidate/action.go b/cli/commands/hclvalidate/action.go index 5bc02e9d3..56b1b2701 100644 --- a/cli/commands/hclvalidate/action.go +++ b/cli/commands/hclvalidate/action.go @@ -2,6 +2,7 @@ package hclvalidate import ( "context" + "sort" "github.com/gruntwork-io/terragrunt/config" "github.com/gruntwork-io/terragrunt/config/hclparse" @@ -42,6 +43,13 @@ func Run(ctx context.Context, opts *Options) (er error) { stackErr := stack.Run(ctx, opts.TerragruntOptions) if len(diags) > 0 { + sort.Slice(diags, func(i, j int) bool { + if diags[i].Range != nil && diags[j].Range != nil && diags[i].Range.Filename > diags[j].Range.Filename { + return false + } + return true + }) + if err := writeDiagnostics(opts, diags); err != nil { return err } @@ -58,8 +66,8 @@ func writeDiagnostics(opts *Options, diags diagnostic.Diagnostics) error { writer := view.NewWriter(opts.Writer, render) - if opts.InvalidConfigPath { - return writer.InvalidConfigPath(diags) + if opts.ShowConfigPath { + return writer.ShowConfigPath(diags) } return writer.Diagnostics(diags) diff --git a/cli/commands/hclvalidate/command.go b/cli/commands/hclvalidate/command.go index 00410c086..35c0c0eb1 100644 --- a/cli/commands/hclvalidate/command.go +++ b/cli/commands/hclvalidate/command.go @@ -11,8 +11,8 @@ import ( const ( CommandName = "hclvalidate" - InvalidFlagName = "terragrunt-hclvalidate-invalid" - InvalidEnvVarName = "TERRAGRUNT_HCLVALIDATE_INVALID" + ShowConfigPathFlagName = "terragrunt-hclvalidate-show-config-path" + ShowConfigPathEnvVarName = "TERRAGRUNT_HCLVALIDATE_SHOW_CONFIG_PATH" JSONOutputFlagName = "terragrunt-hclvalidate-json" JSONOutputEnvVarName = "TERRAGRUNT_HCLVALIDATE_JSON" @@ -21,10 +21,10 @@ const ( func NewFlags(opts *Options) cli.Flags { return cli.Flags{ &cli.BoolFlag{ - Name: InvalidFlagName, - EnvVar: InvalidEnvVarName, + Name: ShowConfigPathFlagName, + EnvVar: ShowConfigPathEnvVarName, Usage: "Show a list of files with invalid configuration.", - Destination: &opts.InvalidConfigPath, + Destination: &opts.ShowConfigPath, }, &cli.BoolFlag{ Name: JSONOutputFlagName, diff --git a/cli/commands/hclvalidate/options.go b/cli/commands/hclvalidate/options.go index dee46ff2a..89aa1c65f 100644 --- a/cli/commands/hclvalidate/options.go +++ b/cli/commands/hclvalidate/options.go @@ -5,8 +5,8 @@ import "github.com/gruntwork-io/terragrunt/options" type Options struct { *options.TerragruntOptions - InvalidConfigPath bool - JSONOutput bool + ShowConfigPath bool + JSONOutput bool } func NewOptions(general *options.TerragruntOptions) *Options { diff --git a/config/config.go b/config/config.go index 4535ceaad..fb1a43f68 100644 --- a/config/config.go +++ b/config/config.go @@ -899,11 +899,11 @@ func decodeAsTerragruntConfigFile(ctx *ParsingContext, file *hclparse.File, eval } if terragruntConfig.Inputs != nil { - inputs, err := updateUnknownCtyValValues(terragruntConfig.Inputs) + inputs, err := updateUnknownCtyValValues(*terragruntConfig.Inputs) if err != nil { return nil, err } - terragruntConfig.Inputs = inputs + terragruntConfig.Inputs = &inputs } return &terragruntConfig, nil @@ -1158,12 +1158,6 @@ func convertToTerragruntConfig(ctx *ParsingContext, configPath string, terragrun } if ctx.Locals != nil && *ctx.Locals != cty.NilVal { - locals, err := updateUnknownCtyValValues(ctx.Locals) - if err != nil { - return nil, err - } - ctx.Locals = locals - localsParsed, err := parseCtyValueToMap(*ctx.Locals) if err != nil { return nil, err diff --git a/config/config_helpers_test.go b/config/config_helpers_test.go index 6fde1146b..063a9e61d 100644 --- a/config/config_helpers_test.go +++ b/config/config_helpers_test.go @@ -589,13 +589,16 @@ func TestResolveCliArgsInterpolationConfigString(t *testing.T) { assert.True(t, containsFoo) fooSlice := toStringSlice(t, foo) - assert.EqualValues(t, testCase.expectedFooInput, fooSlice, "For string '%s' include %v and options %v", testCase.str, testCase.include, testCase.terragruntOptions) }) } } func toStringSlice(t *testing.T, value interface{}) []string { + if value == nil { + return nil + } + asInterfaceSlice, isInterfaceSlice := value.([]interface{}) require.True(t, isInterfaceSlice) diff --git a/config/config_partial.go b/config/config_partial.go index f4d4b4f3b..93db34e27 100644 --- a/config/config_partial.go +++ b/config/config_partial.go @@ -319,12 +319,6 @@ func PartialParseConfig(ctx *ParsingContext, file *hclparse.File, includeFromChi } if decoded.Inputs != nil { - val, err := updateUnknownCtyValValues(decoded.Inputs) - if err != nil { - return nil, err - } - decoded.Inputs = val - inputs, err := parseCtyValueToMap(*decoded.Inputs) if err != nil { return nil, err diff --git a/config/cty_helpers.go b/config/cty_helpers.go index 3dd1c6d60..4687c444c 100644 --- a/config/cty_helpers.go +++ b/config/cty_helpers.go @@ -220,6 +220,12 @@ func deepMergeCtyMapsMapOnly(target cty.Value, source cty.Value, opts ...func(*m // we convert the given value to JSON using cty's JSON library and then convert the JSON back to a // map[string]interface{} using the Go json library. func parseCtyValueToMap(value cty.Value) (map[string]interface{}, error) { + updatedValue, err := updateUnknownCtyValValues(value) + if err != nil { + return nil, err + } + value = updatedValue + jsonBytes, err := ctyjson.Marshal(value, cty.DynamicPseudoType) if err != nil { return nil, errors.WithStackTrace(err) @@ -313,22 +319,44 @@ func includeConfigAsCtyVal(ctx *ParsingContext, includeConfig IncludeConfig) (ct return cty.NilVal, nil } -// updateUnknownCtyValValues updates unknown values with default value -func updateUnknownCtyValValues(value *cty.Value) (*cty.Value, error) { - updatedValue := map[string]cty.Value{} +// updateUnknownCtyValValues deeply updates unknown values with default value +func updateUnknownCtyValValues(value cty.Value) (cty.Value, error) { + var updatedValue any + + switch { + case !value.IsKnown(): + return cty.StringVal(""), nil + case value.IsNull(): + return value, nil + case value.Type().IsMapType(), value.Type().IsObjectType(): + mapVals := value.AsValueMap() + for key, val := range mapVals { + val, err := updateUnknownCtyValValues(val) + if err != nil { + return cty.NilVal, errors.WithStackTrace(err) + } + mapVals[key] = val + } + updatedValue = mapVals - for key, value := range value.AsValueMap() { - if value.IsKnown() { - updatedValue[key] = value - } else { - updatedValue[key] = cty.StringVal("") + case value.Type().IsTupleType(), value.Type().IsListType(): + sliceVals := value.AsValueSlice() + for key, val := range sliceVals { + val, err := updateUnknownCtyValValues(val) + if err != nil { + return cty.NilVal, errors.WithStackTrace(err) + } + sliceVals[key] = val } + updatedValue = sliceVals + + default: + return value, nil } - res, err := gocty.ToCtyValue(updatedValue, value.Type()) + value, err := gocty.ToCtyValue(updatedValue, value.Type()) if err != nil { - return nil, err + return cty.NilVal, errors.WithStackTrace(err) } - - return &res, nil + return value, nil } diff --git a/configstack/stack_test.go b/configstack/stack_test.go index 29a2a19ca..eb274901c 100644 --- a/configstack/stack_test.go +++ b/configstack/stack_test.go @@ -301,7 +301,7 @@ func TestResolveTerraformModulesReadConfigFromParentConfig(t *testing.T) { localsConfigs[name] = map[string]interface{}{ "dependencies": interface{}(nil), "download_dir": "", - "generate": map[string]interface{}{}, + "generate": interface{}(nil), "iam_assume_role_duration": interface{}(nil), "iam_assume_role_session_name": "", "iam_role": "", diff --git a/docs/_docs/04_reference/cli-options.md b/docs/_docs/04_reference/cli-options.md index 65df489a7..22623b323 100644 --- a/docs/_docs/04_reference/cli-options.md +++ b/docs/_docs/04_reference/cli-options.md @@ -64,7 +64,7 @@ This page documents the CLI commands and options available with Terragrunt: - [terragrunt-diff](#terragrunt-diff) - [terragrunt-hclfmt-file](#terragrunt-hclfmt-file) - [terragrunt-hclvalidate-json](#terragrunt-hclvalidate-json) - - [terragrunt-hclvalidate-invalid](#terragrunt-hclvalidate-invalid) + - [terragrunt-hclvalidate-show-config-path](#terragrunt-hclvalidate-show-config-path) - [terragrunt-override-attr](#terragrunt-override-attr) - [terragrunt-json-out](#terragrunt-json-out) - [terragrunt-json-disable-dependent-modules](#terragrunt-json-disable-dependent-modules) @@ -432,12 +432,12 @@ Example: terragrunt hclvalidate --terragrunt-hclvalidate-json ``` -In addition, you can pass the `--terragrunt-hclvalidate-invalid` flag to only output the invalid files, delimited by newlines. This can be especially useful when combined with the [terragrunt-excludes-file](#terragrunt-excludes-file) flag. +In addition, you can pass the `--terragrunt-hclvalidate-show-config-path` flag to only output paths of the invalid config files, delimited by newlines. This can be especially useful when combined with the [terragrunt-excludes-file](#terragrunt-excludes-file) flag. Example: ```bash -terragrunt hclvalidate --terragrunt-hclvalidate-invalid +terragrunt hclvalidate --terragrunt-hclvalidate-show-config-path ``` ### aws-provider-patch @@ -764,7 +764,7 @@ prefix `--terragrunt-` (e.g., `--terragrunt-config`). The currently available op - [terragrunt-diff](#terragrunt-diff) - [terragrunt-hclfmt-file](#terragrunt-hclfmt-file) - [terragrunt-hclvalidate-json](#terragrunt-hclvalidate-json) - - [terragrunt-hclvalidate-invalid](#terragrunt-hclvalidate-invalid) + - [terragrunt-hclvalidate-show-config-path](#terragrunt-hclvalidate-show-config-path) - [terragrunt-override-attr](#terragrunt-override-attr) - [terragrunt-json-out](#terragrunt-json-out) - [terragrunt-json-disable-dependent-modules](#terragrunt-json-disable-dependent-modules) @@ -980,10 +980,10 @@ Path to a file with a list of directories that need to be excluded when running excluded during execution of the commands. If a relative path is specified, it should be relative from [--terragrunt-working-dir](#terragrunt-working-dir). This will only exclude the module, not its dependencies. -This flag has been designed to integrate nicely with the `hclvalidate` command, which can return a list of invalid files delimited by newlines when passed the `--terragrunt-hclvalidate-invalid` flag. To integrate the two, you can run something like the following using bash process substitution: +This flag has been designed to integrate nicely with the `hclvalidate` command, which can return a list of invalid files delimited by newlines when passed the `--terragrunt-hclvalidate-show-config-path` flag. To integrate the two, you can run something like the following using bash process substitution: ```bash -terragrunt run-all plan --terragrunt-excludes-file <(terragrunt hclvalidate --terragrunt-hclvalidate-invalid) +terragrunt run-all plan --terragrunt-excludes-file <(terragrunt hclvalidate --terragrunt-hclvalidate-show-config-path) ``` ### terragrunt-exclude-dir @@ -1130,9 +1130,9 @@ When passed in, run `hclfmt` only on specified hcl file. When passed in, render the output in the JSON format. -### terragrunt-hclvalidate-invalid +### terragrunt-hclvalidate-show-config-path -**CLI Arg**: `--terragrunt-hclvalidate-invalid`
+**CLI Arg**: `--terragrunt-hclvalidate-show-config-path`
**Environment Variable**: `TERRAGRUNT_HCLVALIDATE_INVALID` (set to `true`)
**Commands**: diff --git a/internal/view/human_render.go b/internal/view/human_render.go index fc0167397..146417bb0 100644 --- a/internal/view/human_render.go +++ b/internal/view/human_render.go @@ -40,7 +40,7 @@ func NewHumanRender(disableColor bool) Render { } } -func (render *HumanRender) InvalidConfigPath(filenames []string) (string, error) { +func (render *HumanRender) ShowConfigPath(filenames []string) (string, error) { var buf bytes.Buffer for _, filename := range filenames { diff --git a/internal/view/json_render.go b/internal/view/json_render.go index 4302546c9..119976b6c 100644 --- a/internal/view/json_render.go +++ b/internal/view/json_render.go @@ -17,7 +17,7 @@ func (render *JSONRender) Diagnostics(diags diagnostic.Diagnostics) (string, err return render.toJSON(diags) } -func (render *JSONRender) InvalidConfigPath(filenames []string) (string, error) { +func (render *JSONRender) ShowConfigPath(filenames []string) (string, error) { return render.toJSON(filenames) } diff --git a/internal/view/writer.go b/internal/view/writer.go index 8e714ea7d..23b22409c 100644 --- a/internal/view/writer.go +++ b/internal/view/writer.go @@ -13,8 +13,8 @@ type Render interface { // Diagnostics renders early diagnostics, resulting from argument parsing. Diagnostics(diags diagnostic.Diagnostics) (string, error) - // InvalidConfigPath renders paths to configurations that contain errors. - InvalidConfigPath(filenames []string) (string, error) + // ShowConfigPath renders paths to configurations that contain errors. + ShowConfigPath(filenames []string) (string, error) } // Writer is the base layer for command views, encapsulating a set of I/O streams, a colorize implementation, and implementing a human friendly view for diagnostics. @@ -39,7 +39,7 @@ func (writer *Writer) Diagnostics(diags diagnostic.Diagnostics) error { return writer.output(output) } -func (writer *Writer) InvalidConfigPath(diags diagnostic.Diagnostics) error { +func (writer *Writer) ShowConfigPath(diags diagnostic.Diagnostics) error { var filenames []string for _, diag := range diags { @@ -48,7 +48,7 @@ func (writer *Writer) InvalidConfigPath(diags diagnostic.Diagnostics) error { } } - output, err := writer.render.InvalidConfigPath(filenames) + output, err := writer.render.ShowConfigPath(filenames) if err != nil { return err } diff --git a/test/integration_test.go b/test/integration_test.go index b9c2b28d5..75e46448c 100644 --- a/test/integration_test.go +++ b/test/integration_test.go @@ -277,40 +277,6 @@ func TestHclvalidateDiagnostic(t *testing.T) { HighlightEndOffset: 6, }, }, - &diagnostic.Diagnostic{ - Severity: diagnostic.DiagnosticSeverity(hcl.DiagError), - Summary: "Can't evaluate expression", - Detail: "You can only reference to other local variables here, but it looks like you're referencing something else (\"dependency\" is not defined)", - Range: &diagnostic.Range{ - Filename: filepath.Join(rootPath, "second/c/terragrunt.hcl"), - Start: diagnostic.Pos{Line: 10, Column: 9, Byte: 117}, - End: diagnostic.Pos{Line: 10, Column: 31, Byte: 139}, - }, - Snippet: &diagnostic.Snippet{ - Context: "locals", - Code: " vvv = dependency.a.outputs.z", - StartLine: 10, - HighlightStartOffset: 8, - HighlightEndOffset: 30, - }, - }, - &diagnostic.Diagnostic{ - Severity: diagnostic.DiagnosticSeverity(hcl.DiagError), - Summary: "Can't evaluate expression", - Detail: "You can only reference to other local variables here, but it looks like you're referencing something else (\"dependency\" is not defined)", - Range: &diagnostic.Range{ - Filename: filepath.Join(rootPath, "second/c/terragrunt.hcl"), - Start: diagnostic.Pos{Line: 12, Column: 9, Byte: 149}, - End: diagnostic.Pos{Line: 12, Column: 21, Byte: 161}, - }, - Snippet: &diagnostic.Snippet{ - Context: "locals", - Code: " ddd = dependency.d", - StartLine: 12, - HighlightStartOffset: 8, - HighlightEndOffset: 20, - }, - }, &diagnostic.Diagnostic{ Severity: diagnostic.DiagnosticSeverity(hcl.DiagError), Summary: "Unsupported attribute", @@ -346,6 +312,40 @@ func TestHclvalidateDiagnostic(t *testing.T) { HighlightEndOffset: 16, }, }, + &diagnostic.Diagnostic{ + Severity: diagnostic.DiagnosticSeverity(hcl.DiagError), + Summary: "Can't evaluate expression", + Detail: "You can only reference to other local variables here, but it looks like you're referencing something else (\"dependency\" is not defined)", + Range: &diagnostic.Range{ + Filename: filepath.Join(rootPath, "second/c/terragrunt.hcl"), + Start: diagnostic.Pos{Line: 12, Column: 9, Byte: 149}, + End: diagnostic.Pos{Line: 12, Column: 21, Byte: 161}, + }, + Snippet: &diagnostic.Snippet{ + Context: "locals", + Code: " ddd = dependency.d", + StartLine: 12, + HighlightStartOffset: 8, + HighlightEndOffset: 20, + }, + }, + &diagnostic.Diagnostic{ + Severity: diagnostic.DiagnosticSeverity(hcl.DiagError), + Summary: "Can't evaluate expression", + Detail: "You can only reference to other local variables here, but it looks like you're referencing something else (\"dependency\" is not defined)", + Range: &diagnostic.Range{ + Filename: filepath.Join(rootPath, "second/c/terragrunt.hcl"), + Start: diagnostic.Pos{Line: 10, Column: 9, Byte: 117}, + End: diagnostic.Pos{Line: 10, Column: 31, Byte: 139}, + }, + Snippet: &diagnostic.Snippet{ + Context: "locals", + Code: " vvv = dependency.a.outputs.z", + StartLine: 10, + HighlightStartOffset: 8, + HighlightEndOffset: 30, + }, + }, } stdout, _, err := runTerragruntCommandWithOutput(t, fmt.Sprintf("terragrunt hclvalidate --terragrunt-working-dir %s --terragrunt-hclvalidate-json", rootPath)) @@ -369,7 +369,7 @@ func TestHclvalidateInvalidConfigPath(t *testing.T) { filepath.Join(rootPath, "second/c/terragrunt.hcl"), } - stdout, _, err := runTerragruntCommandWithOutput(t, fmt.Sprintf("terragrunt hclvalidate --terragrunt-working-dir %s --terragrunt-hclvalidate-json --terragrunt-hclvalidate-invalid", rootPath)) + stdout, _, err := runTerragruntCommandWithOutput(t, fmt.Sprintf("terragrunt hclvalidate --terragrunt-working-dir %s --terragrunt-hclvalidate-json --terragrunt-hclvalidate-show-config-path", rootPath)) require.NoError(t, err) var actualPaths []string @@ -3845,7 +3845,7 @@ func TestReadTerragruntConfigFull(t *testing.T) { "suppress_stdout": nil, }, }, - "error_hook": map[string]interface{}{}, + "error_hook": nil, }, ) }