Repository navigation
fix(iam): attach the spored grants as a managed policy (#1573) - #1577
Merged
Merged
Conversation
Since v0.162.0 the versioned spored grants (#1558) were a second inline policy on the shared spored-instance-role. IAM caps a role's inline policies at 10,240 characters together, so on an account where the role carries other tenants' inline policies every default-role launch failed with LimitExceeded before an instance existed. The grants are now a customer-managed policy at /spore-host/, one per document per schema per account (spored-policy-v1, spored-baseline-policy-v1, spored-nested-baseline-policy-v1), attached to the role. It does not count against the inline budget. #1558's properties hold: no released CLI touches it, a newer schema on the role (managed or inline) is deferred to, and an existing policy is never rewritten; one with a different document is left alone and the grants go inline beside it. Superseded inline copies and older managed versions are removed only after the policy is attached. Every failure on the managed path (AccessDenied, quotas, anything else) falls back to the inline write v0.163.0 did; an inline LimitExceeded now lists the role's inline policies with their sizes and names both fixes. Deleters: deleteRole (spawn cleanup, pool drain) deletes a spored policy it detaches from its last role; spawn cleanup --instance-roles deletes unattached spored policies a day old.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Author
|
Live check, spore-host-dev (435415984226) us-east-1, 2026-11-10 The setup is a throwaway caller-named role
Teardown and leak check (independent):
Not exercised live:
|
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1573
What was broken
Since v0.162.0 the versioned spored grants (#1558) were a second inline policy on the shared
spored-instance-role. IAM caps a role's inline policies at 10,240 characters together. On an account where the role carries other tenants' inline policies, every launch on the default role failed withLimitExceededbefore an instance existed. The deletes of the older copies never ran, because the write came first.What this does
The grants are now a customer-managed policy, attached to the role:
spored-instance-rolearn:aws:iam::<acct>:policy/spore-host/spored-policy-v<schema>spawn-instance-*(and a caller-named--iam-role).../spore-host/spored-baseline-policy-v<schema>--nested-spawnrole.../spore-host/spored-nested-baseline-policy-v<schema>Naming. The documents vary only by the account (
bindPolicyAccount). They don't vary by region (IAM is global, and every Resource covers all regions) or by role. So one policy per kind per schema serves the whole account. The nested baseline gets its own name because it is a different document, and a managed policy name is per account. Inline, it sharedspored-baseline-policybecause inline names are per role.#1558's properties still hold. No released CLI touches
/spore-host/. IAM grants the union, so an older CLI rewriting its inline name can only add. A newer schema on the role, managed or inline, means this CLI defers and warns.Exists with a different document. spawn never calls
CreatePolicyVersion, so the 5-version cap never comes into play. It never rewrites an existing policy, and never attaches one it didn't write. It compares the document (semantically: URL-decoded, one-element lists folded, string lists sorted). On a mismatch it warns, naming the ARN and how to remove it, and writes its grants inline beside it.Baseline roles are included. Their inline budget is shared with user input:
--iam-policy-file(spawn-custom-policy, any size), the task staging / nested policy (spawn-scoped-policy), and everything on a caller-named--iam-role.Order. Attach first. Only then delete the superseded inline copies (
spored-policy/spored-baseline-policy, and the-v<=schemainline versions v0.162.0/v0.163.0 wrote). Then detach older managed versions, and delete each one if no other role has it attached (IAM'sDeleteConflictis that check).Fallback, so there's no new failure mode. Every failure on the managed path goes back to v0.163.0's inline write, with a warning that names the actions to grant. That covers AccessDenied on any call, the role's attached-policy quota, the account's policy quota, the conflict above, and any other error. If the inline write then hits
LimitExceeded, the error lists every inline policy on the role with its size (whitespace not counted), the total against 10,240, and both fixes. It does not print the bare 409.Leave no trace.
pkg/aws.deleteRole(spawn cleanup,spawn pool drain) deletes a spored policy it detaches when that was the last attachment.spawn cleanup --instance-rolesalso deletes spored-named policies at/spore-host/withAttachmentCount == 0that are more than 24 h old (instancerole.SweepSporedPolicies).New IAM actions (CLI)
On
arn:aws:iam::*:policy/spore-host/*:iam:GetPolicy,iam:GetPolicyVersion,iam:CreatePolicy,iam:DeletePolicy.On the spawn roles:
iam:ListAttachedRolePolicies,iam:DetachRolePolicy(conditionable oniam:PolicyARN),iam:GetRolePolicy(only for the size listing).iam:AttachRolePolicywas already required, for SSM.spawn cleanup --instance-roles:iam:ListPolicies(*).docs/reference/cli-iam-actions.mdis regenerated. The documented least-privilege baseline lives in spore-host/spore-host (docs/reference/iam-permissions.md): companion PR spore-host/spore-host#691. The portal onboarding template only passesspored-instance-roleand never writes IAM, so it doesn't change.IAM call sequence (for live verification)
Before every sequence:
sts:GetCallerIdentity, thenListRolePolicies(role)andListAttachedRolePolicies(role).spored-policy, maybe inlinespored-policy-v1, plus others' policies):GetPolicy(P)→ NoSuchEntity →CreatePolicy(Path=/spore-host/, Name=spored-policy-v1)→AttachRolePolicy(role, P)→DeleteRolePolicy(role, spored-policy)→DeleteRolePolicy(role, spored-policy-v1). No PutRolePolicy. IfAttachRolePolicyanswers NoSuchEntity (IAM eventual consistency right afterCreatePolicy), it is retried for up to 5 s (0.5, 1, 1.5, 2 s). After that, spawn re-checks with GetPolicy, re-creating the policy once if it is gone, before falling back to inline.GetPolicy(P)→GetPolicyVersion(P, v1)(compare). Nothing written.GetPolicy(P)→GetPolicyVersion(P, v1)→AttachRolePolicy(role2, P).PutRolePolicy(role, spored-policy-v1)→DeleteRolePolicy(role, spored-policy).PutRolePolicy→ LimitExceeded →GetRolePolicyfor each inline policy (for sizes) → an actionable error.spored-policy-v2or inlinespored-policy-v2): a warning, nothing else.GetPolicy(P2)→CreatePolicy(P2)→AttachRolePolicy(role, P2)→ (inline deletes) →DetachRolePolicy(role, P1)→DeletePolicy(P1)(DeleteConflict = still used elsewhere, kept).GetPolicy→GetPolicyVersion(mismatch) → a warning, then case 4's inline write. Nothing is attached or rewritten.The emulator tests run 1, 2, the mixed-version cases and the deleter. The fake runs every case.
Tests
New, in
pkg/aws/iam_spored_managed_test.go. The fake enforces the inline cap, the 6,144 managed cap, the 10-attachment quota and DeleteConflict, and checks the role's grants after every call:Also:
pkg/instancerole/sporedpolicies_test.go(the sweep rule), a cmd test forcleanup --instance-roles, and an emulator test that the baseline policy is deleted with its last role and never with spored-instance-role's.TestSporedRolesFitTheInlineLimitWhenMixednow runs every role kind in both managed and fallback modes. The emulator tests that readspored-policy-v1inline now read the attached managed policy.make checksteps all pass locally. A few fail only because the scratch path contains a space (/Volumes/External HD):hermetic-execin the Makefile, dashboard-api'sTestSetupAllRunsEveryStepFromAnyDirectory, and thecheck-build-tagsfile matcher. I ran the hermetic suite by hand and it is green.semgrep scan --config=auto --erroron the changed files is clean.Not merged: the owner verifies live first.