feat: add default VPC BPA exclusion - #102
Conversation
Create or reuse a VPC-wide allow-bidirectional BPA exclusion for every LowKey pack, disclose the behavior in the final deployment review, and cover the installer and CloudFormation wiring with focused tests.
Reject reused exclusions until they are complete, force exclusion creation for new VPCs, make IAM simulation failures visible, include exclusion lifecycle permissions, and expand regression coverage.
Disclose the VPC-wide bidirectional effect, internet egress, and external ownership of reused exclusions in the final deployment review.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6902f9301f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Separate new-VPC and reused-VPC exclusion resources so shared exclusions survive deletion or replacement of the LowKey stack that originally created them.
Revalidate a complete bidirectional exclusion from the instance before cloning or running any pack, covering direct CloudFormation callers and external exclusion drift.
Use JSON output so AWS CLI combines all exclusion pages before evaluating the pre-bootstrap count.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 915a22bbab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Follow NextToken in the installer and instance checks, make EC2 wait for public routing before UserData, and document the exclusion parameter, VPC-wide scope, and retention behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d884fab712
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Use the API --next-token flag instead of the paginator-only --starting-token, strip the IMDS trailing slash so UserData cannot abort before validation, guard against repeating pagination tokens, and document all four deployment cases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4f98d90e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Inspect only the first 100 exclusions in both callers, keep the malformed-response and IMDS fixes, and document the limit and its failure modes for direct CloudFormation users.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32f1346082
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Fail with a failing command instead of exit so the UserData ERR trap publishes SSM status and signals CloudFormation, avoiding a 30-minute CreationPolicy timeout.
CreateVpcBpaExclusion=false only avoids the duplicate create; the instance-side check reads the same first 100 results and still aborts, so document that the count must be reduced instead.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4e60079fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "$_BPA_READY" != "true" ]]; then | ||
| echo "FATAL: VPC $_TARGET_VPC_ID lacks a complete allow-bidirectional BPA exclusion; refusing to start pack bootstrap" >&2 | ||
| # Fail via a failing command, not 'exit', so the ERR trap reports it. | ||
| false |
There was a problem hiding this comment.
When CreateVpcBpaExclusion=false is used without a valid exclusion while regional BPA is active, the missing exclusion also blocks the instance's IGW path to the public SSM and CloudFormation APIs. Although false now invokes the ERR trap, both status writes and signal-resource therefore fail over the same unavailable network, leaving CloudFormation waiting for the full PT30M CreationPolicy timeout. The fresh evidence after the prior review is that the reporting trap still uses public AWS API calls and this template provisions no private endpoints; perform this validation outside the instance, provide endpoint connectivity, or otherwise avoid depending on the blocked path for the failure signal.
Useful? React with 👍 / 👎.
1 participant
Problem / Motivation
LowKey packs depend on public internet connectivity during EC2 bootstrap, and KiroCrew also exposes an internet-facing ALB. Where VPC Block Public Access is enabled, pack deployment can fail unless the target VPC has an appropriate exclusion before bootstrap begins.
Why it matters
Every LowKey pack must reliably bootstrap in either a newly created or reused public VPC. The deployment wizard must also disclose that the VPC will allow internet ingress before the user chooses Deploy.
What changed (motivation → approach → change)
AWS::EC2::VPCBlockPublicAccessExclusionwith VPC-wideallow-bidirectionalmode for all packs.BPA exclusion: will be createdoralready exists, disclosing VPC-wide bidirectional scope, ingress and egress impact, and external ownership for reused exclusions.DeletionPolicy: RetainandUpdateReplacePolicy: Retain.Tests
bash tests/test-vpc-bpa.sh— 23/23 passed, covering new/reused VPC behavior, wrong-VPC filtering, transitional/failed/deleting state rejection, egress-only rejection, lookup failure, full review disclosure, lifecycle permissions, parameter wiring, exclusion shape, and pre-bootstrap dependency.bash deploy/test-templates.sh— 29/29 passed.bash -n install.sh deploy/test-templates.sh tests/test-vpc-bpa.sh— passed.shellcheck --severity=error install.sh deploy/test-templates.sh tests/test-vpc-bpa.sh— passed.git diff --check— passed.The broader pack suite remains affected only by pre-existing warning-level ShellCheck findings in untouched pack files.
Manual verification
No live deployment was performed because that would create AWS infrastructure. The local AWS account's read-only exclusion query returned no existing exclusions, so non-empty API responses are covered with focused stubs.
Known limitation (accepted)
Exclusion management stays inside the CloudFormation stack by design. Exclusions created for a reused VPC are retained, but a new-VPC exclusion owned by one stack can still be deleted with that stack while another LowKey deployment reuses the same VPC. A teardown-time warning is tracked in #103 rather than splitting networking into a separate stack or blocking VPC reuse here. The instance-side check keeps this fail-closed: bootstrap refuses to start without a complete bidirectional exclusion.
Screenshots / video
Why no screenshot: This changes CloudFormation orchestration and terminal review text, not a rendered web interface.
no linked issue: this change was requested directly and has no tracked issue.