Skip to content

fix(publish): warn when push falls back from OCI 1.1 to OCI 1.0 - #14146

Open
htoyoda18 wants to merge 3 commits into
docker:mainfrom
htoyoda18:fix/oci-push-oci-fallback-warning
Open

fix(publish): warn when push falls back from OCI 1.1 to OCI 1.0#14146
htoyoda18 wants to merge 3 commits into
docker:mainfrom
htoyoda18:fix/oci-push-oci-fallback-warning

Conversation

@htoyoda18

@htoyoda18 htoyoda18 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What I did

PushManifest silently retried in OCI 1.0 format whenever a registry rejected the OCI 1.1 manifest, leaving users unaware their artifact wasn't stored in the newer format. This was left as a TODO because internal/oci intentionally avoids importing logrus.

PushManifest now reports which OCI version was actually used instead of just success/failure, and pkg/compose/publish (which already depends on logrus) logs a warning when that differs from what was requested.

Related issue
N/A

(not mandatory) A picture of a cute animal, if possible in relation to what you did
🐈🐈🐈

Signed-off-by: hiroto.toyoda <hiroto.toyoda@dena.com>
@htoyoda18
htoyoda18 requested review from a team as code owners August 27, 2026 17:50
@htoyoda18
htoyoda18 requested review from glours and ndeloof August 27, 2026 17:50
@htoyoda18

Copy link
Copy Markdown
Contributor Author

Hi @ndeloof @glours 👋 Just a friendly nudge on this one — it's been about a week with no review yet. Happy to address anything if you get a chance to take a look. Thanks!

Copilot AI left a comment

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.

🟢 Approval recommended

The change is small and localized, with only minor wording/doc clarity issues noted.

Pull request overview

This PR makes the publish flow transparent when registries reject OCI 1.1 manifests by having internal/oci.PushManifest report which OCI version was ultimately used, and emitting a warning from pkg/compose/publish when an automatic fallback to OCI 1.0 occurs.

Changes:

  • Extend internal/oci.PushManifest to return the OCI version used (in addition to the manifest descriptor).
  • Update the publish path to log a warning when the default OCI 1.1 push falls back to OCI 1.0.
File summaries
File Description
pkg/compose/publish.go Captures the OCI version used during push and warns when falling back to OCI 1.0.
internal/oci/push.go Returns the OCI version used from PushManifest, including in the fallback path.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/oci/push.go
Comment on lines +97 to +99
// PushManifest pushes the manifest for a Compose OCI artifact and returns
// the OCI version actually used.
func PushManifest(ctx context.Context, resolver remotes.Resolver, named reference.Named, layers []v1.Descriptor, ociVersion api.OCIVersion) (v1.Descriptor, api.OCIVersion, error) {
Comment thread pkg/compose/publish.go
Comment on lines +131 to +133
if options.OCIVersion == "" && usedOCIVersion == api.OCIVersion1_0 {
logrus.Warn("registry does not support OCI 1.1 artifacts; falling back to OCI 1.0 format")
}

@glours glours left a comment

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.

PushManifest currently returns the raw OCI version it ended up using (api.OCIVersion), and the caller reconstructs "did we fall back?" by comparing two independent fields:

if options.OCIVersion == "" && usedOCIVersion == api.OCIVersion1_0 {
    logrus.Warn(...)
}

Returning a bool (e.g. didFallback) instead would be a better fit for what's actually needed right now:

  • PushManifest is the only place that knows why it picked a given version — the moment it decides to retry with api.OCIVersion1_0 is exactly the fallback event itself. A bool captures that fact directly, at the source, instead of asking the caller to re-derive it from two values that happen to correlate today.
  • The caller only ever needs a yes/no signal to decide whether to warn — it never uses the specific api.OCIVersion value for anything else. A richer return type is carrying more information than any consumer reads.
  • It removes a hidden coupling: today, if a second fallback tier were ever added (e.g. 1.1 → 1.0 → some legacy format), the caller's usedOCIVersion == api.OCIVersion1_0 check would need to be updated in lockstep to keep detecting it as a fallback — and nothing would force that update, since it'd still compile and just silently stop warning on the new tier. A bool set at the fallback branch itself doesn't have that failure mode.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants