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

Use directories instead of individual files for the packer_validate hook #38

Merged
merged 2 commits into from
Sep 11, 2024

Conversation

mcdonnnj
Copy link
Member

@mcdonnnj mcdonnnj commented Sep 11, 2024

🗣 Description

This pull request updates the packer_validate hook to process unique directories instead of individual files. It also removes the pass_filenames option from the hook configurations for both the packer_fmt and the packer_validate hooks.

💭 Motivation and context

This change will allow us to support multiple *.pkr.hcl files for a configuration instead of having to cram everything into a single file to ensure it passes the packer_validate hook. This change is also necessary to support converting the Packer configurations in cisagov/cyhy_amis to use HCL2 instead of the legacy JSON format.

The pass_filesnames option is removed from both hooks because it is setting the option to the default value.

🧪 Testing

Automated tests pass. I tested this version of the hook against cisagov/skeleton-packer after decomposing the packer.pkr.hcl file.

Note

The local versions run below have some debug output present just for verification.

$ ls -l src/*.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj  593 Sep 10 17:56 src/amis.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 3779 Sep 10 17:57 src/build.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj   65 Sep 10 17:56 src/locals.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj  402 Sep 10 17:57 src/packer.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1467 Sep 10 17:56 src/variables.pkr.hcl

$ pre-commit run packer_fmt_local --all-files
Test the local version of packer_fmt.....................................Passed
- hook id: packer_fmt_local
- duration: 5.28s

Arguments: -check
Files: src/build.pkr.hcl src/amis.pkr.hcl src/locals.pkr.hcl src/packer.pkr.hcl
Arguments: -check
Files: src/variables.pkr.hcl

$ pre-commit run packer_validate_local --all-files
Test the local version of packer_validate................................Passed
- hook id: packer_validate_local
- duration: 4.9s

Arguments:
Files: src/build.pkr.hcl src/amis.pkr.hcl src/locals.pkr.hcl src/packer.pkr.hcl
Paths: src src src src
Unique paths: src
The configuration is valid.
Arguments:
Files: src/variables.pkr.hcl
Paths: src
Unique paths: src
The configuration is valid.

I also converted the Packer configurations in cisagov/cyhy_amis to HCL2 and used the local versions to verify the configuration:

$ ls -l packer/*.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj  519 Sep 10 15:35 packer/base_images.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1579 Sep 10 15:47 packer/bastion.json.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1592 Sep 10 15:47 packer/dashboard.json.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1596 Sep 10 15:47 packer/docker.json.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj   65 Sep 10 15:34 packer/locals.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1611 Sep 10 15:47 packer/mongo.json.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1573 Sep 10 15:47 packer/nessus.json.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1561 Sep 10 15:47 packer/nmap.json.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj  218 Sep 10 15:34 packer/packer.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj 1589 Sep 10 15:47 packer/reporter.json.pkr.hcl
-rw-r--r-- 1 mcdonnnj mcdonnnj  260 Sep 10 15:40 packer/variables.pkr.hcl

$ pre-commit run packer_fmt_local --all-files
Test the local version of packer_fmt.....................................Passed
- hook id: packer_fmt_local
- duration: 5.32s

Arguments: -write=true
Files: packer/locals.pkr.hcl packer/base_images.pkr.hcl packer/nessus.json.pkr.hcl packer/bastion.json.pkr.hcl
Arguments: -write=true
Files: packer/packer.pkr.hcl packer/variables.pkr.hcl packer/dashboard.json.pkr.hcl packer/mongo.json.pkr.hcl
Arguments: -write=true
Files: packer/docker.json.pkr.hcl packer/nmap.json.pkr.hcl packer/reporter.json.pkr.hcl

$ pre-commit run packer_validate_local --all-files
Test the local version of packer_validate................................Passed
- hook id: packer_validate_local
- duration: 11.75s

Arguments:
Files: packer/base_images.pkr.hcl packer/bastion.json.pkr.hcl packer/dashboard.json.pkr.hcl packer/docker.json.pkr.hcl packer/locals.pkr.hcl packer/mongo.json.pkr.hcl packer/nessus.json.pkr.hcl packer/nmap.json.pkr.hcl packer/packer.pkr.hcl packer/reporter.json.pkr.hcl packer/variables.pkr.hcl
Paths: packer packer packer packer packer packer packer packer packer packer packer
Unique paths: packer
The configuration is valid.

✅ Pre-approval checklist

  • This PR has an informative and human-readable title.
  • Changes are limited to a single goal - eschew scope creep!
  • All relevant type-of-change labels have been added.
  • I have read the CONTRIBUTING document.
  • These code changes follow cisagov code standards.
  • Tests have been added and/or modified to cover the changes in this PR.
  • All new and existing tests pass.

This change updates the `packer_validate` hook to run against
directories instead of against each file. This will correctly allow
configurations to be split across multiple files but be treated as a
single configuration when the hook is run.
@mcdonnnj mcdonnnj added breaking change This issue or pull request involves changes to existing functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use labels Sep 11, 2024
@mcdonnnj mcdonnnj self-assigned this Sep 11, 2024
@mcdonnnj mcdonnnj requested a review from a team September 11, 2024 01:41
.pre-commit-hooks.yaml Outdated Show resolved Hide resolved
After testing we do not appear to need to set `require_serial` for the
`packer_validate` hook. Additionally the default value for
`pass_filenames` is `true` so we do not need to explicitly set the same
value.

Co-authored-by: dav3r <[email protected]>
Copy link
Member

@dav3r dav3r left a comment

Choose a reason for hiding this comment

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

Can you please mention something in the PR description about removing the pass_filenames option?

@mcdonnnj mcdonnnj merged commit bd7429e into develop Sep 11, 2024
4 checks passed
@mcdonnnj mcdonnnj deleted the improvement/use_directories_not_files branch September 11, 2024 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This issue or pull request involves changes to existing functionality improvement This issue or pull request will add or improve functionality, maintainability, or ease of use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants