Skip to content

Redistribute ECR policies and clean up policy files - #343

Open
alinabuzachis wants to merge 2 commits into
mattclay:mainfrom
alinabuzachis:policy-redistribution
Open

Redistribute ECR policies and clean up policy files#343
alinabuzachis wants to merge 2 commits into
mattclay:mainfrom
alinabuzachis:policy-redistribution

Conversation

@alinabuzachis

Copy link
Copy Markdown
Collaborator

Summary

  • Redistribute ECR from paas.yaml to storage-services.yaml — natural fit alongside S3, EFS, Backup, and MemoryDB, and frees up space in paas.yaml for future
    SageMaker/Bedrock growth
  • Fix typo: memorydb:DeleleSnapshotmemorydb:DeleteSnapshot (silent permission bug — delete-snapshot was never actually granted)
  • Remove duplicates: glue:Get* was in both application-services.yaml and data-services.yaml; duplicate arn:aws:dms:...endpoint:* resource ARN in
    data-services.yaml
  • Sort alphabetically: all action lists and resource ARNs across all 8 policy files, making it easier to spot duplicates and review diffs
  • Wildcard read-only actions: lightsail:Get*, bedrock:Invoke*, cloudfront:CreateStreamingDistribution*, ecs:*CapacityProvider*
  • Remove redundancy: eks:DescribeNodegroup was already covered by eks:Describe*

Policy size impact (JSON chars, limit: 6,144)

Policy file Before After Change Headroom
paas.yaml 6,021 5,117 -904 (-15.0%) 1,027
storage-services.yaml 2,749 3,548 +799 (+29.1%) 2,596
application-services.yaml 4,818 4,805 -13 (-0.3%) 1,339
data-services.yaml 5,433 5,384 -49 (-0.9%) 760
application-security.yaml 2,182 2,182 0 3,962
compute.yaml 5,791 5,791 0 353
networking.yaml 5,420 5,420 0 724
security-services.yaml 6,082 6,082 0 62

Sizes calculated using the same method as deploy-test-policy.yml: lookup('template', ...) | from_yaml | to_json | length.

Motivation

PR #342 proposes adding 2 new policy files (paas-sagemaker.yaml, security-services-slr.yaml), which would bring the total to 10 — the AWS default limit for managed policies per role. This redistribution avoids that by freeing up ~900 chars in paas.yaml through moving ECR to storage-services.yaml, keeping the policy count at 8 and leaving room for future growth.

Test plan

  • Verify all policies stay under 6,144 chars: make test_policy STAGE=dev
  • Verify no functional permission changes beyond the fixes noted above
  • Confirm ECR operations still work (permissions moved, not removed)
  • Confirm MemoryDB snapshot deletion works (typo fix grants the permission for the first time)

alinabuzachis and others added 2 commits August 20, 2026 14:45
Move ECR actions and resources from paas.yaml into storage-services.yaml
to free up space in paas.yaml for future sagemaker/bedrock growth.
Also wildcard lightsail:Get* to reduce character count.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix memorydb:DeleleSnapshot typo -> memorydb:DeleteSnapshot
- Remove redundant eks:DescribeNodegroup (covered by eks:Describe*)
- Remove duplicate glue:Get* from application-services (already in data-services)
- Remove duplicate dms endpoint resource ARN in data-services
- Consolidate ecs:*CapacityProvider + ecs:PutClusterCapacityProviders into ecs:*CapacityProvider*
- Consolidate cloudfront:CreateStreamingDistribution* wildcard
- Wildcard bedrock:Invoke* (InvokeAgent + InvokeModel)
- Sort all actions alphabetically within each SID block across all policy files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@oraNod oraNod 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.

LGTM. Thanks @alinabuzachis

@tremble tremble left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As a follow-up I think we need to improve the documentation around these policies.

  • Where do specific policies live (some services could live in multiple files)
  • When is it "ok" to use wildcards (read only; does it need to be resource limited?)

Pointing Claude at it, also raised an interesting point:

  • should we add a linter which enforces a sort order?

- backup-storage:MountCapsule
- ecr:CreateRepository
- ecr:Describe*
- ecr:GetAuthorizationToken

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can move to ecr:Get*

Comment thread aws/policy/paas.yaml
Comment on lines 122 to +123
- bedrock:GetFoundationModel
- bedrock:ListFoundationModels

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can move this to
bedrock:Get*
bedrock:List*

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.

3 participants