feat: push an image to all registries under a deployment - #277
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the build service to support pushing docker images to multiple container registries by renaming FindRegistryConnection to FindRegistryConnections and iterating over the returned slice of registries. Unit tests were also added to cover these scenarios. The reviewer suggested wrapping the registry loop body in an anonymous function to safely use defer for the logout operation, ensuring reliable cleanup on any exit path.
There was a problem hiding this comment.
Code Review
This pull request updates the build service to support multiple container registries by renaming FindRegistryConnection to FindRegistryConnections and iterating over the retrieved registries to log in, push the artifact, and log out. Unit tests have been added to verify this new behavior. The review feedback highlights a potential runtime panic due to dereferencing the config.ProjectId pointer without a nil check, and suggests pluralizing the error message when no registry connections are found to maintain consistency with the renamed method.
There was a problem hiding this comment.
Pull request overview
This PR updates the build flow to support pushing a built Docker image to all container registries associated with a deployment/project, rather than only the first registry returned by the API.
Changes:
- Update registry lookup to return a list of registries (
FindRegistryConnections) instead of a single registry. - Update
RunBuildto iterate through all registries, handling login/push per-registry, and using the first pushed image URI for the build creation payload. - Add unit tests for
FindRegistryConnectionscovering 0/1/many registry responses.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/build/service.go | Return multiple registries and push the image to each registry during RunBuild. |
| internal/build/service_test.go | Add coverage for multi-registry responses and the “no registries found” error case. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
No description provided.