Source OCI image-info from PublishConfiguration and don't pull unless necessary#2161
Open
lbussell wants to merge 9 commits into
Open
Source OCI image-info from PublishConfiguration and don't pull unless necessary#2161lbussell wants to merge 9 commits into
lbussell wants to merge 9 commits into
Conversation
BuildCommand located the previously-published image-info OCI artifact using the generic registry-override/repo-prefix options, which point at the per-build staging registry rather than the publish registry where the artifact actually lives. Inject IOptions<PublishConfiguration> and resolve the image-info registry and repo prefix from PublishRegistry, falling back to the manifest registry when no publish registry is configured. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GetStaleImagesCommand pulled the image-info OCI artifact using the --image-info-registry-override/--image-info-repo-prefix options. Inject IOptions<PublishConfiguration> and resolve the registry and repo prefix from PublishRegistry, falling back to the manifest registry. Remove the now-redundant override options. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GenerateBuildMatrixCommand pulled the image-info OCI artifact using the --image-info-registry-override/--image-info-repo-prefix options. Inject IOptions<PublishConfiguration> and resolve the registry and repo prefix from PublishRegistry, falling back to the manifest registry. Remove the now-redundant override options. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MergeImageInfoCommand pulled the initial (merge-target) image-info OCI artifact using the --initial-image-info-registry-override/--initial-image-info-repo-prefix options. Inject IOptions<PublishConfiguration> and resolve the registry and repo prefix from PublishRegistry, falling back to the manifest registry. Remove the now-redundant override options. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The OCI image-info artifact is only consumed to trim cached images from the matrix, and pulling it requires authenticated access to the publish registry. Skip the pull entirely unless --trim-cached-images was requested, so matrix generation does not need publish-registry access in scenarios that don't trim. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The initial (merge-target) image-info OCI artifact is only relevant when publishing, where it serves as the merge target for newly built image info. Gate the pull on the publish scenario so non-publish merges (which combine freshly produced image-info files) do not attempt to pull from the publish registry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lbussell
changed the title
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| command.Options.ImageInfoRepoPrefix, | ||
| default)) | ||
| .ReturnsAsync(JsonHelper.SerializeObject(imageArtifactDetails)); | ||
| command.Options.ImageInfoPath = Path.Combine(tempFolderContext.Path, "image-info.json"); |
| PublishRegistry = new RegistryEndpoint { Server = publishRegistry, RepoPrefix = repoPrefix } | ||
| }; | ||
| GenerateBuildMatrixCommand command = CreateCommand(imageInfoServiceMock.Object, publishConfig); | ||
| command.Options.Manifest = Path.Combine(tempFolderContext.Path, "manifest.json"); |
mthalman
reviewed
Jun 25, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes how ImageBuilder resolves and pulls the OCI image-info artifact by sourcing the registry/repo-prefix from PublishConfiguration (via appsettings.json) instead of per-command CLI override options, and it reduces registry access requirements by skipping pulls when the artifact isn’t needed.
Changes:
- Resolve
image-infoOCI artifact source fromPublishConfiguration.PublishRegistry(with fallback to the manifest registry) across build-time cache checks and related commands. - Remove command-line options that override the
image-infopull registry/repo-prefix to make behavior consistent across commands. - Gate
image-infopulls to only the scenarios that consume it (generateBuildMatrixonly when--trim-cached-imagesis set;mergeImageInfoonly when publishing), and update tests accordingly.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ImageBuilder/Commands/MergeImageInfoOptions.cs | Removes CLI override options for initial image-info artifact source. |
| src/ImageBuilder/Commands/MergeImageInfoCommand.cs | Pulls initial image-info from publish registry configuration during publish scenario. |
| src/ImageBuilder/Commands/GetStaleImagesOptions.cs | Removes image-info pull override options. |
| src/ImageBuilder/Commands/GetStaleImagesCommand.cs | Pulls image-info from publish registry config (or manifest registry fallback). |
| src/ImageBuilder/Commands/GenerateBuildMatrixOptions.cs | Removes image-info pull override options. |
| src/ImageBuilder/Commands/GenerateBuildMatrixCommand.cs | Pulls image-info only when trimming cached images, using publish registry config. |
| src/ImageBuilder/Commands/BuildCommand.cs | Uses publish registry config for build-time image-info pulls (fallback to manifest registry). |
| src/ImageBuilder.Tests/MergeImageInfoFilesCommandTests.cs | Updates tests for new PublishConfiguration-based behavior. |
| src/ImageBuilder.Tests/GetStaleImagesCommandTests.cs | Updates tests to supply publish registry config instead of CLI overrides. |
| src/ImageBuilder.Tests/GenerateBuildMatrixCommandTests.cs | Adds/updates tests for publish registry sourcing + conditional pulls when trimming. |
| src/ImageBuilder.Tests/BuildCommandTests.cs | Adds test coverage for publish registry + repo-prefix resolution. |
Comment on lines
214
to
217
| string imageInfoRegistry = string.IsNullOrWhiteSpace(_publishConfig.PublishRegistry?.Server) | ||
| ? manifest.Model.Registry | ||
| : _publishConfig.PublishRegistry.Server; | ||
| if (string.IsNullOrWhiteSpace(imageInfoRegistry)) |
Comment on lines
+73
to
+78
| // The image-info artifact lives in the publish registry (where it was last published). | ||
| // Fall back to the manifest registry when no publish registry is configured. | ||
| string imageInfoRegistry = string.IsNullOrWhiteSpace(_publishConfig.PublishRegistry?.Server) | ||
| ? Manifest.Model.Registry | ||
| : _publishConfig.PublishRegistry.Server; | ||
|
|
Comment on lines
+325
to
+331
| // The build-time cache check reads the previously-published image info, which lives in the | ||
| // publish registry (where publishImageInfoArtifact writes it). Fall back to the manifest | ||
| // registry when no publish registry is configured (e.g. public PR builds without appsettings). | ||
| string imageInfoRegistry = string.IsNullOrWhiteSpace(_publishConfig.PublishRegistry?.Server) | ||
| ? Manifest.Model.Registry | ||
| : _publishConfig.PublishRegistry.Server; | ||
|
|
Addresses Copilot review feedback on PR #2161. When falling back to the manifest registry (no publish registry configured), the code still used PublishRegistry.RepoPrefix, which could incorrectly prefix the repo name when the prefix was set but the server was blank. Resolve the registry and repo prefix together so the prefix is null on fallback, and throw a clearer error when no registry is available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Previously, ImageBuilder pulled OCI image info using using CLI options. It also wasn't consistent across commands. This change resolves the artifact's location from
PublishConfiguration(which is already injected viaappsettings.json), so every command pulls from the registry where image-info was last published, falling back to the public registry when no publish registry is configured. The pulls are also gated to the scenarios that actually consume the image info, so commands don't need publish registry access when they don't use it.These are ImageBuilder code-only changes; the corresponding pipeline template updates follow as a separate step per the two-step change process.
PublishConfiguration.getStaleImagesimage-info source fromPublishConfiguration, removing the override options.generateBuildMatriximage-info source fromPublishConfiguration, removing the override options.mergeImageInfoinitial image-info source fromPublishConfiguration, removing the override options.generateBuildMatrixwhen--trim-cached-imagesis set.mergeImageInfowhen publishing.