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

Redundant AWSAFTService Service Catalog Portfolio associations #485

Open
GregCKrause opened this issue Aug 18, 2024 · 2 comments
Open

Redundant AWSAFTService Service Catalog Portfolio associations #485

GregCKrause opened this issue Aug 18, 2024 · 2 comments
Labels
bug Something isn't working pending investigation Issue needs further investigation

Comments

@GregCKrause
Copy link

GregCKrause commented Aug 18, 2024

AFT Version:
1.13.1

Bug Description
The aft-account-request-processor Lambda re-associates the AWSAFTService principal with the AWS Control Tower Account Factory Portfolio, even if the principal already has access. Among other effects, this adds noise to CloudTrail event history:
image

To Reproduce

  1. Invoke aft-account-request-processor Lambda

Expected behavior
The aft-account-request-processor Lambda should only associate the AWSAFTService principal with the AWS Control Tower Account Factory Portfolio if the principal does not already have access.

@GregCKrause GregCKrause added bug Something isn't working pending investigation Issue needs further investigation labels Aug 18, 2024
@GregCKrause GregCKrause changed the title Redundant AWSAFTService Service Catalog Portfolio assignments Redundant AWSAFTService Service Catalog Portfolio associations Aug 18, 2024
@bmorrissirromb
Copy link

I'll also add that this is a surprisingly costly little line of code. Service Catalog charges per API request at a rate of ~$0.0007/API call. By default, AFT runs this Lambda once every 5 minutes (via EventBridge rule aft-lambda-account-request-processor).

In a monthly period, that's 8640 calls to this function, which is about $5 per AFT deployment per month just due to inefficient code. This is probably costing customers -- in aggregate -- a few hundred grand per year.

Even if the call to Service Catalog is necessary, we should be able to reduce the cost by moving the association request within the conditional if sqs_message is not None:, so that it only runs if there's actually a message to process.

@sk-at-amazon
Copy link

Thanks for reporting the issue, we have added this to our backlog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pending investigation Issue needs further investigation
Projects
None yet
3 participants