refactor(ecrref): dedupe the private-ECR-image parser into a leaf package - #491
Merged
Conversation
…kage cmd/app_byo.go and pkg/taskproto/wrapper.go each carried an independent copy of ecrImageAccount/ecrRegistryHost (the taskproto copy explicitly duplicated rather than imported, since taskproto must not depend on cmd). pkg/userdata/container.go (#353/PR #490) was about to become a third divergence point with its own slightly-different variant (ecrImageAccountHost, which also resolves the auth region). New pkg/ecrref (stdlib-only leaf): Account, RegistryHost, and AuthHost (the combined host+region lookup userdata needs, with the image's own region winning over a caller-supplied fallback). Both cmd and taskproto's existing functions become one-line aliases over it -- no call-site or test-name changes, so this is purely internal dedup.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
1 participant
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
cmd/app_byo.goandpkg/taskproto/wrapper.goeach carried an independent copy ofecrImageAccount/ecrRegistryHost(thetaskprotocopy explicitly duplicated rather than imported, per its own comment, sincetaskprotomust not depend oncmd).pkg/userdata/container.go(Headless container-run library primitive (DCV-free docker run of an image ref) #353, PR feat(userdata): headless container-run library primitive #490) added a third, slightly different variant (ecrImageAccountHost, which also resolves the auth region) — flagged as a loose end in that PR's summary.pkg/ecrref(stdlib-only leaf):Account,RegistryHost, andAuthHost(the combined host+region lookupuserdataneeds — the image's own embedded region wins over a caller-supplied fallback, since a cross-region ECR pull must authenticate against its own region).cmd/taskproto's existing functions become one-line aliases over the shared leaf — no call-site or test-name changes anywhere, purely internal dedup.Test plan
go build ./...,go vet ./...,gofmt -lcleanpkg/ecrreftests (TestAccount,TestRegistryHost,TestAuthHost) + existingcmd/taskproto/userdataECR-related tests all pass through the aliases unchangedgolangci-lint runon all 4 touched files — 0 new issues (confirmed the 20 pre-existing repo-wide issues are all in untouched files)make check-docs— no driftTestCatalogValidstill fails locally — pre-existing, unrelated (spawn#489, fix in libs#38, not yet released/bumped)