Skip to content

add ImageIdentifier.repository for custom-registry image lookups#7474

Open
dansola wants to merge 1 commit into
mainfrom
danielsola/se-775-imageidentifier-repository
Open

add ImageIdentifier.repository for custom-registry image lookups#7474
dansola wants to merge 1 commit into
mainfrom
danielsola/se-775-imageidentifier-repository

Conversation

@dansola

@dansola dansola commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Why are the changes needed?

When a flyte v2 image is built with a custom registry override (clone(registry=...)), the
remote-build cache never hits — every deploy rebuilds an image that's already sitting in ECR.

The reason: the existence check (ImageService.GetImage) only receives the image name, so the
dataplane handler always looks in the cluster's default repository, while the build pushed the
image to a different repo. There's currently no way to tell the backend which repository an
image actually lives in, so it can never find custom-registry images and rebuilds them every time.

This PR adds that missing piece of information to the API. (Internal ref: SE-775.)

What changes were proposed in this pull request?

Adds an optional repository field to ImageIdentifier:

message ImageIdentifier {
  string name = 1 [(buf.validate.field).string.min_len = 1];

  // Optional registry/repository the image lives in, e.g.
  // "<account>.dkr.ecr.<region>.amazonaws.com/<repo>" for an image built with a custom registry
  // override (clone(registry=...)). When set, GetImage looks the image up there; when empty, the
  // cluster's default repository is used.
  string repository = 2;
}

It's backwards-compatible — an empty repository preserves today's default-repo behavior. This is
the foundation of a 3-part change: the SDK sets this field on the GetImage request, and the
dataplane GetImage handler uses it to look the image up in the right repo (see Related PRs).

Includes the regenerated Go / Python / swagger stubs.

## How was this patch tested?

NA

### Labels

Please add one or more of the following labels to categorize your PR:
- **added**: For new features.
- **changed**: For changes in existing functionality.
- **deprecated**: For soon-to-be-removed features.
- **removed**: For features being removed.
- **fixed**: For any bug fixed.
- **security**: In case of vulnerabilities

This is important to improve the readability of release notes.

### Setup process

### Screenshots

## Check all the applicable boxes <!-- Follow the above conventions to check the box -->

- [ ] I updated the documentation accordingly.
- [ ] All new and existing tests passed.
- [ ] All commits are signed-off.

## Related PRs

<!-- Add related pull requests for reviewers to check -->

## Stack

If you do use [`git town`](https://www.git-town.com/introduction) to manage PR Stacks, the stack relevant to this PR
will show below. Otherwise, you can ignore this section.

<!-- branch-stack -->

## Docs link

<!-- Add documentation link built by CI jobs here, and specify the changed place -->

@dansola

dansola commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

// "<account>.dkr.ecr.<region>.amazonaws.com/<repo>" for an image built with a custom registry
// override (clone(registry=...)). When set, GetImage looks the image up there; when empty, the
// cluster's default repository is used.
string repository = 2;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit

Suggested change
string repository = 2;
string registry = 2;

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.

Is the idea that we will concat these values to create the final image url to check on?

If that's the case, we do need registry, repo and I'm assuming name already has the version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants