Skip to content

fix: Update ACCOUNT_NAME format in tenant-s3-nuke.sh#77

Merged
venkatamutyala merged 1 commit into
mainfrom
venkatamutyala-patch-2
Feb 12, 2026
Merged

fix: Update ACCOUNT_NAME format in tenant-s3-nuke.sh#77
venkatamutyala merged 1 commit into
mainfrom
venkatamutyala-patch-2

Conversation

@venkatamutyala

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings February 12, 2026 17:20
@venkatamutyala venkatamutyala merged commit f6b08bd into main Feb 12, 2026
6 checks passed
@venkatamutyala venkatamutyala deleted the venkatamutyala-patch-2 branch February 12, 2026 17:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the tenant S3 cleanup script’s AWS Organizations account name lookup to use a DNS-style account name derived from the tenant portion of the captain domain.

Changes:

  • Change ACCOUNT_NAME from tenant-$TENANT_NAME to $TENANT_NAME.onglueops.rocks for Organizations account lookup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ACCOUNT_NAME="tenant-$TENANT_NAME"
ACCOUNT_NAME="$TENANT_NAME.onglueops.rocks"
SUB_ACCOUNT_ID=$(aws organizations list-accounts --output json | jq -r --arg ACCOUNT_NAME "$ACCOUNT_NAME" '.Accounts[] | select(.Name==$ACCOUNT_NAME) | .Id')

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

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

ACCOUNT_NAME is now derived from $TENANT_NAME.onglueops.rocks, which makes the Organizations lookup more brittle. If the computed account name doesn’t exactly match an account, SUB_ACCOUNT_ID becomes empty and the next assume-role call will construct an invalid ARN and fail with a confusing error. Add an explicit check after the list-accounts/jq selection to validate SUB_ACCOUNT_ID is non-empty and exit with a clear message showing the computed ACCOUNT_NAME (and optionally the provided CAPTAIN_DOMAIN_TO_NUKE).

Suggested change
if [ -z "$SUB_ACCOUNT_ID" ]; then
echo "Error: Could not find AWS account with name '$ACCOUNT_NAME'."
echo " This name was derived from CAPTAIN_DOMAIN_TO_NUKE='$CAPTAIN_DOMAIN_TO_NUKE'."
echo " Please verify the captain domain and the AWS Organizations account names."
exit 1
fi

Copilot uses AI. Check for mistakes.
TENANT_NAME=${ADDR[1]}

ACCOUNT_NAME="tenant-$TENANT_NAME"
ACCOUNT_NAME="$TENANT_NAME.onglueops.rocks"

Copilot AI Feb 12, 2026

Copy link

Choose a reason for hiding this comment

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

This change hard-codes the account naming suffix to .onglueops.rocks. Since the script accepts an arbitrary CAPTAIN_DOMAIN_TO_NUKE, passing a domain with a different base (or an extra subdomain level) will still compute ACCOUNT_NAME with .onglueops.rocks, likely causing the account lookup to fail. Consider deriving the base domain from CAPTAIN_DOMAIN (e.g., everything after the tenant label) or explicitly validating that the input ends with .onglueops.rocks before constructing ACCOUNT_NAME.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants