Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow hcl format from stdin #3288

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

alikhil
Copy link
Contributor

@alikhil alikhil commented Jul 23, 2024

Resolves #2926

Description

Fixes #2926 .

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added --terragrunt-hclfmt-stdin flag for hclfmt command

Migration Guide

Copy link
Collaborator

@yhakbar yhakbar left a comment

Choose a reason for hiding this comment

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

This looks great! Left some minor notes.

cli/commands/hclfmt/action_test.go Outdated Show resolved Hide resolved
cli/commands/hclfmt/action_test.go Outdated Show resolved Hide resolved
cli/commands/hclfmt/action_test.go Outdated Show resolved Hide resolved
@alikhil alikhil requested a review from yhakbar July 25, 2024 16:10
@denis256
Copy link
Member

Hi, looks like code failed on linter:

cli/commands/hclfmt/action_test.go:267:2: SA5001: should check returned error before deferring tempStdoutFile.Close() (staticcheck)
        defer tempStdoutFile.Close()

@alikhil
Copy link
Contributor Author

alikhil commented Jul 26, 2024

@denis256 thank you for feedback! Fixed it!

@yhakbar
Copy link
Collaborator

yhakbar commented Aug 1, 2024

Hey @alikhil

We are still getting errors in CI:

=== NAME  TestTerragruntPrintAwsErrors
    integration_test.go:6480: 
                Error Trace:    /home/circleci/project/test/integration_test.go:6480
                Error:          An error is expected but got nil.
                Test:           TestTerragruntPrintAwsErrors
--- FAIL: TestTerragruntPrintAwsErrors (3.81s)

Are you able to run that test locally?

@yhakbar yhakbar changed the title Allow hcl format from stdin WIP - Allow hcl format from stdin Aug 9, 2024
Copy link

sonarcloud bot commented Aug 14, 2024

@alikhil
Copy link
Contributor Author

alikhil commented Aug 14, 2024

It seems like test was failing in main branch when I forked repo.
I have merged recent changes to my branch and test passes now:

micro@homestation MINGW64 /b/Users/micro/projects/opensource/terragrunt (feature/hclfmt-stdin-2926)
$ go test -timeout 30s -run ^TestTerragruntPrintAwsErrors$ github.com/gruntwork-io/terragrunt/test
ok      github.com/gruntwork-io/terragrunt/test 1.335s

@alikhil
Copy link
Contributor Author

alikhil commented Aug 25, 2024

@yhakbar hello!

Does it still fails on tests?

@yhakbar
Copy link
Collaborator

yhakbar commented Sep 9, 2024

Hey @alikhil ,

I just re-ran the test, and this is failing:

    integration_test.go:4397: 
                Error Trace:    /home/circleci/project/test/integration_test.go:4397
                Error:          Should be true
                Test:           TestTerragruntVersionConstraintsPartialParse
--- FAIL: TestTerragruntVersionConstraintsPartialParse (0.54s)

Are you able to run that test locally?

You should be able to with go test -run 'TestTerragruntVersionConstraintsPartialParse' ./test.

@alikhil
Copy link
Contributor Author

alikhil commented Sep 12, 2024

Hi @yhakbar

Yes, when I run the test locally it passes:

go test -run 'TestTerragruntVersionConstraintsPartialParse' ./test
ok      github.com/gruntwork-io/terragrunt/test 1.053s

@yhakbar
Copy link
Collaborator

yhakbar commented Sep 16, 2024

Hey @alikhil , I tried to kick off CI for the changes here, and ran into errors from our linter.

Could you please run make run-lint and take a look at the errors? I'm seeing errors like this:

cli/commands/hclfmt/action_test.go:1: : # github.com/gruntwork-io/terragrunt/cli/commands/hclfmt_test [github.com/gruntwork-io/terragrunt/cli/commands/hclfmt.test]
cli/commands/hclfmt/action_test.go:292:8: undefined: Run (typecheck)
package hclfmt_test

The test does pass for me locally as well, so I suspect that once the code passes linting, we should be able to iron out any remaining issues pretty quickly.

We've left this PR open for quite a while, so please do the following so that we can make sure this PR gets merged ASAP:

  1. Run the linter locally, then address the findings.
  2. Change the title to remove WIP when you think it's ready for another round of review.
  3. Tag me here for another look.

If you receive more feedback that requires changes, please continue to iterate like that. Don't hesitate to remind us to review if we're slow to respond.

@alikhil alikhil changed the title WIP - Allow hcl format from stdin Allow hcl format from stdin Sep 16, 2024
@alikhil
Copy link
Contributor Author

