search

aws-solutions / aws-control-tower-customizations

The Customizations for AWS Control Tower solution combines AWS Control Tower and other highly-available, trusted AWS services to help customers more quickly set up a secure, multi-account AWS environment using AWS best practices.
https://docs.aws.amazon.com/controltower/latest/userguide/cfct-overview.html
Apache License 2.0
353 stars 205 forks source link

Resources not removed when changing/deleting from manifest #24

Open nicoaws opened 3 years ago

nicoaws commented 3 years ago

I noticed that when the OU is changes for an SCP or a CFN resource, that SCP or CFN resource is added to the 'new' OU, but not removed from the old one.

Also, when deleting an SCP or CFN resource, that does not appear to delete it.

groverlalit commented 3 years ago

@dustnic The current version does not support the deletion feature at the stack set level. If an OU, account or region is removed from the CFN resource the pipeline should delete the stack instances.

nicoaws commented 3 years ago

hi Lalit. point taken re: not supporting deletion feature at stack set level. I tested by changing the list of OUs a resource was deployed to from (A,B) to (A,C) the resource was deployed to C, but not removed from B

rcalvachi commented 3 years ago

So what I do is comment out the OU or account and that removes the stack. However, there has to be at least one left in the section and thus, it can't be deleted.

@groverlalit I suggest using the "deploy_method = remove" in a future version. That would be great. Thoughts?

schirag1993 commented 3 years ago

Is this on the roadmap?

mikkelramlov commented 3 years ago

I also believe it should be optional to select retain policy in manifest for each stack. I causes some misconception that a manual deletion procedure is neccesary when removing stacks from manifest file. In my understanding it goes against the IaC idea when you have to make sure the stacks is in sync with the Stacksets.

rakshb commented 3 years ago

Hi all - This feature is on our roadmap and will be evaluated a future release. Thanks!

groverlalit commented 3 years ago

For clarification:

What is already supported?

For SCP resources in manifest :

For CloudFormation resources

What is on the roadmap?

For SCP resources in manifest :

For CloudFormation resources

Speculor commented 3 years ago

I agree this issue is very problematic, counter-intuitive and against general IaC principals. If you add an account or OU to the manifest file and an SCP or CFN template is applied to it, removing the account or OU from the manifest file should cause those resources to be removed. This is how, for example, Terraform or Chef handle things and maintain idempotency: the diff from the existing state is always applied. This issue forces us to manually go in and remove resources that were previously applied with this stack, and at scale this can be extremely time-consuming and error-prone.

kkvinjam commented 2 years ago

+1, one more customer looking for this feature. As per customer, it is not completely idempotent without this capability.

word commented 2 years ago

It has been almost two years since this was reported. Do you have any updates on the progress? It's a rudimentary feature without which this solution isn't very usable beyond the initial set up.

Speculor commented 2 years ago

Yeah this is really bad. Every time we remove an account from our Control Tower the pipeline subsequently fails and we have to go in and manually remove the Stack Set instances for the account that no longer exists. In retrospect we should have used Terraform from the beginning.

randyspainhower commented 2 years ago

agreed we have the same problem. its frustrating, I even created a script to remove them from all our stacksets just for this, but I should not have had to do that.

RichNahra commented 2 years ago

Is this on the roadmap? This is a huge limitation and should be mentioned in the docs.

tomburge commented 2 years ago

Is this on the roadmap? This is a huge limitation and should be mentioned in the docs.

Also a targeted time frame would be good. Some other partners have already built their own customization solutions because deleting objects cleanly is not supported.

kbessas commented 2 years ago

It has been 2 years since this was created. This is fundamental functionality.

suankan commented 2 years ago

Hi @groverlalit Any plans to implement this anytime soon?

balltrev commented 2 years ago

Thanks for the feedback, we understand the pain point here. This is on our roadmap for medium-term implementation, we will circle back with an update as we get closer to deploying a solution.

rcalvachi commented 2 years ago

It has been almost two years since this was reported. Do you have any updates on the progress? It's a rudimentary feature without which this solution isn't very usable beyond the initial set up.

Check out my method: https://github.com/aws-solutions/aws-control-tower-customizations/issues/24#issuecomment-695317733

balltrev commented 1 year ago

We've just published v2.5.0 containing an opt-in flag to enable Stack Set resource deletion.

suankan commented 1 year ago

MANY THANKS GUYS!!! YOU'RE THE BEST!!!

I'll be trying it out somewhere soon and will come back how it went.

suankan commented 1 year ago

My apology for the late update on this.

Tested on version v2.5.1 and flag enable_stack_set_deletion: true works as described for the StackSet resources which is great!

This config parameter name also suggests that it was only meant for StackSets and according to my testing Service Control Policies (SCP) type of resources are still missing similar feature. That would be great if similar feature would be there for SCPs as well!

But anyways, many thanks to you guys from University of New South Wales for getting it done!