Stacks: stack values #3961
Stacks: stack values #3961
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis change refactors the Terragrunt configuration handling by replacing the use of the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Test
participant T as Terragrunt CLI
participant RV as ReadValues
participant RS as ReadStackConfigFile
participant WV as writeValues
U->>T: Invoke generate/apply stack command
T->>RV: Call ReadValues (unit/stack values)
RV-->>T: Return values (cty.Value)
T->>RS: Read stack config passing values
RS-->>T: Return stack configuration
T->>WV: Write values file based on config
WV-->>T: Confirm file creation
T-->>U: Return success and output results
Possibly related PRs
Suggested reviewers
🪧 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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
config/config.go (1)
900-900: Function has been renamed from ReadUnitValues to ReadValues.The function call has been updated from
ReadUnitValuestoReadValueswhich appears to be part of the enhancement to support values within stack blocks. This change maintains consistent parameter structure while expanding functionality.To verify this change is consistently applied throughout the codebase, run:
#!/bin/bash # Check for any remaining instances of ReadUnitValues that might need updating grep -r "ReadUnitValues" --include="*.go" .docs/_docs/04_reference/04-config-blocks-and-attributes.md (1)
1721-1722: Small inconsistency in documentation explanation.Line 1721 refers to the stack being generated at
.terragrunt-stack/services, but line 1722 mentions custom values forprojectandenv, while the example above showsprojectandcidr. This creates a slight discrepancy.-The stack is also provided with custom values for `project` and `env`, which can be used within the stack's configuration files. +The stack is also provided with custom values for `project` and `cidr`, which can be used within the stack's configuration files.test/fixtures/stacks/stack-values/units/app/main.tf (3)
2-6: Define variable type constraints and descriptions for better maintainability.The variables are declared without type constraints or descriptions, which makes it harder to understand their intended use and acceptable values.
-variable "project" {} +variable "project" { + description = "The project identifier" + type = string +} -variable "env" {} +variable "env" { + description = "The environment name" + type = string +} -variable "data" {} +variable "data" { + description = "Additional data for the configuration" + type = string +}
12-15: Consider adding more file properties and validation.The
local_fileresource could benefit from additional properties such as file permissions and validation to ensure the content is meaningful.resource "local_file" "file" { content = local.config filename = "${path.module}/file.txt" + file_permission = "0644" + + # Validate that we have meaningful content + lifecycle { + precondition { + condition = length(local.config) > 0 + error_message = "Config content cannot be empty." + } + } }
17-31: Consider adding descriptions to outputs.Adding descriptions to outputs would improve the usability of this module when referenced by other modules or in the console output.
output "config" { value = local.config + description = "The combined configuration string" } output "project" { value = var.project + description = "The project identifier" } output "env" { value = var.env + description = "The environment name" } output "data" { value = var.data + description = "Additional data used in the configuration" }config/stack.go (4)
378-403: Robust approach for writing values.This function:
- Validates directory presence.
- Auto-creates the folder structure.
- Writes a generated HCL file, including a comment header.
Potential enhancement: If partial merges or user overrides become necessary, consider a merging strategy instead of blunt overwrites.
🧰 Tools
🪛 golangci-lint (1.62.2)
382-382: if statements should only be cuddled with assignments
(wsl)
389-389: expressions should not be cuddled with blocks
(wsl)
119-119: Minor style lint: reduce cuddle before the if statement.According to static analysis, only one statement may be cuddled before an
if. Consider adding a line break or consolidating assignments. This is purely cosmetic and does not affect logic.- values, err := ReadValues(ctx, opts, dir) - if err != nil { + values, err := ReadValues(ctx, opts, dir) + if err != nil { return nil, errors.New(err) }🧰 Tools
🪛 golangci-lint (1.62.2)
119-119: only one cuddle assignment allowed before if statement
(wsl)
382-382: Minor style lint: avoid cuddling the if statement directly after a blank line.A small formatting detail from the linter suggests aligning the
ifstatement more cohesively.🧰 Tools
🪛 golangci-lint (1.62.2)
382-382: if statements should only be cuddled with assignments
(wsl)
389-389: Minor style lint: expressions should not be cuddled with blocks.Separate the block closing brace from the subsequent line to comply with the linter’s style rules. This is purely stylistic.
🧰 Tools
🪛 golangci-lint (1.62.2)
389-389: expressions should not be cuddled with blocks
(wsl)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
config/config.go(1 hunks)config/config_partial.go(1 hunks)config/stack.go(11 hunks)docs/_docs/04_reference/04-config-blocks-and-attributes.md(2 hunks)test/fixtures/stacks/stack-values/project/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/stack-values/stacks/dev/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/stack-values/stacks/prod/terragrunt.stack.hcl(1 hunks)test/fixtures/stacks/stack-values/units/app/main.tf(1 hunks)test/fixtures/stacks/stack-values/units/app/terragrunt.hcl(1 hunks)test/integration_stacks_test.go(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/fixtures/stacks/stack-values/units/app/terragrunt.hcl
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.go`: Review the Go code for quality and correctness. M...
**/*.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.
config/config.goconfig/config_partial.gotest/integration_stacks_test.goconfig/stack.go
`docs/**/*.md`: Review the documentation for clarity, gramma...
docs/**/*.md: Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation indocsto the Starlight + Astro based documentation indocs-starlight. Whenever changes are made to thedocsdirectory, ensure that an equivalent change is made in thedocs-starlightdirectory to keep thedocs-starlightdocumentation accurate.
docs/_docs/04_reference/04-config-blocks-and-attributes.md
🪛 golangci-lint (1.62.2)
config/stack.go
119-119: only one cuddle assignment allowed before if statement
(wsl)
382-382: if statements should only be cuddled with assignments
(wsl)
389-389: expressions should not be cuddled with blocks
(wsl)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Pull Request has non-contributor approval
🔇 Additional comments (26)
config/config_partial.go (1)
353-353: Refactoring to useReadValuesinstead ofReadUnitValuesThe change from
ReadUnitValuestoReadValuesis part of the implementation that adds support for values within stack blocks. This aligns with the PR objective of enhancing configuration flexibility.test/fixtures/stacks/stack-values/stacks/prod/terragrunt.stack.hcl (2)
1-9: Well-structured unit configuration with valuesThis unit configuration correctly implements the new
valuesattribute and showcases referencing parent stack values (values.projectandvalues.env). The structure follows best practices with clear source path and identifier.
11-19: Consistent unit configuration implementationThe second unit follows the same pattern as the first, maintaining consistency in the configuration structure. Good use of the values attribute to provide different data for each unit while sharing common project and environment variables.
test/fixtures/stacks/stack-values/stacks/dev/terragrunt.stack.hcl (2)
1-9: Properly structured development unit with valuesThis development unit configuration correctly implements the new values attribute, maintaining consistent structure with the production configuration while using appropriate naming conventions for the development environment.
11-19: Consistent development unit implementationThe second development unit follows the same pattern as the first, providing good consistency across the configuration. The values are properly referenced and the unit-specific data is clearly defined.
test/fixtures/stacks/stack-values/project/terragrunt.stack.hcl (2)
2-9: Well-defined stack with values for development environmentThis stack configuration properly implements the new
valuesattribute, providing environment-specific variables that can be accessed by child configurations. Good use of theget_repo_root()function to maintain path flexibility.
11-18: Well-structured stack with values for production environmentThe production stack configuration follows the same pattern as the development stack, providing environment-specific variables while maintaining a consistent structure. This demonstrates the hierarchical value passing capability introduced in this PR.
docs/_docs/04_reference/04-config-blocks-and-attributes.md (2)
1704-1704: Documentation added for newvaluesattribute in stack blocks.The new optional
valuesattribute has been properly documented, explaining that it allows passing custom values to stacks.
1713-1716: Example values for stack block properly included in documentation.The documentation includes a helpful example showing how to set custom values like
projectandcidrin a stack block.test/integration_stacks_test.go (3)
507-523: Good coverage on generating stack values.This test function effectively verifies that the stack generation creates the expected directory and values file.
525-552: Thorough validation of dev and prod outputs.This coverage of both dev and prod environments ensures that the correct values are populated in each environment’s file and console output.
554-578: Clear JSON output testing.By unmarshaling the JSON output and verifying all expected keys, this test robustly confirms that multiple stacks are included.
config/stack.go (14)
27-27: Renaming to a more generic constant.Using
valuesFileinstead of a unit-specific name improves clarity and aligns with the broader usage of values across stacks and units.
52-55: Expanded stack struct to include values.Adding the
Valuesfield enables direct configuration within the stack, providing more flexibility and consistency.
116-122: Reading stack values before configuring stack.Fetching values early ensures they’re available for parsing the configuration. The logic here is straightforward and error checks are handled properly.
🧰 Tools
🪛 golangci-lint (1.62.2)
119-119: only one cuddle assignment allowed before if statement
(wsl)
148-150: Improved docstring clarity.This documentation accurately explains the expanded responsibilities of generating stack files, including reading the values.
154-157: Fetching values prior to reading stack config.This sequencing correctly ensures that any relevant user-defined values can be applied within the subsequent stack file parsing.
159-160: Passing values into the config file reader.Ensuring the newly read values are injected into the parser provides a flexible mechanism for parameterizing stacks.
182-184: Updated documentation for units generation.The revised comments clearly reflect the expanded logic for writing values for each unit.
204-204: Writing unit values.Generating the unit’s
terragrunt.values.hclis a clean approach to store per-unit configuration parameters.
232-235: Writing stack values similarly to units.Applying the same mechanism for stacks creates a uniform approach for all levels of configuration, promoting consistency.
340-340: Docstring updates for stack config reading.Clearer explanation helps maintainers understand how values are read and applied to the stack config.
347-349: Injecting values into the parser.Tying the
parserwithvaluesensures local references or evaluations can use the newly provided data.
376-377: Revised function comments to match functionality.The docstring now correctly highlights that the function generates and writes values to a file if the input is non-nil.
412-413: Docstring refresh for reading values.This documentation now correctly reflects the usage of
terragrunt.values.hclin each directory.
414-447: Comprehensive value reading logic.Key highlights:
- Checks for empty directory early to prevent confusion.
- Skips gracefully if no file is found.
- Properly parses the file into a
cty.ObjectVal.This design ensures that absent files don’t cause errors, maintaining flexibility in configuration.
|
Take a look at the docs bug mentioned here. It might need fixing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
config/stack.go (3)
154-160: Improve error message precisionThe error message doesn't accurately reflect what operation failed. It mentions the stack file, but the error is related to reading values from a directory.
- return errors.Errorf("Failed to read values file %s in %s %v", stackFilePath, stackSourceDir, err) + return errors.Errorf("Failed to read values from directory %s: %v", stackSourceDir, err)
380-381: Update function comment for clarityThe comment should be updated to reflect that the function now handles both unit and stack values, not just unit values.
-// writeValues generates and writes unit values to a terragrunt.values.hcl file in the specified unit directory. +// writeValues generates and writes values to a terragrunt.values.hcl file in the specified directory.
418-419: Update function comment for clarityThe comment should be updated to reflect that the function now reads both unit and stack values, not just unit values.
-// ReadValues reads the unit values from the terragrunt.values.hcl file. +// ReadValues reads values from the terragrunt.values.hcl file in the specified directory.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
config/stack.go(11 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.go`: Review the Go code for quality and correctness. M...
**/*.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.
config/stack.go
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-and-test
- GitHub Check: unessential
- GitHub Check: Pull Request has non-contributor approval
🔇 Additional comments (7)
config/stack.go (7)
27-27: LGTM: Renamed constant to reflect broader usageThe constant
unitValuesFilehas been appropriately renamed tovaluesFileto represent its broader usage for both stack and unit values.
52-56: LGTM: Enhanced Stack struct with values supportThe Stack struct has been properly extended with a
Valuesfield to support key-value pairs at the stack level, matching the PR objective to support values within stack blocks.
116-125: LGTM: Stack values are now properly read before configurationThe StackOutput function now correctly reads values from the stack directory before processing the stack configuration file, allowing the configuration to reference these values.
205-205: LGTM: Unit values handling refactoredThe implementation now correctly uses the common
writeValuesfunction for unit values, promoting code reuse.
233-236: LGTM: Stack values support addedThe implementation now properly writes stack values to the destination directory, fulfilling the PR objective.
344-344: LGTM: ReadStackConfigFile signature updatedThe function signature has been properly updated to accept values that can be used during configuration parsing.
349-351: LGTM: Values integration with parsing contextValues are now correctly integrated with the parsing context when provided, allowing them to be referenced in the stack configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/_docs/04_reference/04-config-blocks-and-attributes.md (2)
1704-1704: Clarify the newvaluesattribute documentation.
The addition of the new optionalvaluesattribute is clearly explained as a map of custom values that can be passed to the stack. Consider adding a brief note on the expected data types (e.g. the keys should be strings and the values can be any valid HCL type) or linking to additional documentation if available.
1713-1716: Example usage of thevaluesattribute looks correct.
The example in the stack block now includes avaluesblock with keys such asprojectandcidr, which is clear and helpful for users. Please ensure that the indentation and formatting match the style used throughout the document and that similar changes are applied to the parallel documentation in thedocs-starlightdirectory for consistency.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
config/stack.go(9 hunks)docs/_docs/04_reference/04-config-blocks-and-attributes.md(2 hunks)test/fixtures/stacks/stack-values/units/app/main.tf(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- test/fixtures/stacks/stack-values/units/app/main.tf
- config/stack.go
🧰 Additional context used
📓 Path-based instructions (1)
`docs/**/*.md`: Review the documentation for clarity, gramma...
docs/**/*.md: Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation indocsto the Starlight + Astro based documentation indocs-starlight. Whenever changes are made to thedocsdirectory, ensure that an equivalent change is made in thedocs-starlightdirectory to keep thedocs-starlightdocumentation accurate.
docs/_docs/04_reference/04-config-blocks-and-attributes.md
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test
- GitHub Check: Pull Request has non-contributor approval
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/_docs/04_reference/04-config-blocks-and-attributes.md (2)
1704-1704: NewvaluesAttribute Documentation in Stack BlockThe new
valuesattribute is clearly documented here, explaining that it accepts a map of custom values and can be referenced within the stack’s configuration files. This addition is well written. Please ensure that analogous updates are made in thedocs-starlightdirectory to maintain consistency across the documentation formats.
1713-1717: Example Usage of thevaluesAttributeThe example provided for the stack block effectively demonstrates how to pass custom key-value pairs (such as
projectandcidr) using the newvaluesattribute. This hands-on snippet should help users understand how to leverage this new feature. If possible, consider adding a brief note to highlight any potential validations or additional context regarding the acceptable keys and types, which could further assist users.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/_docs/04_reference/04-config-blocks-and-attributes.md(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`docs/**/*.md`: Review the documentation for clarity, gramma...
docs/**/*.md: Review the documentation for clarity, grammar, and spelling. Make sure that the documentation is easy to understand and follow. There is currently a migration underway from the Jekyll based documentation indocsto the Starlight + Astro based documentation indocs-starlight. Whenever changes are made to thedocsdirectory, ensure that an equivalent change is made in thedocs-starlightdirectory to keep thedocs-starlightdocumentation accurate.
docs/_docs/04_reference/04-config-blocks-and-attributes.md
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: unessential
- GitHub Check: build-and-test
- GitHub Check: Pull Request has non-contributor approval
Description
TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Summary by CodeRabbit
New Features
Documentation
Tests