Feature proposal: Automate close account thanks to the recently released CloseAccount API

[fredericbarthelet]

Feature proposal: Automate close account thanks to the recently released CloseAccount API

AWS recently release a new API to automate account deletion from Organization master account. Instead of ignoring account removal operations from organization.yml (current behavior), this new API will actually enable planning for account deletion in the change set.

AWS article regarding release of this API

WDYT ?

[DonMagee]

This should be configurable as it may not be a desired behavior for all use cases.

[fredericbarthelet]

Agreed @DonMagee. In addition, such feature would be a BC if not disabled by default. I drafted a first PR just to have a base for conversation. I added the CloseAccountsOnRemoval property on OrganizationRoot resource to customize change set behavior.

[OlafConijn]

I've been going back and forth on how to implement this, I think I really like the suggestion put forward in the PR. I think how most people would likely use this is to have the "closeAccount: true" on their "suspended ou". though maybe on test/sandbox organisations you might as well put it on the root.

one thing I wonder about: if I have the intent to close 20 accounts but this exceeds the maximum of 10% this month, should the build process "just fail" without committing state?

I would be inclined to say yes (all other options seem to get complicated or have unmanaged ghost accounts lingering in the org). I think in practice the commit would just have to be reverted to re-add the account. so be it?

[OperationalFallacy]

one thing I wonder about: if I have the intent to close 20 accounts but this exceeds the maximum of 10% this month, should the build process "just fail" without committing state?

Is it possible to fail and provide a warning? So committer can adjust the amount of account and re-run the process? Or the api call will close some accounts (so it is not like all-or-nothing)?

[fredericbarthelet]

@OlafConijn @OperationalFallacy thanks for your comments.

I've updated my PR with the following :

• Added unit tests to cover new type of tasks
• Added recursive check on account status to ensure account is now suspended before proceeding to next task. This is similar to account creation status check, both operations are asynchronous and need to be periodically checked. Skipping status check may result in hitting another Organization service quota, L-6A88197C, prohibiting closing more than 3 accounts concurrently

Screenshot below extracted from Organization default quotas

One way to ensure people never exceeds the 10% limitation, quota L-55309E5F, is to rely on currently SUSPENDED account being part of the organization. Account will remain suspended for 90 days before being removed from the organization, while the quota only runs for 30 consecutive days. This implementation will therefor be conservative. The logic will run as detailed below. This should be implemented in src/org-binder/org-task-runner.ts or in src/commands/update-organization.ts as it needs to access all tasks before any are run:

• call ListAccounts API recursively to list all accounts in the organization and their corresponding status
• calculate ratio of futur SUSPENDED accounts (currently SUSPENDED accounts + quantity of Delete account tasks) over the quantity of ACTIVE accounts
• fail before proceeding any other tasks if the ratio exceeds 0.1

Some edge cases may have to be coded as well (when there are less than 10 accounts in an organization, you still can suspend one even if this would result in a ratio larger than 0.1).

Would the PR in its current state suffice or would you like me to implement this conservative check as a mean to fail fast and ensure all-or-nothing behavior ?

[fredericbarthelet]

Hey @OlafConijn @OperationalFallacy 🖐 What do you think of the currently proposed implementation ? Would you be up to merge the PR as is and enable this feature without suspension quota limitation ?

I can add to the current PR the limitation mechanism I proposed above if you deem it necessary beforehand.

Thanks for letting me know how to move forward on this :)

[OlafConijn]

apologies for letting this wait so long. I'll look into this before beginning of next week. appreciate the note! thanks

[OlafConijn]

merged the branch into main, and published a beta that contains this (and other unreleased changes): npm i [email protected]

[fredericbarthelet]

Thanks @OlafConijn, will give it a go to check if everything is working smoothly and as expected for account deletion :)