resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model #12423

srikanthchelluri · 2020-03-17T13:59:05Z

This updates the aws_cloudformation_stack_set resource to support the SERVICE_MANAGED permission model. This is helpful if a consumer intends to use CloudFormation StackSets to manage resources across an AWS Organization.

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

Closes #12422.

Release note for CHANGELOG:

* resource/aws_cloudformation_stack_set: Support the `SERVICE_MANAGED` permission model [GH-12422]

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSCloudFormationStackSet_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAWSCloudFormationStackSet_ -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSCloudFormationStackSet_basic
=== PAUSE TestAccAWSCloudFormationStackSet_basic
=== RUN   TestAccAWSCloudFormationStackSet_disappears
=== PAUSE TestAccAWSCloudFormationStackSet_disappears
=== RUN   TestAccAWSCloudFormationStackSet_AdministrationRoleArn
=== PAUSE TestAccAWSCloudFormationStackSet_AdministrationRoleArn
=== RUN   TestAccAWSCloudFormationStackSet_Description
=== PAUSE TestAccAWSCloudFormationStackSet_Description
=== RUN   TestAccAWSCloudFormationStackSet_ExecutionRoleName
=== PAUSE TestAccAWSCloudFormationStackSet_ExecutionRoleName
=== RUN   TestAccAWSCloudFormationStackSet_Name
=== PAUSE TestAccAWSCloudFormationStackSet_Name
=== RUN   TestAccAWSCloudFormationStackSet_Parameters
=== PAUSE TestAccAWSCloudFormationStackSet_Parameters
=== RUN   TestAccAWSCloudFormationStackSet_Parameters_Default
--- SKIP: TestAccAWSCloudFormationStackSet_Parameters_Default (0.00s)
    resource_aws_cloudformation_stack_set_test.go:358: this resource does not currently ignore unconfigured CloudFormation template parameters with the Default property
=== RUN   TestAccAWSCloudFormationStackSet_Parameters_NoEcho
--- SKIP: TestAccAWSCloudFormationStackSet_Parameters_NoEcho (0.00s)
    resource_aws_cloudformation_stack_set_test.go:409: this resource does not currently ignore CloudFormation template parameters with the NoEcho property
=== RUN   TestAccAWSCloudFormationStackSet_PermissionModel_ServiceManaged
=== PAUSE TestAccAWSCloudFormationStackSet_PermissionModel_ServiceManaged
=== RUN   TestAccAWSCloudFormationStackSet_Tags
=== PAUSE TestAccAWSCloudFormationStackSet_Tags
=== RUN   TestAccAWSCloudFormationStackSet_TemplateBody
=== PAUSE TestAccAWSCloudFormationStackSet_TemplateBody
=== RUN   TestAccAWSCloudFormationStackSet_TemplateUrl
=== PAUSE TestAccAWSCloudFormationStackSet_TemplateUrl
=== CONT  TestAccAWSCloudFormationStackSet_basic
=== CONT  TestAccAWSCloudFormationStackSet_TemplateUrl
=== CONT  TestAccAWSCloudFormationStackSet_disappears
=== CONT  TestAccAWSCloudFormationStackSet_Parameters
=== CONT  TestAccAWSCloudFormationStackSet_Description
=== CONT  TestAccAWSCloudFormationStackSet_Name
=== CONT  TestAccAWSCloudFormationStackSet_AdministrationRoleArn
=== CONT  TestAccAWSCloudFormationStackSet_ExecutionRoleName
=== CONT  TestAccAWSCloudFormationStackSet_Tags
=== CONT  TestAccAWSCloudFormationStackSet_TemplateBody
=== CONT  TestAccAWSCloudFormationStackSet_PermissionModel_ServiceManaged
--- PASS: TestAccAWSCloudFormationStackSet_disappears (19.89s)
--- PASS: TestAccAWSCloudFormationStackSet_basic (22.95s)
--- PASS: TestAccAWSCloudFormationStackSet_PermissionModel_ServiceManaged (36.97s)
--- PASS: TestAccAWSCloudFormationStackSet_AdministrationRoleArn (42.16s)
--- PASS: TestAccAWSCloudFormationStackSet_ExecutionRoleName (43.69s)
--- PASS: TestAccAWSCloudFormationStackSet_TemplateBody (45.05s)
--- PASS: TestAccAWSCloudFormationStackSet_Name (46.27s)
--- PASS: TestAccAWSCloudFormationStackSet_Description (58.74s)
--- PASS: TestAccAWSCloudFormationStackSet_Tags (79.10s)
--- PASS: TestAccAWSCloudFormationStackSet_TemplateUrl (88.26s)
--- PASS: TestAccAWSCloudFormationStackSet_Parameters (97.51s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       97.622s
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap      0.012s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags 0.108s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/naming       0.047s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/service/batch/equivalency    0.029s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/service/eks/token    0.035s [no tests to run]
?       github.com/terraform-providers/terraform-provider-aws/awsproviderlint   [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes    0.053s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSAT001   0.060s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSR001    0.013s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/fmtsprintfcallexpr 0.012s [no tests to run]
...

srikanthchelluri · 2020-03-25T21:52:39Z

Forced pushed a change to remove computed fields that I added since it was preventing updating StackSets that were already created.

srikanthchelluri · 2020-03-30T15:02:20Z

Currently hashing out issues with computed fields - will remove WIP label when it's ready for review again.

…D permission model

srikanthchelluri · 2020-03-30T15:21:58Z

Fixed the issue - see inline comment on nil-ing out some of the fields on update. Validated acceptance tests pass.

srikanthchelluri · 2020-05-19T15:09:30Z

@bflad Hey Brian - tagging you since you're pretty active in this repo. Any chance I can get a code review on this? Thanks!

cgetzen · 2020-05-29T22:40:31Z

@appilon seems to be active too 😬 🙏 I'd like to contribute to aws_cloudformation_stack_set_instance, and this PR is a prereq.

piersf · 2020-08-27T22:19:20Z

Hello there,

Are there any updates on this? It seems that there are several issues opened for the same need which goes to show that people are really wanting this feature.

Thank you in advance!

aldegoeij · 2020-10-02T14:35:18Z

Hi @bflad could you review and/or approve this? Would be super helpful to have!

chancez · 2020-12-08T21:28:12Z

This and the SSO work going on right now both feel super important as everyone's trying to adopt the AWS tooling to make multi-account less awful. Could this be prioritized @bflad ?

daveadams · 2021-01-22T23:28:23Z

Any news on what's the hold-up on this commit? I just started looking at this myself and came across this now year-old pull request that looks like exactly what I was considering writing. Will want to pair this with a PR for #12425 since service-managed stack set instances will go along with service-managed stack sets.

srikanthchelluri · 2021-02-25T02:40:58Z

bump

myoung34 · 2021-03-02T19:40:27Z

Also bump @bflad or anyone else

This is a real blocker for some POC work were doing

piersf · 2021-03-03T15:23:27Z

@myoung34 see #12422 (comment) if it works for you

myoung34 · 2021-03-03T15:40:44Z

@piersf Thats what were currently doing but its extremely kludgey

piersf · 2021-03-03T15:45:44Z

@myoung34 what do you find kludgy about it?

We're using it to deploy a baseline configuration to every new account as they are added to their respective OU, by using the SERVICE_MANAGED permission model with auto-deployment enabled.

The only thing that I've seen missing from CloudFormation StackSets so far is the ability to cherry-pick stack instances to remove from a stack set resource and the fact that you can't configure an SNS Topic in stack instances(like you can for a normal stack resource). The latter can't be solved by Terraform either as the API itself does not support this.

bflad

Hi @srikanthchelluri 👋 Thank you for submitting this. It looks pretty good, with just some minor adjustments due to some newer CI checks we have in place. We also need to adjust the new test to work only in Organizations administrative accounts, but that is outside the scope of what we would typically would expect from a community contribution. To speed up everything here, we will apply these changes on merge now so this can be included in this week's release. Thank you again and apologies for the delayed review.

Output from acceptance testing:

=== CONT  TestAccAWSCloudFormationStackSet_PermissionModel_ServiceManaged
    resource_aws_cloudformation_stack_set_test.go:465: Step 1/2 error: Error running apply: exit status 1

        Error: error creating CloudFormation StackSet: ValidationError: You must be the master or delegated admin account of an organization before operating a SERVICE_MANAGED stack set
        	status code: 400, request id: 0578577e-948d-4fa3-b856-68e8d90a77f3

          on terraform_plugin_test.tf line 2, in resource "aws_cloudformation_stack_set" "test":
           2: resource "aws_cloudformation_stack_set" "test" {


--- FAIL: TestAccAWSCloudFormationStackSet_PermissionModel_ServiceManaged (20.32s)
--- PASS: TestAccAWSCloudFormationStackSet_disappears (29.55s)
--- PASS: TestAccAWSCloudFormationStackSet_basic (37.19s)
--- PASS: TestAccAWSCloudFormationStackSet_ExecutionRoleName (52.08s)
--- PASS: TestAccAWSCloudFormationStackSet_Description (52.94s)
--- PASS: TestAccAWSCloudFormationStackSet_TemplateBody (56.51s)
--- PASS: TestAccAWSCloudFormationStackSet_Name (60.38s)
--- PASS: TestAccAWSCloudFormationStackSet_TemplateUrl (76.66s)
--- PASS: TestAccAWSCloudFormationStackSet_Tags (86.81s)
--- PASS: TestAccAWSCloudFormationStackSet_Parameters (88.40s)
--- PASS: TestAccAWSCloudFormationStackSet_AdministrationRoleArn (89.54s)

bflad · 2021-03-31T16:26:42Z

aws/resource_aws_cloudformation_stack_set.go

+				ValidateFunc: validation.StringInSlice([]string{
+					cloudformation.PermissionModelsServiceManaged,
+					cloudformation.PermissionModelsSelfManaged,
+				}, false),


Nit: Since this pull request was originally introduced, the AWS SDK Go created functions to retrieve all (modeled) enumeration values. See also: #14601

Suggested change

ValidateFunc: validation.StringInSlice([]string{

cloudformation.PermissionModelsServiceManaged,

cloudformation.PermissionModelsSelfManaged,

}, false),

ValidateFunc: validation.StringInSlice(cloudformation.PermissionModels_Values(), false),

bflad · 2021-03-31T16:28:25Z

aws/resource_aws_cloudformation_stack_set_test.go

+		PreCheck:     func() { testAccPreCheck(t); testAccPreCheckAWSCloudFormationStackSet(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSCloudFormationStackSetDestroy,


Newer linter since this pull request was introduced:

Suggested change

PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSCloudFormationStackSet(t) },

Providers: testAccProviders,

CheckDestroy: testAccCheckAWSCloudFormationStackSetDestroy,

PreCheck: func() { testAccPreCheck(t); testAccPreCheckAWSCloudFormationStackSet(t) },

ErrorCheck: testAccErrorCheck(t, cloudformation.EndpointsID),

Providers: testAccProviders,

CheckDestroy: testAccCheckAWSCloudFormationStackSetDestroy,

bflad · 2021-03-31T16:28:58Z

aws/resource_aws_cloudformation_stack_set_test.go

+		enabled                          = true
+		retain_stacks_on_account_removal = false


Newer linter since this pull request was introduced:

Suggested change

enabled = true

retain_stacks_on_account_removal = false

enabled = true

retain_stacks_on_account_removal = false

bflad · 2021-03-31T16:29:34Z

website/docs/r/cloudformation_stack_set.html.markdown

+  * `enabled` - Whether or not auto-deployment is enabled.
+  * `retain_stacks_on_account_removal` - Whether or not to retain stacks when the account is removed.


Newer linter since this pull request was introduced:

Suggested change

* `enabled` - Whether or not auto-deployment is enabled.

* `retain_stacks_on_account_removal` - Whether or not to retain stacks when the account is removed.

* `enabled` - (Optional) Whether or not auto-deployment is enabled.

* `retain_stacks_on_account_removal` - (Optional) Whether or not to retain stacks when the account is removed.

Reference: #12423 (review)

srikanthchelluri · 2021-03-31T19:27:29Z

thanks for merging and taking care of the minor changes, @bflad!

ghost · 2021-04-01T22:57:23Z

This has been released in version 3.35.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

ghost · 2021-05-01T17:10:30Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

srikanthchelluri marked this pull request as ready for review March 17, 2020 14:33

srikanthchelluri requested a review from a team March 17, 2020 14:33

bdwyertech mentioned this pull request Mar 24, 2020

CloudFormation.Stackset - Plan Requires AdministrationRoleARN while Apply says "not supported" #12125

Closed

srikanthchelluri force-pushed the stacksets-permissions branch from 005e193 to 966cf04 Compare March 25, 2020 21:51

srikanthchelluri force-pushed the stacksets-permissions branch from 966cf04 to 2f8d823 Compare March 30, 2020 14:53

srikanthchelluri changed the title ~~resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model~~ WIP: resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model Mar 30, 2020

resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGE…

675b02a

…D permission model

srikanthchelluri force-pushed the stacksets-permissions branch from 2f8d823 to 675b02a Compare March 30, 2020 15:19

srikanthchelluri changed the title ~~WIP: resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model~~ resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model Mar 30, 2020

anGie44 force-pushed the master branch from 54db70d to 48ba2f7 Compare July 21, 2020 18:27

Base automatically changed from master to main January 23, 2021 00:57

breathingdust requested a review from a team as a code owner January 23, 2021 00:57

rclark mentioned this pull request Mar 2, 2021

Support the SERVICE_MANAGED permission model for CloudFormation StackSets #12422

Closed

bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Mar 31, 2021

bflad self-assigned this Mar 31, 2021

bflad approved these changes Mar 31, 2021

View reviewed changes

bflad added a commit that referenced this pull request Mar 31, 2021

resource/aws_cloudformation_stack_set: Finalize SERVICE_MANAGED support

f400978

Reference: #12423 (review)

bflad merged commit 2bbc75f into hashicorp:main Mar 31, 2021

github-actions bot added this to the v3.35.0 milestone Mar 31, 2021

ghost mentioned this pull request Apr 7, 2021

Support StackSet instances for StackSets that use the SERVICE_MANAGED permission model #12425

Closed

ghost locked as resolved and limited conversation to collaborators May 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model #12423

resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model #12423

srikanthchelluri commented Mar 17, 2020 •

edited

Loading

srikanthchelluri commented Mar 25, 2020

srikanthchelluri commented Mar 30, 2020

srikanthchelluri commented Mar 30, 2020

srikanthchelluri commented May 19, 2020

cgetzen commented May 29, 2020

piersf commented Aug 27, 2020

aldegoeij commented Oct 2, 2020

chancez commented Dec 8, 2020

daveadams commented Jan 22, 2021

srikanthchelluri commented Feb 25, 2021

myoung34 commented Mar 2, 2021

piersf commented Mar 3, 2021

myoung34 commented Mar 3, 2021 •

edited

Loading

piersf commented Mar 3, 2021

bflad left a comment

bflad Mar 31, 2021

bflad Mar 31, 2021

bflad Mar 31, 2021

bflad Mar 31, 2021

srikanthchelluri commented Mar 31, 2021

ghost commented Apr 1, 2021

ghost commented May 1, 2021

		* `enabled` - Whether or not auto-deployment is enabled.
		* `retain_stacks_on_account_removal` - Whether or not to retain stacks when the account is removed.

resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model #12423

resource/aws_cloudformation_stack_set: Add support for SERVICE_MANAGED permission model #12423

Conversation

srikanthchelluri commented Mar 17, 2020 • edited Loading

Community Note

srikanthchelluri commented Mar 25, 2020

srikanthchelluri commented Mar 30, 2020

srikanthchelluri commented Mar 30, 2020

srikanthchelluri commented May 19, 2020

cgetzen commented May 29, 2020

piersf commented Aug 27, 2020

aldegoeij commented Oct 2, 2020

chancez commented Dec 8, 2020

daveadams commented Jan 22, 2021

srikanthchelluri commented Feb 25, 2021

myoung34 commented Mar 2, 2021

piersf commented Mar 3, 2021

myoung34 commented Mar 3, 2021 • edited Loading

piersf commented Mar 3, 2021

bflad left a comment

Choose a reason for hiding this comment

bflad Mar 31, 2021

Choose a reason for hiding this comment

bflad Mar 31, 2021

Choose a reason for hiding this comment

bflad Mar 31, 2021

Choose a reason for hiding this comment

bflad Mar 31, 2021

Choose a reason for hiding this comment

srikanthchelluri commented Mar 31, 2021

ghost commented Apr 1, 2021

ghost commented May 1, 2021

srikanthchelluri commented Mar 17, 2020 •

edited

Loading

myoung34 commented Mar 3, 2021 •

edited

Loading