hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.61k stars 9k forks source link

[Bug]: Impossible to add Organizational Unit on existing aws_cloudformation_stack_set_instance #27877

Closed florenp closed 1 day ago

florenp commented 1 year ago

Terraform Core Version

1.3.1

AWS Provider Version

4.39.0

Affected Resource(s)

Expected Behavior

Adding a new Organization Unit to list of organizational_unit_ids should deploy the stack_set_instance to the new OU.

Actual Behavior

aws_cloudformation_stack_set_instance.my-stack-set-instance["us-west-2"] will be updated in-place
  ~ resource "aws_cloudformation_stack_set_instance" "my-stack-set-instance" {
        id                     = "xxxxxxx,xxxxxxxxxx,us-west-2"
        # (8 unchanged attributes hidden)

      ~ deployment_targets {
          ~ organizational_unit_ids = [
              + "ou-xxxxxxxxx",
                # (1 unchanged element hidden)
            ]
        }

        # (2 unchanged blocks hidden)
    }

Relevant Error/Panic Output Snippet

Error: error updating CloudFormation StackSet Instance (xxxxxxx,123456789012,us-west-2): ValidationError: Organizational unit ou-xxxxxxxxx not found in StackSet
│       status code: 400, request id: 1826eb23-8df6-4538-ad5c-1b0563701c8b
│
│   with aws_cloudformation_stack_set_instance.my-stack-set-instance["us-west-2"],
│   on stack-set.tf line 66, in resource "aws_cloudformation_stack_set_instance" "my-stack-set-instance":
│   66: resource "aws_cloudformation_stack_set_instance" "my-stack-set-instance" {

Terraform Configuration Files

.

Steps to Reproduce

Create a stack set & stack set instance applying to an Organizational Unit.

Try to modify the stack set instance adding a new OU to the list of targets

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

No response

Would you like to implement a fix?

No response

github-actions[bot] commented 1 year ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue

hellokhush commented 1 year ago

What is the solution for this, is that resolved?

rkno82 commented 1 year ago

We are also facing the issue. Any ideas?

hellokhush commented 1 year ago

I have found a work around for this, I have added a stack in the existing cloudformation stack set for New OU and deployed cloudformation through that manually using the AWS console, and then I ran the "Terraform apply" command. It worked for me

image

jjf130 commented 1 year ago

Manual workaround resolves the issue, but would be nice if there was an automated workaround (or better yet a fix). Trying to manage an AWS organization with consistent baseline tooling in accounts deployed with stacksets becomes very tedious because of this bug and https://github.com/hashicorp/terraform-provider-aws/issues/25253

MarcNaameh commented 1 year ago

Any Updates on this bug? It is really causing a lot of issues...

cedricbastin commented 1 year ago

As it is not possible to update the target OUs you can use this workaround which creates a separate StackSet instance for each OU such that no update API calls are used

old code which throws the error: ValidationError: Organizational unit ou-xx-xxx has no instances in [eu-central-1]

resource "aws_cloudformation_stack_set_instance" "instance" {
  deployment_targets {
    organizational_unit_ids = var.target_ous
  }
}

new code which uses a loop to create a StackSet instance for every OU:

resource "aws_cloudformation_stack_set_instance" "instance" {
  for_each = toset(var.target_ous)
  deployment_targets {
    organizational_unit_ids = [each.key]
  }
}
jvanenckevort commented 11 months ago

Any update on this?

framirezcruz commented 10 months ago

any updates on this

jgarcia-pluralsight commented 9 months ago

Any updates on this?

aglittle commented 9 months ago

Any updates on this bug?

evantlueck commented 8 months ago

I found a solution for this. I put it on a similar ticket here: https://github.com/hashicorp/terraform-provider-aws/issues/33785#issuecomment-1780233282

jvanenckevort commented 8 months ago

@evantlueck in your comment you are talking about parent and child OUs. For me this happens to every OU I later add, without them having an inheritance relationship. So this does not solve this issue.

evantlueck commented 8 months ago

@evantlueck in your comment you are talking about parent and child OUs. For me this happens to every OU I later add, without them having an inheritance relationship. So this does not solve this issue.

Hey! I was not clear enough likely in how the solution works. I added a summary providing context. But I've been actively using it to add/subtract OUs at my leisure in my stacksets so I can testify that it works :D

Ultimately, the issue you're speaking of is resolved by this line:

for_each       = { for ou in local.final_ous : ou => [ou] }

within the aws_cloudformation_stack_set_instance resource.

What I am doing is creating an entirely unique aws_cloudformation_stack_set_instance resource for each ou; rather than providing what the resource expects (which is a list of OUs we wish to target). Ultimately, the provider for that resource fails when we attempt to modify the list we provide it. So this solution works around that by deploying multiple stackset instances. One for each OU passed to the module. This works properly with terraform state and is accepted by the underlying provider, but only if the parent/child relationship issue is prevented. Which leads me to the more complex portion of the solution below:

locals {
  deduped_ous = distinct(var.target_ous)
}

data "aws_organizations_organizational_unit_descendant_accounts" "targeted_accounts" {
  for_each = { for ou in local.deduped_ous : ou => ou }
  parent_id = each.key
}

locals {
  ou_descendants_map = { for ou, child_accounts in data.aws_organizations_organizational_unit_descendant_accounts.targeted_accounts : ou => [ for account in child_accounts.accounts : account.id ] }
  # Identify OUs whose account list is a subset of any other OU's account list
  subset_ous = [for ou_id, accounts in local.ou_descendants_map :
    ou_id if length([
      for other_ou_id, other_accounts in local.ou_descendants_map :
        other_ou_id if (other_ou_id != ou_id && length(setsubtract(accounts, other_accounts)) == 0)
    ]) > 0
  ]
  # list of OUs which are not a subset of any other OU's account list which should each have their own aws_cloudformation_stack_set_instance resource.
  final_ous = setsubtract(local.deduped_ous, local.subset_ous)
}

This excerpt of code prevents terraform state issues by helping assist the module user by not deploying a new instance for OUs which would deploy to a subset of the accounts already technically defined by a different OU being passed.

As you mentioned, you would not run into this use-case if your OUs all contain unique accounts. That is not the case for my org. So I needed the additional layer of complexity.

So just this:

variable "target_ous" {
  type        = list(string)
  description = "Where would you like to deploy the stackset?"
}

resource "aws_cloudformation_stack_set_instance" "instance" {
  for_each       = { for ou in var.target_ous : ou => [ou] }
  stack_set_name = aws_cloudformation_stack_set.stackset.name
  region         = data.aws_region.this.id
  retain_stack   = false
  deployment_targets {
    organizational_unit_ids = each.value
  }
  operation_preferences { # ref: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/cloudformation_stack_set#operation_preferences-argument-reference
    failure_tolerance_percentage = 50
    max_concurrent_percentage    = 50
    region_concurrency_type      = "PARALLEL"
  }
}

would resolve your issue.

That being said, doing the additional recursive lookup can't hurt. Regardless, all of this is so I could use stacksets in terraform however I wanted while waiting on hashicorp/aws to publish an official fix for the bug.

franciscojose-sanchezsanchez commented 5 months ago

I know that there are several workarounds, but all of them involve, in many cases, low-level solutions that are bad and can break certain operations of previously deployed stacksets.

However, I would like to know if there is any information on the progress to fix this bug. I think it's an important issue to resolve, whenever possible.

Thanks

tabasku commented 4 months ago

Seems like there was an attempt to change deployment_targets to force new resource if changed but it is not currently working : https://github.com/hashicorp/terraform-provider-aws/commit/c6965befe6cd31a7b1584289290a43fc3f2cac3a

In my opinion this makes sense as aws_cloudformation_stack_set_instance or at least deployment_targets is immutable. Any following updates are done against all existing stack instances.

aiell0 commented 3 months ago

I am also having this problem and would love a solution here.

ollytheninja commented 2 months ago

I'm using the same workaround as @cedricbastin but boy is this one a doozy. I'm glad I'm only managing IAM roles which can easily be re-created but it would be fantastic if there was a better solution!

palmobar commented 6 days ago

Hi, sorry to post that comment but this is really a pain do we know if fix is in the works?

github-actions[bot] commented 1 day ago

[!WARNING] This issue has been closed, meaning that any additional comments are hard for our team to see. Please assume that the maintainers will not see them.

Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed.