aws-ia / terraform-aws-control_tower_account_factory

AWS Control Tower Account Factory
Apache License 2.0
605 stars 386 forks source link

enroll log_archive account into aft-account-customizations #353

Closed ivansamartino closed 10 months ago

ivansamartino commented 1 year ago

This is not a bug it is rather a question about the framework

Question What would be the suggested approach to apply aft-account-customizations to the log_archive_account. During the deployment of the framework in my AWS organization, I already had a log account that I re-used for the framework deployment and it is not tracked within the aft-account-request repository.

Would it be harmful to add to the aft-account-request repository the existing log archive account?

snebhu3 commented 1 year ago

@ivansamartino thank you for reaching out. You can onboard the log archive account into AFT just like onboarding any other pre-existing account of your organization into AFT. You can find more details in the 'Update an existing account' documentation.

devalibvr commented 1 year ago

@snebhu3 I'm trying to add control tower management account to "aft-account-request" since I needed to provide alternate contact information. In this process, I already added all control tower accounts without any issue, but when I tried to add CT management account, I received an error relating to the account:PutAlternateContact permission, which it seems AFT cannot manage CT management account. Are there any other ways I can add alternate contacts to the AFT code for CT management account?

balltrev commented 1 year ago

Hey @devalibvr, just to confirm, this failure occurs at account request and you do not make it to the account customization process correct?

The way I read this, you may be providing alternate credentials in the account request payload for your Control Tower management account. To manage any existing Control Tower account, including the management account, you provide the account request parameters as they are. AFT is unable to modify SSO parameters outside of creating a new account.

devalibvr commented 1 year ago

@balltrev yes this is related to account request because adding alternate contacts is part of custom_fields in aft-account-request repo. I spent some time investigating AFT pipeline creation for CT management account and I realized that the issue is related to alternate contacts. To create pipeline, I removed them and the pipeline has been created, but when I added alternate-contacts again, it can not add them into CT management account because of the permission error account:PutAlternateContact. the following is the error showing in aft-alternate-contacts state machine

error: errorMessage": "An error occurred (AccessDeniedException) when calling the PutAlternateContact operation: User: arn:aws:sts::{account_id}:assumed-role/aft-alternate-contacts-add-lambda-role/aft-alternate-contacts-add is not authorized to perform: account:PutAlternateContact"

I checked the role aft-alternate-contacts-add-lambda-role and the permission is allowed for all resources and as I said earlier I've done this for all accounts across control tower.

balltrev commented 1 year ago

The core solution does support management of Control Tower shared accounts, and as I read, I think you've confirmed that functionality as well, the bug here seems to be with the alternate contacts software you're using.

For context, this is supported by our partner Solution's Architect team. I've reached out to them internally and they'll investigate as they have bandwidth.

devalibvr commented 1 year ago

@balltrev Thanks! please update me here if you get more information. this issue only affects the CT management account, as it's working for all other CT accounts

wellsiau-aws commented 1 year ago

relates to #314 and will be investigated as part of the sample repo AFT Workshop Sample

wellsiau-aws commented 1 year ago

From my initial investigation and test, we can solve this problem by:

  1. Assuming role AWSAFTExecution on the AWS Control Tower Management account
  2. To achieve the first requirement, the Lambda function aft-alternate-contacts-add also have to first assume the role AWSAFTAdmin on the AFT Management account id
  3. Execute put_alternate_contact without specifying the AccountId

Consideration

Alternative

The alternative while not elegant it's actually safer from my perspective. @devalibvr would you be incline with the alternative option as explained above ?

wellsiau-aws commented 1 year ago

I have made changes to the aft-alternate-contacts-add to skip put_alternate_contact on CT Management account id.

I believe that should address the remaining open question on this github issue.