fix(ecr): remove read section, cross-account role, and fix setup.policy drift#389
Merged
sebastiancorrea81 merged 2 commits intoJun 10, 2026
Merged
Conversation
agustincelentano
approved these changes
Jun 10, 2026
- Remove cross_account_pull_role_arn variable and read block from provider config — the read section is deprecated for new installations and caused perpetual drift due to PATCH merge semantics (absent keys are preserved by the API rather than cleared) - Always include setup.policy using var.repository_policy directly instead of a conditional merge — the nullplatform API preserves empty strings as-is, avoiding drift when cross-account pull is disabled - Refactor attributes into a local for readability Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The ecr_cross_account_pull role was used exclusively to populate read.role_arn in the nullplatform provider config, which is a deprecated installation pattern. Cross-account pull is now handled entirely via ECR repository policy (setup.policy), making this role orphaned. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sebastiancorrea81
force-pushed
the
fix/ecr-remove-read-use-policy-empty-string
branch
from
7355843 to
d1b1f56
Compare
sebastiancorrea81
deleted the
fix/ecr-remove-read-use-policy-empty-string
branch
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
2 participants
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.
Summary
nullplatform/asset/ecr: Removes the deprecatedreadblock andcross_account_pull_role_arnvariable from the ECR provider config. Thereadsection was used by a legacy installation pattern (role assumption-based cross-account pull) that is no longer used for new installations. Also simplifiessetup.policyhandling — always includes the field usingvar.repository_policydirectly (empty string when not set), which the nullplatform API preserves as-is, avoiding perpetual drift on enable/disable cycles.infrastructure/aws/iam/ecr: Removes theecr_cross_account_pullIAM role, policy, and attachment. This role was exclusively used to populateread.role_arnin the provider config, which is now removed. Cross-account pull access is now handled entirely via ECR repository policy (setup.policy). Removes thecross_account_pull_role_arnoutput accordingly.Context
The root cause of the drift was PATCH merge semantics in the nullplatform API: keys absent from the PATCH body are preserved rather than cleared. This made it impossible to remove the
readsection once set without sending an explicit removal signal. Sinceread.role_arnrejects empty strings and nulls at the API level (unlikesetup.policywhich accepts""), the cleanest fix is to removereadfrom the module entirely.Test plan
tofu validatepassesNo changeson subsequent planNo changeson subsequent plan (no IAM role created, onlysetup.policyupdated)No changeson subsequent plan (setup.policycleared correctly)🤖 Generated with Claude Code