Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

remove duplicate paths in layering #237

Merged

Conversation

byron70
Copy link
Contributor

@byron70 byron70 commented May 19, 2022

This is a 🐞 bug fix.

  • I've added tests (if it's a bug, feature or enhancement)
  • I've adjusted the documentation (if it's a feature or enhancement)
  • The test suite passes (run bundle exec rspec to verify this)

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
Copy link
Contributor

tongueroo commented Jul 4, 2022

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants