Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aws organizations account idempotency #90

Merged

Conversation

ramsriu
Copy link
Contributor

@ramsriu ramsriu commented Jan 6, 2025

Title: Implement idempotency for account creation in CreateHandler

Description:
This PR implements idempotency for account creation in the CreateHandler of the AWS Organizations Account resource. The changes ensure that attempting to create an account that already exists results in an appropriate error response rather than attempting to create a duplicate account.

Key Changes:

  1. Modified CreateHandler to check for existing accounts before attempting creation.
  2. Implemented a new checkIfAccountExists method to perform the existence check.
  3. Updated handleRequest method to return an AlreadyExists error when an account with the same email is found.
  4. Updated CallbackContext to track the pre-existence check and account creation status.
  5. Refactored unit tests to align with the new idempotent behavior.

Specific Updates:

  • In CreateHandler:

    • Added checkIfAccountExists method to query existing accounts.
    • Updated handleRequest to use checkIfAccountExists and handle already existing accounts.
    • Modified error handling to return appropriate error codes and messages.
  • In CreateHandlerTest:

    • Updated existing test cases to account for the new idempotency check.
    • Added new test cases:
      • handleRequest_checkIfAccountExists_AccountAlreadyExists
      • handleRequest_checkIfAccountExists_AccountDoesNotExist
    • Modified other affected test cases to align with the new behavior.
  • In CallbackContext:

    • Added isPreExistenceCheckComplete and isDidResourceAlreadyExist flags.

Testing:

  • All existing unit tests have been updated and pass.
  • New unit tests have been added to cover the idempotency scenarios.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@@ -21,6 +21,8 @@ public void setCurrentRetryAttempt(final AccountConstants.Action actionName, fin
}
// used in CREATE handler
private boolean isAccountCreated = false;
private boolean isPreExistenceCheckComplete = false;
private boolean isDidResourceAlreadyExist = false;
Copy link
Contributor

@pns-amzn pns-amzn Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isDidResourceAlreadyExist -> didResourceAlreadyExist

Without this update, will isDidResourceAlreadyExist and setDidResourceAlreadyExist work correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes the Functionality of it doesn't have any affect(if it has any may be I would have caught in the unit tests), it is the name that looks a bit redundant may be not following the naming conventions in java properly, I followed because to maintain similarity with the other intializations

@ramsriu ramsriu merged commit d20c265 into aws-cloudformation:main Jan 8, 2025
1 check passed
@ramsriu ramsriu deleted the aws-organizations-account-idempotency branch January 8, 2025 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants