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

Install terraform and packer for the linting job #82

Merged
merged 10 commits into from
Jul 19, 2021

Conversation

jsf9k
Copy link
Member

@jsf9k jsf9k commented Jul 9, 2021

🗣 Description

This pull request modifies the tasks for the lint job to install the versions of Terraform and Packer that we support.

Please note that this pull request requires that cisagov/setup-env-github-action#31 be merged first.

💭 Motivation and context

As described in #74, the Packer and Terraform pre-commit hooks leverage the corresponding executables; therefore, it makes sense to go ahead and install the particular versions of those executables that we support.

Resolves #74.

🧪 Testing

All pre-commit hooks and GitHub Actions pass.

✅ 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.
  • All new and existing tests pass.

@jsf9k jsf9k added blocked This issue or pull request is awaiting the outcome of another issue or pull request improvement This issue or pull request will add or improve functionality, maintainability, or ease of use labels Jul 9, 2021
@jsf9k jsf9k self-assigned this Jul 9, 2021
We should be doing this because the Packer and Terraform pre-commit
hooks leverage the corresponding executables; therefore, it makes
sense to go ahead and install the particular versions of those
executables that we support.  Also add support for optionally
debugging via tmate.

See also #74.
@jsf9k jsf9k force-pushed the improvement/install-tf-and-packer-for-linting branch from da12b0a to 106af21 Compare July 9, 2021 18:59
@jsf9k jsf9k marked this pull request as ready for review July 9, 2021 19:05
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of implementing this improvement! I have some suggestions and also some items I would like consensus on before approving.

.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
jsf9k and others added 3 commits July 9, 2021 22:43
There is no reason to create /usr/bin/terraform.  This is a vestige of
an earlier age.

Co-authored-by: Nick M. <[email protected]>
The Terraform installation does not destroy the existing system
Terraform installation, and neither should the Packer installation.

Co-authored-by: Nick M. <[email protected]>
Note that this change is dependent on the merging of
cisagov/setup-env-github-action#31.

Co-authored-by: Nick M. <[email protected]>
.github/workflows/build.yml Outdated Show resolved Hide resolved
Some variables defined in the go installation are used in the cache
task, so the go installation must happen first.

Co-authored-by: Nick M. <[email protected]>
@jsf9k jsf9k requested a review from mcdonnnj July 12, 2021 02:04
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Just some consistency suggestions. Thanks for your work on implementing this 🏆

.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
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.

Groovy baby, yeah! 🇬🇧

jsf9k added a commit to cisagov/skeleton-packer that referenced this pull request Jul 13, 2021
jsf9k added a commit to cisagov/skeleton-packer that referenced this pull request Jul 13, 2021
jsf9k added a commit to cisagov/skeleton-packer that referenced this pull request Jul 13, 2021
This mirrors what was done in cisagov/skeleton-generic#82.

Co-authored-by: Nick M. <[email protected]>
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

Belated change from reading Go documentation/release notes for something else I'm working on.

.github/workflows/build.yml Outdated Show resolved Hide resolved
As of [Go 1.16](https://tip.golang.org/doc/go1.16#go-command) the `GO111MODULE` environment variable defaults to `on` and `go get` has been deprecated for module installation.

Co-authored-by: Nick M. <[email protected]>
Copy link
Member

@mcdonnnj mcdonnnj left a comment

Choose a reason for hiding this comment

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

:shipit:

@mcdonnnj mcdonnnj merged commit 0e4fc41 into develop Jul 19, 2021
@mcdonnnj mcdonnnj deleted the improvement/install-tf-and-packer-for-linting branch July 19, 2021 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue or pull request is awaiting the outcome of another issue or pull request improvement This issue or pull request will add or improve functionality, maintainability, or ease of use
Projects
Development

Successfully merging this pull request may close these issues.

All GitHub Actions lint Workflows Should Use cisagov/setup-env-github-action
3 participants