Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[terraform/utils] Lint terraform variables generation #1102

Merged
merged 18 commits into from
Sep 17, 2024

Conversation

ethangraham2001
Copy link
Contributor

@ethangraham2001 ethangraham2001 commented Sep 6, 2024

This PR adds a lint stage for checking that locally-stored .gen.tf files are the same as the generated output of calling variables.py. This is done by adding a new --lint flag to variables.py that checks for differences between the generated output the local files without writing anything out to disk - variables.py --lint will exit with status code 0 on success, and 1 on failure. The terraform-lint section of the top-level makefile is updated to call this accordingly.

This is intended to be used as a CI check.

Relates to issue #1084

Copy link

linux-foundation-easycla bot commented Sep 6, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@ethangraham2001 ethangraham2001 changed the title [terraform][utils] Diff tool for terrform output [terraform/utils] Diff tool for terraform output Sep 6, 2024
Copy link
Contributor

@barroco barroco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ethangraham2001 , thank you for this PR. Please find small comments inline.

if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument("--diff", action="store_true")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an argument help="..." and describe the action and make sure the exit code is mentionned.

deploy/infrastructure/utils/variables.py Outdated Show resolved Hide resolved
deploy/infrastructure/utils/variables.py Show resolved Hide resolved
deploy/infrastructure/utils/variables.py Show resolved Hide resolved
if __name__ == "__main__":
parser = argparse.ArgumentParser()
parser.add_argument("--diff", action="store_true")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add an argument help="..." and describe the action and make sure the exit code is mentionned.

@barroco barroco changed the title [terraform/utils] Diff tool for terraform output [terraform/utils] Diff tool for terraform variables generation output Sep 6, 2024
@barroco
Copy link
Contributor

barroco commented Sep 6, 2024

Could you please add this command to Makefile target: terraform-lint
This will add it to the CI checks.

Makefile Outdated Show resolved Hide resolved
deploy/infrastructure/utils/variables.py Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
deploy/infrastructure/utils/variables.py Outdated Show resolved Hide resolved
deploy/infrastructure/utils/variables.py Outdated Show resolved Hide resolved
@ethangraham2001 ethangraham2001 changed the title [terraform/utils] Diff tool for terraform variables generation output [terraform/utils] Lint terraform variables generation Sep 9, 2024
Copy link
Contributor

@barroco barroco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ethangraham2001 !

@barroco barroco merged commit 54fce70 into interuss:master Sep 17, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants