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

feat(controlTower): add validation of status for ControlTower #567

Closed

Conversation

richardkeit
Copy link
Contributor

*As an output of:

Description of changes:

Per the linked issue, the AWS Control Tower launch failed.
When subsequently re-running the pipeline,


$ /codebuild/output/src2276/src/s3/00/source/node_modules/.bin/ts-node packages/@aws-accelerator/modules/bin/runner.ts --module control-tower --partition aws --use-existing-role No --config-dir /codebuild/output/src2276/src/s3/01
--
143 | (node:138) NOTE: The AWS SDK for JavaScript (v2) will enter maintenance mode
144 | on September 8, 2024 and reach end-of-support on September 8, 2025.
145 |  
146 | Please migrate your code to use AWS SDK for JavaScript (v3).
147 | For more information, check blog post at https://a.co/cUPnyil
148 | (Use `node --trace-warnings ...` to show where the warning was created)
149 | 2024-09-13 03:39:28.261 \| info \| runner \| There were no changes found to update or reset the Landing Zone.

There were no changes found to update or reset the Landing Zone.

The actual status of the CT was as follows:

{
    "landingZone": {
        "arn": "arn:aws:controltower:ap-southeast-2:XXXXXXX:landingzone/Zzzzzz",
        "driftStatus": {
            "status": "IN_SYNC"
        },
        "latestAvailableVersion": "3.3",
        "manifest": {
            "accessManagement": {
                "enabled": true
            },
            "securityRoles": {
                "accountId": "AAAAAA"
            },
            "governedRegions": [
                "ap-southeast-2",
                "us-east-1"
            ],
            "organizationStructure": {
                "sandbox": {
                    "name": "Infrastructure"
                },
                "security": {
                    "name": "Security"
                }
            },
            "centralizedLogging": {
                "accountId": "BBBBBBB",
                "configurations": {
                    "loggingBucket": {
                        "retentionDays": 365
                    },
                    "kmsKeyArn": "arn:aws:kms:ap-southeast-2:XXXXXXX:key/YYYYYYYYYY",
                    "accessLoggingBucket": {
                        "retentionDays": 3650
                    }
                },
                "enabled": true
            }
        },
        "status": "FAILED",
        "version": "3.3"
    }
}

Based on missing status, it proceeded to run the prepare stack - which failed on:

AWSAccelerator-PrepareStack-418295722774-ap-southeast-2 \|  28/106 \| 3:42:24 AM \| CREATE_IN_PROGRESS   \| Custom::GetPortfolioId                             \| GetPortFolioId/Resource/Default (GetPortFolioId5CA7347E) Resource creation Initiated
--
374 | AWSAccelerator-PrepareStack-418295722774-ap-southeast-2 \|  28/106 \| 3:42:24 AM \| CREATE_FAILED        \| Custom::GetPortfolioId                             \| GetPortFolioId/Resource/Default (GetPortFolioId5CA7347E) Received response status [FAILED] from custom resource. Message returned: Error: No portfolio ID was found for AWS Control Tower Account Factory Portfolio AWS Control Tower in the account

Testing of change

image

@richardkeit
Copy link
Contributor Author

Hi @erwaxler, @bo1984,

Would love this simple validation to be merged in / assessed

@erwaxler
Copy link
Contributor

Hey @richardkeit , thank you for this contribution! I am currently reviewing this code and aim to incorporate it into the next release. I will keep you updated with any requests for change, thank you for your support of the LZA!

@erwaxler erwaxler self-assigned this Feb 13, 2025
@erwaxler erwaxler added the in-progress This issue is actively being developed label Feb 13, 2025
@erwaxler erwaxler removed their assignment Feb 13, 2025
@richardkeit richardkeit force-pushed the feature/control-tower-error branch from a3bbc9b to 813c8b2 Compare February 13, 2025 23:52
@richardkeit
Copy link
Contributor Author

Hey @erwaxler ,

I am attempting to bring in the latest changes from main, but it appears it's missing 1.11.1 release.
How do contributors typically rebase for new features/fixes?

Only commit 96e880f is relevant for my change

@erwaxler
Copy link
Contributor

Hey @richardkeit , starting with v1.11.1 we started making the repository's default branch the latest release to simplify the way we publish new releases. In the future, target the repository's default branch for new pull requests.

Don't worry about rebasing this request, I will just cherry-pick that commit since there are no conflicts.

@erwaxler
Copy link
Contributor

Hey @richardkeit , I'm pleased to say this has been merged into the code base and will be available in the next major release. Thank you again for your contribution to the Landing Zone Accelerator!

@erwaxler erwaxler closed this Feb 17, 2025
@erwaxler erwaxler removed the in-progress This issue is actively being developed label Feb 20, 2025
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.

9 participants