boltops-tools / terraspace

Terraspace: The Terraform Framework
https://terraspace.cloud
Apache License 2.0
678 stars 46 forks source link

remove duplicate paths in layering #237

Closed byron70 closed 2 years ago

byron70 commented 2 years ago

This is a 🐞 bug fix.

Summary

Removes duplicate entries from layering paths that occur when you add a layer name mapping that matches TS_ENV.

Eventually this leads to tfvar files with duplicated variables - 1-sbx.auto.tfvar 2-sbx.auto.tfvar.

Context

This really extended from my improper use of layer name mappings. It was a simple fix once I tracked it down (in the config), but thought it could warrant a fix anyway to prevent user frustration in the future.

How to Test

TS_SHOW_ALL_LAYERS=1 was showing duplicate entries when running TS_ENV=sbx terraspace build mystack. See output below.

    ....
    config/terraform/tfvars/base.tfvars
    # this is duplicated below
    config/terraform/tfvars/sbx.tfvars
    config/terraform/tfvars/sbx/base.tfvars
    config/terraform/tfvars/sbx/sbx.tfvars
    config/terraform/tfvars/us-east-1.tfvars
    config/terraform/tfvars/us-east-1/base.tfvars
    config/terraform/tfvars/us-east-1/sbx.tfvars
    config/terraform/tfvars/us-east-1/sbx/base.tfvars
    config/terraform/tfvars/us-east-1/sbx/sbx.tfvars
    # duplication starts here of `sbx`
    config/terraform/tfvars/sbx.tfvars
    config/terraform/tfvars/sbx/base.tfvars
    ...

I tracked this back to namespace mappings where I had added to config/app.rb, see below.

Terraspace.configure do |config|
  config.layering.names = {
    "123456789123" => "dev",
    "123456789124" => "prd",
    "123456789125" => "stg",  <--- changing this to acct-stg prevents the duplicates
    "123456789126" => "sbx",
  }
end

Please feel free to reject this fix if you feel it is not necessary due to the improper use of mappings.

tongueroo commented 2 years ago

Took some time to think about this one. Here's some review:

When Friendly Account Name Does Not Match TS_ENV

config/app.rb

Terraspace.configure do |config|
  config.layering.names = {
    "111111111111" => "dev-account",
  }
  config.layering.mode = "namespace" # simple, namespace, or provider
end
$ export TS_SHOW_ALL_LAYERS=1
$ terraspace build demo 2>&1 | grep config
    config/terraform/tfvars/base.tfvars
    config/terraform/tfvars/dev.tfvars
    config/terraform/tfvars/us-west-2.tfvars
    config/terraform/tfvars/us-west-2/base.tfvars
    config/terraform/tfvars/us-west-2/dev.tfvars
    config/terraform/tfvars/dev-account.tfvars # <= ALL OK
    config/terraform/tfvars/dev-account/base.tfvars
    config/terraform/tfvars/dev-account/dev.tfvars
    config/terraform/tfvars/dev-account/us-west-2.tfvars
    config/terraform/tfvars/dev-account/us-west-2/base.tfvars
    config/terraform/tfvars/dev-account/us-west-2/dev.tfvars
$

When Friendly Account Name Matches TS_ENV

config/app.rb

Terraspace.configure do |config|
  config.layering.names = {
    "111111111111" => "dev",
  }
  config.layering.mode = "namespace" # simple, namespace, or provider
end
$ export TS_SHOW_ALL_LAYERS=1
$ terraspace build demo 2>&1 | grep config
    config/terraform/tfvars/base.tfvars
    config/terraform/tfvars/dev.tfvars
    config/terraform/tfvars/us-west-2.tfvars
    config/terraform/tfvars/us-west-2/base.tfvars
    config/terraform/tfvars/us-west-2/dev.tfvars
    config/terraform/tfvars/dev.tfvars # <= DUPLICATED
    config/terraform/tfvars/dev/base.tfvars
    config/terraform/tfvars/dev/dev.tfvars
    config/terraform/tfvars/dev/us-west-2.tfvars
    config/terraform/tfvars/dev/us-west-2/base.tfvars
    config/terraform/tfvars/dev/us-west-2/dev.tfvars
$

It was a bit of a struggle. Think it's better to remove the duplicated layer like you've done. Think the namespace layer is less used and so it's more expected that config/terraform/tfvars/dev.tfvars essentially takes higher precedence. Thanks for the PR and explanation.

With the Change

The second config/terraform/tfvars/dev/base.tfvars is removed.

$ export TS_SHOW_ALL_LAYERS=1
$ terraspace build demo 2>&1 | grep config
    config/terraform/tfvars/base.tfvars
    config/terraform/tfvars/dev.tfvars
    config/terraform/tfvars/us-west-2.tfvars
    config/terraform/tfvars/us-west-2/base.tfvars
    config/terraform/tfvars/us-west-2/dev.tfvars
    config/terraform/tfvars/dev/base.tfvars
    config/terraform/tfvars/dev/dev.tfvars
    config/terraform/tfvars/dev/us-west-2.tfvars
    config/terraform/tfvars/dev/us-west-2/base.tfvars
    config/terraform/tfvars/dev/us-west-2/dev.tfvars
$

Also updated the docs with a note https://terraspace.cloud/docs/layering/friendly-names/ Also note, in Terraspace 2.0 layering was tweaked. It was simplified.