expose env variable ATMOS_CLI_CONFIG_PATH and ATMOS_BASE_PATH before running terraform and helm cmd #1072
Conversation
📝 WalkthroughWalkthroughThis pull request introduces enhancements to environment configuration handling across various components. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AtmosCLI
participant ConfigLoader
participant HelmfileExec
User->>AtmosCLI: Run helmfile command
AtmosCLI->>ConfigLoader: Load configuration
ConfigLoader-->>AtmosCLI: Return CLI config paths
AtmosCLI->>HelmfileExec: Execute helmfile with env vars (ATMOS_CLI_CONFIG_PATH, ATMOS_BASE_PATH)
HelmfileExec-->>AtmosCLI: Return command output
AtmosCLI-->>User: Output result
sequenceDiagram
participant User
participant AtmosCLI
participant ConfigLoader
participant TerraformExec
User->>AtmosCLI: Run terraform apply command
AtmosCLI->>ConfigLoader: Load configuration
ConfigLoader-->>AtmosCLI: Return CLI config paths
AtmosCLI->>TerraformExec: Execute terraform with env vars (ATMOS_CLI_CONFIG_PATH, ATMOS_BASE_PATH)
TerraformExec-->>AtmosCLI: Return execution output
AtmosCLI-->>User: Output result
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous 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
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 (1)
tests/fixtures/components/terraform/env-example/outputs.tf (1)
22-26: Variable Declaration for Deployment Stage.The
stagevariable is clearly declared. As a suggestion for maintainability, consider moving variable declarations to a dedicated variables file (e.g.,variables.tf) if this module expands.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
internal/exec/helmfile.go(1 hunks)internal/exec/terraform.go(1 hunks)pkg/config/config.go(7 hunks)pkg/schema/schema.go(1 hunks)tests/fixtures/components/terraform/env-example/main.tf(1 hunks)tests/fixtures/components/terraform/env-example/outputs.tf(1 hunks)tests/fixtures/scenarios/env/atmos.yaml(1 hunks)tests/fixtures/scenarios/env/stacks/catalog/example.yaml(1 hunks)tests/fixtures/scenarios/env/stacks/deploy/dev.yaml(1 hunks)tests/fixtures/scenarios/env/stacks/deploy/prod.yaml(1 hunks)tests/fixtures/scenarios/env/stacks/deploy/staging.yaml(1 hunks)tests/test-cases/env.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- tests/fixtures/scenarios/env/stacks/catalog/example.yaml
- tests/fixtures/scenarios/env/stacks/deploy/prod.yaml
- tests/fixtures/scenarios/env/stacks/deploy/dev.yaml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (19)
internal/exec/helmfile.go (1)
243-244: LGTM! Environment variables added at the right location.The environment variables
ATMOS_CLI_CONFIG_PATHandATMOS_BASE_PATHare correctly added to theenvVarsslice just before executing the helmfile command.pkg/config/config.go (1)
142-142: LGTM! Robust implementation of config path tracking and handling.The changes properly track the config file path and ensure it's converted to an absolute path before storing in
atmosConfig.CliConfigPath. The implementation:
- Tracks the path through all possible config locations
- Handles both absolute and relative paths
- Includes proper error handling
Also applies to: 164-164, 180-180, 195-195, 209-209, 224-224, 265-273
internal/exec/terraform.go (1)
225-226: LGTM! Environment variables added consistently with helmfile implementation.The environment variables
ATMOS_CLI_CONFIG_PATHandATMOS_BASE_PATHare correctly added toinfo.ComponentEnvListbefore executing the terraform command, maintaining consistency with the helmfile implementation.pkg/schema/schema.go (1)
39-40: LGTM! Clean addition of CliConfigPath field.The
CliConfigPathfield is properly added to theAtmosConfigurationstruct with consistent formatting and appropriate serialization tags.tests/fixtures/scenarios/env/stacks/deploy/staging.yaml (4)
1-2: Schema Declaration is on point.The YAML schema directive is correctly specified to aid language server validation for Atmos manifests.
3-5: Vars Block Clarity.The
varssection setsstageto "staging" clearly—just ensure that this aligns with your deployment pipeline expectations.
6-8: Import Directive Verified.The import of
catalog/exampleis straightforward. Double-check that the referenced catalog exists and contains the expected components.
9-12: Component Configuration Looks Solid.The
componentssection properly defines the Terraform componentenv-examplewith an empty vars block, serving as an extensible base.tests/fixtures/components/terraform/env-example/main.tf (2)
1-8: Terraform Provider Block is Well Defined.The provider declaration for
environmentuses the correct source and version constraint—with a reminder in the comment to check for the latest version, which is good practice.
10-13: Data Block for Environment Variables.The data block efficiently captures environment variables matching the regex pattern "^ATMOS_.*|^EXAMPLE$". This filtering is clear and concise.
tests/fixtures/scenarios/env/atmos.yaml (4)
1-2: Base Path Declaration is Correct.Setting
base_pathto "./" aligns with expectations for a local deployment reference.
3-9: Terraform Component Configuration is Clear.The components block sets the Terraform base path and includes logical flags for initialization and auto-approval. Everything appears clean and purposeful.
11-18: Stacks Configuration Verified.The stacks block defines the
base_path, along with the included and excluded paths, which is crucial for managing environment-specific deployments.
19-21: Log Configuration is Acute.Directing logs to standard error with a level of Info is clear and should support efficient debugging.
tests/test-cases/env.yaml (1)
1-23: Test Case for Environment Export is Solid.This test case is comprehensive—it correctly sets the working directory, provides the relevant command and arguments, and validates that
atmos_base_pathandatmos_cli_config_pathare exported as intended. The use of regular expressions in the expected stdout adds needed flexibility.tests/fixtures/components/terraform/env-example/outputs.tf (4)
1-4: Output Definition for atmos_cli_config_path.The output for
atmos_cli_config_pathcorrectly sources its value from the environment variables data block, with a clear and informative description.
6-9: Output Definition for atmos_base_path.This output correctly mirrors the previous definition to capture the base path. Everything appears well organized.
11-14: Output for 'example' is in Place.The output for the "EXAMPLE" variable is clearly defined and described, ensuring transparency in the configuration.
16-20: Aggregate Output for all Atmos Variables.Aggregating all matched environment variables is useful for debugging and further processing. The configuration here is neat.
There was a problem hiding this comment.
@haitham911 LGTM, a few comments
Also, in the PR description, add ## what and ## why
aknysh
added
the
no-release
|
These changes were released in v1.163.0. |
What
Why
references
Summary by CodeRabbit
New Features
Tests