Skip to content

feat: support shared ECS image registry mode - #80

Merged
javiercm1410 merged 6 commits into
mainfrom
feat-shared-registry-mode
Sep 9, 2026
Merged

javiercm1410 merged 6 commits into
mainfrom
feat-shared-registry-mode

Conversation

@javiercm1410

Copy link
Copy Markdown
Contributor

Summary

  • add registry.mode: shared for ECS task definitions that pull <ECR_URL>/leopard-platform/<app>:<tag>
  • retain the existing environment repository layout as the default
  • show the resolved registry mode in ops config

Test plan

  • go test ./...

Allow ECS deployments to pull platform images from the shared leopard-platform repository while preserving environment repositories by default.
@changeset-bot

changeset-bot Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 50bccbd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 111bcbbb-ad50-4f53-bdba-6d02f39ef8a0

📥 Commits

Reviewing files that changed from the base of the PR and between bc12e49 and 50bccbd.

📒 Files selected for processing (9)
  • .ops/config.yaml
  • CLAUDE.md
  • cmd/config/config.go
  • cmd/ecs/ecs.go
  • pkg/config/registry.go
  • pkg/config/root_test.go
  • pkg/ecs/base.go
  • pkg/ecs/taskdef.go
  • pkg/ecs/taskdef_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🔇 Additional comments (9)
pkg/config/registry.go (1)

11-12: LGTM!

Also applies to: 15-21

cmd/config/config.go (1)

70-70: LGTM!

pkg/config/root_test.go (2)

109-125: LGTM!


34-42: LGTM!

Also applies to: 49-49, 59-72, 335-355

.ops/config.yaml (1)

40-42: LGTM!

CLAUDE.md (1)

66-70: LGTM!

cmd/ecs/ecs.go (1)

102-105: LGTM!

pkg/ecs/taskdef_test.go (1)

267-272: 🎯 Functional Correctness

Do not add coverage for registry.mode: shared.

The repository defines no registry.mode field or leopard-platform mapping. RegistryRepository() returns the configured repository or {env}/{service}, so the proposed boundary case does not match the configuration contract.

pkg/ecs/base.go (1)

8-8: 🗄️ Data Integrity & Integration

Do not add a registry_mode migration

BaseConfig is built from .ops/config.yaml; the supported input is registry.repository. No repository code or fixture deserializes BaseConfig from base.toml or YAML, and no registry_mode key exists. The repository template reaches resolveImage through BaseAWS.RegistryRepository.


Walkthrough

The change adds configurable ECS registry repository templates. The default template is {env}/{service}. cfg.RegistryRepository() exposes the configured or default template. ECS passes this value to image resolution. ECR image paths expand {env} and {service} before adding the image tag. Configuration output and documentation show the repository setting. Tests cover default and custom repository templates.

Suggested reviewers: angelmadames

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 50bcc

No actionable merge-blocking risk remains after confirming that slashless image values behave as before.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Docstring Coverage ❌ Error Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ⚠️ Warning The title identifies ECS image registry changes, but it claims support for a registry.mode: shared feature that is not implemented. The changes add configurable registry.repository templates inste… Update the title to describe configurable ECS registry repository templates, such as feat: support configurable ECS image repositories.},{
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title identifies ECS image registry changes, but it claims support for a registry.mode: shared feature that is not implemented. The changes add configurable registry.repository templates instead.

Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI

Comment @coderabbitai help to get the list of available commands.

Remove platform-specific registry configuration and keep shared image resolution independent of repository naming.
b-nicasio
b-nicasio previously approved these changes Sep 8, 2026
Replace registry modes with a template that preserves environment-specific repositories by default and supports custom service paths.
b-nicasio
b-nicasio previously approved these changes Sep 9, 2026
byFrederick
byFrederick previously approved these changes Sep 9, 2026
Use registry.repository as the public template setting while preserving its existing expansion and default path.
@javiercm1410
javiercm1410 dismissed stale reviews from byFrederick and b-nicasio via 50bccbd September 9, 2026 00:24
@javiercm1410
javiercm1410 merged commit 0297375 into main Sep 9, 2026
4 checks passed
@javiercm1410
javiercm1410 deleted the feat-shared-registry-mode branch September 9, 2026 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants