_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

Hi @Gowiem! Thanks for putting up this PR! I did want to weigh in with some unfortunate shortcomings with this approach:

• Trunk Check does not officially support running formatters on file A as input and file B as output, we expect A and B to be one and the same.

With that being said, there are a couple options here:

• Land this basically as-is. If it seems to be working for your team so far, that's great! However, we won't be able to provide some of the guarantees that check provides, i.e. with the upstream and HTL.
• Semantically, this should really be a Trunk Action, which you can set up with git- or file-triggers to automatically run and update the file in the background.

Let me know your thoughts, I'd like to do what I can to unblock your team

@TylerJang27 thanks for the info! Great stuff to know.

I just checked out running terraform-docs as a Trunk Action locally.

Pretty simple:

# Rest of trunk.yaml
actions:
  enabled:
    - terraform-docs
    - trunk-announce
    - trunk-check-pre-push
    - trunk-fmt-pre-commit
    - trunk-upgrade-available
  definitions:
    - id: terraform-docs
      triggers:
        - git_hooks: [pre-push, pre-commit]
      run: terraform-docs markdown --output-file=README.md .

This does the trick and AFTER my commit, I see changes in README.md if I updated something that would trigger those changes.

I could see a path forward with that... but it seems less capable than a linter plugin from the examples I am looking at in actions/. As a result I'm wondering the following:

1. Can we make it so that if the action is triggered on pre-commit, it rejects the commit if there are changes new from terraform-docs?
2. Can we configure it to run recursively for all TF modules? e.g. If I change some path in my project like ./root-modules/rds-cluster/variables.tf I realize I can trigger it with triggers.files: ["*.tf"] or something similar, but will it run in ./root-modules/rds-cluster/?

Can I bug you to expand on these guarantees so I can better understand option 1?

we won't be able to provide some of the guarantees that check provides, i.e. with the upstream and HTL.

@Gowiem here's some more info:

1. You could set run: terraform-docs markdown --output-file=README.md . && git add README.md, which should achieve the desired effect. If you just wanted to block, you could use --output-check
2. You may be able to do this with the terraform-docs --recursive flag, but based on your question I assume you've already tried that. You could write a wrapper script to do this, but it's hacky and I'll let you weigh the tradeoffs of that
3. This requires understanding a bit more about Trunk's implementation/execution model.
   1. In order to run Hold-The-Line, trunk runs on your workspace changes and a sandboxed version of the upstream (main). This sandbox only snapshots input files A (not output B) before and after running the formatter, so you won't get any results when running on the upstream. You'll still get workspace results, which is probably why the linter is working for you currently.
   2. Similarly, when running git-hooks, e.g. pre-commit, trunk runs on a sandboxed version of the index you're committing and a sandboxed upstream. In this case, you won't get results for either.
   3. You could hack around this by forcing Trunk to run on the workspace all the time, but this could interfere with other linters (e.g. markdownlint) detecting that the file it's running on has changed. This would also not auto-stage your changes when running with pre-commit. This would look something like:

lint:
  definitions:
    - name: terraform-docs
      files: [terraform]
      main_tool: terraform-docs
      description: Generate terraform + tofu module documentation
      commands:
        - name: terraform-docs
          output: pass_fail
          formatter: true
          target: ${parent}
          run: bash -c 'terraform-docs markdown --output-file=README.md ${WORKSPACE}/${target}'
          read_output_from: stdout
          success_codes: [0, 1]
          disable_upstream: true
      environment:
        - name: PATH
          list: ["${linter}", "${env.PATH}"]
        - name: WORKSPACE
          # there is a bug in our template resolver requiring us to pass this as an env var and resolve in bash
          value: "${workspace}"
      ...

Let me know what you think. I personally recommend that Trunk Actions route, although I recognize that this exposes some limitations of our meta-linter.

@TylerJang27 gotcha gotcha. Thanks for digging into terraform-docs and trying to explain + make this work -- I really appreciate this help!

I am not fully able to grok all the internals of the problem with the HTL or pre-commit linter issues, but I don't think I need to. You're basically saying "There be dragons, don't do that", which I do get.

So I just played around with the following, which while I don't necessarily like... I believe it'll work. Here's the setup:

# .trunk/trunk.yaml
# ...
actions:
  enabled:
    - no-readme-changes-check
    - terraform-docs-check
    - terraform-docs
    - trunk-announce
    - trunk-check-pre-push
    - trunk-fmt-pre-commit
    - trunk-upgrade-available
  definitions:

    # Check for any README.md changes that are either staged or unstaged. Don't allow a push if there are any changes.
    # We run this check before terraform-docs-check because --output-check will succeed even if README.md is not checked in.
    - id: no-readme-changes-check
      triggers:
        - git_hooks: [pre-push]
      run: git status --porcelain | grep -qE '^(.M|M).*README\.md$' && echo "README.md changes found" && exit 1 || exit 0

    # --output-check does not update README.md, it only passes judgement on the content of README.md (regardless of status in git)
    - id: terraform-docs-check
      triggers:
        - git_hooks: [pre-push]
      run: terraform-docs . --output-check

    - id: terraform-docs
      triggers:
        - git_hooks: [pre-commit]
      run: terraform-docs .

# .terraform-docs.yml
formatter: markdown

recursive:
  enabled: false
  path: modules
  include-main: true

output:
  file: README.md
  mode: inject
  template: |-
    <!-- BEGIN_TF_DOCS -->
    {{ .Content }}
    <!-- END_TF_DOCS -->

I'm willing to try all that out by rolling this config out across a few of internal + client repos to see how all this works in practice. But this solution is definitely not ideal: it requires a good bit of copy / pasta and feels custom.

My goal with this PR was to get my team's process down to simply adding another linter (an action is fine too) so we can ditch terraform-pre-commit internally. terraform-pre-commit requires a separate dependency, separate workflow to activate per-repo, and there is overlap with what trunk is providing so it's painful.

It's not immediately apparent to me if I could accomplish the intended workflow by creating an action plugin. It feels like I can't from looking at the limited examples -- would you agree? If not, mind pointing me in the right direction? And if you do agree, should we try to create some GH issues that will improve the capabilities of actions so that this sort of workflow could be accomplished?

@TylerJang27 nevermind -- I took a 2nd look at the existing actions and it seems there is likely a path forward using custom logic like what the npm-check action does.

I'm going to retract my "It's not immediately apparent to me if I could accomplish the intended workflow by creating an action plugin". I'll give an action plugin a go and report back 👍

@Gowiem that action seems like your best bet. A couple other reasonable options:

1. A Trunk Action that runs the updates and then stages the changes, i.e. run: terraform-docs . && git add README.md
2. A Trunk Action that runs the updates, and a linter (not formatter) that uses the definition I provided above and uses --output-check, i.e. run: bash -c 'terraform-docs markdown --output-file=README.md ${WORKSPACE}/${target}' --output-check

Closing this PR in favor of #966. Will continue the terraform-docs functionality there.