Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add fly:bats rake task for running BATs from a local branch #2753
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
base: main
Are you sure you want to change the base?
Add fly:bats rake task for running BATs from a local branch #2753
Changes from all commits
d13c35c67d7b23File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Checksum verification will always fail — filename mismatch breaks every run.
The zip is downloaded to
${TMP_TF_DIR}/terraform.zip(line 39), but the verification step greps the SHA256SUMS line forterraform_${TERRAFORM_VERSION}_linux_amd64.zipand pipes it tosha256sum -c -/shasum -c -from insideTMP_TF_DIR. That checker looks for a file with that exact name relative to cwd, which never exists (onlyterraform.zipdoes). Withset -euo pipefail, this failure aborts the whole task on every fresh container run — the new checksum step (added to fix the prior "unverified Terraform download" finding) now breaks Terraform installation entirely.🐛 Proposed fix
echo "--- Installing terraform ${TERRAFORM_VERSION} ---" TMP_TF_DIR="$(mktemp -d /tmp/terraform-install-XXXXXX)" + TF_ZIP="terraform_${TERRAFORM_VERSION}_linux_amd64.zip" curl -sSL \ "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_linux_amd64.zip" \ - -o "${TMP_TF_DIR}/terraform.zip" + -o "${TMP_TF_DIR}/${TF_ZIP}" curl -sSL \ "https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_SHA256SUMS" \ -o "${TMP_TF_DIR}/SHA256SUMS" if command -v sha256sum &>/dev/null; then - (cd "${TMP_TF_DIR}" && grep "terraform_${TERRAFORM_VERSION}_linux_amd64.zip" SHA256SUMS | sha256sum -c -) + (cd "${TMP_TF_DIR}" && grep "${TF_ZIP}" SHA256SUMS | sha256sum -c -) elif command -v shasum &>/dev/null; then - (cd "${TMP_TF_DIR}" && grep "terraform_${TERRAFORM_VERSION}_linux_amd64.zip" SHA256SUMS | shasum -a 256 -c -) + (cd "${TMP_TF_DIR}" && grep "${TF_ZIP}" SHA256SUMS | shasum -a 256 -c -) fi - unzip -qo "${TMP_TF_DIR}/terraform.zip" -d /usr/local/bin terraform + unzip -qo "${TMP_TF_DIR}/${TF_ZIP}" -d /usr/local/bin terraform📝 Committable suggestion
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win
Avoid passing the GCP service-account key via
-varon the command line.terraform apply/terraform destroyreceivegcp_credentials_json(the full service-account JSON) as a CLI argument. That puts the secret into the process argv, readable via/proc/<pid>/cmdlineby anything else in the container. Prefer Terraform'sTF_VAR_<name>environment-variable convention for secret inputs, which the CLI reads directly without exposing them as arguments.♻️ Proposed fix
Apply the same pattern to the
terraform destroycall inteardown.Also applies to: 144-149
🤖 Prompt for AI Agents
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
No-op regex substitution.
.sub(/\.git\z/, '.git')replaces a trailing.gitwith the identical string.git, so it never changes the value — this line has no effect regardless of input. If the intent was to normalize the URL (e.g. strip or guarantee a.gitsuffix), this doesn't do it and should be fixed or removed to avoid misleading future readers.♻️ Proposed fix (strip `.git` suffix)
repo = `git -C .. remote get-url origin`.strip .sub(/\Agit@github\.com:/, 'https://github.com/') - .sub(/\.git\z/, '.git') + .sub(/\.git\z/, '')📝 Committable suggestion
🤖 Prompt for AI Agents
Uh oh!
There was an error while loading. Please reload this page.