alikhil commented Sep 16, 2024

Hi @yhakbar !

I think I fixed original linting issue, but in my dev enviroment (windows) I can not make linter green because of other issues not related to my PR:

golangci-lint run -v --timeout=5m -c .strict.golangci.yml --new-from-rev master ./...
level=info msg="golangci-lint has version 1.61.0 built with go1.23.1 from a1d6c560 on 2024-09-09T17:44:42Z"
level=info msg="[config_reader] Used config file .strict.golangci.yml"
level=info msg="[lintersdb] Active 97 linters: [asasalint asciicheck bidichk bodyclose canonicalheader containedctx contextcheck copyloopvar cyclop decorder dogsled dupl durationcheck err113 errcheck errchkjson errname errorlint exhaustive fatcontext forbidigo forcetypeassert funlen ginkgolinter gocheckcompilerdirectives gochecknoglobals gochecknoinits gochecksumtype gocognit goconst gocritic godot godox goheader goimports gomoddirectives gomodguard goprintffuncname gosimple gosmopolitan govet grouper importas inamedparam ineffassign interfacebloat intrange ireturn lll loggercheck maintidx makezero mirror misspell mnd musttag nakedret nestif nilerr nilnil nlreturn noctx nolintlint nonamedreturns nosprintfhostport paralleltest perfsprint prealloc predeclared promlinter protogetter reassign revive rowserrcheck sloglint spancheck sqlclosecheck staticcheck stylecheck tagalign tagliatelle tenv testableexamples testifylint testpackage thelper tparallel unconvert unparam unused usestdlibvars varnamelen wastedassign whitespace wrapcheck wsl zerologlint]"
level=info msg="[lintersdb] Active presets: [bugs complexity performance style test unused]"
level=info msg="[loader] Go packages loading at mode 575 (exports_file|files|name|types_sizes|compiled_files|deps|imports) took 7.969282s"
level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 35.9544ms"
level=info msg="[linters_context] importas settings found, but no aliases listed. List aliases under alias: key."
level=error msg="[linters_context] typechecking error: : # github.com/gruntwork-io/terragrunt/shell [github.com/gruntwork-io/terragrunt/shell.test]\nshell\\run_shell_cmd_windows_test.go:34:23: cannot use testLogger.WithContext(context.Background()) (value of type *logrus.Entry) as \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger value in assignment: *logrus.Entry does not implement \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger (missing method Clone)\nshell\\run_shell_cmd_windows_test.go:53:19: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:60:21: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:79:39: undefined: forwardSignals"
level=error msg="[linters_context] typechecking error: B:\\Users\\micro\\projects\\opensource\\terragrunt\\shell\\error_explainer_test.go:7:2: could not import github.com/gruntwork-io/terragrunt/shell (-: # github.com/gruntwork-io/terragrunt/shell [github.com/gruntwork-io/terragrunt/shell.test]\nshell\\run_shell_cmd_windows_test.go:34:23: cannot use testLogger.WithContext(context.Background()) (value of type *logrus.Entry) as \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger value in assignment: *logrus.Entry does not implement \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger (missing method Clone)\nshell\\run_shell_cmd_windows_test.go:53:19: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:60:21: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:79:39: undefined: forwardSignals)"
level=error msg="[linters_context] typechecking error: : # github.com/gruntwork-io/terragrunt/test_test [github.com/gruntwork-io/terragrunt/test.test]\ntest\\integration_windows_test.go:79:43: undefined: testFixtureTflintNoIssuesFound\ntest\\integration_windows_test.go:80:40: undefined: testFixtureTflintNoIssuesFound"
level=info msg="[linters_context/goanalysis] analyzers took 4m11.8592396s with top 10 stages: the_only_name: 35.2096731s, goimports: 31.0032048s, musttag: 11.3977133s, buildir: 9.6888642s, wastedassign: 6.0419758s, printf: 3.0985622s, fact_deprecated: 3.054973s, exhaustive: 2.9193669s, dupl: 2.9077815s, ctrlflow: 2.8261665s"
level=info msg="[runner] Issues before processing: 6030, after processing: 0"
level=info msg="[runner] Processors filtering stat (in/out): exclude: 6030/6030, uniq_by_line: 5200/4016, diff: 4016/0, autogenerated_exclude: 6030/6030, identifier_marker: 6030/6030, exclude-rules: 6030/5310, nolint: 5310/5200, filename_unadjuster: 6030/6030, skip_files: 6030/6030, cgo: 6030/6030, invalid_issue: 6030/6030, path_prettifier: 6030/6030, skip_dirs: 6030/6030"
level=info msg="[runner] processing took 355.8069ms with stages: diff: 111.2997ms, path_prettifier: 109.8715ms, identifier_marker: 64.1366ms, nolint: 53.8274ms, autogenerated_exclude: 12.5603ms, skip_dirs: 1.5378ms, exclude-rules: 999.8µs, uniq_by_line: 537.7µs, cgo: 527.1µs, invalid_issue: 509µs, path_shortener: 0s, severity-rules: 0s, max_from_linter: 0s, max_same_issues: 0s, skip_files: 0s, exclude: 0s, sort_results: 0s, filename_unadjuster: 0s, source_code: 0s, fixer: 0s, max_per_file_from_linter: 0s, path_prefixer: 0s"
level=info msg="[runner] linters took 18.0225496s with stages: goanalysis_metalinter: 17.6662394s"
level=info msg="File cache stats: 266 entries of total size 1.6MiB"
level=info msg="Memory: 261 samples, avg is 342.7MB, max is 1135.1MB"
level=info msg="Execution took 26.0656064s"
make: *** [Makefile:48: run-strict-lint] Error 7

