hcllint: add a linting check for HCL files

[tgross]

Running make dev runs hclfmt, but this isn't checked as part of CI. That makes it possible to merge un-formatted HCL and Nomad jobspecs that later will make for dirty git staging areas when developers pull master. (Examples: #7584 #7011 #6724)

This adds a make hcllint target and wires it up to CI. Note that hclfmt doesn't return a non-0 exit code if it makes changes, which is why we can't just re-use make hclfmt here.

---

Looks like the following when run:

▶ make hcllint
6a7
>
17c18
< }
\ No newline at end of file
---
> }
6a7
>
11a13
>
15a18
>
19a23
>
26a31
>
33a39
>
42c48
< }
\ No newline at end of file
---
> }
6a7
>
8c9,10
<         sidecar_service {}
---
>         sidecar_service = {}
>
15c17
< }
\ No newline at end of file
---
> }
make: *** [hcllint] Error 1

▶ echo $?
2

▶ make hclfmt
--> Formatting HCL

▶ make hcllint

▶ echo $?
0

[notnoop]

It would be nice if the output emitted the name of files that need to be formatted, rather than plain diff.

When faced with a similar issue in managing protobuf generation being out of sync, I used another way in https://github.com/hashicorp/nomad/blob/v0.11.1/GNUmakefile#L205-L207 . Since we effectively always want to run generation and they are idempotent, we can just run generation and see if any files changed. In CI it will emit the failed files. When run locally, it will update and report the files, and you can just commit then without any additional steps. What do you think about that approach?

[shoenig]

At some point it might be worth to make a hcldiff.sh with some comments 😅

[shoenig]

Thanks for doing this, @tgross !!

[tgross]

I like @notnoop's suggested approach, and that'll read easier in the Makefile too. Fix incoming.

[tgross]

@notnoop how's this look? I ended up using git status -s to just get the files because otherwise the whole output invariably seemed to include nomad and I was getting false positives 😀

[notnoop]

Good point.. You can also do a git diff --name-only to print out only the file changes. We probably could do that with the pb.go check.

Also, I find using -e a tiny bit more readable, e.g. grep -e '\.hcl$' -e '\.nomad$'?

[notnoop]

I think we only need a single $ here.

Suggested change

	@if (git status -s | grep -q -e '\.hcl$$' -e '\.nomad$$'); then echo the following HCL files are out of sync; git status -s | grep -e '\.hcl$$' -e '\.nomad$$'; exit 1; fi
	@if (git status -s | grep -q -e '\.hcl$' -e '\.nomad$'); then echo the following HCL files are out of sync; git status -s | grep -e '\.hcl$' -e '\.nomad$'; exit 1; fi

[tgross]

I thought so at first but when I tested it, I realized Make wants you to escape the $, so the anchor was being ignored without that and the results were wrong.

[notnoop]

:til: Thanks for your testing :). Ship it!

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.