diff --git a/NOTICE b/NOTICE index 6d2d46559c..55b2e4f6d8 100644 --- a/NOTICE +++ b/NOTICE @@ -99,7 +99,7 @@ APACHE 2.0 LICENSED DEPENDENCIES - github.com/aws/aws-sdk-go-v2 License: Apache-2.0 - URL: https://github.com/aws/aws-sdk-go-v2/blob/v1.40.1/LICENSE.txt + URL: https://github.com/aws/aws-sdk-go-v2/blob/v1.41.0/LICENSE.txt - github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream License: Apache-2.0 @@ -560,7 +560,7 @@ BSD LICENSED DEPENDENCIES - github.com/aws/aws-sdk-go-v2/internal/sync/singleflight License: BSD-3-Clause - URL: https://github.com/aws/aws-sdk-go-v2/blob/v1.40.1/internal/sync/singleflight/LICENSE + URL: https://github.com/aws/aws-sdk-go-v2/blob/v1.41.0/internal/sync/singleflight/LICENSE - github.com/aws/aws-sdk-go/internal/sync/singleflight License: BSD-3-Clause diff --git a/errors/errors.go b/errors/errors.go index 07ac4a0632..e563431838 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -94,16 +94,25 @@ var ( ErrDescribeComponent = errors.New("failed to describe component") ErrReadTerraformState = errors.New("failed to read Terraform state") ErrEvaluateTerraformBackendVariable = errors.New("failed to evaluate terraform backend variable") - ErrUnsupportedBackendType = errors.New("unsupported backend type") - ErrProcessTerraformStateFile = errors.New("error processing terraform state file") - ErrLoadAwsConfig = errors.New("failed to load AWS config") - ErrGetObjectFromS3 = errors.New("failed to get object from S3") - ErrReadS3ObjectBody = errors.New("failed to read S3 object body") - ErrCreateGCSClient = errors.New("failed to create GCS client") - ErrGetObjectFromGCS = errors.New("failed to get object from GCS") - ErrReadGCSObjectBody = errors.New("failed to read GCS object body") - ErrGCSBucketRequired = errors.New("bucket is required for gcs backend") - ErrInvalidBackendConfig = errors.New("invalid backend configuration") + + // Recoverable YAML function errors - use YQ default if available. + // These errors indicate the data is not available but do not represent API failures. + ErrTerraformStateNotProvisioned = errors.New("terraform state not provisioned") + ErrTerraformOutputNotFound = errors.New("terraform output not found") + + // API/infrastructure errors - should cause non-zero exit. + // These errors indicate backend API failures that should not use YQ defaults. + ErrTerraformBackendAPIError = errors.New("terraform backend API error") + ErrUnsupportedBackendType = errors.New("unsupported backend type") + ErrProcessTerraformStateFile = errors.New("error processing terraform state file") + ErrLoadAwsConfig = errors.New("failed to load AWS config") + ErrGetObjectFromS3 = errors.New("failed to get object from S3") + ErrReadS3ObjectBody = errors.New("failed to read S3 object body") + ErrCreateGCSClient = errors.New("failed to create GCS client") + ErrGetObjectFromGCS = errors.New("failed to get object from GCS") + ErrReadGCSObjectBody = errors.New("failed to read GCS object body") + ErrGCSBucketRequired = errors.New("bucket is required for gcs backend") + ErrInvalidBackendConfig = errors.New("invalid backend configuration") // Azure Blob Storage specific errors. ErrGetBlobFromAzure = errors.New("failed to get blob from Azure Blob Storage") diff --git a/go.mod b/go.mod index 82d1935382..2b542315d7 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( github.com/alicebob/miniredis/v2 v2.35.0 github.com/arsham/figurine v1.3.0 github.com/atotto/clipboard v0.1.4 - github.com/aws/aws-sdk-go-v2 v1.40.1 + github.com/aws/aws-sdk-go-v2 v1.41.0 github.com/aws/aws-sdk-go-v2/config v1.32.3 github.com/aws/aws-sdk-go-v2/credentials v1.19.3 github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.20.13 diff --git a/go.sum b/go.sum index dec5bb1bf0..8bc00e2b5b 100644 --- a/go.sum +++ b/go.sum @@ -171,8 +171,8 @@ github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevB github.com/aws/aws-sdk-go v1.34.0/go.mod h1:5zCpMtNQVjRREroY7sYe8lOMRSxkhG6MZveU8YkpAk0= github.com/aws/aws-sdk-go v1.55.7 h1:UJrkFq7es5CShfBwlWAC8DA077vp8PyVbQd3lqLiztE= github.com/aws/aws-sdk-go v1.55.7/go.mod h1:eRwEWoyTWFMVYVQzKMNHWP5/RV4xIUGMQfXQHfHkpNU= -github.com/aws/aws-sdk-go-v2 v1.40.1 h1:difXb4maDZkRH0x//Qkwcfpdg1XQVXEAEs2DdXldFFc= -github.com/aws/aws-sdk-go-v2 v1.40.1/go.mod h1:MayyLB8y+buD9hZqkCW3kX1AKq07Y5pXxtgB+rRFhz0= +github.com/aws/aws-sdk-go-v2 v1.41.0 h1:tNvqh1s+v0vFYdA1xq0aOJH+Y5cRyZ5upu6roPgPKd4= +github.com/aws/aws-sdk-go-v2 v1.41.0/go.mod h1:MayyLB8y+buD9hZqkCW3kX1AKq07Y5pXxtgB+rRFhz0= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.4 h1:489krEF9xIGkOaaX3CE/Be2uWjiXrkCH6gUX+bZA/BU= github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.4/go.mod h1:IOAPF6oT9KCsceNTvvYMNHy0+kMF8akOjeDvPENWxp4= github.com/aws/aws-sdk-go-v2/config v1.32.3 h1:cpz7H2uMNTDa0h/5CYL5dLUEzPSLo2g0NkbxTRJtSSU= diff --git a/internal/exec/describe_component_nested_authmanager_test.go b/internal/exec/describe_component_nested_authmanager_test.go index 24f949595d..82da59e4e6 100644 --- a/internal/exec/describe_component_nested_authmanager_test.go +++ b/internal/exec/describe_component_nested_authmanager_test.go @@ -11,6 +11,94 @@ import ( "github.com/cloudposse/atmos/pkg/schema" ) +// setupMockStateGetter configures the global stateGetter to return mock values +// for terraform state lookups. This allows tests to run without actual terraform +// state files. Returns a cleanup function that must be deferred to restore the +// original state getter. +func setupMockStateGetter(t *testing.T, ctrl *gomock.Controller) func() { + t.Helper() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + + // Configure mock to return values for all components in the fixture. + + // Level 3 components have no dependencies. + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "level3-component", "subnet_id", gomock.Any(), gomock.Any(), gomock.Any()). + Return("subnet-level3-12345", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "level3-component", "cidr_block", gomock.Any(), gomock.Any(), gomock.Any()). + Return("10.0.3.0/24", nil). + AnyTimes() + + // Level 2 components depend on level 3. + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "level2-component", "vpc_id", gomock.Any(), gomock.Any(), gomock.Any()). + Return("vpc-level2-67890", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "level2-component", "level3_subnet_id", gomock.Any(), gomock.Any(), gomock.Any()). + Return("subnet-level3-12345", nil). + AnyTimes() + + // Auth override scenario components test middle-level auth override. + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "auth-override-level3", "database_host", gomock.Any(), gomock.Any(), gomock.Any()). + Return("db.example.com", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "auth-override-level2", "service_name", gomock.Any(), gomock.Any(), gomock.Any()). + Return("api-service", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "auth-override-level2", "database_config", gomock.Any(), gomock.Any(), gomock.Any()). + Return("db.example.com", nil). + AnyTimes() + + // Multi-auth scenario components test multiple auth overrides in chain. + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "multi-auth-level3", "shared_resource_id", gomock.Any(), gomock.Any(), gomock.Any()). + Return("shared-12345", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "multi-auth-level2", "vpc_id", gomock.Any(), gomock.Any(), gomock.Any()). + Return("vpc-account-b", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "multi-auth-level2", "shared_resource", gomock.Any(), gomock.Any(), gomock.Any()). + Return("shared-12345", nil). + AnyTimes() + + // Mixed inheritance scenario components test selective auth override. + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "mixed-inherit-component", "config_value", gomock.Any(), gomock.Any(), gomock.Any()). + Return("inherited-auth-config", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "mixed-override-component", "override_value", gomock.Any(), gomock.Any(), gomock.Any()). + Return("override-specific-value", nil). + AnyTimes() + + // Deep nesting scenario components test 4-level deep auth override. + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "deep-level4", "data_source", gomock.Any(), gomock.Any(), gomock.Any()). + Return("primary-db", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "deep-level3", "data_ref", gomock.Any(), gomock.Any(), gomock.Any()). + Return("primary-db", nil). + AnyTimes() + mockStateGetter.EXPECT(). + GetState(gomock.Any(), gomock.Any(), "test", "deep-level2", "nested_data", gomock.Any(), gomock.Any(), gomock.Any()). + Return("primary-db", nil). + AnyTimes() + + stateGetter = mockStateGetter + return func() { stateGetter = originalGetter } +} + // TestNestedAuthManagerPropagation verifies that AuthManager propagates correctly // through multiple levels of nested !terraform.state YAML functions. // @@ -34,6 +122,10 @@ func TestNestedAuthManagerPropagation(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() + // Setup mock state getter to return expected values without actual terraform state. + cleanup := setupMockStateGetter(t, ctrl) + defer cleanup() + // Setup: Create AuthContext with AWS credentials. expectedAuthContext := &schema.AuthContext{ AWS: &schema.AWSAuthContext{ @@ -94,6 +186,10 @@ func TestNestedAuthManagerPropagationLevel2(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() + // Setup mock state getter to return expected values without actual terraform state. + cleanup := setupMockStateGetter(t, ctrl) + defer cleanup() + expectedAuthContext := &schema.AuthContext{ AWS: &schema.AWSAuthContext{ Profile: "test-level2-identity", @@ -293,12 +389,15 @@ func TestNestedAuthManagerWithNilAuthContext(t *testing.T) { } // setupNestedAuthTest creates a mock AuthManager and sets up the test directory. -// Returns the mock controller and AuthManager for use in nested auth tests. -func setupNestedAuthTest(t *testing.T, profile, region string) (*gomock.Controller, types.AuthManager) { +// Returns the mock controller, AuthManager, and cleanup function for use in nested auth tests. +func setupNestedAuthTest(t *testing.T, profile, region string) (*gomock.Controller, types.AuthManager, func()) { t.Helper() ctrl := gomock.NewController(t) + // Setup mock state getter to return expected values without actual terraform state. + stateCleanup := setupMockStateGetter(t, ctrl) + parentAuthContext := &schema.AuthContext{ AWS: &schema.AWSAuthContext{ Profile: profile, @@ -320,7 +419,7 @@ func setupNestedAuthTest(t *testing.T, profile, region string) (*gomock.Controll workDir := "../../tests/fixtures/scenarios/authmanager-nested-propagation" t.Chdir(workDir) - return ctrl, mockAuthManager + return ctrl, mockAuthManager, stateCleanup } // TestNestedAuthManagerScenario2_AuthOverrideAtMiddleLevel verifies that @@ -340,8 +439,9 @@ func setupNestedAuthTest(t *testing.T, profile, region string) (*gomock.Controll // - Level 3 inherits Level 2's AuthManager // - No IMDS timeout errors at any level. func TestNestedAuthManagerScenario2_AuthOverrideAtMiddleLevel(t *testing.T) { - ctrl, mockAuthManager := setupNestedAuthTest(t, "parent-profile", "us-east-1") + ctrl, mockAuthManager, stateCleanup := setupNestedAuthTest(t, "parent-profile", "us-east-1") defer ctrl.Finish() + defer stateCleanup() componentSection, err := ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ Component: "auth-override-level1", @@ -378,8 +478,9 @@ func TestNestedAuthManagerScenario2_AuthOverrideAtMiddleLevel(t *testing.T) { // - Level 3 uses account 333333333333 (inherited from Level 2) // - Each level with auth config creates its own AuthManager. func TestNestedAuthManagerScenario3_MultipleAuthOverrides(t *testing.T) { - ctrl, mockAuthManager := setupNestedAuthTest(t, "global-profile", "us-east-1") + ctrl, mockAuthManager, stateCleanup := setupNestedAuthTest(t, "global-profile", "us-east-1") defer ctrl.Finish() + defer stateCleanup() componentSection, err := ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ Component: "multi-auth-level1", @@ -415,8 +516,9 @@ func TestNestedAuthManagerScenario3_MultipleAuthOverrides(t *testing.T) { // - mixed-override-component uses account 555555555555 (its own auth) // - When mixed-override calls mixed-inherit, it inherits account 555555555555. func TestNestedAuthManagerScenario4_MixedInheritance(t *testing.T) { - ctrl, mockAuthManager := setupNestedAuthTest(t, "parent-profile", "us-west-2") + ctrl, mockAuthManager, stateCleanup := setupNestedAuthTest(t, "parent-profile", "us-west-2") defer ctrl.Finish() + defer stateCleanup() componentSection, err := ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ Component: "mixed-top-level", @@ -455,8 +557,9 @@ func TestNestedAuthManagerScenario4_MixedInheritance(t *testing.T) { // - Level 3 switches to account 666666666666 (Level3Access) - its own override // - Level 4 inherits account 666666666666 from Level 3. func TestNestedAuthManagerScenario5_DeepNesting(t *testing.T) { - ctrl, mockAuthManager := setupNestedAuthTest(t, "global-profile", "us-east-1") + ctrl, mockAuthManager, stateCleanup := setupNestedAuthTest(t, "global-profile", "us-east-1") defer ctrl.Finish() + defer stateCleanup() componentSection, err := ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ Component: "deep-level1", diff --git a/internal/exec/spinner_utils.go b/internal/exec/spinner_utils.go index 36836faf63..274d2b246d 100644 --- a/internal/exec/spinner_utils.go +++ b/internal/exec/spinner_utils.go @@ -6,9 +6,9 @@ import ( "github.com/charmbracelet/bubbles/spinner" tea "github.com/charmbracelet/bubbletea" - log "github.com/charmbracelet/log" "github.com/cloudposse/atmos/internal/tui/templates/term" + log "github.com/cloudposse/atmos/pkg/logger" "github.com/cloudposse/atmos/pkg/terminal" "github.com/cloudposse/atmos/pkg/ui/theme" ) diff --git a/internal/exec/terraform_state_utils.go b/internal/exec/terraform_state_utils.go index 945a352fcc..fea6c8a3cf 100644 --- a/internal/exec/terraform_state_utils.go +++ b/internal/exec/terraform_state_utils.go @@ -130,9 +130,10 @@ func GetTerraformState( // Cache the result. terraformStateCache.Store(stackSlug, backend) - // If `backend` is `nil`, return `nil` (the component in the stack has not been provisioned yet). + // If `backend` is `nil`, return a recoverable error (the component in the stack has not been provisioned yet). + // This allows callers to use YQ defaults if available. if backend == nil { - return nil, nil + return nil, fmt.Errorf("%w for component `%s` in stack `%s`", errUtils.ErrTerraformStateNotProvisioned, component, stack) } // Get the output. diff --git a/internal/exec/yaml_func_aws_test.go b/internal/exec/yaml_func_aws_test.go index 67b955c47c..cebb4b82f4 100644 --- a/internal/exec/yaml_func_aws_test.go +++ b/internal/exec/yaml_func_aws_test.go @@ -444,7 +444,8 @@ func TestProcessSimpleTagsWithAWSFunctions(t *testing.T) { stackInfo := &schema.ConfigAndStacksInfo{} // Test !aws.account_id through processSimpleTags. - result, handled := processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsAccountID, "", nil, stackInfo) + result, handled, err := processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsAccountID, "", nil, stackInfo) + assert.NoError(t, err) assert.True(t, handled) assert.Equal(t, "333333333333", result) @@ -452,7 +453,8 @@ func TestProcessSimpleTagsWithAWSFunctions(t *testing.T) { ClearAWSIdentityCache() // Test !aws.caller_identity_arn through processSimpleTags. - result, handled = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityArn, "", nil, stackInfo) + result, handled, err = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityArn, "", nil, stackInfo) + assert.NoError(t, err) assert.True(t, handled) assert.Equal(t, "arn:aws:iam::333333333333:user/integration-test", result) @@ -460,7 +462,8 @@ func TestProcessSimpleTagsWithAWSFunctions(t *testing.T) { ClearAWSIdentityCache() // Test !aws.caller_identity_user_id through processSimpleTags. - result, handled = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityUserID, "", nil, stackInfo) + result, handled, err = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityUserID, "", nil, stackInfo) + assert.NoError(t, err) assert.True(t, handled) assert.Equal(t, "AIDAINTEGRATION", result) @@ -468,7 +471,8 @@ func TestProcessSimpleTagsWithAWSFunctions(t *testing.T) { ClearAWSIdentityCache() // Test !aws.region through processSimpleTags. - result, handled = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsRegion, "", nil, stackInfo) + result, handled, err = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsRegion, "", nil, stackInfo) + assert.NoError(t, err) assert.True(t, handled) assert.Equal(t, "us-west-2", result) } @@ -479,25 +483,29 @@ func TestProcessSimpleTagsSkipsAWSFunctions(t *testing.T) { // Test that skipping works for aws.account_id. skip := []string{"aws.account_id"} - result, handled := processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsAccountID, "", skip, stackInfo) + result, handled, err := processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsAccountID, "", skip, stackInfo) + assert.NoError(t, err) assert.False(t, handled) assert.Nil(t, result) // Test that skipping works for aws.caller_identity_arn. skip = []string{"aws.caller_identity_arn"} - result, handled = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityArn, "", skip, stackInfo) + result, handled, err = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityArn, "", skip, stackInfo) + assert.NoError(t, err) assert.False(t, handled) assert.Nil(t, result) // Test that skipping works for aws.caller_identity_user_id. skip = []string{"aws.caller_identity_user_id"} - result, handled = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityUserID, "", skip, stackInfo) + result, handled, err = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsCallerIdentityUserID, "", skip, stackInfo) + assert.NoError(t, err) assert.False(t, handled) assert.Nil(t, result) // Test that skipping works for aws.region. skip = []string{"aws.region"} - result, handled = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsRegion, "", skip, stackInfo) + result, handled, err = processSimpleTags(atmosConfig, u.AtmosYamlFuncAwsRegion, "", skip, stackInfo) + assert.NoError(t, err) assert.False(t, handled) assert.Nil(t, result) } diff --git a/internal/exec/yaml_func_resolution_context_bench_test.go b/internal/exec/yaml_func_resolution_context_bench_test.go index b65c76b1c9..67089f7973 100644 --- a/internal/exec/yaml_func_resolution_context_bench_test.go +++ b/internal/exec/yaml_func_resolution_context_bench_test.go @@ -117,7 +117,7 @@ func BenchmarkProcessCustomYamlTagsOverhead(b *testing.B) { b.Run("WithoutCycleDetection", func(b *testing.B) { // Simulate old behavior (direct processing without context). for i := 0; i < b.N; i++ { - result := processNodes(atmosConfig, input, "test-stack", nil, nil) + result, _ := processNodes(atmosConfig, input, "test-stack", nil, nil) _ = result } }) diff --git a/internal/exec/yaml_func_terraform_output.go b/internal/exec/yaml_func_terraform_output.go index 635d790cdf..68119ec3f8 100644 --- a/internal/exec/yaml_func_terraform_output.go +++ b/internal/exec/yaml_func_terraform_output.go @@ -19,20 +19,21 @@ func processTagTerraformOutput( input string, currentStack string, stackInfo *schema.ConfigAndStacksInfo, -) any { +) (any, error) { return processTagTerraformOutputWithContext(atmosConfig, input, currentStack, nil, stackInfo) } // trackOutputDependency records the dependency in the resolution context and returns a cleanup function. +// It returns an error if cycle detection fails. func trackOutputDependency( atmosConfig *schema.AtmosConfiguration, resolutionCtx *ResolutionContext, component string, stack string, input string, -) func() { +) (func(), error) { if resolutionCtx == nil { - return func() {} + return func() {}, nil } node := DependencyNode{ @@ -44,11 +45,11 @@ func trackOutputDependency( // Check and record this dependency. if err := resolutionCtx.Push(atmosConfig, node); err != nil { - errUtils.CheckErrorPrintAndExit(err, "", "") + return nil, err } // Return cleanup function. - return func() { resolutionCtx.Pop(atmosConfig) } + return func() { resolutionCtx.Pop(atmosConfig) }, nil } // processTagTerraformOutputWithContext processes `!terraform.output` YAML tag with cycle detection. @@ -58,13 +59,15 @@ func processTagTerraformOutputWithContext( currentStack string, resolutionCtx *ResolutionContext, stackInfo *schema.ConfigAndStacksInfo, -) any { +) (any, error) { defer perf.Track(atmosConfig, "exec.processTagTerraformOutputWithContext")() - log.Debug("Executing Atmos YAML function", "function", input) + log.Debug("Executing Atmos YAML function", log.FieldFunction, input) str, err := getStringAfterTag(input, u.AtmosYamlFuncTerraformOutput) - errUtils.CheckErrorPrintAndExit(err, "", "") + if err != nil { + return nil, err + } var component string var stack string @@ -74,7 +77,9 @@ func processTagTerraformOutputWithContext( // while also ignoring leading and trailing whitespace. // SplitStringByDelimiter splits a string by the delimiter, not splitting inside quotes. parts, err := u.SplitStringByDelimiter(str, ' ') - errUtils.CheckErrorPrintAndExit(err, "", "") + if err != nil { + return nil, err + } partsLen := len(parts) @@ -88,16 +93,19 @@ func processTagTerraformOutputWithContext( stack = currentStack output = strings.TrimSpace(parts[1]) log.Debug("Executing Atmos YAML function with component and output parameters; using current stack", - "function", input, + log.FieldFunction, input, "stack", currentStack, ) default: - er := fmt.Errorf("%w %s", errUtils.ErrYamlFuncInvalidArguments, input) - errUtils.CheckErrorPrintAndExit(er, "", "") + return nil, fmt.Errorf("%w %s", errUtils.ErrYamlFuncInvalidArguments, input) } - // Track dependency and defer cleanup. - defer trackOutputDependency(atmosConfig, resolutionCtx, component, stack, input)() + // Track dependency and get cleanup function. + cleanup, err := trackOutputDependency(atmosConfig, resolutionCtx, component, stack, input) + if err != nil { + return nil, err + } + defer cleanup() // Extract authContext and authManager from stackInfo if available. var authContext *schema.AuthContext @@ -109,17 +117,49 @@ func processTagTerraformOutputWithContext( value, exists, err := outputGetter.GetOutput(atmosConfig, stack, component, output, false, authContext, authManager) if err != nil { - er := fmt.Errorf("failed to get terraform output for component %s in stack %s, output %s: %w", component, stack, output, err) - errUtils.CheckErrorPrintAndExit(er, "", "") + // Only use YQ defaults for recoverable terraform errors (state not provisioned, output not found). + // Non-recoverable errors (API failures, auth errors, infrastructure issues) should fail hard. + if isRecoverableTerraformError(err) && hasYqDefault(output) { + log.Debug("Evaluating YQ default for recoverable error", + log.FieldFunction, input, + "error", err.Error(), + ) + // Evaluate YQ against an empty map to get the default value. + defaultValue, yqErr := evaluateYqDefault(atmosConfig, output) + if yqErr != nil { + // If YQ evaluation fails, return the original error. + return nil, fmt.Errorf("failed to get terraform output for component %s in stack %s, output %s: %w", component, stack, output, err) + } + return defaultValue, nil + } + return nil, fmt.Errorf("failed to get terraform output for component %s in stack %s, output %s: %w", component, stack, output, err) } - // If the output doesn't exist, return nil (backward compatible). - // This allows YAML functions to reference outputs that don't exist yet. - // Use yq fallback syntax (.output // "default") for default values. + // If the output doesn't exist, check if we can use YQ default. if !exists { - return nil + if hasYqDefault(output) { + log.Debug("Evaluating YQ default for non-existent output", + log.FieldFunction, input, + "component", component, + "stack", stack, + "output", output, + ) + // Evaluate YQ against an empty map to get the default value. + defaultValue, yqErr := evaluateYqDefault(atmosConfig, output) + if yqErr != nil { + // If YQ evaluation fails, return nil (backward compatible). + log.Debug("YQ default evaluation failed, returning nil", + log.FieldFunction, input, + "error", yqErr.Error(), + ) + return nil, nil + } + return defaultValue, nil + } + // No default available, return nil (backward compatible). + return nil, nil } // value may be nil here if the terraform output is legitimately null, which is valid. - return value + return value, nil } diff --git a/internal/exec/yaml_func_terraform_output_test.go b/internal/exec/yaml_func_terraform_output_test.go index 35800c5e02..55e30f68ed 100644 --- a/internal/exec/yaml_func_terraform_output_test.go +++ b/internal/exec/yaml_func_terraform_output_test.go @@ -61,13 +61,16 @@ func TestYamlFuncTerraformOutput(t *testing.T) { atmosConfig, err := cfg.InitCliConfig(info, true) assert.NoError(t, err) - d := processTagTerraformOutput(&atmosConfig, "!terraform.output component-1 foo", stack, nil) + d, err := processTagTerraformOutput(&atmosConfig, "!terraform.output component-1 foo", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-a", d) - d = processTagTerraformOutput(&atmosConfig, "!terraform.output component-1 bar", stack, nil) + d, err = processTagTerraformOutput(&atmosConfig, "!terraform.output component-1 bar", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-b", d) - d = processTagTerraformOutput(&atmosConfig, "!terraform.output component-1 nonprod baz", "", nil) + d, err = processTagTerraformOutput(&atmosConfig, "!terraform.output component-1 nonprod baz", "", nil) + assert.NoError(t, err) assert.Equal(t, "component-1-c", d) res, err := ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ @@ -102,13 +105,16 @@ func TestYamlFuncTerraformOutput(t *testing.T) { t.Fatalf("Failed to execute 'ExecuteTerraform': %v", err) } - d = processTagTerraformOutput(&atmosConfig, "!terraform.output component-2 foo", stack, nil) + d, err = processTagTerraformOutput(&atmosConfig, "!terraform.output component-2 foo", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-a", d) - d = processTagTerraformOutput(&atmosConfig, "!terraform.output component-2 nonprod bar", stack, nil) + d, err = processTagTerraformOutput(&atmosConfig, "!terraform.output component-2 nonprod bar", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-b", d) - d = processTagTerraformOutput(&atmosConfig, "!terraform.output component-2 nonprod baz", "", nil) + d, err = processTagTerraformOutput(&atmosConfig, "!terraform.output component-2 nonprod baz", "", nil) + assert.NoError(t, err) assert.Equal(t, "component-1-c", d) res, err = ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ diff --git a/internal/exec/yaml_func_terraform_output_yq_defaults_test.go b/internal/exec/yaml_func_terraform_output_yq_defaults_test.go new file mode 100644 index 0000000000..b92304dc49 --- /dev/null +++ b/internal/exec/yaml_func_terraform_output_yq_defaults_test.go @@ -0,0 +1,534 @@ +package exec + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + + errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/schema" +) + +// TestTerraformOutput_RecoverableErrorWithDefaultUsesDefault verifies that when +// GetOutput returns a recoverable error (state not provisioned) AND the expression +// has a YQ default, the default is used. +func TestTerraformOutput_RecoverableErrorWithDefaultUsesDefault(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.bucket_name // "default-bucket"` + + // Mock returns a recoverable error - state not provisioned. + // This is a recoverable error that should use the YQ default. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, fmt.Errorf("component not provisioned: %w", errUtils.ErrTerraformStateNotProvisioned)). + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": `!terraform.output vpc test-stack ".bucket_name // ""default-bucket"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + // With a YQ default and a recoverable error, the default should be used. + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "default-bucket", result["bucket"]) +} + +// TestTerraformOutput_APIErrorWithDefaultReturnsError verifies that when GetOutput +// returns a non-recoverable API error, even with a YQ default, the error propagates. +// This ensures that infrastructure/API failures don't silently use defaults. +func TestTerraformOutput_APIErrorWithDefaultReturnsError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.bucket_name // "default-bucket"` + + // Mock returns a non-recoverable API error (S3 connection failure). + // This should NOT use the YQ default - it should return the error. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, fmt.Errorf("S3 connection timeout: %w", errUtils.ErrGetObjectFromS3)). + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": `!terraform.output vpc test-stack ".bucket_name // ""default-bucket"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + // API errors should propagate even when a YQ default is present. + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "S3 connection timeout") +} + +// TestTerraformOutput_YqDefaultWhenOutputNotExists verifies that YQ default +// values work when the output doesn't exist (exists=false). +func TestTerraformOutput_YqDefaultWhenOutputNotExists(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.bucket_name // "default-bucket"` + + // Mock returns exists=false - output doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, nil). // exists=false, no error + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": `!terraform.output vpc test-stack ".bucket_name // ""default-bucket"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "default-bucket", result["bucket"]) +} + +// TestTerraformOutput_YqDefaultWithListFallback verifies that YQ default +// values work with list fallback expressions. +func TestTerraformOutput_YqDefaultWithListFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.subnets // ["subnet-1", "subnet-2"]` + + // Mock returns exists=false - output doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, nil). + Times(1) + + input := schema.AtmosSectionMapType{ + "subnets": `!terraform.output vpc test-stack ".subnets // [""subnet-1"", ""subnet-2""]"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, []any{"subnet-1", "subnet-2"}, result["subnets"]) +} + +// TestTerraformOutput_NoDefaultExpressionReturnsNil verifies that expressions +// WITHOUT YQ defaults still return nil when output doesn't exist. +// This is the expected behavior - no default means nil is acceptable. +func TestTerraformOutput_NoDefaultExpressionReturnsNil(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + // Mock returns exists=false - output doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + ".bucket_name", // No default expression (no //) + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, nil). + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": "!terraform.output vpc test-stack .bucket_name", + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Without a default, nil is the expected result (backward compatible). + assert.Nil(t, result["bucket"]) +} + +// TestTerraformOutput_YqDefaultWhenValueIsNilButExists verifies that when +// the output exists but has a nil value, the value is returned (nil is valid). +// YQ evaluation happens at this level, and YQ's // operator handles null. +func TestTerraformOutput_YqDefaultWhenValueIsNilButExists(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.bucket_name // "default-bucket"` + + // Mock returns nil value but exists=true (output is explicitly null in terraform). + // When the output exists with a nil value, that's valid - return nil. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, true, nil). // value=nil, exists=true (terraform null output) + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": `!terraform.output vpc test-stack ".bucket_name // ""default-bucket"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // When the output exists (even with nil value), return the value. + // The YQ evaluation already happened in GetOutput. + assert.Nil(t, result["bucket"]) +} + +// TestTerraformOutput_APIErrorWithoutDefaultReturnsError verifies that API errors +// without YQ defaults properly return errors. +func TestTerraformOutput_APIErrorWithoutDefaultReturnsError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + // Mock returns error - simulating terraform output failure. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + ".bucket_name", // No default expression + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, fmt.Errorf("S3 connection timeout")). + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": "!terraform.output vpc test-stack .bucket_name", + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + // Without a default, API errors should propagate. + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "S3 connection timeout") +} + +// TestTerraformOutput_OutputNotFoundWithDefaultUsesDefault verifies that when +// GetOutput returns ErrTerraformOutputNotFound (output key doesn't exist in state) +// AND the expression has a YQ default, the default is used. +func TestTerraformOutput_OutputNotFoundWithDefaultUsesDefault(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.missing_output // "fallback-value"` + + // Mock returns ErrTerraformOutputNotFound - the output key doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, fmt.Errorf("output key not found: %w", errUtils.ErrTerraformOutputNotFound)). + Times(1) + + input := schema.AtmosSectionMapType{ + "value": `!terraform.output vpc test-stack ".missing_output // ""fallback-value"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + // With a YQ default and ErrTerraformOutputNotFound, the default should be used. + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "fallback-value", result["value"]) +} + +// TestTerraformOutput_YqDefaultWithMapFallback verifies that YQ default +// values work with map/object fallback expressions. +func TestTerraformOutput_YqDefaultWithMapFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.tags // {"env": "dev", "team": "platform"}` + + // Mock returns exists=false - output doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "config", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, nil). + Times(1) + + input := schema.AtmosSectionMapType{ + "tags": `!terraform.output config test-stack ".tags // {""env"": ""dev"", ""team"": ""platform""}"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Verify the map structure is returned correctly. + expectedMap := map[string]any{"env": "dev", "team": "platform"} + assert.Equal(t, expectedMap, result["tags"]) +} + +// TestTerraformOutput_YqDefaultWithEmptyStringFallback verifies behavior +// when using an empty string as a YQ default value. +// Note: YQ evaluates empty strings to nil in the current implementation. +func TestTerraformOutput_YqDefaultWithEmptyStringFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.optional_value // ""` + + // Mock returns exists=false - output doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "config", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, nil). + Times(1) + + input := schema.AtmosSectionMapType{ + "optional": `!terraform.output config test-stack ".optional_value // """""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Note: YQ evaluates empty string default to nil in current implementation. + // This documents the actual behavior. + assert.Nil(t, result["optional"]) +} + +// TestTerraformOutput_YqDefaultWithNumericFallback verifies that +// numeric defaults work correctly. +func TestTerraformOutput_YqDefaultWithNumericFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.port // 8080` + + // Mock returns exists=false - output doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "config", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, nil). + Times(1) + + input := schema.AtmosSectionMapType{ + "port": `!terraform.output config test-stack ".port // 8080"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Numeric default should work. + assert.Equal(t, 8080, result["port"]) +} + +// TestTerraformOutput_YqDefaultWithBooleanFallback verifies that +// boolean defaults work correctly. +func TestTerraformOutput_YqDefaultWithBooleanFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockOutputGetter := NewMockTerraformOutputGetter(ctrl) + originalGetter := outputGetter + outputGetter = mockOutputGetter + defer func() { outputGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.enabled // true` + + // Mock returns exists=false - output doesn't exist. + mockOutputGetter.EXPECT(). + GetOutput( + atmosConfig, + "test-stack", + "config", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, false, nil). + Times(1) + + input := schema.AtmosSectionMapType{ + "enabled": `!terraform.output config test-stack ".enabled // true"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Boolean default should work. + assert.Equal(t, true, result["enabled"]) +} diff --git a/internal/exec/yaml_func_terraform_state.go b/internal/exec/yaml_func_terraform_state.go index 247c3b9a7e..1f5c897975 100644 --- a/internal/exec/yaml_func_terraform_state.go +++ b/internal/exec/yaml_func_terraform_state.go @@ -1,10 +1,12 @@ package exec import ( + "errors" "fmt" "strings" errUtils "github.com/cloudposse/atmos/errors" + tb "github.com/cloudposse/atmos/internal/terraform_backend" log "github.com/cloudposse/atmos/pkg/logger" "github.com/cloudposse/atmos/pkg/perf" "github.com/cloudposse/atmos/pkg/schema" @@ -19,10 +21,26 @@ func processTagTerraformState( input string, currentStack string, stackInfo *schema.ConfigAndStacksInfo, -) any { +) (any, error) { return processTagTerraformStateWithContext(atmosConfig, input, currentStack, nil, stackInfo) } +// isRecoverableTerraformError checks if an error is recoverable (can use YQ default). +func isRecoverableTerraformError(err error) bool { + return errors.Is(err, errUtils.ErrTerraformStateNotProvisioned) || + errors.Is(err, errUtils.ErrTerraformOutputNotFound) +} + +// hasYqDefault checks if a YQ expression contains a default (fallback) operator. +func hasYqDefault(yqExpr string) bool { + return strings.Contains(yqExpr, "//") +} + +// evaluateYqDefault evaluates a YQ expression against an empty map to get the default value. +func evaluateYqDefault(atmosConfig *schema.AtmosConfiguration, yqExpr string) (any, error) { + return tb.GetTerraformBackendVariable(atmosConfig, map[string]any{}, yqExpr) +} + // processTagTerraformStateWithContext processes `!terraform.state` YAML tag with cycle detection. func processTagTerraformStateWithContext( atmosConfig *schema.AtmosConfiguration, @@ -30,13 +48,15 @@ func processTagTerraformStateWithContext( currentStack string, resolutionCtx *ResolutionContext, stackInfo *schema.ConfigAndStacksInfo, -) any { +) (any, error) { defer perf.Track(atmosConfig, "exec.processTagTerraformStateWithContext")() log.Debug("Executing Atmos YAML function", "function", input) str, err := getStringAfterTag(input, u.AtmosYamlFuncTerraformState) - errUtils.CheckErrorPrintAndExit(err, "", "") + if err != nil { + return nil, err + } var component string var stack string @@ -46,7 +66,9 @@ func processTagTerraformStateWithContext( // while also ignoring leading and trailing whitespace. // SplitStringByDelimiter splits a string by the delimiter, not splitting inside quotes. parts, err := u.SplitStringByDelimiter(str, ' ') - errUtils.CheckErrorPrintAndExit(err, "", "") + if err != nil { + return nil, err + } partsLen := len(parts) @@ -64,8 +86,7 @@ func processTagTerraformStateWithContext( "stack", currentStack, ) default: - er := fmt.Errorf("%w %s", errUtils.ErrYamlFuncInvalidArguments, input) - errUtils.CheckErrorPrintAndExit(er, "", "") + return nil, fmt.Errorf("%w %s", errUtils.ErrYamlFuncInvalidArguments, input) } // Check for circular dependencies if resolution context is provided. @@ -79,7 +100,7 @@ func processTagTerraformStateWithContext( // Check and record this dependency. if err := resolutionCtx.Push(atmosConfig, node); err != nil { - errUtils.CheckErrorPrintAndExit(err, "", "") + return nil, err } // Defer pop to ensure we clean up even if there's an error. @@ -95,6 +116,24 @@ func processTagTerraformStateWithContext( } value, err := stateGetter.GetState(atmosConfig, input, stack, component, output, false, authContext, authManager) - errUtils.CheckErrorPrintAndExit(err, "", "") - return value + if err != nil { + // Check if this is a recoverable error AND the expression has a YQ default. + if isRecoverableTerraformError(err) && hasYqDefault(output) { + log.Debug("Evaluating YQ default for recoverable error", + "function", input, + "error", err.Error(), + ) + // Evaluate YQ against an empty map to get the default value. + defaultValue, yqErr := evaluateYqDefault(atmosConfig, output) + if yqErr != nil { + // If YQ evaluation fails, return the original error. + return nil, fmt.Errorf("%w: failed to evaluate YQ default: %w", err, yqErr) + } + return defaultValue, nil + } + // Non-recoverable error or no default available. + return nil, err + } + + return value, nil } diff --git a/internal/exec/yaml_func_terraform_state_test.go b/internal/exec/yaml_func_terraform_state_test.go index cef5139675..a678b9a324 100644 --- a/internal/exec/yaml_func_terraform_state_test.go +++ b/internal/exec/yaml_func_terraform_state_test.go @@ -61,13 +61,16 @@ func TestYamlFuncTerraformState(t *testing.T) { atmosConfig, err := cfg.InitCliConfig(info, true) assert.NoError(t, err) - d := processTagTerraformState(&atmosConfig, "!terraform.state component-1 foo", stack, nil) + d, err := processTagTerraformState(&atmosConfig, "!terraform.state component-1 foo", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-a", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-1 bar", stack, nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-1 bar", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-b", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-1 nonprod baz", "", nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-1 nonprod baz", "", nil) + assert.NoError(t, err) assert.Equal(t, "component-1-c", d) res, err := ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ @@ -102,13 +105,16 @@ func TestYamlFuncTerraformState(t *testing.T) { t.Fatalf("Failed to execute 'ExecuteTerraform': %v", err) } - d = processTagTerraformState(&atmosConfig, "!terraform.state component-2 foo", stack, nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-2 foo", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-a", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod bar", stack, nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod bar", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-b", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod baz", "", nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod baz", "", nil) + assert.NoError(t, err) assert.Equal(t, "component-1-c", d) res, err = ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ diff --git a/internal/exec/yaml_func_terraform_state_yq_defaults_test.go b/internal/exec/yaml_func_terraform_state_yq_defaults_test.go new file mode 100644 index 0000000000..0e82b0e176 --- /dev/null +++ b/internal/exec/yaml_func_terraform_state_yq_defaults_test.go @@ -0,0 +1,520 @@ +package exec + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/mock/gomock" + + errUtils "github.com/cloudposse/atmos/errors" + "github.com/cloudposse/atmos/pkg/schema" +) + +// TestIsRecoverableTerraformError tests the error classification helper function. +func TestIsRecoverableTerraformError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "ErrTerraformStateNotProvisioned is recoverable", + err: errUtils.ErrTerraformStateNotProvisioned, + expected: true, + }, + { + name: "Wrapped ErrTerraformStateNotProvisioned is recoverable", + err: fmt.Errorf("component not found: %w", errUtils.ErrTerraformStateNotProvisioned), + expected: true, + }, + { + name: "ErrTerraformOutputNotFound is recoverable", + err: errUtils.ErrTerraformOutputNotFound, + expected: true, + }, + { + name: "Wrapped ErrTerraformOutputNotFound is recoverable", + err: fmt.Errorf("output missing: %w", errUtils.ErrTerraformOutputNotFound), + expected: true, + }, + { + name: "ErrGetObjectFromS3 is not recoverable", + err: errUtils.ErrGetObjectFromS3, + expected: false, + }, + { + name: "ErrTerraformBackendAPIError is not recoverable", + err: errUtils.ErrTerraformBackendAPIError, + expected: false, + }, + { + name: "Generic error is not recoverable", + err: errors.New("some random error"), + expected: false, + }, + { + name: "Nil error is not recoverable", + err: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isRecoverableTerraformError(tt.err) + assert.Equal(t, tt.expected, result) + }) + } +} + +//nolint:dupl // Test structure is similar to TestHasSchemeSeparator but tests completely different function (YQ expressions vs URI schemes). +func TestHasYqDefault(t *testing.T) { + tests := []struct { + name string + yqExpr string + expected bool + }{ + { + name: "Expression with string default", + yqExpr: `.bucket_name // "default"`, + expected: true, + }, + { + name: "Expression with list default", + yqExpr: `.subnets // ["a", "b"]`, + expected: true, + }, + { + name: "Expression with map default", + yqExpr: `.tags // {"env": "dev"}`, + expected: true, + }, + { + name: "Expression with numeric default", + yqExpr: `.port // 8080`, + expected: true, + }, + { + name: "Expression with boolean default", + yqExpr: `.enabled // true`, + expected: true, + }, + { + name: "Expression with empty string default", + yqExpr: `.value // ""`, + expected: true, + }, + { + name: "Expression with null default", + yqExpr: `.value // null`, + expected: true, + }, + { + name: "Simple field access without default", + yqExpr: `.bucket_name`, + expected: false, + }, + { + name: "Array access without default", + yqExpr: `.subnets[0]`, + expected: false, + }, + { + name: "Nested field access without default", + yqExpr: `.config.database.host`, + expected: false, + }, + { + name: "Field name without dot", + yqExpr: `bucket_name`, + expected: false, + }, + { + name: "Empty string", + yqExpr: ``, + expected: false, + }, + { + name: "Expression with pipe but no default", + yqExpr: `.items | length`, + expected: false, + }, + { + name: "Expression with multiple defaults (chained)", + yqExpr: `.value // .fallback // "default"`, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasYqDefault(tt.yqExpr) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestTerraformState_YqDefaultWhenBackendReturnsNil verifies that YQ default +// values work when the backend returns nil (component not provisioned). +// The mock returns the ErrTerraformStateNotProvisioned sentinel error to simulate +// the real behavior when a component is not provisioned. +func TestTerraformState_YqDefaultWhenBackendReturnsNil(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + // The YQ expression after CSV parsing will have the inner quotes preserved. + // Input: ".bucket_name // ""default-bucket""" + // After CSV parse: .bucket_name // "default-bucket" + expectedYqExpr := `.bucket_name // "default-bucket"` + + // Mock returns ErrTerraformStateNotProvisioned - simulating component not provisioned. + // This is the sentinel error that triggers YQ default evaluation. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), // yamlFunc (full function call string) + "test-stack", + "vpc", + expectedYqExpr, // YQ expression with default + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("%w for component `vpc` in stack `test-stack`", errUtils.ErrTerraformStateNotProvisioned)). + Times(1) + + // Input using YAML-style double quotes for escaping (like the fixture files). + // In YAML: !terraform.state vpc test-stack ".bucket_name // ""default-bucket""" + // The CSV parser handles the outer quotes and unescapes inner double quotes. + input := schema.AtmosSectionMapType{ + "bucket": `!terraform.state vpc test-stack ".bucket_name // ""default-bucket"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "default-bucket", result["bucket"]) +} + +// TestTerraformState_APIErrorReturnsError verifies that API errors (non-recoverable) +// properly return an error instead of using YQ defaults. +// API errors like S3 timeouts should cause the command to fail, not silently use defaults. +func TestTerraformState_APIErrorReturnsError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.bucket_name // "default-bucket"` + + // Mock returns a non-recoverable API error - S3 failure. + // This type of error should NOT use YQ defaults. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("S3 GetObject timeout after 30s")). + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": `!terraform.state vpc test-stack ".bucket_name // ""default-bucket"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + // API errors should propagate as errors, not use defaults. + assert.Error(t, err) + assert.Nil(t, result) + assert.Contains(t, err.Error(), "S3 GetObject timeout") +} + +// TestTerraformState_YqDefaultWithListFallback verifies that YQ default +// values work with list fallback expressions. +func TestTerraformState_YqDefaultWithListFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.subnets // ["subnet-1", "subnet-2"]` + + // Mock returns ErrTerraformStateNotProvisioned - triggers YQ default evaluation. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("%w for component `vpc` in stack `test-stack`", errUtils.ErrTerraformStateNotProvisioned)). + Times(1) + + input := schema.AtmosSectionMapType{ + "subnets": `!terraform.state vpc test-stack ".subnets // [""subnet-1"", ""subnet-2""]"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, []any{"subnet-1", "subnet-2"}, result["subnets"]) +} + +// TestTerraformState_NoDefaultExpressionReturnsError verifies that expressions +// WITHOUT YQ defaults return an error when the component is not provisioned. +// This is expected - without a default, the user must handle the error. +func TestTerraformState_NoDefaultExpressionReturnsError(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + // Mock returns ErrTerraformStateNotProvisioned. + // Without a default, this error should propagate. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), + "test-stack", + "vpc", + ".bucket_name", // No default expression (no //) + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("%w for component `vpc` in stack `test-stack`", errUtils.ErrTerraformStateNotProvisioned)). + Times(1) + + input := schema.AtmosSectionMapType{ + "bucket": "!terraform.state vpc test-stack .bucket_name", + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + // Without a default, the error should propagate. + assert.Error(t, err) + assert.Nil(t, result) + assert.ErrorIs(t, err, errUtils.ErrTerraformStateNotProvisioned) +} + +// TestTerraformState_OutputNotFoundWithDefaultUsesDefault verifies that when +// GetState returns ErrTerraformOutputNotFound (output key doesn't exist in state) +// AND the expression has a YQ default, the default is used. +func TestTerraformState_OutputNotFoundWithDefaultUsesDefault(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.missing_output // "fallback-value"` + + // Mock returns ErrTerraformOutputNotFound - the output key doesn't exist. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("output key not found: %w", errUtils.ErrTerraformOutputNotFound)). + Times(1) + + input := schema.AtmosSectionMapType{ + "value": `!terraform.state vpc test-stack ".missing_output // ""fallback-value"""`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + // With a YQ default and ErrTerraformOutputNotFound, the default should be used. + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "fallback-value", result["value"]) +} + +// TestTerraformState_YqDefaultWithMapFallback verifies that YQ default +// values work with map/object fallback expressions. +func TestTerraformState_YqDefaultWithMapFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.tags // {"env": "dev", "team": "platform"}` + + // Mock returns ErrTerraformStateNotProvisioned - component not provisioned. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), + "test-stack", + "config", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("%w for component `config` in stack `test-stack`", errUtils.ErrTerraformStateNotProvisioned)). + Times(1) + + input := schema.AtmosSectionMapType{ + "tags": `!terraform.state config test-stack ".tags // {""env"": ""dev"", ""team"": ""platform""}"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Verify the map structure is returned correctly. + expectedMap := map[string]any{"env": "dev", "team": "platform"} + assert.Equal(t, expectedMap, result["tags"]) +} + +// TestTerraformState_YqDefaultWithNumericFallback verifies that +// numeric defaults work correctly. +func TestTerraformState_YqDefaultWithNumericFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.replicas // 3` + + // Mock returns ErrTerraformStateNotProvisioned. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), + "test-stack", + "app", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("%w for component `app` in stack `test-stack`", errUtils.ErrTerraformStateNotProvisioned)). + Times(1) + + input := schema.AtmosSectionMapType{ + "replicas": `!terraform.state app test-stack ".replicas // 3"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Numeric default should work. + assert.Equal(t, 3, result["replicas"]) +} + +// TestTerraformState_YqDefaultWithEmptyListFallback verifies that +// empty list defaults work correctly. +func TestTerraformState_YqDefaultWithEmptyListFallback(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockStateGetter := NewMockTerraformStateGetter(ctrl) + originalGetter := stateGetter + stateGetter = mockStateGetter + defer func() { stateGetter = originalGetter }() + + atmosConfig := &schema.AtmosConfiguration{ + BasePath: t.TempDir(), + } + + expectedYqExpr := `.security_groups // []` + + // Mock returns ErrTerraformStateNotProvisioned. + mockStateGetter.EXPECT(). + GetState( + atmosConfig, + gomock.Any(), + "test-stack", + "vpc", + expectedYqExpr, + false, + gomock.Any(), + gomock.Any(), + ). + Return(nil, fmt.Errorf("%w for component `vpc` in stack `test-stack`", errUtils.ErrTerraformStateNotProvisioned)). + Times(1) + + input := schema.AtmosSectionMapType{ + "security_groups": `!terraform.state vpc test-stack ".security_groups // []"`, + } + + result, err := ProcessCustomYamlTags(atmosConfig, input, "test-stack", nil, nil) + + assert.NoError(t, err) + assert.NotNil(t, result) + // Empty list default should work. + assert.Equal(t, []any{}, result["security_groups"]) +} diff --git a/internal/exec/yaml_func_utils.go b/internal/exec/yaml_func_utils.go index d8d9ac877e..5a13c7673b 100644 --- a/internal/exec/yaml_func_utils.go +++ b/internal/exec/yaml_func_utils.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - errUtils "github.com/cloudposse/atmos/errors" "github.com/cloudposse/atmos/pkg/perf" "github.com/cloudposse/atmos/pkg/schema" u "github.com/cloudposse/atmos/pkg/utils" @@ -26,7 +25,7 @@ func ProcessCustomYamlTags( // Get the fresh context we just installed. resolutionCtx := GetOrCreateResolutionContext() - return processNodesWithContext(atmosConfig, input, currentStack, skip, resolutionCtx, stackInfo), nil + return processNodesWithContext(atmosConfig, input, currentStack, skip, resolutionCtx, stackInfo) } func ProcessCustomYamlTagsWithContext( @@ -39,7 +38,7 @@ func ProcessCustomYamlTagsWithContext( ) (schema.AtmosSectionMapType, error) { defer perf.Track(atmosConfig, "exec.ProcessCustomYamlTagsWithContext")() - return processNodesWithContext(atmosConfig, input, currentStack, skip, resolutionCtx, stackInfo), nil + return processNodesWithContext(atmosConfig, input, currentStack, skip, resolutionCtx, stackInfo) } func processNodes( @@ -48,7 +47,7 @@ func processNodes( currentStack string, skip []string, stackInfo *schema.ConfigAndStacksInfo, -) map[string]any { +) (map[string]any, error) { return processNodesWithContext(atmosConfig, data, currentStack, skip, nil, stackInfo) } @@ -59,14 +58,25 @@ func processNodesWithContext( skip []string, resolutionCtx *ResolutionContext, stackInfo *schema.ConfigAndStacksInfo, -) map[string]any { +) (map[string]any, error) { newMap := make(map[string]any) - var recurse func(any) any + var firstErr error + var recurse func(any) any recurse = func(node any) any { + // If we already have an error, skip processing. + if firstErr != nil { + return node + } + switch v := node.(type) { case string: - return processCustomTagsWithContext(atmosConfig, v, currentStack, skip, resolutionCtx, stackInfo) + result, err := processCustomTagsWithContext(atmosConfig, v, currentStack, skip, resolutionCtx, stackInfo) + if err != nil { + firstErr = err + return v + } + return result case map[string]any: newNestedMap := make(map[string]any) @@ -91,7 +101,11 @@ func processNodesWithContext( newMap[k] = recurse(v) } - return newMap + if firstErr != nil { + return nil, firstErr + } + + return newMap, nil } func processCustomTags( @@ -100,7 +114,7 @@ func processCustomTags( currentStack string, skip []string, stackInfo *schema.ConfigAndStacksInfo, -) any { +) (any, error) { return processCustomTagsWithContext(atmosConfig, input, currentStack, skip, nil, stackInfo) } @@ -110,6 +124,7 @@ func matchesPrefix(input, prefix string, skip []string) bool { } // processContextAwareTags processes tags that support cycle detection. +// Returns (result, handled, error) where handled indicates if a matching tag was found. func processContextAwareTags( atmosConfig *schema.AtmosConfiguration, input string, @@ -117,57 +132,64 @@ func processContextAwareTags( skip []string, resolutionCtx *ResolutionContext, stackInfo *schema.ConfigAndStacksInfo, -) (any, bool) { +) (any, bool, error) { if matchesPrefix(input, u.AtmosYamlFuncTerraformOutput, skip) { - return processTagTerraformOutputWithContext(atmosConfig, input, currentStack, resolutionCtx, stackInfo), true + result, err := processTagTerraformOutputWithContext(atmosConfig, input, currentStack, resolutionCtx, stackInfo) + return result, true, err } if matchesPrefix(input, u.AtmosYamlFuncTerraformState, skip) { - return processTagTerraformStateWithContext(atmosConfig, input, currentStack, resolutionCtx, stackInfo), true + result, err := processTagTerraformStateWithContext(atmosConfig, input, currentStack, resolutionCtx, stackInfo) + return result, true, err } - return nil, false + return nil, false, nil } // processSimpleTags processes tags that don't need cycle detection. +// Returns (result, handled, error) where handled indicates if a matching tag was found. func processSimpleTags( atmosConfig *schema.AtmosConfiguration, input string, currentStack string, skip []string, stackInfo *schema.ConfigAndStacksInfo, -) (any, bool) { +) (any, bool, error) { if matchesPrefix(input, u.AtmosYamlFuncTemplate, skip) { - return processTagTemplate(input), true + return processTagTemplate(input), true, nil } if matchesPrefix(input, u.AtmosYamlFuncExec, skip) { res, err := u.ProcessTagExec(input) - errUtils.CheckErrorPrintAndExit(err, "", "") - return res, true + if err != nil { + return nil, true, err + } + return res, true, nil } if matchesPrefix(input, u.AtmosYamlFuncStoreGet, skip) { - return processTagStoreGet(atmosConfig, input, currentStack), true + return processTagStoreGet(atmosConfig, input, currentStack), true, nil } if matchesPrefix(input, u.AtmosYamlFuncStore, skip) { - return processTagStore(atmosConfig, input, currentStack), true + return processTagStore(atmosConfig, input, currentStack), true, nil } if matchesPrefix(input, u.AtmosYamlFuncEnv, skip) { res, err := u.ProcessTagEnv(input, stackInfo) - errUtils.CheckErrorPrintAndExit(err, "", "") - return res, true + if err != nil { + return nil, true, err + } + return res, true, nil } // AWS YAML functions - note these check for exact match since they take no arguments. if input == u.AtmosYamlFuncAwsAccountID && !skipFunc(skip, u.AtmosYamlFuncAwsAccountID) { - return processTagAwsAccountID(atmosConfig, input, stackInfo), true + return processTagAwsAccountID(atmosConfig, input, stackInfo), true, nil } if input == u.AtmosYamlFuncAwsCallerIdentityArn && !skipFunc(skip, u.AtmosYamlFuncAwsCallerIdentityArn) { - return processTagAwsCallerIdentityArn(atmosConfig, input, stackInfo), true + return processTagAwsCallerIdentityArn(atmosConfig, input, stackInfo), true, nil } if input == u.AtmosYamlFuncAwsCallerIdentityUserID && !skipFunc(skip, u.AtmosYamlFuncAwsCallerIdentityUserID) { - return processTagAwsCallerIdentityUserID(atmosConfig, input, stackInfo), true + return processTagAwsCallerIdentityUserID(atmosConfig, input, stackInfo), true, nil } if input == u.AtmosYamlFuncAwsRegion && !skipFunc(skip, u.AtmosYamlFuncAwsRegion) { - return processTagAwsRegion(atmosConfig, input, stackInfo), true + return processTagAwsRegion(atmosConfig, input, stackInfo), true, nil } - return nil, false + return nil, false, nil } func processCustomTagsWithContext( @@ -177,19 +199,19 @@ func processCustomTagsWithContext( skip []string, resolutionCtx *ResolutionContext, stackInfo *schema.ConfigAndStacksInfo, -) any { +) (any, error) { // Try context-aware tags first. - if result, handled := processContextAwareTags(atmosConfig, input, currentStack, skip, resolutionCtx, stackInfo); handled { - return result + if result, handled, err := processContextAwareTags(atmosConfig, input, currentStack, skip, resolutionCtx, stackInfo); handled { + return result, err } // Try simple tags. - if result, handled := processSimpleTags(atmosConfig, input, currentStack, skip, stackInfo); handled { - return result + if result, handled, err := processSimpleTags(atmosConfig, input, currentStack, skip, stackInfo); handled { + return result, err } // If any other YAML explicit tag (not currently supported by Atmos) is used, return it w/o processing. - return input + return input, nil } func skipFunc(skip []string, f string) bool { diff --git a/internal/exec/yaml_func_utils_context_test.go b/internal/exec/yaml_func_utils_context_test.go index a32fbac404..78265b3d7d 100644 --- a/internal/exec/yaml_func_utils_context_test.go +++ b/internal/exec/yaml_func_utils_context_test.go @@ -85,8 +85,9 @@ func TestProcessNodesWithContextNestedMaps(t *testing.T) { }, } - result := processNodesWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + result, err := processNodesWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + require.NoError(t, err) require.NotNil(t, result) assert.Equal(t, input, result) } @@ -105,8 +106,9 @@ func TestProcessNodesWithContextSlices(t *testing.T) { }, } - result := processNodesWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + result, err := processNodesWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + require.NoError(t, err) require.NotNil(t, result) assert.Equal(t, input, result) } @@ -126,8 +128,9 @@ func TestProcessNodesWithContextMixedTypes(t *testing.T) { "complex": []any{map[string]any{"nested": true}}, } - result := processNodesWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + result, err := processNodesWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + require.NoError(t, err) require.NotNil(t, result) assert.Equal(t, input, result) } @@ -141,16 +144,19 @@ func TestProcessCustomTagsWithContextSkipFunctions(t *testing.T) { // These should not be processed (just returned as-is). input1 := "!terraform.state vpc dev output" - result1 := processCustomTagsWithContext(atmosConfig, input1, "test-stack", skip, ctx, nil) + result1, err := processCustomTagsWithContext(atmosConfig, input1, "test-stack", skip, ctx, nil) + require.NoError(t, err) assert.Equal(t, input1, result1) input2 := "!terraform.output vpc dev output" - result2 := processCustomTagsWithContext(atmosConfig, input2, "test-stack", skip, ctx, nil) + result2, err := processCustomTagsWithContext(atmosConfig, input2, "test-stack", skip, ctx, nil) + require.NoError(t, err) assert.Equal(t, input2, result2) // Non-skipped functions should still be processed. input3 := "!env HOME" - result3 := processCustomTagsWithContext(atmosConfig, input3, "test-stack", skip, ctx, nil) + result3, err := processCustomTagsWithContext(atmosConfig, input3, "test-stack", skip, ctx, nil) + require.NoError(t, err) // Result should be different (env var value or empty string). assert.IsType(t, "", result3) } @@ -161,7 +167,8 @@ func TestProcessCustomTagsWithContextTemplateFunction(t *testing.T) { // Template function should work without errors. input := "!template {{ .test }}" - result := processCustomTagsWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + result, err := processCustomTagsWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + require.NoError(t, err) // Should return a string (the template). assert.IsType(t, "", result) @@ -173,7 +180,8 @@ func TestProcessCustomTagsWithContextEnvFunction(t *testing.T) { // Test !env function. input := "!env USER" - result := processCustomTagsWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + result, err := processCustomTagsWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + require.NoError(t, err) // Should return a string. assert.IsType(t, "", result) @@ -185,7 +193,8 @@ func TestProcessCustomTagsWithContextUnknownTag(t *testing.T) { // Unknown tags should be returned as-is. input := "!unknown.function arg1 arg2" - result := processCustomTagsWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + result, err := processCustomTagsWithContext(atmosConfig, input, "test-stack", nil, ctx, nil) + require.NoError(t, err) assert.Equal(t, input, result) } diff --git a/internal/exec/yaml_func_utils_test.go b/internal/exec/yaml_func_utils_test.go index 9f155dd1bc..b45341967e 100644 --- a/internal/exec/yaml_func_utils_test.go +++ b/internal/exec/yaml_func_utils_test.go @@ -279,13 +279,16 @@ func TestProcessCustomYamlTags(t *testing.T) { atmosConfig, err := cfg.InitCliConfig(info, true) assert.NoError(t, err) - d := processTagTerraformState(&atmosConfig, "!terraform.state component-1 foo", stack, nil) + d, err := processTagTerraformState(&atmosConfig, "!terraform.state component-1 foo", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-a", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-1 bar", stack, nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-1 bar", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-b", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-1 nonprod baz", "", nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-1 nonprod baz", "", nil) + assert.NoError(t, err) assert.Equal(t, "component-1-c", d) res, err := ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ @@ -320,13 +323,16 @@ func TestProcessCustomYamlTags(t *testing.T) { t.Fatalf("Failed to execute 'ExecuteTerraform': %v", err) } - d = processTagTerraformState(&atmosConfig, "!terraform.state component-2 foo", stack, nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-2 foo", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-a", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod bar", stack, nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod bar", stack, nil) + assert.NoError(t, err) assert.Equal(t, "component-1-b", d) - d = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod baz", "", nil) + d, err = processTagTerraformState(&atmosConfig, "!terraform.state component-2 nonprod baz", "", nil) + assert.NoError(t, err) assert.Equal(t, "component-1-c", d) res, err = ExecuteDescribeComponent(&ExecuteDescribeComponentParams{ @@ -471,6 +477,7 @@ func TestProcessCustomYamlTagsStackInfoThreading(t *testing.T) { // The real test: Verify that when we process nodes, the stackInfo // is available for YAML function processing. // This is a white-box test that ensures the parameter flows through. - processedNodes := processNodes(atmosConfig, input, "test-stack", nil, stackInfo) + processedNodes, err := processNodes(atmosConfig, input, "test-stack", nil, stackInfo) + assert.NoError(t, err) assert.NotNil(t, processedNodes) } diff --git a/internal/exec/yaml_processor.go b/internal/exec/yaml_processor.go index d04fb27b98..f17cdb0032 100644 --- a/internal/exec/yaml_processor.go +++ b/internal/exec/yaml_processor.go @@ -55,5 +55,5 @@ func (p *ComponentYAMLProcessor) ProcessYAMLFunctionString(value string) (any, e p.skip, p.resolutionCtx, p.stackInfo, - ), nil + ) } diff --git a/internal/terraform_backend/terraform_backend_azurerm.go b/internal/terraform_backend/terraform_backend_azurerm.go index 3b6f38a7b0..0183645e67 100644 --- a/internal/terraform_backend/terraform_backend_azurerm.go +++ b/internal/terraform_backend/terraform_backend_azurerm.go @@ -253,6 +253,8 @@ func ReadTerraformBackendAzurermInternal( time.Sleep(backoff) continue } + // Retries exhausted - log warning with error details to help diagnose the issue. + logAzureRetryExhausted(err, tfStateFilePath, containerName, maxRetryCountAzure) cancel() return nil, fmt.Errorf(errWrapFormat, errUtils.ErrGetBlobFromAzure, lastErr) } @@ -270,3 +272,33 @@ func ReadTerraformBackendAzurermInternal( return nil, fmt.Errorf(errWrapFormat, errUtils.ErrGetBlobFromAzure, lastErr) } + +// logAzureRetryExhausted logs a warning when all retries are exhausted for Azure Blob operations. +// This helps users report issues by providing the HTTP status code and details. +func logAzureRetryExhausted(err error, tfStateFilePath, containerName string, maxRetries int) { + defer perf.Track(nil, "terraform_backend.logAzureRetryExhausted")() + + // Extract Azure HTTP status code if available. + statusCode := 0 + errorCode := "unknown" + var respErr *azcore.ResponseError + if errors.As(err, &respErr) { + statusCode = respErr.StatusCode + errorCode = fmt.Sprintf("HTTP_%d", statusCode) + } + + // Check for context timeout. + if errors.Is(err, context.DeadlineExceeded) { + errorCode = "timeout" + } + + log.Warn( + "Failed to read Terraform state after all retries exhausted", + "file", tfStateFilePath, + "container", containerName, + "attempts", maxRetries+1, + "error_code", errorCode, + "status_code", statusCode, + "error", err, + ) +} diff --git a/internal/terraform_backend/terraform_backend_gcs.go b/internal/terraform_backend/terraform_backend_gcs.go index 72456c02b3..556b9001ad 100644 --- a/internal/terraform_backend/terraform_backend_gcs.go +++ b/internal/terraform_backend/terraform_backend_gcs.go @@ -13,13 +13,13 @@ import ( "time" "cloud.google.com/go/storage" - log "github.com/charmbracelet/log" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" errUtils "github.com/cloudposse/atmos/errors" "github.com/cloudposse/atmos/internal/gcp" cfg "github.com/cloudposse/atmos/pkg/config" + log "github.com/cloudposse/atmos/pkg/logger" "github.com/cloudposse/atmos/pkg/perf" "github.com/cloudposse/atmos/pkg/schema" ) @@ -236,16 +236,26 @@ func ReadTerraformBackendGCSInternal( // Check if the error is because the object doesn't exist. // If the state file does not exist (the component in the stack has not been provisioned yet), return a `nil` result and no error. if errors.Is(err, storage.ErrObjectNotExist) || status.Code(err) == codes.NotFound { - log.Debug("Terraform state file doesn't exist in the GCS bucket; returning 'null'", "file", tfStateFilePath, "bucket", bucket) + log.Debug("Terraform state file doesn't exist in the GCS bucket; returning 'null'", "file", tfStateFilePath, log.FieldBucket, bucket) return nil, nil } lastErr = err if attempt < maxGCSRetryCount { - log.Debug("Failed to read Terraform state file from GCS bucket", "attempt", attempt+1, "file", tfStateFilePath, "bucket", bucket, "error", err) - time.Sleep(time.Second * 2) // backoff + // Exponential backoff: 1s, 2s, 4s for attempts 0, 1, 2. + backoff := time.Second * time.Duration(1< + +## What Changed + +YQ expressions with default values now work correctly in all scenarios: + +```yaml +# This now works reliably when vpc component isn't provisioned +vars: + vpc_id: !terraform.output vpc {{ .stack }} ".vpc_id // ""default-vpc""" + subnets: !terraform.state vpc {{ .stack }} ".subnets // [""subnet-1"", ""subnet-2""]" +``` + +## The Problem + +Previously, when a terraform component wasn't provisioned or an output didn't exist, the YAML function wrappers would return `nil` or exit before the YQ pipeline could evaluate default expressions. This caused sporadic failures where users expected the YQ `//` operator to provide fallback values. + +The issue manifested as: +- Stack configurations failing when referencing unprovisioned components +- Inconsistent behavior depending on component state +- No way to gracefully handle missing outputs with defaults + +## The Solution + +We refactored the YAML function processing to: + +1. **Properly classify errors**: Distinguish between recoverable errors (component not provisioned, output missing) and non-recoverable API errors (S3 timeouts, network failures) + +2. **Evaluate YQ defaults for recoverable errors**: When a component isn't provisioned but the expression includes a default (`//`), evaluate the YQ expression against an empty map to extract the default value + +3. **Propagate API errors**: Infrastructure failures like S3 timeouts correctly propagate as errors rather than silently using defaults + +## Examples + +### String Defaults + +```yaml +vars: + bucket_name: !terraform.output s3 {{ .stack }} ".bucket_name // ""default-bucket""" +``` + +If the `s3` component isn't provisioned, `bucket_name` will be `"default-bucket"`. + +### List Defaults + +```yaml +vars: + subnets: !terraform.state vpc {{ .stack }} ".private_subnets // [""subnet-a"", ""subnet-b""]" +``` + +If the `vpc` component isn't provisioned, `subnets` will be `["subnet-a", "subnet-b"]`. + +### Map Defaults + +```yaml +vars: + tags: !terraform.output common {{ .stack }} ".tags // {\"env\": \"dev\", \"team\": \"platform\"}" +``` + +### No Default (Error on Missing) + +```yaml +vars: + # This will error if vpc component isn't provisioned (expected behavior) + vpc_id: !terraform.output vpc {{ .stack }} .vpc_id +``` + +## Error Handling Behavior + +| Scenario | Has Default (`//`) | Result | +|----------|-------------------|--------| +| Component not provisioned | Yes | Uses default value | +| Component not provisioned | No | Returns error | +| Output missing | Yes | Uses default value | +| Output missing | No | Returns `nil` | +| API error (S3 timeout) | Yes/No | Returns error | + +## Migration + +No migration is required. Existing configurations that use YQ defaults will now work as expected. Configurations without defaults maintain their existing behavior. + +## Related Links + +- [PR #1836: Fix YQ defaults for terraform.state/output YAML functions](https://github.com/cloudposse/atmos/pull/1836) +- [YAML Functions Documentation](/functions/yaml) +- [Terraform Output Function](/functions/yaml/terraform.output) +- [Terraform State Function](/functions/yaml/terraform.state) diff --git a/website/docs/cli/commands/devcontainer/shell.mdx b/website/docs/cli/commands/devcontainer/shell.mdx index bd7a19f91d..43ad7d2cbf 100644 --- a/website/docs/cli/commands/devcontainer/shell.mdx +++ b/website/docs/cli/commands/devcontainer/shell.mdx @@ -152,4 +152,4 @@ This pattern provides a consistent experience with traditional shell wrappers li - [`atmos devcontainer start`](/cli/commands/devcontainer/start) - [`atmos devcontainer attach`](/cli/commands/devcontainer/attach) - [`atmos devcontainer list`](/cli/commands/devcontainer/list) -- [Command Aliases](/cli/configuration/commands#aliases) +- [Command Aliases](/cli/configuration/aliases) diff --git a/website/docs/functions/yaml/terraform.output.mdx b/website/docs/functions/yaml/terraform.output.mdx index 0387f4fee9..5c5c5321ff 100644 --- a/website/docs/functions/yaml/terraform.output.mdx +++ b/website/docs/functions/yaml/terraform.output.mdx @@ -169,16 +169,21 @@ This quoting pattern works for every Atmos YAML function that accepts YQ express ## Using YQ Expressions to provide a default value -If the component for which you are reading the output has not been provisioned yet, the `!terraform.output` function -will return `null` unless you specify a [default value](https://mikefarah.gitbook.io/yq/operators/alternative-default-value) -in the YQ expression, in which case the function will return the default value. +If the component for which you are reading the output has not been provisioned yet, or if the specific output doesn't exist, +you can specify a [default value](https://mikefarah.gitbook.io/yq/operators/alternative-default-value) +in the YQ expression using the `//` operator. Atmos will evaluate the default when the data is unavailable. -This will allow you to mock outputs when executing `atmos terraform plan` where there are dependencies between components, +This allows you to mock outputs when executing `atmos terraform plan` where there are dependencies between components, and the dependent components are not provisioned yet. -:::tip -When an output doesn't exist, the function returns `null` to maintain backward compatibility with existing workflows. -To explicitly handle missing outputs, use the YQ `//` operator to provide a default value. +:::tip Default Value Behavior +Atmos distinguishes between **recoverable errors** (component not provisioned, output missing) and **non-recoverable errors** (API failures, network timeouts): + +- **Recoverable errors with defaults**: When a component isn't provisioned or an output doesn't exist, AND you specify a YQ default (`//`), Atmos uses the default value +- **Recoverable errors without defaults**: Returns `null` for missing outputs (backward compatible) or an error for unprovisioned components +- **Non-recoverable errors**: API failures (S3 timeouts, authentication errors, network issues) always propagate as errors, even if a default is specified + +This ensures that infrastructure failures are never silently masked by default values. ::: :::note @@ -485,5 +490,4 @@ vars: - Be mindful of disaster recovery (DR) implications when using it across regions. - - Consider cold-start scenarios: if the dependent component has not yet been provisioned, the function will return `null`. - Use the YQ `//` operator to provide default values for components that may not be provisioned yet. + - Consider cold-start scenarios: if the dependent component has not yet been provisioned, the function will return an error unless you provide a YQ default value using the `//` operator. See [Using YQ Expressions to provide a default value](#using-yq-expressions-to-provide-a-default-value) for details on error handling behavior. diff --git a/website/docs/functions/yaml/terraform.state.mdx b/website/docs/functions/yaml/terraform.state.mdx index fd8bcb58a3..492a238168 100644 --- a/website/docs/functions/yaml/terraform.state.mdx +++ b/website/docs/functions/yaml/terraform.state.mdx @@ -164,13 +164,23 @@ This quoting pattern works for every Atmos YAML function that accepts YQ express ## Using YQ Expressions to provide a default value -If the component for which you are reading the output has not been provisioned yet, the `!terraform.state` function -will return `null`, unless you specify a [default value](https://mikefarah.gitbook.io/yq/operators/alternative-default-value) -in the YQ expression, in which case the function will return the default value. +If the component for which you are reading the state has not been provisioned yet, or if the specific output doesn't exist, +you can specify a [default value](https://mikefarah.gitbook.io/yq/operators/alternative-default-value) +in the YQ expression using the `//` operator. Atmos will evaluate the default when the data is unavailable. -This will allow you to mock outputs when executing `atmos terraform plan` where there are dependencies between components, +This allows you to mock outputs when executing `atmos terraform plan` where there are dependencies between components, and the dependent components are not provisioned yet. +:::tip Default Value Behavior +Atmos distinguishes between **recoverable errors** (component not provisioned, output missing) and **non-recoverable errors** (backend API failures): + +- **Recoverable errors with defaults**: When a component's state doesn't exist or an output is missing, AND you specify a YQ default (`//`), Atmos uses the default value +- **Recoverable errors without defaults**: Returns an error when state doesn't exist, or `null` for missing outputs +- **Non-recoverable errors**: Backend API failures (S3 access denied, GCS timeouts, Azure connectivity issues) always propagate as errors, even if a default is specified + +This ensures that infrastructure failures are never silently masked by default values. +::: + :::note To provide a default value, you use the `//` YQ operator. The whole YQ expression contains spaces, and to make it a single parameter, you need to double-quote it. @@ -515,5 +525,4 @@ vars: - Be mindful of disaster recovery (DR) implications when using it across regions. - - Consider cold-start scenarios: if the referenced component's state file doesn't exist (e.g., not yet provisioned), `!terraform.state` returns `null`. - Use the YQ `//` operator to provide default values for components that may not be provisioned yet (see [Using YQ Expressions to provide a default value](#using-yq-expressions-to-provide-a-default-value)). + - Consider cold-start scenarios: if the referenced component's state file doesn't exist (e.g., not yet provisioned), `!terraform.state` returns an error unless you provide a YQ default value using the `//` operator. See [Using YQ Expressions to provide a default value](#using-yq-expressions-to-provide-a-default-value) for details on error handling behavior.