terraform-provider-aws icon indicating copy to clipboard operation
terraform-provider-aws copied to clipboard

Add support for additional deploymen_targets config for aws_cloudformation_stack_set_instance

Open hhughes0 opened this issue 2 years ago • 2 comments

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Release Note:

  • resource/aws_cloudformation_stack_set_instance: Extend deployment_targets argument

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccCloudFormationStackSet_\|TestAccCloudFormationStackSetInstance_' PKG=cloudformation ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/cloudformation/... -v -count 1 -parallel 3  -run=TestAccCloudFormationStackSet_\|TestAccCloudFormationStackSetInstance_ -timeout 180m
=== RUN   TestAccCloudFormationStackSetInstance_basic
=== PAUSE TestAccCloudFormationStackSetInstance_basic
=== RUN   TestAccCloudFormationStackSetInstance_disappears
=== PAUSE TestAccCloudFormationStackSetInstance_disappears
=== RUN   TestAccCloudFormationStackSetInstance_Disappears_stackSet
=== PAUSE TestAccCloudFormationStackSetInstance_Disappears_stackSet
=== RUN   TestAccCloudFormationStackSetInstance_parameterOverrides
=== PAUSE TestAccCloudFormationStackSetInstance_parameterOverrides
=== RUN   TestAccCloudFormationStackSetInstance_retainStack
=== PAUSE TestAccCloudFormationStackSetInstance_retainStack
=== RUN   TestAccCloudFormationStackSetInstance_deploymentTargets
=== PAUSE TestAccCloudFormationStackSetInstance_deploymentTargets
=== RUN   TestAccCloudFormationStackSetInstance_operationPreferences
=== PAUSE TestAccCloudFormationStackSetInstance_operationPreferences
=== RUN   TestAccCloudFormationStackSet_basic
=== PAUSE TestAccCloudFormationStackSet_basic
=== RUN   TestAccCloudFormationStackSet_disappears
=== PAUSE TestAccCloudFormationStackSet_disappears
=== RUN   TestAccCloudFormationStackSet_administrationRoleARN
=== PAUSE TestAccCloudFormationStackSet_administrationRoleARN
=== RUN   TestAccCloudFormationStackSet_description
=== PAUSE TestAccCloudFormationStackSet_description
=== RUN   TestAccCloudFormationStackSet_executionRoleName
=== PAUSE TestAccCloudFormationStackSet_executionRoleName
=== RUN   TestAccCloudFormationStackSet_name
=== PAUSE TestAccCloudFormationStackSet_name
=== RUN   TestAccCloudFormationStackSet_operationPreferences
=== PAUSE TestAccCloudFormationStackSet_operationPreferences
=== RUN   TestAccCloudFormationStackSet_parameters
=== PAUSE TestAccCloudFormationStackSet_parameters
=== RUN   TestAccCloudFormationStackSet_Parameters_default
    acctest.go:69: this resource does not currently ignore unconfigured CloudFormation template parameters with the Default property
--- SKIP: TestAccCloudFormationStackSet_Parameters_default (0.00s)
=== RUN   TestAccCloudFormationStackSet_Parameters_noEcho
    acctest.go:69: this resource does not currently ignore CloudFormation template parameters with the NoEcho property
--- SKIP: TestAccCloudFormationStackSet_Parameters_noEcho (0.00s)
=== RUN   TestAccCloudFormationStackSet_PermissionModel_serviceManaged
    acctest.go:69: API does not support enabling Organizations access (in particular, creating the Stack Sets IAM Service-Linked Role)
--- SKIP: TestAccCloudFormationStackSet_PermissionModel_serviceManaged (0.00s)
=== RUN   TestAccCloudFormationStackSet_tags
=== PAUSE TestAccCloudFormationStackSet_tags
=== RUN   TestAccCloudFormationStackSet_templateBody
=== PAUSE TestAccCloudFormationStackSet_templateBody
=== RUN   TestAccCloudFormationStackSet_templateURL
=== PAUSE TestAccCloudFormationStackSet_templateURL
=== CONT  TestAccCloudFormationStackSetInstance_basic
=== CONT  TestAccCloudFormationStackSet_administrationRoleARN
=== CONT  TestAccCloudFormationStackSet_templateURL
--- PASS: TestAccCloudFormationStackSet_administrationRoleARN (30.16s)
=== CONT  TestAccCloudFormationStackSet_templateBody
--- PASS: TestAccCloudFormationStackSet_templateURL (30.90s)
=== CONT  TestAccCloudFormationStackSet_tags
--- PASS: TestAccCloudFormationStackSet_templateBody (28.75s)
=== CONT  TestAccCloudFormationStackSet_parameters
--- PASS: TestAccCloudFormationStackSetInstance_basic (77.61s)
=== CONT  TestAccCloudFormationStackSet_operationPreferences
--- PASS: TestAccCloudFormationStackSet_tags (59.29s)
=== CONT  TestAccCloudFormationStackSet_name
--- PASS: TestAccCloudFormationStackSet_name (27.78s)
=== CONT  TestAccCloudFormationStackSet_executionRoleName
--- PASS: TestAccCloudFormationStackSet_parameters (59.18s)
=== CONT  TestAccCloudFormationStackSet_description
--- PASS: TestAccCloudFormationStackSet_description (28.95s)
=== CONT  TestAccCloudFormationStackSetInstance_deploymentTargets
--- PASS: TestAccCloudFormationStackSet_executionRoleName (29.45s)
=== CONT  TestAccCloudFormationStackSet_disappears
--- PASS: TestAccCloudFormationStackSet_disappears (11.79s)
=== CONT  TestAccCloudFormationStackSet_basic
--- PASS: TestAccCloudFormationStackSet_operationPreferences (89.50s)
=== CONT  TestAccCloudFormationStackSetInstance_operationPreferences
--- PASS: TestAccCloudFormationStackSet_basic (13.91s)
=== CONT  TestAccCloudFormationStackSetInstance_parameterOverrides
=== CONT  TestAccCloudFormationStackSetInstance_deploymentTargets
    stack_set_instance_test.go:232: Failed state verification, resource with ID o-7gui9hebdk not found
