Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@yhakbar has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 Walkthrough""" WalkthroughThis update removes the Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Setup Script
participant Test Runner
GitHub Actions->>Setup Script: Run setup script (engine/gcp/sops/ssh/generate-secrets/run-setup-scripts)
Setup Script-->>GitHub Actions: Environment prepared
GitHub Actions->>Test Runner: Run tests (unit/integration/pre-commit)
Test Runner-->>GitHub Actions: Test results (JUnit XML/artifacts)
GitHub Actions->>GitHub Actions: Upload artifacts & display summary
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
yhakbar
force-pushed
the
feat/adding-integration-tests
branch
from
a9e328d to
c35e08a
Compare
yhakbar
force-pushed
the
feat/adding-integration-tests
branch
from
928e948 to
73e2929
Compare
yhakbar
force-pushed
the
feat/adding-integration-tests
branch
from
6462179 to
7a7bd23
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (14)
.github/scripts/setup/aws.sh (1)
1-4: Add environment variable validation.
The script enters strict mode but doesn’t confirm that AWS credentials are present. To fail fast, consider adding checks such as:: "${AWS_ACCESS_KEY_ID?AWS_ACCESS_KEY_ID is required}" : "${AWS_SECRET_ACCESS_KEY?AWS_SECRET_ACCESS_KEY is required}".github/scripts/setup/sops.sh (1)
1-6: Check forgpgavailability.
To provide a clearer error when GPG is missing, add:command -v gpg >/dev/null 2>&1 || { echo "gpg not found. Please install GPG."; exit 1; }This helps avoid confusing failures during the key import.
.github/scripts/setup/gcp.sh (1)
3-6: Validate and secure GCP service key.
Add a check to ensureGCLOUD_SERVICE_KEYis set and restrict the credentials file permissions:set -euo pipefail +: "${GCLOUD_SERVICE_KEY?GCLOUD_SERVICE_KEY is required for GCP auth}" echo "$GCLOUD_SERVICE_KEY" > "${HOME}/gcloud-service-key.json" +chmod 600 "${HOME}/gcloud-service-key.json" export GOOGLE_APPLICATION_CREDENTIALS="${HOME}/gcloud-service-key.json".github/scripts/setup/engine.sh (1)
3-13: Improve robustness and cleanup for engine download.
Consider using a temporary directory, validating required tools, and cleaning up after download:-set -euo pipefail -export TOFU_ENGINE_VERSION="v0.0.16" -export REPO="gruntwork-io/terragrunt-engine-opentofu" -export ASSET_NAME="terragrunt-iac-engine-opentofu_rpc_${TOFU_ENGINE_VERSION}_linux_amd64.zip" -pushd . -# Download the engine binary -mkdir -p /tmp/engine -cd /tmp/engine -wget -O "engine.zip" "https://github.com/${REPO}/releases/download/${TOFU_ENGINE_VERSION}/${ASSET_NAME}" -unzip -o "engine.zip" -popd +set -euo pipefail +command -v wget >/dev/null 2>&1 || { echo "wget is required"; exit 1; } +command -v unzip >/dev/null 2>&1 || { echo "unzip is required"; exit 1; } +TMP_DIR=$(mktemp -d) +trap 'rm -rf "$TMP_DIR"' EXIT +pushd "$TMP_DIR" +# Download the engine binary +wget -qO engine.zip "https://github.com/${REPO}/releases/download/${TOFU_ENGINE_VERSION}/${ASSET_NAME}" +unzip -o engine.zip +popdThis approach isolates files in a temp folder and removes them on exit.
.github/scripts/setup/tflint.sh (1)
1-1: Add descriptive header.
Consider adding a top‐of‐file comment explaining the purpose of this script, e.g.:# Configure TFLint version in mise.toml for CI integration tests.mise.cicd.toml (1)
1-7: Lock CI tool versions for reproducibility.
Pinning versions for critical tools is excellent. One consideration: setting"go:golang.org/x/tools/cmd/goimports" = "latest"could introduce non-determinism. If strict reproducibility is required, you may want to specify an exact version.
Please also verify that these versions are in sync with the.github/scripts/setup/*scripts.test/integration_units_reading_test.go (2)
1-2: Backward-compatible build tags.
You’ve added the//go:build sopsdirective. If this codebase supports Go versions <1.17, consider adding the legacy tag on the next line:// +build sopsto maintain compatibility with older tooling.
29-29: Align file naming with test content.
Renaming the function toTestSOPSUnitsReadingclarifies that it’s a SOPS‐specific test. For consistency, you may want to rename the file tointegration_sops_units_reading_test.go, matching other SOPS test filenames.internal/cas/race_test.go (1)
46-63: Consider enhancing test validations.While the test correctly verifies that no errors occur during the get operation and that the destination matches, consider adding more assertions to verify the fetched repository structure (e.g., checking for specific files or directories).
assert.Equal(t, tmpDir, res.Dst) + + // Verify that essential files were fetched correctly + essentialFiles := []string{".github", "go.mod", "README.md"} + for _, file := range essentialFiles { + assert.FileExists(t, filepath.Join(tmpDir, file)) + }.github/workflows/integration-test.yml (5)
90-119: Double-quote matrix expansions in the secrets-generation script.
Unquoted expansions can lead to word splitting or globbing. Please update as follows:- for SECRET in ${{ join(matrix.integration.secrets, ' ') }}; do + for SECRET in "${{ join(matrix.integration.secrets, ' ') }}"; doAlso quote the redirect path:
- echo "export SCRIPT_PATHS='${{ matrix.integration.setup_scripts }}'" > $ENV_FILE + echo "export SCRIPT_PATHS='${{ matrix.integration.setup_scripts }}'" > "$ENV_FILE"🧰 Tools
🪛 actionlint (1.7.4)
92-92: shellcheck reported issue in this script: SC2086:info:2:72: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:7:78: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:9:84: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:11:92: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:13:86: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:14:95: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:16:90: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:18:88: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:20:92: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:22:84: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type array
(expression)
🪛 YAMLlint (1.35.1)
[error] 95-95: trailing spaces
(trailing-spaces)
95-95: Remove trailing whitespace.
Lines 95 and 125 contain trailing spaces, which cause YAML lint errors. Please delete the extra spaces at the end of those lines.Also applies to: 125-125
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 95-95: trailing spaces
(trailing-spaces)
121-134: Quote script path expansions in the Setup step.
To guard against splitting if a path contains spaces, wrap the expansion in quotes:- for SCRIPT in ${{ join(matrix.integration.setup_scripts, ' ') }}; do + for SCRIPT in "${{ join(matrix.integration.setup_scripts, ' ') }}"; do🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 125-125: trailing spaces
(trailing-spaces)
155-164: Quotego testarguments for stability.
EnsureTAGSandRUNparameters are passed intact even if they include special characters:- go test -v ${TAGS:+-tags "$TAGS"} ${RUN:+-run "$RUN"} "${TARGET}" + go test -v ${TAGS:+-tags "${TAGS}"} ${RUN:+-run "${RUN}"} "${TARGET}"
90-134: Consider using$GITHUB_ENVover a custom.env.secretsfile.
Writing secrets directly to$GITHUB_ENVis more idiomatic and auto-injects them into subsequent steps, removing the need tosourcea separate file:- echo "export AWS_ACCESS_KEY_ID='${{ secrets.AWS_ACCESS_KEY_ID }}'" >> $ENV_FILE + echo "AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }}" >> $GITHUB_ENV🧰 Tools
🪛 actionlint (1.7.4)
92-92: shellcheck reported issue in this script: SC2086:info:2:72: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:7:78: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:9:84: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:11:92: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:13:86: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:14:95: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:16:90: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:18:88: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:20:92: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:22:84: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type array
(expression)
🪛 YAMLlint (1.35.1)
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.circleci/config.yml(0 hunks).github/scripts/setup/aws.sh(1 hunks).github/scripts/setup/engine.sh(1 hunks).github/scripts/setup/gcp.sh(1 hunks).github/scripts/setup/sops.sh(1 hunks).github/scripts/setup/ssh.sh(1 hunks).github/scripts/setup/tflint.sh(1 hunks).github/workflows/base-test.yml(3 hunks).github/workflows/build-no-proxy.yml(0 hunks).github/workflows/integration-test.yml(1 hunks).github/workflows/precommit.yml(1 hunks).pre-commit-config.yaml(1 hunks)internal/cas/getter_ssh_test.go(2 hunks)internal/cas/git.go(2 hunks)internal/cas/race_test.go(1 hunks)mise.cicd.toml(1 hunks)test/fixtures/download/remote-ref/terragrunt.hcl(1 hunks)test/fixtures/source-map/slashes-in-ref/terragrunt.hcl(1 hunks)test/integration_sops_test.go(4 hunks)test/integration_tflint_test.go(2 hunks)test/integration_units_reading_test.go(1 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/build-no-proxy.yml
- .circleci/config.yml
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
**/*.go: Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
internal/cas/getter_ssh_test.gotest/integration_units_reading_test.gointernal/cas/git.gointernal/cas/race_test.gotest/integration_tflint_test.gotest/integration_sops_test.go
🧬 Code Graph Analysis (1)
test/integration_sops_test.go (2)
test/helpers/package.go (5)
CleanupTerraformFolder(719-726)CopyEnvironment(83-96)RunTerragrunt(799-803)RunTerragruntCommandWithOutput(827-831)TerraformOutput(77-81)util/file.go (1)
JoinPath(472-474)
🪛 actionlint (1.7.4)
.github/workflows/precommit.yml
38-38: property "os" is not defined in object type {}
(expression)
43-43: property "os" is not defined in object type {}
(expression)
44-44: property "arch" is not defined in object type {}
(expression)
.github/workflows/integration-test.yml
92-92: shellcheck reported issue in this script: SC2086:info:2:72: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:7:78: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:9:84: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:11:92: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:13:86: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:14:95: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:16:90: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:18:88: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:20:92: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: shellcheck reported issue in this script: SC2086:info:22:84: Double quote to prevent globbing and word splitting
(shellcheck)
92-92: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type array
(expression)
🪛 YAMLlint (1.35.1)
.github/workflows/integration-test.yml
[error] 95-95: trailing spaces
(trailing-spaces)
[error] 125-125: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test (macos)
- GitHub Check: Pull Request has non-contributor approval
🔇 Additional comments (32)
.pre-commit-config.yaml (2)
3-3: Approve version bump.
Bumping therevtov0.1.29aligns with the new GHA workflow and ensures you pick up the latest fixes.
5-6:Details
✅ Verification successful
Verify the new hook exists in the updated revision.
Ensure that thetofu-fmthook is available in thev0.1.29revision of thegruntwork-io/pre-commitrepo.
🏁 Script executed:
#!/bin/bash # Verify that `tofu-fmt` is defined in the hooks manifest at rev v0.1.29 curl -sL https://raw.githubusercontent.com/gruntwork-io/pre-commit/v0.1.29/.pre-commit-hooks.yaml | grep -A2 'id: tofu-fmt'Length of output: 240
tofu-fmt hook confirmed in v0.1.29
Thetofu-fmthook is defined in thegruntwork-io/pre-commitrepo at revisionv0.1.29, matching your.pre-commit-config.yamlentry. No further action required.test/fixtures/source-map/slashes-in-ref/terragrunt.hcl (1)
2-2:Details
✅ Verification successful
Ensure test fixture path correctness.
Thesourcenow points tohello-world-no-remote, which matches the updated fixture path. Please verify that the directorytest/fixtures/download/hello-world-no-remoteexists and contains the expected Terraform module for this integration test.
Run the following to confirm:
🏁 Script executed:
#!/bin/bash fd hello-world-no-remote test/fixtures/downloadLength of output: 95
Verified fixture path
The directorytest/fixtures/download/hello-world-no-remote/exists as expected. No further action needed.test/fixtures/download/remote-ref/terragrunt.hcl (1)
6-6:Details
✅ Verification successful
Confirm Git reference and path.
The updatedsourceURL includeshello-world-no-remoteand?ref=v0.77.22. Verify that the tagv0.77.22exists in thegruntwork-io/terragruntrepository and that the pathtest/fixtures/download/hello-world-no-remoteis present in that revision.
🏁 Script executed:
#!/bin/bash git ls-remote --tags git@github.com:gruntwork-io/terragrunt.git | grep 'refs/tags/v0.77.22'Length of output: 162
To verify both the tag and the path, let’s try over HTTPS and then shallow-clone that revision to check the directory:
🏁 Script executed:
#!/bin/bash # Check that tag v0.77.22 exists git ls-remote --tags https://github.com/gruntwork-io/terragrunt.git | grep 'refs/tags/v0.77.22' # Shallow-clone just that tag and verify the fixture path git clone --depth 1 --branch v0.77.22 https://github.com/gruntwork-io/terragrunt.git temp-repo if [ -d temp-repo/test/fixtures/download/hello-world-no-remote ]; then echo "PATH_EXISTS" else echo "PATH_MISSING" fiLength of output: 1021
Git tag and source path verified
- Tag
v0.77.22exists in thegruntwork-io/terragruntrepo.- Directory
test/fixtures/download/hello-world-no-remoteis present at that revision.No further changes needed.
.github/scripts/setup/tflint.sh (2)
3-3: Enable strict error handling.
Great use ofset -euo pipefailto ensure failures are caught early.
5-5: Pin TFLint version in CI configuration.
Appendingtflint = "0.47.0"ensures consistency with the integration-test workflow matrix entry. Confirm alignment with other tool versions inmise.cicd.toml..github/scripts/setup/ssh.sh (1)
1-9: Good implementation of the SSH setup script.This script properly sets up SSH by:
- Using appropriate error handling with
set -euo pipefail- Validating the required environment variable
- Setting correct permissions (600) for the private key
The script follows shell scripting best practices and security standards for handling SSH keys.
test/integration_tflint_test.go (2)
132-132: Function rename for consistent test naming conventions.The function has been properly renamed from
TestExternalTflinttoTestTflintExternalTflintto follow a consistent naming pattern, which improves test organization and discoverability.
148-148: Function rename for consistent test naming conventions.The function has been properly renamed from
TestTfvarsArePassedToTflinttoTestTflintTfvarsArePassedToTflintto follow a consistent naming pattern, which improves test organization and discoverability.internal/cas/getter_ssh_test.go (2)
14-14: Added import for filepath package.The import is properly added to support the new changes in the test.
50-61: Improved test isolation by scoping resources to each subtest.This change enhances test reliability by:
- Moving the CAS client initialization inside the subtest loop
- Creating a unique store path for each test
- Properly scoping all resources (client, options, logger, getter) to each subtest
These changes prevent potential cross-test interference and improve parallel test execution reliability.
internal/cas/git.go (2)
10-11: Added required imports for timestamp generation.The imports for
strconvandtimeare correctly added to support the timestamp functionality.Also applies to: 12-13
148-153: Enhanced temporary directory naming with timestamps.This is an excellent improvement that:
- Reduces the likelihood of directory name collisions in concurrent environments
- Uses nanosecond precision for high uniqueness
- Maintains backward compatibility with the existing function signature
This change will help prevent race conditions when running tests in parallel.
.github/workflows/base-test.yml (5)
18-19: LGTM! Environment setup for CI/CD.Setting up a specific mise profile for CI/CD is a good practice to ensure consistent tooling environments.
29-29: LGTM! Enabling experimental features.Enabling experimental features in mise-action will allow access to newer features that might be beneficial for CI/CD workflows.
53-56: Great improvement to test result handling!The changes properly capture test output in multiple formats:
- Setting pipefail ensures command failures are properly propagated
- Using go-junit-report generates structured test results
- Preserving raw logs while also creating XML reports
This approach significantly improves test result visibility and debugging capabilities.
62-67: LGTM! Artifact upload.Uploading test reports as artifacts makes them easily accessible for later analysis, which is particularly helpful for debugging test failures.
68-75: Great addition of structured test reporting!Using action-junit-report provides a well-formatted, easily readable summary of test results directly in the GitHub Actions UI. The configuration options for detailed summaries and time inclusion will make test failures much easier to diagnose.
internal/cas/race_test.go (3)
1-14: LGTM! Well-structured test file setup.The file is properly organized with clear package declaration, imports, and documentation. Using the
cas_testpackage (rather thancas) follows Go best practices for testing.
16-32: Good parallel test configuration with proper resource initialization.The test is correctly set up to run in parallel with
t.Parallel()and initializes the CAS instance and getter client properly. The setup correctly creates isolated resources for concurrent testing.
33-44: Good use of table-driven testing pattern.The table-driven test approach makes it easy to add more test cases in the future, improving maintainability.
.github/workflows/precommit.yml (1)
1-29: LGTM! Well-structured workflow setup.The workflow is properly configured to run on appropriate events and includes necessary setup steps for tooling and dependencies.
test/integration_sops_test.go (7)
32-32: LGTM! Standardized test naming.Standardizing test names with uppercase "SOPS" improves consistency across the test suite.
65-65: LGTM! Standardized test naming.Consistently applying the "SOPS" capitalization pattern across test functions.
72-73: LGTM! Updated command format.The command now properly uses the
--queue-include-dirparameter to target specific directories in the run-all operation.
74-74: LGTM! Updated command format with improved output handling.The command format is updated to match line 72, and the function now returns the output directly for processing.
78-78: LGTM! Direct string unmarshaling.The code now directly unmarshals from the stdout string rather than converting to bytes first, which is cleaner.
95-95: LGTM! Standardized test naming.Test name updated to maintain consistent capitalization of "SOPS".
111-111: LGTM! Standardized test naming.Test name updated to maintain consistent capitalization of "SOPS".
.github/workflows/integration-test.yml (3)
1-9: Triggers configuration looks correct and comprehensive.
The workflow fires on pushes tomainand pull request events (opened,synchronize,reopened), which aligns with the intended integration test coverage.
10-18: Matrix strategy withfail-fast: falseis appropriate for full feedback.
Allowing all matrix jobs to run even if one fails ensures you get complete insight into each integration scenario.
19-85: Integration matrix entries are well-structured.
Each suite clearly defines itsname,target, optionalsetup_scripts,tags,runfilters, and requiredsecrets. The use ofskip: truefor credentialed tests avoids CI failures when secrets aren’t provided.
| - id: go-cache-paths | ||
| run: | | ||
| echo "go-build=$(go env GOCACHE)" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Go Build Cache | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: ${{ steps.go-cache-paths.outputs.go-build }} | ||
| key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }}-${{ matrix.os }}-${{ matrix.arch }} |
There was a problem hiding this comment.
Fix matrix references in cache key.
The cache key references matrix.os and matrix.arch, but there's no matrix strategy defined in this workflow.
- key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }}-${{ matrix.os }}-${{ matrix.arch }}
+ key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - id: go-cache-paths | |
| run: | | |
| echo "go-build=$(go env GOCACHE)" >> "$GITHUB_OUTPUT" | |
| - name: Go Build Cache | |
| uses: actions/cache@v4 | |
| with: | |
| path: ${{ steps.go-cache-paths.outputs.go-build }} | |
| key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }}-${{ matrix.os }}-${{ matrix.arch }} | |
| - name: Go Build Cache | |
| uses: actions/cache@v4 | |
| with: | |
| path: ${{ steps.go-cache-paths.outputs.go-build }} | |
| key: ${{ runner.os }}-go-build-${{ hashFiles('**/go.sum') }} |
🧰 Tools
🪛 actionlint (1.7.4)
38-38: property "os" is not defined in object type {}
(expression)
| - name: Run pre-commit hooks | ||
| env: | ||
| GOPROXY: direct | ||
| GOOS: ${{ matrix.os }} | ||
| GOARCH: ${{ matrix.arch }} | ||
| run: | | ||
| pre-commit install | ||
| pre-commit run --all-files |
There was a problem hiding this comment.
Fix matrix references in environment variables.
The environment variables reference matrix.os and matrix.arch, but there's no matrix strategy defined in this workflow.
env:
GOPROXY: direct
- GOOS: ${{ matrix.os }}
- GOARCH: ${{ matrix.arch }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Run pre-commit hooks | |
| env: | |
| GOPROXY: direct | |
| GOOS: ${{ matrix.os }} | |
| GOARCH: ${{ matrix.arch }} | |
| run: | | |
| pre-commit install | |
| pre-commit run --all-files | |
| - name: Run pre-commit hooks | |
| env: | |
| GOPROXY: direct | |
| run: | | |
| pre-commit install | |
| pre-commit run --all-files |
🧰 Tools
🪛 actionlint (1.7.4)
43-43: property "os" is not defined in object type {}
(expression)
44-44: property "arch" is not defined in object type {}
(expression)
| id: generate-secrets | ||
| run: | | ||
| ENV_FILE="${GITHUB_WORKSPACE}/.env.secrets" | ||
| echo "export SCRIPT_PATHS='${{ matrix.integration.setup_scripts }}'" > $ENV_FILE |
There was a problem hiding this comment.
We should never directly interpolate these in the workflow. Please forward this through an env to avoid attacks involving shell injection.
| source "${GITHUB_WORKSPACE}/.env.secrets" | ||
|
|
||
| # Loop through setup scripts and execute them | ||
| for SCRIPT in ${{ join(matrix.integration.setup_scripts, ' ') }}; do |
There was a problem hiding this comment.
Make sure to forward this through an env var as well.
| source "${GITHUB_WORKSPACE}/.env.secrets" | ||
| go test -v ${TAGS:+-tags "$TAGS"} ${RUN:+-run "$RUN"} "${TARGET}" | ||
| else | ||
| echo "Skipping tests for ${{ matrix.integration.name }} as the skip flag is true." |
There was a problem hiding this comment.
Make sure to forward this through an env var as well.
| pre-commit = "4.2.0" | ||
| gcloud = "520.0.0" | ||
| awscli = "2.27.7" | ||
| "go:golang.org/x/tools/cmd/goimports" = "latest" |
There was a problem hiding this comment.
This is really cool. I had no idea you could do this.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/integration-test.yml (1)
86-94: 🛠️ Refactor suggestionAvoid direct interpolation of matrix variables in the shell script.
Past reviews flagged direct usage of${{ matrix.integration.setup_scripts }}and${{ matrix.integration.secrets }}inside therunblock due to potential injection risks and Actionlint errors (evaluating an array in a template). Instead, define these values underenv:and reference them as shell variables:id: generate-secrets +env: + SCRIPT_PATHS: ${{ join(matrix.integration.setup_scripts, ' ') }} + SECRET_LIST: ${{ join(matrix.integration.secrets, ' ') }} run: | ENV_FILE="${GITHUB_WORKSPACE}/.env.secrets" - echo "export SCRIPT_PATHS='${{ matrix.integration.setup_scripts }}'" > $ENV_FILE + echo "export SCRIPT_PATHS='$SCRIPT_PATHS'" > $ENV_FILE # Manually export each secret listed in matrix.integration.secrets - for SECRET in ${{ join(matrix.integration.secrets, ' ') }}; do + for SECRET in $SECRET_LIST; do ...This also resolves the Actionlint complaint about evaluating an array directly in the template.
🧰 Tools
🪛 actionlint (1.7.4)
88-88: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type array
(expression)
🪛 YAMLlint (1.35.1)
[error] 91-91: trailing spaces
(trailing-spaces)
🧹 Nitpick comments (1)
.github/workflows/integration-test.yml (1)
91-91: Remove trailing whitespace.
YAMLlint flags trailing spaces on these blank lines. Please delete the extra spaces to satisfy lint rules.Also applies to: 121-121
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 91-91: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/base-test.yml(3 hunks).github/workflows/integration-test.yml(1 hunks)mise.cicd.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- mise.cicd.toml
- .github/workflows/base-test.yml
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/integration-test.yml
88-88: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type array
(expression)
🪛 YAMLlint (1.35.1)
.github/workflows/integration-test.yml
[error] 91-91: trailing spaces
(trailing-spaces)
[error] 121-121: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test (macos)
- GitHub Check: Test (ubuntu)
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: build-and-test
🔇 Additional comments (3)
.github/workflows/integration-test.yml (3)
1-8: Workflow triggers look correct.
Theonsection properly configures the workflow to run on pushes tomainand on pull request events (opened,synchronize,reopened).
16-24: Matrix strategy is well-defined.
Usingfail-fast: falseensures all jobs run even if one fails, and defining multiple integration categories (Fixtures, SSH, SOPS, etc.) in the matrix is a clean way to parallelize.
150-158: Run Tests step with conditional skipping is solid.
The use of theskipflag to selectively bypass tests is correctly implemented, and re‐sourcing the secrets file before executinggo testensures proper environment setup.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/scripts/setup/generate-secrets.sh(1 hunks).github/scripts/setup/run-setup-scripts.sh(1 hunks).github/workflows/integration-test.yml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .github/scripts/setup/run-setup-scripts.sh
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/integration-test.yml
91-91: object, array, and null values should not be evaluated in template with ${{ }} but evaluating the value of type array
(expression)
🪛 GitHub Actions: Integration Tests
.github/workflows/integration-test.yml
[error] 91-91: The template is not valid. A sequence was not expected at line 91, column 25.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test (ubuntu)
- GitHub Check: Test (macos)
- GitHub Check: build-and-test
- GitHub Check: Pull Request has non-contributor approval
2 participants
Description
Adds integration tests, including the system where we'll add optional extra tests that have particular build tags and prefixed test names.
Included changes:
Example execution:
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added integration tests to GHA.
Migration Guide
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Tests