antonbabenko / pre-commit-terraform

pre-commit git hooks to take care of Terraform configurations 🇺🇦
MIT License
3.21k stars 540 forks source link

wrong arguments passed to terraform-docs (v0.10.0 and v0.10.1 #154

Closed aidan-melen closed 3 years ago

aidan-melen commented 4 years ago

Related: https://github.com/terraform-docs/terraform-docs/issues/324

PASS

# .pre-commit-config.yaml
repos:
- repo: git://github.com/antonbabenko/pre-commit-terraform
  rev: v1.43.0
  hooks:
    - id: terraform_docs
      language: docker_image
      entry: quay.io/terraform-docs/terraform-docs:0.9.1

result

$ git add -A && pre-commit run
Terraform docs...........................................................Passed

FAIL

# .pre-commit-config.yaml
repos:
- repo: git://github.com/antonbabenko/pre-commit-terraform
  rev: v1.43.0
  hooks:
    - id: terraform_docs
      language: docker_image
      entry: quay.io/terraform-docs/terraform-docs:0.10.0

result

$ git add -A && pre-commit run
Terraform docs...........................................................Failed
- hook id: terraform_docs
- exit code: 1

Error: accepts at most 1 arg(s), received 5

FAIL

# .pre-commit-config.yaml
repos:
- repo: git://github.com/antonbabenko/pre-commit-terraform
  rev: v1.43.0
  hooks:
    - id: terraform_docs
      language: docker_image
      entry: quay.io/terraform-docs/terraform-docs:0.10.1

result

$ git add -A && pre-commit run
Terraform docs...........................................................Failed
- hook id: terraform_docs
- exit code: 1

Error: accepts at most 1 arg(s), received 5

context

The maintainers of terraform-docs upgraded the cli to use the popular cobra package in version v0.10.0 and for some reason -- that change did not play nice with the terraform_doc.sh. I don't think the new release of terraform-docs is the issue because when I run it natively I don't run into any issues e.g.

$ brew install terraform-docs
...
$ terraform-docs markdown .                                                                                                                                                                                                               HEAD
## Requirements

| Name | Version |
|------|---------|
| terraform | >= 0.13.0 |
| aws | >= 2.70.0 |

## Providers

| Name | Version |
|------|---------|
| aws | >= 2.70.0 |

## Inputs

| Name | Description | Type | Default | Required |
|------|-------------|------|---------|:--------:|
| ec2\_instance\_type | AWS EC2 instance type. | `string` | `"t2.small"` | no |

## Outputs

| Name | Description |
|------|-------------|
| instance\_id | EC2 instance id |
1oglop1 commented 4 years ago

I'm not sure if this deserves it's own issue but I find argument parsing of terraform-docs broken in general. I suspect this issue is of parsing arguments is related to the usage of https://github.com/agriffis/pure-getopt/ in lib_getopt file. After reading some SO posts it says that normal getopts stops parsing arguments after the first non-option argument. And since terraform-docs are using sub-commands which are non-option arguments getopt or any other version of it is probably confused.

https://github.com/antonbabenko/pre-commit-terraform/blob/84374f64a05e53a23d8e23177db09ae964434f64/terraform_docs.sh#L30-L31

This works

repos:

- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: v1.43.0
  hooks:
    - id: terraform_docs
      args:
        - '--args=--hide-all'

The above snippet only works because --hide-all is a global argument.

This does not

repos:

- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: v1.43.0
  hooks:
    - id: terraform_docs
      args:
        - '--args=md document

Neither this

repos:

- repo: https://github.com/antonbabenko/pre-commit-terraform
  rev: v1.43.0
  hooks:
    - id: terraform_docs
      args:
        - 'md --indent=3 document'

While these 2 do not work because md and document are sub-commands (non-option arguments)

This issue also prevents more granular configuration of terraform-docs without its config file.

So the challenge left here is to separate .tf files from a string like this --hide-all md document main.tf output.tf into args --hide-all md document and files main.tf output.tf (which is basically what parse_cmdline_ is trying to do)

MaxymVlasov commented 3 years ago

Bug for docker image confirmed

repos:
- repo: git://github.com/antonbabenko/pre-commit-terraform
  rev: v1.50.0
  hooks:
    - id: terraform_docs
      language: docker_image
      entry: quay.io/terraform-docs/terraform-docs:0.15.0

The good news is that pre-commit-terraform with locally installed terraform-docs work fine.

$ terraform-docs -v
terraform-docs version v0.15.0 f3b6eab linux/amd64
repos:
- repo: git://github.com/antonbabenko/pre-commit-terraform
  rev: v1.50.0
  hooks:
    - id: terraform_docs
MaxymVlasov commented 3 years ago

Looks like there are 2 issues. First - #155 (introduce breaking-changes) Second - not supported docker_image at all. Need to check after fix first issue

MaxymVlasov commented 3 years ago

Tools versions used for the tests:

pre-commit 2.15.0
Terraform v1.0.8
Python 3.8.5
Python 3.8.5
checkov 2.0.395
terraform-docs version v0.15.0 f3b6eab linux/amd64
terragrunt SKIPPED
terrascan version: v1.10.0
TFLint version 0.32.1
+ ruleset.aws (0.7.2)
tfsec 0.58.6

The first example now not works during hide-all option was removed from terraform_docs config

The second example has problems, see usage examples

The third example works fine, as expected.

Fixed Example 2 ```bash repos: - repo: git://github.com/antonbabenko/pre-commit-terraform rev: v1.52.0 hooks: - id: terraform_docs args: - md document ```
Example 3 ```bash repos: - repo: git://github.com/antonbabenko/pre-commit-terraform rev: v1.52.0 hooks: - id: terraform_docs args: - md --indent=3 document ```

Conclusion

Looks like there are 2 issues. First - #155 (introduce breaking-changes) Second - not supported docker_image at all. Need to check after fix first issue

The first issue was effectively fixed.

The second issue is related to use terraform_docs as docker image, I'm right @aidan-melen? docker_image option works only for local hooks.

So, I propose to use a fully Dockerized pre-commit-terraform or setup repo local hook