--- FAIL: TestAccCloudFormationStackSetInstance_deploymentTargets (99.12s)
=== CONT  TestAccCloudFormationStackSetInstance_retainStack
--- PASS: TestAccCloudFormationStackSetInstance_operationPreferences (124.49s)
=== CONT  TestAccCloudFormationStackSetInstance_Disappears_stackSet
--- PASS: TestAccCloudFormationStackSetInstance_parameterOverrides (182.90s)
=== CONT  TestAccCloudFormationStackSetInstance_disappears
--- PASS: TestAccCloudFormationStackSetInstance_Disappears_stackSet (88.77s)
--- PASS: TestAccCloudFormationStackSetInstance_retainStack (153.78s)
--- PASS: TestAccCloudFormationStackSetInstance_disappears (81.91s)
FAIL
FAIL	github.com/hashicorp/terraform-provider-aws/internal/service/cloudformation	439.617s
FAIL
make: *** [testacc] Error 1

...

Closes

Closes #26917

Other Notes

I've mostly adapted the work done by @jnixon-blue here https://github.com/hashicorp/terraform-provider-aws/pull/23908

We have a use case where it would be great to have this support added to target multiple accounts within an organization with the use of SERVICE_MANAGED permissions.

hhughes0 avatar Sep 22 '22 14:09 hhughes0

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Please review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

github-actions[bot] avatar Sep 22 '22 14:09 github-actions[bot]

@ewbankkit I noticed you reviewed the PR I linked in the description, would you be able to cast your eyes over this one too by any chance?

Acceptance tests are now passing.

hhughes0 avatar Sep 22 '22 20:09 hhughes0

@hhughes0

Hey hope you're doing well.

Your PR has helped a lot with some internal projects. I would love to see this merged into main.

However, in our usage we're seeing something a bit strange.

Creation works like a charm. But if i add another account_id to the accounts list in the stackset instance, it doesn't actually do anything. Similar if i delete the account from the list.

as an example:

Running this change:

deployment_targets {
    organizational_unit_ids = ["r-1abc"]
    account_filter_type = "INTERSECTION"
    accounts = ["12345678910"]
  }

to

deployment_targets {
    organizational_unit_ids = ["r-1abc"]
    account_filter_type = "INTERSECTION"
    accounts = ["12345678910", "12345678911"]
  }

Doesn't actually deploy another stack instance to the new account ("12345678911"). Similar occurs if we remove the account id from the list.

I'm looking at the code but not 100% sure where to start here regarding debugging.

Is this something you've experienced as well?

mdrdannyr avatar Jan 10 '23 08:01 mdrdannyr

I would also like to see this get merged.

I do not know much about terraform programming or debugging, but I can see:

  • https://github.com/hashicorp/terraform-provider-aws/pull/26935/files#diff-d9f95211f49cb64e3e599050bee4ca1e1f9071418fcc34594928c4dc2fb28d68R201-R203
  • https://github.com/hashicorp/terraform-provider-aws/pull/26935/files#diff-d9f95211f49cb64e3e599050bee4ca1e1f9071418fcc34594928c4dc2fb28d68R229
  • https://github.com/hashicorp/terraform-provider-aws/pull/26935/files#diff-d9f95211f49cb64e3e599050bee4ca1e1f9071418fcc34594928c4dc2fb28d68R290-R302

It seems to me like it is using the OU ID(s) to determine whether or not the deployment target has changed, in which case, that "ID" would probably need to change to include the filter operation and the account ID, for CRUD and idempotence purposes.

If my theory is correct, then I would expect the addition of resources to result in a "Nothing to do" change, because the deployment target "remained the same" because the presence of accounts are not factored into change detection. However, if something else were to trigger a change (a change in the Stackset?), from reading the code, I would expect the update operation to complete successfully and alter the deployment targets. Can anyone support whether my speculation might be valid?

sosheskaz avatar Oct 30 '23 17:10 sosheskaz

Really need this to get going, this is a basic feature and has been going on for about a year, any ETA for when this will be merged?

JaimePoloTeck avatar Apr 30 '24 15:04 JaimePoloTeck

I have submitted a PR to ensure that the resource is updated even when changes are made to the deployment_targets argument. #37898

yhamano0312 avatar Jun 10 '24 13:06 yhamano0312