CI/Linters Need Some Love

YakDriver commented 2 years ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

The role CI plays in reducing risk and improving code quality cannot be overstated. We need more and continuing checks. However, we have (at least) three problems that would be excellent to address:

.semgrep.yml is getting bloated - semgrep is fast and is our preferred way going forward for analysis. However, the config file needs to be managed. We could break it into multiple files. We need to start checking off exclusions by making fixes to clean up the file.
Acceptance Test Linting / validate-terraform is slow - This check often takes over 30 minutes. I suspect we can get the same benefit with a faster approach (semgrep?). (Acceptance Test Linting / validate-terraform (pull_request) Successful in 31m)
awsproviderlint is feeling legacy - We used this tool to great effect. However, it is vendored so there are many extraneous files included with the provider. In addition, for maintainability, it would be better to simplify the toolset we're using. We should avoid having to understand and support too many tools.

References

18376
19039
15014

gdavison commented 2 years ago

For validate-terraform, it's downloading the terraform executable and providers for each Terraform block found. We can probably cache both of those to speed it up significantly.

At a minimum, we should try removing vendoring from awsproviderlint and removing AWSR002, since we've had to disable it since adding default tags

github-actions[bot] commented 11 months ago

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

obounaim commented 2 months ago

In addition to the current linters, would it make sense to configure git pre-commit hook checks ?

I run the command bellow, it detected a lot of issues related to mixed line endings and other findings :

repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
  rev: v4.5.0
  hooks:
    - id: trailing-whitespace
    - id: end-of-file-fixer
    - id: mixed-line-ending

pre-commit run --all-files

A total of 6k file was fixed.

hashicorp / terraform-provider-aws