Repository navigation
Fix CreateFleet authorization for GPU host launches - #694
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
CreateFleet authorization grants fcvm-fleet.tf, scripts/fixtures/fcvm-fleet-create-fleet-context.json, scripts/test-fcvm-fleet.py |
CreateFleet grants no longer check aws:RequestedRegion. Tests check the fleet and volume grants, host instance resources, referenced-resource condition, and recorded authorization context. |
RunInstances restrictions fcvm-fleet.tf, scripts/test-fcvm-fleet.py |
Instance-type and Purpose denials apply to RunInstances, not CreateFleet. Host and tile template instances must use Purpose=fcvm-fleet-host and Purpose=fcvm-fleet-tile, respectively. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related issues
ejc3/fcvm#1289— The issue plans to measureCreateFleetstaging. This change updatesCreateFleetauthorization.ejc3/fcvm#1257— The issue includesCreateFleetlaunch timing. This change affects the coordinator's host launch authorization, but does not add timing instrumentation or fallback work.
Possibly related PRs
- ejc3/aws#633 — That PR adds the on-demand tile launch template; this change applies a
Purposerestriction to tile-template instances.
Merge Risk: 
8b5d5
-
The change scopes CreateFleet parent authorization to account and region resource ARNs and keeps the type and Purpose restrictions on RunInstances. No concrete merge-blocking defect was established. Confirm the pending real-context authorization simulations and the negative boundary matrix before relying on this in production.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: fixing CreateFleet authorization for GPU host launches. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
@codex security review |
|
@coderabbitai review |
|
@greptileai review |
✅ Action performed
|
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
The coordinator cannot launch GPU hosts: CreateFleet parent authorization omits instance-type and Purpose context, so the policy added in #676 explicitly denies the intended calls.
Scope CreateFleet parent authorization through the existing account/region resource ARNs. Keep instance type, template, image, subnet, security group and role restrictions on RunInstances, and enforce each template kind’s Purpose on its instances. Add a sanitized fixture from the actual authorization failure.
Validation: 50 focused ARM tests, 35 authorizations in the primary matrix, and 22 independent IAM simulation cases pass. The real-context baseline denial is reproduced; the corrected parent request is allowed while negative resource, type, template, Purpose and PassRole cases are denied. Independent code/security review passes; Access Analyzer reports zero findings. The policy is 9,662 compact bytes, below the 10,240-byte role limit.