Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

fix(caf_solution/locals.remote_tfstates.tf): changing order of global… #448

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(caf_solution/locals.remote_tfstates.tf): changing order of global… #448

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions caf_solution/locals.remote_tfstates.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ locals {
sas_token = try(value.sas_token, null) != null ? var.sas_token : null
use_azuread_auth = try(value.use_azuread_auth, true)
} if try(value.backend_type, "azurerm") == "azurerm"
}
}
remote = {
for key, value in try(var.landingzone.tfstates, {}) : key => {
hostname = try(value.hostname, null)
Expand All @@ -46,14 +46,21 @@ locals {
}
}

tags = merge(try(local.global_settings.tags, {}), { "level" = var.landingzone.level }, try({ "environment" = local.global_settings.environment }, {}), { "rover_version" = var.rover_version }, var.tags)
tags = merge(
try(local.global_settings.tags, {}),
{ "level" = var.landingzone.level },
try({ "environment" = local.global_settings.environment }, {}),
{ "rover_version" = var.rover_version },
var.tags,
try(var.global_settings.tags, {}),
try(local.custom_variables.tags, {}),
)

global_settings = merge(
var.global_settings,
try(data.terraform_remote_state.remote[var.landingzone.global_settings_key].outputs.objects[var.landingzone.global_settings_key].global_settings, null),
try(data.terraform_remote_state.remote[var.landingzone.global_settings_key].outputs.global_settings, null),
try(data.terraform_remote_state.remote[keys(var.landingzone.tfstates)[0]].outputs.global_settings, null),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This entry was put for backward compatibility as originally the global_setting_key was not always defined, so we took the global settings from the tfstate in the map.
Why not keeping it?

I think it is a good idea to add the var.global_settings as it will override any previous settings. Good spot

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LaurentLesle Lets say we have the following use-case:

  1. Lower Landingzones has global_settings { passthrough = true } and we deployed all the resources in that landingzone without any prefix, these are shared resources hence no prefix.

  2. Then I define two new higher landingzones (same level) one for prod with global_settings = { prefix=prod} and another for non-prod with global_settings = { prefix=nonprod }

  3. Now when I reference any lower level resource in my prod/nonprod spoke landingzones, my expectation is to just use tfstate to reference lower level landingzone objects but not inherit its global_settings unless explicitly intended by defining a "global_settings_key = "mylowerlevel-landingzone"". If not, the moment we define global_settings_key, which does not have any prefix set, Terraform would start deleting resources defined with prefix=prod and prefix=nonprod

  4. The overriding with var.global_settings may not work due to the merge function order.
    "If more than one given map or object defines the same key or attribute, then the one that is later in the argument sequence takes precedence. " Ref: https://developer.hashicorp.com/terraform/language/functions/merge

local.custom_variables
try(var.global_settings, {}),
try(local.custom_variables, {}),
)


Expand Down