@yhakbar
Copy link
Collaborator

yhakbar commented Sep 16, 2024

Hey @alikhil ,

I'm sorry, I don't have a Windows machine to check on this. I'll ask my colleague @denis256 to confirm.

To be clear, you don't have to pass the strict lints in .strict.golangci.yml to get merged. As long as the lints in make run-lint pass in CI, the tests will also run.

@alikhil
Copy link
Contributor Author

alikhil commented Sep 16, 2024

Same error in make run-lint:

$ golangci-lint run -v --timeout=5m --new-from-rev master ./...
level=info msg="golangci-lint has version 1.61.0 built with go1.23.1 from a1d6c560 on 2024-09-09T17:44:42Z"
level=info msg="[config_reader] Config search paths: [./ B:\\Users\\micro\\projects\\opensource\\terragrunt B:\\Users\\micro\\projects\\opensource B:\\Users\\micro\\projects B:\\Users\\micro B:\\Users B:\\ C:\\Users\\micro]"
level=info msg="[config_reader] Used config file .golangci.yml"
level=info msg="[lintersdb] Active 49 linters: [asasalint asciicheck bidichk bodyclose contextcheck dupl durationcheck errcheck errchkjson errorlint exhaustive fatcontext gocheckcompilerdirectives gochecksumtype goconst gocritic goimports gosimple gosmopolitan govet ineffassign loggercheck makezero misspell mnd musttag nilerr noctx paralleltest perfsprint prealloc protogetter reassign rowserrcheck spancheck sqlclosecheck staticcheck tenv testableexamples testifylint testpackage thelper tparallel unconvert unparam unused wastedassign wsl zerologlint]"
level=info msg="[lintersdb] Active presets: [bugs performance test unused]"
level=info msg="[loader] Go packages loading at mode 575 (compiled_files|exports_file|files|types_sizes|deps|imports|name) took 1m21.8997333s"
level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 29.4338ms"
level=error msg="[linters_context] typechecking error: : # github.com/gruntwork-io/terragrunt/shell [github.com/gruntwork-io/terragrunt/shell.test]\nshell\\run_shell_cmd_windows_test.go:34:23: cannot use testLogger.WithContext(context.Background()) (value of type *logrus.Entry) as \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger value in assignment: *logrus.Entry does not implement \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger (missing method Clone)\nshell\\run_shell_cmd_windows_test.go:53:19: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:60:21: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:79:39: undefined: forwardSignals"
level=error msg="[linters_context] typechecking error: B:\\Users\\micro\\projects\\opensource\\terragrunt\\shell\\error_explainer_test.go:7:2: could not import github.com/gruntwork-io/terragrunt/shell (-: # github.com/gruntwork-io/terragrunt/shell [github.com/gruntwork-io/terragrunt/shell.test]\nshell\\run_shell_cmd_windows_test.go:34:23: cannot use testLogger.WithContext(context.Background()) (value of type *logrus.Entry) as \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger value in assignment: *logrus.Entry does not implement \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger (missing method Clone)\nshell\\run_shell_cmd_windows_test.go:53:19: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:60:21: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:79:39: undefined: forwardSignals)"
level=error msg="[linters_context] typechecking error: : # github.com/gruntwork-io/terragrunt/test_test [github.com/gruntwork-io/terragrunt/test.test]\ntest\\integration_windows_test.go:79:43: undefined: testFixtureTflintNoIssuesFound\ntest\\integration_windows_test.go:80:40: undefined: testFixtureTflintNoIssuesFound"
level=info msg="[linters_context/goanalysis] analyzers took 1m39.0444883s with top 10 stages: goimports: 22.7806484s, musttag: 6.7210218s, buildir: 5.7475996s, wastedassign: 4.1789491s, dupl: 2.5547631s, unconvert: 1.8089643s, bidichk: 1.8003896s, buildssa: 1.4576907s, printf: 1.2733183s, gocritic: 1.2729486s"
level=info msg="[runner] Issues before processing: 640, after processing: 0"
level=info msg="[runner] Processors filtering stat (in/out): invalid_issue: 640/640, skip_dirs: 640/640, autogenerated_exclude: 640/640, exclude-rules: 640/328, uniq_by_line: 272/272, cgo: 640/640, nolint: 328/272, diff: 272/0, exclude: 640/640, path_prettifier: 640/640, skip_files: 640/640, identifier_marker: 640/640, filename_unadjuster: 640/640"
level=info msg="[runner] processing took 245.4345ms with stages: diff: 119.2377ms, path_prettifier: 57.353ms, nolint: 53.8302ms, autogenerated_exclude: 8.9941ms, identifier_marker: 3.5135ms, skip_dirs: 1.5067ms, exclude-rules: 999.3µs, source_code: 0s, exclude: 0s, skip_files: 0s, invalid_issue: 0s, sort_results: 0s, fixer: 0s, cgo: 0s, path_prefixer: 0s, max_same_issues: 0s, path_shortener: 0s, severity-rules: 0s, filename_unadjuster: 0s, max_per_file_from_linter: 0s, uniq_by_line: 0s, max_from_linter: 0s"
level=info msg="[runner] linters took 15.295535s with stages: goanalysis_metalinter: 15.0495977s"
level=info msg="File cache stats: 266 entries of total size 1.6MiB"
level=info msg="Memory: 974 samples, avg is 85.4MB, max is 891.5MB"
level=info msg="Execution took 1m37.2886741s"

But initial error undefined: Run (typecheck) has gone.
It seems to be CI checks don't run automatically in CircleCI. Could you start them please?

@denis256
Copy link
Member

Hello,
make sure you have installed the same version of the linter as used in CICD
from your log:

level=info msg="golangci-lint has version 1.61.0 built

CICD uses different version - v1.59.1
https://github.com/gruntwork-io/terragrunt/blob/main/Makefile#L42

@alikhil
Copy link
Contributor Author

alikhil commented Sep 17, 2024

@denis256 hello!

Sure! Output fov 1.59.1 is the same:

$ golangci-lint run -v --timeout=5m ./... --new-from-rev master ./...
level=info msg="golangci-lint has version v1.59.1 built with go1.22.5 from (unknown, modified: ?, mod sum: \"h1:CRRLu1JbhK5avLABFJ/OHVSQ0Ie5c4ulsOId1h3TTks=\") on (unknown)"
level=info msg="[config_reader] Config search paths: [./ B:\\Users\\micro\\projects\\opensource\\terragrunt B:\\Users\\micro\\projects\\opensource B:\\Users\\micro\\projects B:\\Users\\micro B:\\Users B:\\ C:\\Users\\micro]"
level=info msg="[config_reader] Used config file .golangci.yml"
level=info msg="[lintersdb] Active 49 linters: [asasalint asciicheck bidichk bodyclose contextcheck dupl durationcheck errcheck errchkjson errorlint exhaustive exportloopref fatcontext gocheckcompilerdirectives gochecksumtype goconst gocritic goimports gosimple gosmopolitan govet ineffassign loggercheck makezero misspell mnd musttag nilerr noctx paralleltest perfsprint prealloc protogetter reassign rowserrcheck spancheck sqlclosecheck staticcheck testableexamples testifylint testpackage thelper tparallel unconvert unparam unused wastedassign wsl zerologlint]"
level=info msg="[lintersdb] Active presets: [bugs performance test unused]"
level=info msg="[loader] Go packages loading at mode 575 (name|types_sizes|imports|compiled_files|deps|exports_file|files) took 1m39.4087965s"
level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 39.6572ms"
level=error msg="[linters_context] typechecking error: : # github.com/gruntwork-io/terragrunt/shell [github.com/gruntwork-io/terragrunt/shell.test]\nshell\\run_shell_cmd_windows_test.go:34:23: cannot use testLogger.WithContext(context.Background()) (value of type *logrus.Entry) as \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger value in assignment: *logrus.Entry does not implement \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger (missing method Clone)\nshell\\run_shell_cmd_windows_test.go:53:19: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:60:21: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:79:39: undefined: forwardSignals"
level=error msg="[linters_context] typechecking error: B:\\Users\\micro\\projects\\opensource\\terragrunt\\shell\\error_explainer_test.go:7:2: could not import github.com/gruntwork-io/terragrunt/shell (-: # github.com/gruntwork-io/terragrunt/shell [github.com/gruntwork-io/terragrunt/shell.test]\nshell\\run_shell_cmd_windows_test.go:34:23: cannot use testLogger.WithContext(context.Background()) (value of type *logrus.Entry) as \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger value in assignment: *logrus.Entry does not implement \"github.com/gruntwork-io/terragrunt/pkg/log\".Logger (missing method Clone)\nshell\\run_shell_cmd_windows_test.go:53:19: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:60:21: undefined: GetExitCode\nshell\\run_shell_cmd_windows_test.go:79:39: undefined: forwardSignals)"
level=error msg="[linters_context] typechecking error: : # github.com/gruntwork-io/terragrunt/test_test [github.com/gruntwork-io/terragrunt/test.test]\ntest\\integration_windows_test.go:79:43: undefined: testFixtureTflintNoIssuesFound\ntest\\integration_windows_test.go:80:40: undefined: testFixtureTflintNoIssuesFound"
level=info msg="[linters_context/goanalysis] analyzers took 12m38.8432198s with top 10 stages: buildir: 2m14.0825081s, exhaustive: 57.9786269s, printf: 54.985132s, nilness: 54.9205568s, ctrlflow: 53.4878902s, fact_purity: 53.0860956s, fact_deprecated: 52.959708s, contextcheck: 51.2333017s, typedness: 50.2242019s, SA5012: 46.3549058s"
level=info msg="[runner] Issues before processing: 635, after processing: 0"
level=info msg="[runner] Processors filtering stat (out/in): cgo: 635/635, identifier_marker: 635/635, uniq_by_line: 267/267, diff: 0/267, filename_unadjuster: 635/635, invalid_issue: 635/635, skip_files: 635/635, nolint: 267/323, autogenerated_exclude: 635/635, exclude: 635/635, path_prettifier: 635/635, skip_dirs: 635/635, exclude-rules: 323/635"
level=info msg="[runner] processing took 238.2881ms with stages: diff: 110.4967ms, path_prettifier: 58.4285ms, nolint: 52.6069ms, autogenerated_exclude: 10.8708ms, identifier_marker: 4.3794ms, skip_dirs: 998.9µs, cgo: 506.9µs, uniq_by_line: 0s, invalid_issue: 0s, path_shortener: 0s, severity-rules: 0s, exclude: 0s, fixer: 0s, filename_unadjuster: 0s, skip_files: 0s, max_per_file_from_linter: 0s, max_same_issues: 0s, max_from_linter: 0s, path_prefixer: 0s, sort_results: 0s, source_code: 0s, exclude-rules: 0s"
level=info msg="[runner] linters took 34.2223712s with stages: goanalysis_metalinter: 33.9837228s"
level=info msg="File cache stats: 266 entries of total size 1.6MiB"
level=info msg="Memory: 1324 samples, avg is 420.4MB, max is 2282.2MB"
level=info msg="Execution took 2m13.7183368s"

@denis256
Copy link
Member

INFO [runner] linters took 54.697135838s with stages: goanalysis_metalinter: 54.61638833s 
cli/commands/hclfmt/action_test.go:266:1: Function TestHCLFmtStdin missing the call to method parallel (paralleltest)
func TestHCLFmtStdin(t *testing.T) {
^
cli/commands/hclfmt/action.go:38:11: fmt.Errorf can be replaced with errors.New (perfsprint)
                        return fmt.Errorf("both stdin and path flags are specified")
                               ^
cli/commands/hclfmt/action.go:96:62: block should not start with a whitespace (wsl)
func formatFromStdin(opts *options.TerragruntOptions) error {
                                                             ^
cli/commands/hclfmt/action.go:115:2: only one cuddle assignment allowed before if statement (wsl)
        if err != nil {
        ^
INFO File cache stats: 299 entries of total size 1.9MiB 
INFO Memory: 982 samples, avg is 1034.9MB, max is 2766.6MB 
INFO Execution took 1m41.20436888s                
make: *** [Makefile:45: run-lint] Error 1

@alikhil
Copy link
Contributor Author

alikhil commented Sep 20, 2024

@denis256 could you recheck please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve HCL formatting to work with stdin and stdout
3 participants