Update atmos.Component template function for helmfile components
#1217
Conversation
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
📝 WalkthroughWalkthroughThis update introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant DescribeComponent
participant Config
participant TemplateFuncs
User->>CLI: Run "atmos describe component"
CLI->>DescribeComponent: ExecuteDescribeComponent
DescribeComponent->>Config: Load stacks and config
DescribeComponent->>DescribeComponent: Determine component type (terraform/helmfile)
DescribeComponent->>DescribeComponent: Populate ComponentSection with component_type
DescribeComponent->>TemplateFuncs: Prepare section outputs (using constants)
DescribeComponent->>CLI: Return merged configuration with component_type section
CLI->>User: Output component configuration (includes component_type)
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/exec/template_funcs_component.go (2)
51-52: Add validation for component type existence and type.Current implementation assumes
sections[cfg.ComponentTypeSectionName]exists and is comparable tocfg.TerraformComponentType. Consider adding validation to handle cases where the component type might be missing or of unexpected type.-componentType := sections[cfg.ComponentTypeSectionName] -if componentType == cfg.TerraformComponentType { +componentType, ok := sections[cfg.ComponentTypeSectionName].(string) +if !ok { + log.Debug("Component type not found or not a string", "component", component, "stack", stack) + componentType = "" +} +if componentType == cfg.TerraformComponentType {🧰 Tools
🪛 GitHub Check: golangci-lint
[warning] 52-52:
if componentType == cfg.TerraformComponentTypehas complex nested blocks (complexity: 5)
52-91: Consider refactoring to reduce complexity.The static analysis tool flags high complexity in this conditional block. Consider extracting the terraform-specific logic into a separate function to improve readability and maintainability.
func componentFunc( atmosConfig *schema.AtmosConfiguration, component string, stack string, ) (any, error) { // ... existing code ... sections, err := ExecuteDescribeComponent(component, stack, true, true, nil) if err != nil { return nil, err } componentType, ok := sections[cfg.ComponentTypeSectionName].(string) if !ok { log.Debug("Component type not found or not a string", "component", component, "stack", stack) componentType = "" } // Process Terraform remote state if componentType == cfg.TerraformComponentType { + err = processTerraformComponent(atmosConfig, component, stack, sections) + if err != nil { + return nil, err + } - // Check if the component in the stack is configured with the 'static' remote state backend, - // in which case get the `output` from the static remote state instead of executing `terraform output` - remoteStateBackendStaticTypeOutputs, err := GetComponentRemoteStateBackendStaticType(sections) - if err != nil { - return nil, err - } - - if remoteStateBackendStaticTypeOutputs != nil { - // Return the static backend outputs - terraformOutputs = remoteStateBackendStaticTypeOutputs - } else { - // Execute `terraform output` - terraformOutputs, err = execTerraformOutput(atmosConfig, component, stack, sections) - if err != nil { - return nil, err - } - } - - outputs := map[string]any{ - cfg.OutputsSectionName: terraformOutputs, - } - - sections = lo.Assign(sections, outputs) } // Cache the result componentFuncSyncMap.Store(stackSlug, sections) log.Debug("Executed template function", "function", functionName) - // Print the `outputs` section of the Terraform component - if componentType == cfg.TerraformComponentType { - y, err2 := u.ConvertToYAML(terraformOutputs) - if err2 != nil { - log.Error(err2) - } else { - log.Debug("'outputs' of the template function", "function", functionName, cfg.OutputsSectionName, y) - } - } + // Print debug information if this is a terraform component + if componentType == cfg.TerraformComponentType { + printTerraformOutputDebug(functionName, sections) + } return sections, nil } + // processTerraformComponent handles terraform-specific logic for component outputs + func processTerraformComponent( + atmosConfig *schema.AtmosConfiguration, + component string, + stack string, + sections map[string]any, + ) error { + // Check if the component has static remote state backend + terraformOutputs, err := GetComponentRemoteStateBackendStaticType(sections) + if err != nil { + return err + } + + // If no static outputs, execute terraform output command + if terraformOutputs == nil { + terraformOutputs, err = execTerraformOutput(atmosConfig, component, stack, sections) + if err != nil { + return err + } + } + + // Update sections with terraform outputs + outputs := map[string]any{ + cfg.OutputsSectionName: terraformOutputs, + } + + lo.Assign(sections, outputs) + return nil + } + + // printTerraformOutputDebug logs terraform outputs for debugging + func printTerraformOutputDebug(functionName string, sections map[string]any) { + if outputs, ok := sections[cfg.OutputsSectionName]; ok { + y, err := u.ConvertToYAML(outputs) + if err != nil { + log.Error(err) + } else { + log.Debug("'outputs' of the template function", "function", functionName, cfg.OutputsSectionName, y) + } + } + }🧰 Tools
🪛 GitHub Check: golangci-lint
[warning] 52-52:
if componentType == cfg.TerraformComponentTypehas complex nested blocks (complexity: 5)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
examples/quick-start-advanced/Dockerfile(1 hunks)internal/exec/describe_component.go(2 hunks)internal/exec/template_funcs.go(1 hunks)internal/exec/template_funcs_component.go(4 hunks)internal/exec/template_funcs_component_test.go(2 hunks)pkg/config/const.go(1 hunks)tests/fixtures/scenarios/stack-templates-3/atmos.yaml(1 hunks)tests/fixtures/scenarios/stack-templates-3/stacks/deploy/nonprod.yaml(1 hunks)website/docs/cli/commands/describe/describe-component.mdx(1 hunks)website/docs/integrations/atlantis.mdx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
examples/quick-start-advanced/Dockerfile (1)
Learnt from: aknysh
PR: cloudposse/atmos#775
File: examples/quick-start-advanced/Dockerfile:9-9
Timestamp: 2024-11-12T03:15:15.627Z
Learning: It is acceptable to set `ARG ATMOS_VERSION` to a future version like `1.105.0` in `examples/quick-start-advanced/Dockerfile` if that will be the next release.
🧬 Code Graph Analysis (2)
internal/exec/template_funcs_component_test.go (1)
pkg/utils/yaml_utils.go (1)
ConvertToYAML(84-90)
internal/exec/template_funcs_component.go (2)
pkg/config/const.go (3)
OutputsSectionName(74-74)ComponentTypeSectionName(73-73)TerraformComponentType(44-44)pkg/utils/yaml_utils.go (1)
ConvertToYAML(84-90)
🪛 LanguageTool
website/docs/cli/commands/describe/describe-component.mdx
[typographical] ~106-~106: To join two clauses or introduce examples, consider using an em dash.
Context: ...ovides configuration - component_type - the type of the component (terraform o...
(DASH_RULE)
🪛 GitHub Check: golangci-lint
internal/exec/template_funcs_component.go
[warning] 52-52:
if componentType == cfg.TerraformComponentType has complex nested blocks (complexity: 5)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (19)
website/docs/integrations/atlantis.mdx (1)
676-676: Bump ATMOS_VERSION to 1.173.0
Aligns the Atlantis integration example with the newly released Atmos CLI version.examples/quick-start-advanced/Dockerfile (1)
9-9: Update ATMOS_VERSION build argument
Synchronizes the Docker image build with Atmos CLI v1.173.0.pkg/config/const.go (1)
73-74: Introduce constants forcomponent_typeandoutputs
AddsComponentTypeSectionNameandOutputsSectionNameto avoid hardcoded strings and improve maintainability.tests/fixtures/scenarios/stack-templates-3/atmos.yaml (1)
10-11: Register new Helmfile component section
Defines thehelmfilecomponent with itsbase_pathto enable Helmfile support in test fixtures.website/docs/cli/commands/describe/describe-component.mdx (1)
106-107: Document newcomponent_typeoutput section
Adds thecomponent_typebullet to the describe-component docs, reflecting the enhanced output for Terraform and Helmfile components.🧰 Tools
🪛 LanguageTool
[typographical] ~106-~106: To join two clauses or introduce examples, consider using an em dash.
Context: ...ovides configuration -component_type- the type of the component (terraformo...(DASH_RULE)
internal/exec/template_funcs.go (1)
40-42: Correct signature update for Component function callThe modification properly aligns with the refactored
componentFuncimplementation that now dynamically determines the component type rather than requiring it as a parameter.tests/fixtures/scenarios/stack-templates-3/stacks/deploy/nonprod.yaml (1)
29-41: Good test coverage for helmfile componentsThe added helmfile components provide good test coverage for cross-component references:
- Component-3 correctly references vars from the terraform component
- Component-4 demonstrates proper handling of outputs from terraform components and missing outputs from helmfile components
The comment on line 40 appropriately documents the expected behavior for invalid output references.
internal/exec/template_funcs_component_test.go (3)
65-67: Function call updated correctlyThe test has been updated to match the new signature of
componentFuncwhich no longer requires theconfigAndStacksInfoparameter.
76-85: Good test coverage for helmfile component-3The test appropriately verifies that helmfile component-3 can reference:
- Variables from terraform component-1
- Configuration settings from terraform component-1
This ensures cross-component references work correctly between terraform and helmfile components.
87-97: Comprehensive test for helmfile component-4The test properly verifies:
- That helmfile components can reference outputs from terraform components
- That helmfile components can reference vars from other helmfile components
- That attempting to access outputs from helmfile components results in
<no value>This covers the key scenarios needed to validate the PR objective of gracefully handling missing outputs in helmfile components.
internal/exec/describe_component.go (6)
13-14: Minor documentation improvementAdded period to the function comment for consistency.
109-109: Good initialization of ComponentSection mapProperly initializes the map that will track component type information.
116-119: Improved component type handling for terraform componentsThe changes properly:
- Use a constant for the component type instead of a hardcoded string
- Store the component type in the ComponentSection map for consumer access
120-123: Proper fallback to helmfile component typeGood implementation of fallback logic that:
- Tries helmfile if terraform component processing fails
- Records the component type appropriately in the ComponentSection map
124-125: Appropriate error handlingCorrectly clears the component type in the ComponentSection map if both component types fail, preventing incorrect type information from being returned with the error.
132-133: Minor documentation improvementAdded period to the function comment for consistency.
internal/exec/template_funcs_component.go (3)
32-38: Good use of constants instead of hardcoded strings.Replacing hardcoded "outputs" with
cfg.OutputsSectionNameimproves maintainability and consistency across the codebase. This change aligns with best practices for avoiding magic strings.
72-73: Good use of constant for output section.Using
cfg.OutputsSectionNameinstead of hardcoded "outputs" string here is consistent with other changes.
83-91: Conditional output based on component type is a good improvement.This change properly handles the PR's objective by only attempting to print terraform outputs for terraform components, preventing potential errors with helmfile components.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1217 +/- ##
==========================================
+ Coverage 30.87% 31.31% +0.43%
==========================================
Files 206 206
Lines 23038 23044 +6
==========================================
+ Hits 7114 7216 +102
+ Misses 14858 14747 -111
- Partials 1066 1081 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
atmos.Component template functions for helmfile componentsatmos.Component template function for helmfile components
atmos.Component template function for helmfile componentsatmos.Component template function for helmfile components
|
These changes were released in v1.173.0. |
what
atmos.Componenttemplate function forhelmfilecomponentswhy
atmos.Componenttemplate function allows reading any section or attribute from the component (vars,settings, etc.), including theoutputssection (Terraform state). Helmfile components don't have Terraform state. This PR adds detecting the component type (terraformorhelmfile) when executingatmos.Component, and don't try to read the remote state for thehelmfilecomponentsSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores