RFC 66: Self Managed Stack Set Support #357

linsona · 2021-07-08T19:01:42Z

Tracking issue: StackSets Support #66
Rendered version

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache-2.0 license

text/66-self-managed-stack-sets.md

skinny85

This is a great first attempt @linsona!

Some comments from me, but the general direction looks really promising!

text/66-self-managed-stack-sets.md

Pull request has been modified.

skinny85

This is shaping up very nicely!

A few comments still.

text/66-self-managed-stack-sets.md

Pull request has been modified.

skinny85

Next round of comments. We definitely need to iron out the Roles that were surfaced in this iteration - that seems to be a huge thing that's completely omitted from the current RFC. Also, let's expand the current "working backwards" section to include actually creating StackSet instances (as that's the final purpose of SackSets after all). That should illuminate what is the experience we want for the Roles here.

Also, make sure the linting pases on your document (you can run it locally to make sure it doesn't fail).

text/66-self-managed-stack-sets.md

nija-at · 2021-07-20T12:51:57Z

RFC Bar Raiser: @eladb

Pull request has been modified.

text/66-self-managed-stack-sets.md

Pull request has been modified.

skinny85

We're getting there! These are basically cosmetic issues, I think we've pretty much settled on the customer experience.

text/66-self-managed-stack-sets.md

skinny85 · 2021-07-21T17:09:41Z

Also, the linter is still failing 😉.

Pull request has been modified.

eladb · 2021-07-27T06:03:13Z

text/66-self-managed-stack-sets.md

+
+### README
+
+The `cdk.StackSet` construct in CDK defines AWS resources in a stack set and stack set


Please replace "stack sets" (or any variation thereof) with "StackSets" (one word) across this doc. I believe this is the how it is used by the service documentation.

eladb · 2021-07-27T06:03:49Z

text/66-self-managed-stack-sets.md

+ This is different from the `cdk.CfnStackSet` construct, which requires a CloudFormation stack that then deploys a CloudFormation stack set.
+ The `cdk.StackSet` construct creates the stack set directly if it does not exist,
+or updates a stack set and existing stack set instances while monitoring the stack set operation.
+


Add some information about what StackSets are (from the official CFN docs) and a reference to the official docs entry point

eladb · 2021-07-27T06:07:24Z

text/66-self-managed-stack-sets.md

+* **Stack Set Administration Role:** Role in the parent account, which is used by CloudFormation to assume the stack set execution role in the child account.
+* **Stack Set Execution Role:** Role in the child accounts, which allows the stack set administration role in parent account to assume,
+and perform the stack set instance deployment.


These roles sounds very much like the ones we use for CDK pipelines. Can't we reuse them?

+1. I'm thinking the same.

If all we need is some slight adjustment to the existing role, then we don't need to change the cdk bootstrap command.

Yeah, so after discussing with Adam, we decided to go the route of introducing new roles:

The Stack Set Execution Role names should be identical in all accounts, which is a requirement for how stack sets operate. The current roles in the bootstrap template are suffixed with specific account ids

The Stack Set Execution Role also must trust the Stack Set Administration Role, to assume and provision the stack set instance.

In the current bootstrap, the trust flag would give any IAM entity in the account permissions to assume the execution role, which may have a large scope of permission.

This is different from trust given to the DeploymentActionRole, where it only has permissions to to create stack/changeset, and the actual role with large scope of permissions is CloudFormationExecutionRole which is not assumable to by the trusted account

eladb · 2021-07-27T06:08:20Z

text/66-self-managed-stack-sets.md

+import * as cdk from '@aws-cdk/core';
+import * as iam from '@aws-cdk/aws-iam';
+
+class PocStackSet extends cdk.StackSet {


Our convention is not to use namespaced imports for core types:

Suggested change

class PocStackSet extends cdk.StackSet {

class MyStackSet extends StackSet {

eladb · 2021-07-27T06:08:53Z

text/66-self-managed-stack-sets.md

+
+        new iam.Role(this, 'ExampleRole', {
+          assumedBy: new iam.ServicePrincipal('s3.amazonaws.com')
+        });


Change to 2 space indentation please

eladb · 2021-07-27T06:18:24Z

text/66-self-managed-stack-sets.md

+
+The handling of CDK assets is more complex than a normal stack CDK app.
+Stack set instances are deployed independently from CDK context, and exist in different accounts/regions.
+This is dicussed more in the design details.


Suggested change

This is dicussed more in the design details.

This is discussed more in the design details.

eladb · 2021-07-27T06:19:19Z

text/66-self-managed-stack-sets.md

+
+### What alternative solutions did you consider?
+
+An alternative is the usage of `cdk.CfnStackSet`, which is not ideal for customers with existing stack sets since there is significant migration effort.


I have to say that this does not sound like a good enough reason not to implement this. Migrating from existing infrastructure (either CFN-based or not) is hard in most cases...

What are other reasons for us to support this?

eladb · 2021-07-27T06:20:55Z

text/66-self-managed-stack-sets.md

+
+### CDK Bootstrap CLI Changes
+
+To support stack sets,


Repeat from above: can we use/expand the CDK pipelines roles we already have in the bootstrap stack? Intuitively these should suffice since CDK Pipelines is basically doing the same thing as stack sets.

eladb · 2021-07-27T06:21:39Z

text/66-self-managed-stack-sets.md

+
+**CLI Changes:**
+
+* Update bootstrap CLI to include new flag `--stack-set`


Suggested change

* Update bootstrap CLI to include new flag `--stack-set`

* Update bootstrap CLI to include new flag `--stackset`

eladb · 2021-07-27T06:22:00Z

text/66-self-managed-stack-sets.md

+
+**CLI Changes:**
+
+* Update bootstrap CLI to include new flag `--stack-set`


If possible, it would be nice if we didn't need any of this

nija-at · 2021-07-29T09:41:07Z

text/66-self-managed-stack-sets.md

+* **Stack Set Administration Role:** Role in the parent account, which is used by CloudFormation to assume the stack set execution role in the child account.
+* **Stack Set Execution Role:** Role in the child accounts, which allows the stack set administration role in parent account to assume,
+and perform the stack set instance deployment.


+1. I'm thinking the same.

If all we need is some slight adjustment to the existing role, then we don't need to change the cdk bootstrap command.

nija-at · 2021-07-29T09:42:59Z

text/66-self-managed-stack-sets.md

+ The `cdk.StackSet` construct creates the stack set directly if it does not exist,
+or updates a stack set and existing stack set instances while monitoring the stack set operation.
+
+#### Prerequisites


minor:

Suggested change

#### Prerequisites

#### Bootstrapping

nija-at · 2021-07-29T09:44:52Z

text/66-self-managed-stack-sets.md

+ This is different from the `cdk.CfnStackSet` construct, which requires a CloudFormation stack that then deploys a CloudFormation stack set.
+ The `cdk.StackSet` construct creates the stack set directly if it does not exist,
+or updates a stack set and existing stack set instances while monitoring the stack set operation.


Why is this better than what CfnStackSet does?

nija-at · 2021-07-29T09:50:19Z

text/66-self-managed-stack-sets.md

+      and the Stack Set Execution roles for the child accounts.
+   * The administration role and execution role name should be specified as part of the StackSet properties.
+
+#### Usage


The main value of a stack set is to deploy a template to different accounts (and regions).

The usage should talk about this primary use case - how to specify the list of target accounts and regions for this stack set.

And also cover, how to configure parameters that will have different values in the target accounts/regions.

nija-at · 2021-07-29T09:51:32Z

text/66-self-managed-stack-sets.md

+  administrationRoleName: 'AWSCloudFormationStackSetAdministrationRole',
+  executionRoleName: 'AWSCloudFormationStackSetExecutionRole',


Isn't this assuming that the execution role will be the same in all accounts? Should it be more flexible than that?

nija-at · 2021-07-29T09:52:32Z

text/66-self-managed-stack-sets.md

+### What are we launching today?
+
+The core construct StackSet, which will enable users to define stack sets directly from CDK.
+A new Cloud Assembly artifact type for stack sets. CLI changes to enable deploying stack sets through CDK.


The CDK's cloud assembly is a public API. If we're making changes there (to the schema), the working backwards section should include the API changes there.

nija-at · 2021-07-29T09:53:38Z

text/66-self-managed-stack-sets.md

+### Why should I use this feature?
+
+This core constuct directly creates a CloudFormation stack set vs. provisioning a single stack that deploys a stack set.
+The deployment also updates the stack set, updates existing stack set instances, and monitors the stack set operation.


This is not answering the question. What is the benefit of this change? "Why should I use it?"

nija-at · 2021-07-29T09:58:23Z

text/66-self-managed-stack-sets.md

+### What is not supported?
+
+* Service Managed Stack Sets
+* Stack Set Instance Add/Remove through CDK


From what I understand, 'remove instance from stack set' is a very important use case for stack sets.
Without this, we will corner users into a situation where a production stack is bound to a stack set and they lose flexibility.

Will users be able to do this outside of the cdk? If so, we should have a section in the README to talk about this and the impact of an 'out of band' action.

nija-at · 2021-07-29T10:01:38Z

text/66-self-managed-stack-sets.md

+**Creating Stack Set Instances:**
+
+Before creating stack set instances, child accounts should be bootstrapped (or custom stack set roles created) and the initial stack set deployed.
+Stack set instances can be added to the stack set by console/cli/api by specifying the child account id and region.


How exactly? Why not via the cdk?

nija-at · 2021-07-29T10:04:49Z

text/66-self-managed-stack-sets.md

+### Why are we doing this?
+
+Today, CDK does not support the creation and updating of CloudFormation stack sets directly.
+This solution would also enable deploying to existing stack sets with low effort.
+
+CDK does provide the `cdk.CfnStackSet` construct, which could accomplish similar behavior.
+The difference is that the `cdk.CfnStackSet` construct is a resource in a single CloudFormation stack with the resource type `AWS::CloudFormation::StackSet`,
+where stack provisions the actual stack set.
+This requires a 1:1 mapping of parameters and tags that need to be passed from the stack to the stack set.
+Customer can deploy to existing stack sets, but requires some migration steps.
+The customer would need to create a new stack,
+and import the stack set into the resource `AWS::CloudFormation::StackSet`.


I'm mostly repeating myself here, but this doesn't cover why this approach is better than using CfnStackSet.

We should be talking about this in terms of what we are enabling for users here that didn't previously exist.

eladb · 2021-10-11T09:13:48Z

Sadly this has been abandoned for now. Reopen if you wish to continue to work on this.

linsona mentioned this pull request Jul 8, 2021

RFC 66: StackSets Support (self-managed) #347

Closed

eladb reviewed Jul 8, 2021

View reviewed changes

text/66-self-managed-stack-sets.md Show resolved Hide resolved

text/66-self-managed-stack-sets.md Outdated Show resolved Hide resolved

text/66-self-managed-stack-sets.md Outdated Show resolved Hide resolved

text/66-self-managed-stack-sets.md Outdated Show resolved Hide resolved

skinny85 previously requested changes Jul 13, 2021

View reviewed changes

skinny85 previously requested changes Jul 15, 2021

View reviewed changes

skinny85 previously requested changes Jul 19, 2021

View reviewed changes

nija-at assigned eladb Jul 20, 2021

skinny85 previously requested changes Jul 20, 2021

View reviewed changes

text/66-self-managed-stack-sets.md Show resolved Hide resolved

text/66-self-managed-stack-sets.md Outdated Show resolved Hide resolved

text/66-self-managed-stack-sets.md Outdated Show resolved Hide resolved

skinny85 previously requested changes Jul 21, 2021

View reviewed changes

linsona force-pushed the rfc-66 branch from 94d56e8 to 438b4b2 Compare July 21, 2021 23:52

linsona added 8 commits July 22, 2021 07:41

RFC 66: Self Managed Stack Set Support

8ad7686

Address PR:357 comments

813262b

Add line breaks to make more readable in plain text

40cba6a

Address PR:357 2nd revision comments

b5caa51

Fix linting errors

e320fe0

Address PR:357 3rd revision comments

9e9e3c1

Address PR:357 4th revision comments

995c36b

Address PR:357 5th revision comments

7042c9a

linsona force-pushed the rfc-66 branch from 438b4b2 to 7042c9a Compare July 22, 2021 14:42

eladb suggested changes Jul 27, 2021

View reviewed changes

nija-at suggested changes Jul 29, 2021

View reviewed changes

eladb closed this Oct 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC 66: Self Managed Stack Set Support #357

RFC 66: Self Managed Stack Set Support #357

linsona commented Jul 8, 2021

skinny85 left a comment

skinny85 left a comment

skinny85 left a comment

nija-at commented Jul 20, 2021

skinny85 left a comment

skinny85 commented Jul 21, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

nija-at Jul 29, 2021

linsona Jul 29, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

eladb Jul 27, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

nija-at Jul 29, 2021

eladb commented Oct 11, 2021


		### README

		The `cdk.StackSet` construct in CDK defines AWS resources in a stack set and stack set

	class PocStackSet extends cdk.StackSet {
	class MyStackSet extends StackSet {

	This is dicussed more in the design details.
	This is discussed more in the design details.


		### What alternative solutions did you consider?

		An alternative is the usage of `cdk.CfnStackSet`, which is not ideal for customers with existing stack sets since there is significant migration effort.


		CLI Changes:

		* Update bootstrap CLI to include new flag `--stack-set`

		administrationRoleName: 'AWSCloudFormationStackSetAdministrationRole',
		executionRoleName: 'AWSCloudFormationStackSetExecutionRole',

RFC 66: Self Managed Stack Set Support #357

RFC 66: Self Managed Stack Set Support #357

Conversation

linsona commented Jul 8, 2021

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

nija-at commented Jul 20, 2021

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 commented Jul 21, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eladb commented Oct 11, 2021