-
Notifications
You must be signed in to change notification settings - Fork 561
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
allow cache values to be overridable for terraform and terragrunt #1873
Conversation
WalkthroughThe pull request modifies the GitHub Action configuration by introducing a new optional input parameter: Changes
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
action.yml (2)
381-392
: Fix inconsistency in environment variable comments.The comment for
TERRAGRUNT_PROVIDER_CACHE_DIR
is missing while it's present in the release mode configuration (line 435).Add the missing comment:
TF_PLUGIN_CACHE_DIR: ${{ inputs.terraform-cache-dir == '' && format('{0}/cache', github.workspace) || inputs.terraform-cache-dir }} TERRAGRUNT_PROVIDER_CACHE: ${{ inputs.cache-dependencies == 'true' && 1 || 0 }} + # if terragrunt-cache-dir is set then we set it to that otherwise set it to '${{github.workspace}}/cache' TERRAGRUNT_PROVIDER_CACHE_DIR: ${{
428-440
: Consider extracting repeated logic into composite expressions.The cache directory configuration logic is duplicated between development and release modes. This could lead to maintenance issues if the logic needs to be updated.
Consider defining these as composite expressions at the top of the file:
# Add at the top level of the action.yml env: TERRAFORM_CACHE_DIR: ${{ inputs.terraform-cache-dir == '' && format('{0}/cache', github.workspace) || inputs.terraform-cache-dir }} TERRAGRUNT_CACHE_DIR: ${{ inputs.terragrunt-cache-dir == '' && format('{0}/cache', github.workspace) || inputs.terragrunt-cache-dir }}Then reference these in both development and release mode sections:
TF_PLUGIN_CACHE_DIR: ${{ env.TERRAFORM_CACHE_DIR }} TERRAGRUNT_PROVIDER_CACHE_DIR: ${{ env.TERRAGRUNT_CACHE_DIR }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
action.yml
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (71)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm, windows)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm, freebsd)
- GitHub Check: Build
- GitHub Check: binary (arm, windows)
- GitHub Check: binary (arm, windows)
- GitHub Check: binary (arm, windows)
- GitHub Check: binary
- GitHub Check: binary (arm, linux)
- GitHub Check: Build
- GitHub Check: binary (arm, freebsd)
- GitHub Check: binary (arm, freebsd)
- GitHub Check: binary (arm, freebsd)
- GitHub Check: binary
- GitHub Check: binary (arm, linux)
- GitHub Check: binary (arm, linux)
- GitHub Check: binary (arm, linux)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (2)
action.yml (2)
170-177
: LGTM! Well-structured input parameters.The new input parameters are well-defined with clear descriptions and appropriate defaults. The optional nature allows for backward compatibility.
Line range hint
170-440
: Verify the impact on existing cache implementations.The new cache directory overrides might affect existing implementations that rely on the default cache location.
Let's check for any hardcoded references to the default cache path:
✅ Verification successful
Cache directory overrides are safely implemented
The implementation uses environment variables and proper fallback mechanisms without any hardcoded dependencies on the default cache location. The changes maintain backward compatibility while adding flexibility for custom cache paths.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for hardcoded references to the default cache path # This helps identify any places that might break with custom cache directories echo "Searching for hardcoded cache path references..." rg -l "/cache" --type yaml --type typescript --type javascript --type go echo "Searching for cache-related environment variable usage..." rg -l "TF_PLUGIN_CACHE_DIR|TERRAGRUNT_PROVIDER_CACHE_DIR" --type yaml --type typescript --type javascript --type goLength of output: 492
Script:
#!/bin/bash # Search with explicit file extensions and broader patterns echo "Searching for cache path and workspace references..." rg -l "cache|workspace" --type yaml rg -l "cache|workspace" -g "*.ts" -g "*.js" -g "*.go" echo "Searching for environment variables..." rg -l "TF_PLUGIN_CACHE_DIR|TERRAGRUNT_PROVIDER_CACHE_DIR" --type yaml rg -l "TF_PLUGIN_CACHE_DIR|TERRAGRUNT_PROVIDER_CACHE_DIR" -g "*.ts" -g "*.js" -g "*.go" echo "Searching for github.workspace references..." rg -l "github.workspace" --type yamlLength of output: 1562
Script:
#!/bin/bash # Examine cache implementation details echo "Checking cache implementation in Go files..." rg -A 5 -B 5 "cache.*dir" libs/execution/tf.go libs/execution/opentofu.go backend/models/cache.go echo "Checking for workspace path handling..." rg -A 5 -B 5 "workspace.*path" libs/execution/tf.go libs/execution/opentofu.go backend/models/cache.go echo "Looking for cache directory initialization..." ast-grep --pattern 'func $_($$_) { $$$ cache $$$ }'Length of output: 553
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
action.yml (3)
170-178
: Improve input parameter descriptions for consistency and clarity.The descriptions could be more precise and consistent:
- description: "allows overriding of the terraform cache dir which defaults to ${github.workspace}/cache" + description: "Override the terraform plugin cache directory (defaults to ${github.workspace}/cache)" - description: "allows overriding of the terragrunt cache dir to ${github.workspace}/cache" + description: "Override the terragrunt provider cache directory (defaults to ${github.workspace}/cache)"
381-384
: LGTM! Consider simplifying the comment.The conditional logic for setting
TF_PLUGIN_CACHE_DIR
is correct.- # if terraform-cache-dir is set then we set it to that otherwise set it to '${{github.workspace}}/cache' + # Set terraform plugin cache directory (defaults to workspace cache)
424-432
: Fix trailing spaces and consider extracting environment variables.
- Remove trailing spaces in the conditional expressions
- Consider extracting common environment variables to reduce duplication between the two run blocks
- TF_PLUGIN_CACHE_DIR: ${{ inputs.terraform-cache-dir == '' && - format('{0}/cache', github.workspace) || + TF_PLUGIN_CACHE_DIR: ${{ inputs.terraform-cache-dir == '' && + format('{0}/cache', github.workspace) || inputs.terraform-cache-dir }}Consider moving the environment variable definitions to a composite step or a shell script that can be reused by both run blocks to reduce duplication.
🧰 Tools
🪛 yamllint (1.35.1)
[error] 425-425: trailing spaces
(trailing-spaces)
[error] 426-426: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
action.yml
(3 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
action.yml
[error] 425-425: trailing spaces
(trailing-spaces)
[error] 426-426: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (75)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (386, windows)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (386, freebsd)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (386, linux)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, windows)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (amd64, freebsd)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (amd64, darwin)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (amd64, linux)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, windows)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm64, freebsd)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm64, darwin)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm64, linux)
- GitHub Check: binary (arm, windows)
- GitHub Check: binary (arm, windows)
- GitHub Check: binary (arm, windows)
- GitHub Check: binary (arm, freebsd)
- GitHub Check: binary (arm, windows)
- GitHub Check: binary (arm, freebsd)
- GitHub Check: binary (arm, linux)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: build-and-push-image
- GitHub Check: binary
- GitHub Check: build-and-push-image
- GitHub Check: binary (arm, freebsd)
- GitHub Check: binary (arm, linux)
- GitHub Check: binary (arm, freebsd)
- GitHub Check: Build
- GitHub Check: build-and-push-image
- GitHub Check: build-and-push-image
- GitHub Check: binary (arm, linux)
- GitHub Check: binary
- GitHub Check: binary (arm, linux)
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
- GitHub Check: Build
🔇 Additional comments (1)
action.yml (1)
386-388
: LGTM! The implementation is consistent with terraform cache.The conditional logic for setting
TERRAGRUNT_PROVIDER_CACHE_DIR
follows the same pattern asTF_PLUGIN_CACHE_DIR
, maintaining consistency.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/libs_test.yml (1)
19-19
: Remove trailing whitespace.There are trailing spaces on this line that should be removed.
- +🧰 Tools
🪛 yamllint (1.35.1)
[error] 19-19: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/libs_test.yml
(1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/libs_test.yml
[error] 19-19: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build
- GitHub Check: Build
…erhq/digger into feat/allow-overridable-cache-values
Summary by CodeRabbit