cloudposse / terraform-aws-components

Opinionated, self-contained Terraform root modules that each solve one, specific problem
https://docs.cloudposse.com/components/
Apache License 2.0
494 stars 209 forks source link

spacelift admin/stack component broken by v1.400.0 #996

Open MReggler opened 6 months ago

MReggler commented 6 months ago

Describe the Bug

After updating the component to 1.400.0 from a prior version, all stacks are moved to the root Spacelift space, unless the new optional Spacelift settings keyword space_name_pattern is defined in every stack.

Why this is happening

The change to calculation space_id in the child-stacks.tf and spaces.tf involves a guard condition that presupposes the existence of a new key space_name_pattern. Excerpt of the new logic that appears in both files is included below:

  space_id = local.spaces[
    try(
      coalesce(
        # if `space_name` is specified, use it
        each.value.settings.spacelift.space_name,
        # otherwise, try to replace the context tokens in `space_name_template` and use it
        # `space_name_template` accepts the following context tokens: {namespace}, {tenant}, {environment}, {stage}
        each.value.settings.spacelift.space_name_pattern != "" && each.value.settings.spacelift.space_name_pattern != null ? (
          replace(
            replace(
              replace(
                replace(
                  each.value.settings.spacelift.space_name_pattern,
                  "{namespace}", module.this.namespace
                ),
                "{tenant}", module.this.tenant
              ),
              "{environment}", module.this.environment
            ),
          "{stage}", module.this.stage)
        ) : ""
      ),
      var.space_id
    )
  ]

The condition checking that each.value.settings.spacelift.space_name_pattern does not equal the empty string or null requires that key to exist. As all of the logic in this block occurs within a Terraform try() function, every component is mapped to the default string "root".

The value settings.spacelift.space_name_pattern key is intended to be optional, but is not optional in practice.

If you remove the try() statement, the hidden error appears

╷
│ Error: Unsupported attribute
│ 
│   on spaces.tf line 13, in locals:
│   13:         v.settings.spacelift.space_name_pattern != "" && v.settings.spacelift.space_name_pattern != null ? (
│     ├────────────────
│     │ v.settings.spacelift is object with 8 attributes
│ 
│ This object does not have an attribute named "space_name_pattern".

Expected Behavior

For an Atmos component definition, if the spacelift settings key space_name key has been defined, and the new space_name_pattern key has not been defined, then the resultant stack should be assigned to the named space in Spacelift

Steps to Reproduce

components:
  terraform:
    spacelift/admin-stack:
      metadata:
        component: spacelift/admin-stack
        inherits:
          - spacelift/admin-stack/default
      settings:
        spacelift:
          space_name: example
          labels:
            - admin-stack-name:example
      vars:
          ...

You will notice the the Spacelift stacks created for the "child components" of this administrative stack will be created in the root space, not the example space as defined in the above configuration.

Screenshots

No response

Environment

No response

Additional Context

No response

MReggler commented 6 months ago

This is essentially the risk with using the try(coalesce(<option_a>,<option_b>), <default>) pattern without unit tests, any unforeseen error in the internal coalesce is masked to the default value, which is valid Terraform