Conversation
✅ Deploy Preview for docs-kargo-io ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
E2E Test ResultsTested the Setup
Promotionsteps:
- uses: oci-push
as: retag-chart
config:
imageRef: "oci://ghcr.io/eronwright/charts/mychart:${{ chartFrom('oci://ghcr.io/eronwright/charts/mychart').Version }}"
destRef: "oci://ghcr.io/eronwright/charts/mychart:promoted"
annotations:
io.kargo.promoted-by: "kargo"
io.kargo.stage: "${{ ctx.stage }}"Result: ✅ SucceededStep outputs: Manifest annotations verified on GHCR: {
"io.kargo.promoted-by": "kargo",
"io.kargo.stage": "retag",
"org.opencontainers.image.created": "2026-02-23T16:06:15-08:00",
"org.opencontainers.image.description": "A minimal chart for testing oci-push",
"org.opencontainers.image.title": "mychart",
"org.opencontainers.image.version": "0.2.0"
} |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5782 +/- ##
==========================================
+ Coverage 56.75% 56.84% +0.09%
==========================================
Files 467 471 +4
Lines 39228 39543 +315
==========================================
+ Hits 22262 22479 +217
- Misses 15636 15705 +69
- Partials 1330 1359 +29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
E2E Test Report: Cross-repo Helm chart push with annotations on GHCRTested cross-repository OCI push of a Helm chart with annotations to a brand-new GHCR repository (no pre-existing blobs). Test setup (namespace
Result: Manifest on GHCR ( {
"config": { "mediaType": "application/vnd.cncf.helm.config.v1+json" },
"layers": [{ "mediaType": "application/vnd.cncf.helm.chart.content.v1.tar+gzip" }],
"annotations": {
"io.kargo.promoted-by": "kargo",
"io.kargo.source-repo": "ghcr.io/eronwright/charts/mychart",
"io.kargo.test": "custom-wrapper",
"org.opencontainers.image.created": "2026-02-23T16:06:15-08:00",
"org.opencontainers.image.description": "A minimal chart for testing oci-push",
"org.opencontainers.image.title": "mychart",
"org.opencontainers.image.version": "0.2.0"
}
}
Previously tested in earlier iterations: retag-in-place (same repo), cross-repo without annotations, image index push. All passed. |
E2E Test: Container Image (multi-arch nginx)Tested Pure copy (no annotations)Digest is preserved across registries:
The pushed image was verified with With scoped annotationsAnnotations used: annotations:
io.kargo.promoted-by: kargo
io.kargo.source-repo: public.ecr.aws/nginx/nginx
"index:io.kargo.index-test": index-annotation-value
"manifest:io.kargo.manifest-test": manifest-annotation-valueAll digests changed as expected (annotations mutate manifest content):
Annotation scoping worked correctly:
|
|
Added a 1 GiB size limit for cross-repository copies in Same-repository retags skip the check since no blob transfer occurs. Tested against the live cluster:
|
|
Verified that same-repo retag + annotate correctly bypasses the size limit:
So the three cases work as expected:
|
|
Closing this PR for now, in favor of a more limited solution focusing on tagging, not replication. To recap, this PR offers an |
|
Re-opening after further offline discussions. |
EronWright
force-pushed
the
EronWright/issue-3762
branch
from
6c78442 to
8a81a2b
Compare
|
|
||
| | Name | Type | Required | Description | | ||
| |------|------|----------|-------------| | ||
| | `imageRef` | `string` | Y | Reference to the source OCI artifact. Supports both tag format `registry/repository:tag` and digest format `registry/repository@sha256:digest`. For Helm OCI artifacts, the `oci://` prefix is supported (e.g., `oci://registry/repository:tag`) and will use Helm-specific credential lookup. | |
There was a problem hiding this comment.
I'm iffy on imageRef. If we're going to the trouble to generalize this as oci-push and already plan to use it for charts, and will likely use it for other sorts of artifacts in the future, I think I'd try to be more general here also. srcRef? artifactRef?
There was a problem hiding this comment.
This is patterned after oci-download:
imageRef- Reference to the OCI artifact to download. Supports both tag formatregistry/repository:tagand digest formatregistry/repository@sha256:digest. For Helm OCI artifacts, theoci://prefix is supported (e.g.,oci://registry/repository:tag) and will use Helm-specific credential lookup.
I think srcRef would be a good alternative but would rather leave it.
docs/docs/50-user-guide/60-reference-docs/30-promotion-steps/oci-push.md
Outdated
Show resolved
Hide resolved
| // image wraps a v1.Image to overlay annotations on its manifest. All methods | ||
| // delegate to the base image; only the manifest is modified. | ||
| type image struct { | ||
| base v1.Image | ||
| annotations map[string]string | ||
|
|
||
| computed bool | ||
| manifest *v1.Manifest | ||
| sync.Mutex | ||
| } | ||
|
|
||
| var _ v1.Image = (*image)(nil) | ||
|
|
||
| func (i *image) compute() error { | ||
| i.Lock() | ||
| defer i.Unlock() | ||
| if i.computed { | ||
| return nil | ||
| } | ||
| m, err := i.base.Manifest() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| manifest := m.DeepCopy() | ||
| if manifest.Annotations == nil { | ||
| manifest.Annotations = map[string]string{} | ||
| } | ||
| maps.Copy(manifest.Annotations, i.annotations) | ||
| i.manifest = manifest | ||
| i.computed = true | ||
| return nil | ||
| } | ||
|
|
||
| func (i *image) MediaType() (types.MediaType, error) { | ||
| return i.base.MediaType() | ||
| } | ||
|
|
||
| func (i *image) Layers() ([]v1.Layer, error) { | ||
| return i.base.Layers() | ||
| } | ||
|
|
||
| func (i *image) ConfigName() (v1.Hash, error) { | ||
| return i.base.ConfigName() | ||
| } | ||
|
|
||
| func (i *image) ConfigFile() (*v1.ConfigFile, error) { | ||
| return i.base.ConfigFile() | ||
| } | ||
|
|
||
| func (i *image) RawConfigFile() ([]byte, error) { | ||
| return i.base.RawConfigFile() | ||
| } | ||
|
|
||
| func (i *image) LayerByDigest(h v1.Hash) (v1.Layer, error) { | ||
| return i.base.LayerByDigest(h) | ||
| } | ||
|
|
||
| func (i *image) LayerByDiffID(h v1.Hash) (v1.Layer, error) { | ||
| return i.base.LayerByDiffID(h) | ||
| } | ||
|
|
||
| func (i *image) Manifest() (*v1.Manifest, error) { | ||
| if err := i.compute(); err != nil { | ||
| return nil, err | ||
| } | ||
| return i.manifest.DeepCopy(), nil | ||
| } | ||
|
|
||
| func (i *image) RawManifest() ([]byte, error) { | ||
| if err := i.compute(); err != nil { | ||
| return nil, err | ||
| } | ||
| return json.Marshal(i.manifest) | ||
| } | ||
|
|
||
| func (i *image) Digest() (v1.Hash, error) { | ||
| if err := i.compute(); err != nil { | ||
| return v1.Hash{}, err | ||
| } | ||
| return partial.Digest(i) | ||
| } | ||
|
|
||
| func (i *image) Size() (int64, error) { | ||
| if err := i.compute(); err != nil { | ||
| return -1, err | ||
| } | ||
| return partial.Size(i) | ||
| } |
There was a problem hiding this comment.
Can you explain what's going on here? By all appearances, compute() performs some one-time initialization of the image and many methods of image call it up front as if to guarantee initialization. This seems a bit unconventional. Is there a reason not to perform all initialization with a constructor-like function like newImage()?
There was a problem hiding this comment.
Similar question for index. I see newIndex() actually exists, but doesn't handle the initialization performed by its own compute() method, so it's still piquing my curiosity.
There was a problem hiding this comment.
The rationale is that it mimics the pattern seen in upstream go-containerregistry. This file contains an alternative implementation of the Image and Index interfaces, to overcome a bug in the upstream implementation that is triggered by non-container images (i.e. Helm charts).
There was a problem hiding this comment.
Looking a bit deeper, the rationale for lazy computation is that computing the manifest requires calling i.base.Manifest() which can fail, and so the constructor approach would require returning an error from newImage(). So, the compute() approach is well-justified.
Filed google/go-containerregistry#2251 for the problem that motivated having our own image implementation.
Implements a new `oci-push` builtin promotion step that copies or retags OCI artifacts (container images and Helm charts) between registries as part of a promotion pipeline. Uses go-containerregistry for server-side copy with support for multi-arch images and manifest annotations. Also extracts shared OCI registry helpers (reference parsing, credential resolution, HTTP transport) from oci-download into oci_common.go for reuse by both steps. Refs: #3762 Signed-off-by: Eron Wright <eron.wright@akuity.io>
Signed-off-by: Eron Wright <eron.wright@akuity.io>
mutate.Annotations wraps images in mutate.image, whose Layers() enumerates layers via ConfigFile().RootFS.DiffIDs. For non-Docker OCI artifacts (e.g. Helm charts), the config blob has no RootFS field, so DiffIDs is empty and Layers() returns nothing. This causes blobs to be omitted from cross-repository pushes, resulting in MANIFEST_BLOB_UNKNOWN errors on registries like GHCR. Replace mutate.Annotations with thin annotatedImage and annotatedIndex wrappers that override only manifest-related methods (Manifest, RawManifest, Digest, Size) while delegating Layers() and all other methods to the base image. This preserves MountableLayer wrapping for cross-repo blob mounting and avoids the broken DiffIDs enumeration path. Signed-off-by: Eron Wright <eron.wright@akuity.io>
Allow annotation keys to be prefixed with "index:" or "manifest:" to control whether they target the index manifest or child image manifests. Unprefixed keys default to the image manifest. For single images, "index:"-prefixed keys are silently ignored. When manifest annotations are applied to an image index, child descriptors are rewritten with updated digests and sizes to stay consistent with the annotated content. Signed-off-by: Eron Wright <eron.wright@akuity.io> Entire-Checkpoint: e87ef149e998
Move annotatedImage/annotatedIndex types from oci_pusher.go into a dedicated mutate.go with a unified Annotations() entrypoint that type-checks for v1.Image vs v1.ImageIndex. Adopt the compute() pattern with sync.Mutex for thread-safe lazy evaluation, matching the go-containerregistry mutate package style. Consolidate pushImage/pushIndex into a single push() method that delegates annotation handling to Annotations(). Add mutate_test.go covering Docker images, OCI/Helm images (empty DiffIDs regression), and multi-arch indexes. Signed-off-by: Eron Wright <eron.wright@akuity.io>
Enforce a 1 GiB maximum compressed artifact size when oci-push copies artifacts across repositories, preventing accidental promotion of very large images through the pipeline. The limit is skipped for same-repo retags since no blob transfer occurs. Size is computed from manifest metadata without downloading blobs. Signed-off-by: Eron Wright <eron.wright@akuity.io>
Signed-off-by: Eron Wright <eron.wright@akuity.io>
…_SIZE Allow hosted environments to tune, disable, or block cross-repo OCI pushes by reading an integer env var at runner creation time. Unset defaults to 1 GiB (preserving current behavior), 0 blocks all cross-repo pushes, and -1 disables the limit entirely. Signed-off-by: Eron Wright <eron.wright@akuity.io>
Use human-friendly byte formatting (GiB/MiB/KiB/bytes) via new pkg/fmt.FormatBytes helper. Add a clear "cross-repository push is disabled" message when the limit is set to zero. Signed-off-by: Eron Wright <eron.wright@akuity.io>
Expose MAX_OCI_PUSH_ARTIFACT_SIZE as a first-class Helm value instead of requiring raw env var overrides via controller.env. Signed-off-by: Eron Wright <eron.wright@akuity.io>
- Rename FormatBytes to FormatByteCount and add unit tests - Convert standalone helpers to ociPusher methods - Remove unnecessary libmutate import alias - Parse max artifact size eagerly in init() using envconfig - Clarify "retagging" doc example as a dedicated release Stage Signed-off-by: Eron Wright <eron.wright@akuity.io>
EronWright
force-pushed
the
EronWright/issue-3762
branch
from
8a81a2b to
f87a6d0
Compare
Mirror the regex fix from PR #5894 (oci-download) to also support non-standard registry ports in oci-push imageRef and destRef fields. Signed-off-by: Eron Wright <eron.wright@akuity.io>
Resolve conflict in oci_downloader_test.go: keep removal of test functions (parseImageReference, buildRemoteOptions, buildHTTPTransport, getAuthOption) that were moved to oci_common_test.go as shared helpers. Add Test_parseOCIReference case for standard registry reference with port. Signed-off-by: Eron Wright <eron.wright@akuity.io>
2 participants
Summary
Adds a new
oci-pushpromotion step that copies/retags OCI artifacts (container images and Helm charts) between registries.parseOCIReference,buildOCIRemoteOptions, etc.) fromoci-downloadintooci_common.goso both steps reuse the same credential resolution and transport logicoci-pushwithStepCapabilityAccessCredentialsoci://prefix)annotationsfield to set OCI manifest annotations on the pushed artifact, with scoped prefixes (index:,manifest:) for controlling placement on image indexes vs child manifestsmutate.go) to work around a limitation ofgo-containerregistry.image(destination ref),digest(sha256),tagCloses #3762
Test plan
go test -race ./pkg/promotion/runner/builtin/...)make lint-go— no issues in changed files)compressed artifact size 6.1 GiB exceeds maximum allowed size of 1.0 GiB