8 ms·

I wouldn't want hooks modifying the code. They should be only approve/reject. Ideally landlock rules would give them only ro access to repo dir

sgarland 8mo ago
It depends. I wrote a pre-commit hook (in shell, not precommit the tool) at a previous job that ran terraform fmt on any staged files (and add the changes to the commit) because I was really tired of having people push commits that would then fail for trivial things. It was overrideable with an env var. IMO if there’s a formatting issue, and the tool knows how it should look, it should fix it for you.
- pxc 8mo ago
  The standard way for this with current tools is to have the formatter/linter make the changes but exit with a non-zero status, failing the hook. Then the person reviews the changes, stages, and commits. (That's what our setup currently has `tofu fmt` do.) But if you don't want to have hooks modify code, in a case like this you can also just use `tofu validate`. Our setup does `tflint` and `tofu validate` for this purpose, neither of which modifies the code. This is also, of course, a reasonable place to have people use `tofu plan`. It you want bad code to fail as quickly as possible, you can do: tflint -> tfsec -> tofu validate -> tofu plan That'll catch everything Terraform will let you catch before deploy time— most of it very quickly— without modifying any code.
  - sgarland 7mo ago
    > make the changes but exit with a non-zero status That's reasonable. My personal (and that of my team at the time) take was that I was willing to let formatting - and only formatting - be auto-merged into the commit, since that isn't going to impact logic. For anything else, though, I would definitely want to let submitter review the changes.
jdxcode 8mo ago
ok but I was replying to a comment about a tool which advertises precisely that feature
timhh 7mo ago
It's going to be optional - the hooks will always fix the code if they can, but then you can supply a `--no-fix` flag (or config) if you want to tell it to not actually apply those changes to the real filesystem. It doesn't need Landlock because WASI already provides a VFS.

sgarland 8mo ago

It depends. I wrote a pre-commit hook (in shell, not precommit the tool) at a previous job that ran terraform fmt on any staged files (and add the changes to the commit) because I was really tired of having people push commits that would then fail for trivial things. It was overrideable with an env var. IMO if there’s a formatting issue, and the tool knows how it should look, it should fix it for you.

pxc 8mo ago
The standard way for this with current tools is to have the formatter/linter make the changes but exit with a non-zero status, failing the hook. Then the person reviews the changes, stages, and commits. (That's what our setup currently has `tofu fmt` do.) But if you don't want to have hooks modify code, in a case like this you can also just use `tofu validate`. Our setup does `tflint` and `tofu validate` for this purpose, neither of which modifies the code. This is also, of course, a reasonable place to have people use `tofu plan`. It you want bad code to fail as quickly as possible, you can do: tflint -> tfsec -> tofu validate -> tofu plan That'll catch everything Terraform will let you catch before deploy time— most of it very quickly— without modifying any code.
- sgarland 7mo ago
  > make the changes but exit with a non-zero status That's reasonable. My personal (and that of my team at the time) take was that I was willing to let formatting - and only formatting - be auto-merged into the commit, since that isn't going to impact logic. For anything else, though, I would definitely want to let submitter review the changes.

jdxcode 8mo ago

ok but I was replying to a comment about a tool which advertises precisely that feature

timhh 7mo ago

It's going to be optional - the hooks will always fix the code if they can, but then you can supply a `--no-fix` flag (or config) if you want to tell it to not actually apply those changes to the real filesystem. It doesn't need Landlock because WASI already provides a VFS.