Skip to content

Commit 8b1caba

Browse files
fix for copying digest artifacts (backport #673) (#683)
Co-authored-by: Adam Martin <adam.martin@ranchergovernment.com>
1 parent 2c4b798 commit 8b1caba

2 files changed

Lines changed: 125 additions & 4 deletions

File tree

cmd/hauler/cli/store/copy.go

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,26 +229,32 @@ func CopyCmd(ctx context.Context, o *flags.CopyOpts, s *store.Layout, targetRef
229229
if ext, isSigKind := sigExts[kind]; isSigKind {
230230
if imgDigest, ok := refDigest[baseRef]; ok {
231231
digestTag := strings.ReplaceAll(imgDigest, ":", "-")
232-
repo := baseRef
233-
if colon := strings.LastIndex(baseRef, ":"); colon != -1 {
234-
repo = baseRef[:colon]
235-
}
232+
repo := repoFromBaseRef(baseRef)
236233
destRef = repo + ":" + digestTag + ext
237234
}
238235
} else if strings.HasPrefix(kind, consts.KindAnnotationReferrers) {
239236
// OCI 1.1 referrer (cosign v3 new-bundle-format): push by manifest digest so
240237
// the target registry wires it up via the OCI Referrers API (subject field).
241238
// For registries that don't support the Referrers API natively, the manifest
239+
<<<<<<< HEAD
242240
// is still pushed intact; the subject linkage depends on registry support.
243241
repo := baseRef
244242
if colon := strings.LastIndex(baseRef, ":"); colon != -1 {
245243
repo = baseRef[:colon]
246244
}
245+
=======
246+
// is still pushed intact... the subject linkage depends on registry support.
247+
repo := repoFromBaseRef(baseRef)
248+
>>>>>>> 0f4a8ba (fix for copying digest artifacts (#673))
247249
destRef = repo + "@" + desc.Digest.String()
248250
}
249251

250252
toRef, err := content.RewriteRefToRegistry(destRef, components[1])
251253
if err != nil {
254+
if !ro.IgnoreErrors {
255+
fatalErr = fmt.Errorf("rewriting ref [%s]: %w", baseRef, err)
256+
return nil
257+
}
252258
l.Warnf("failed to rewrite ref [%s]: %v", baseRef, err)
253259
return nil
254260
}
@@ -288,6 +294,22 @@ func CopyCmd(ctx context.Context, o *flags.CopyOpts, s *store.Layout, targetRef
288294
return nil
289295
}
290296

297+
// repoFromBaseRef strips any digest and/or tag from a stored ref name, yielding
298+
// just the repository path. AnnotationRefName never contains a registry host, so
299+
// the only colons come from a tag or the digest algorithm separator. A digest-only
300+
// ref (myorg/myimage@sha256:<hex>) must strip the "@sha256:<hex>" suffix rather
301+
// than the last colon, which would otherwise land inside the digest (#667).
302+
func repoFromBaseRef(baseRef string) string {
303+
repo := baseRef
304+
if at := strings.Index(repo, "@"); at != -1 {
305+
repo = repo[:at]
306+
}
307+
if colon := strings.LastIndex(repo, ":"); colon != -1 {
308+
repo = repo[:colon]
309+
}
310+
return repo
311+
}
312+
291313
// extractManifestContent extracts a manifest's layers through a mapper target
292314
// This is used for child manifests in indexes that aren't in the store's nameMap
293315
func extractManifestContent(ctx context.Context, s *store.Layout, desc ocispec.Descriptor, m ocispec.Manifest, target content.Target) error {

cmd/hauler/cli/store/copy_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,15 @@ import (
1212

1313
"github.com/google/go-containerregistry/pkg/name"
1414
"github.com/google/go-containerregistry/pkg/v1/remote"
15+
digest "github.com/opencontainers/go-digest"
16+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
1517
"github.com/rs/zerolog"
1618

1719
"hauler.dev/go/hauler/v2/internal/flags"
1820
v1 "hauler.dev/go/hauler/v2/pkg/apis/hauler.cattle.io/v1"
21+
"hauler.dev/go/hauler/v2/pkg/consts"
22+
"hauler.dev/go/hauler/v2/pkg/content"
23+
"hauler.dev/go/hauler/v2/pkg/store"
1924
)
2025

2126
// --------------------------------------------------------------------------
@@ -78,6 +83,47 @@ func TestCopyCmd_UnknownProtocol(t *testing.T) {
7883
}
7984
}
8085

86+
// --------------------------------------------------------------------------
87+
// repoFromBaseRef / destination ref derivation tests (#667)
88+
// --------------------------------------------------------------------------
89+
90+
func TestRepoFromBaseRef(t *testing.T) {
91+
cases := map[string]string{
92+
"myorg/myimage@sha256:" + strings.Repeat("a", 64): "myorg/myimage",
93+
"myorg/myimage:v1.0.2": "myorg/myimage",
94+
"myorg/myimage": "myorg/myimage",
95+
"nested/path/img@sha256:" + strings.Repeat("b", 64): "nested/path/img",
96+
}
97+
for in, want := range cases {
98+
if got := repoFromBaseRef(in); got != want {
99+
t.Errorf("repoFromBaseRef(%q) = %q, want %q", in, got, want)
100+
}
101+
}
102+
}
103+
104+
// TestDestRef_DigestOnly_Parses is a regression test for #667: destination refs
105+
// derived for artifacts of a digest-only image must be parseable by the real
106+
// RewriteRefToRegistry.
107+
func TestDestRef_DigestOnly_Parses(t *testing.T) {
108+
imgDigest := "sha256:" + strings.Repeat("a", 64)
109+
refDigestHex := strings.Repeat("c", 64)
110+
base := "myorg/myimage@" + imgDigest // tag@digest ingests to digest-only
111+
112+
repo := repoFromBaseRef(base)
113+
114+
// sig/att/sbom cosign tag
115+
sigDest := repo + ":" + strings.ReplaceAll(imgDigest, ":", "-") + ".sig"
116+
if _, err := content.RewriteRefToRegistry(sigDest, "target.example.com"); err != nil {
117+
t.Errorf("sig destRef %q failed to rewrite: %v", sigDest, err)
118+
}
119+
120+
// referrer by manifest digest
121+
refDest := repo + "@sha256:" + refDigestHex
122+
if _, err := content.RewriteRefToRegistry(refDest, "target.example.com"); err != nil {
123+
t.Errorf("referrer destRef %q failed to rewrite: %v", refDest, err)
124+
}
125+
}
126+
81127
// --------------------------------------------------------------------------
82128
// Registry copy tests
83129
// --------------------------------------------------------------------------
@@ -232,6 +278,59 @@ func TestCopyCmd_Registry_IgnoreErrors(t *testing.T) {
232278
}
233279
}
234280

281+
// TestCopy_UndeliverableArtifact_RespectsIgnoreErrors verifies that when
282+
// CopyCmd's registry branch derives an unparseable destination ref for an
283+
// artifact (RewriteRefToRegistry failure), the walk fails by default and
284+
// only swallows the error when --ignore-errors is set (#667).
285+
//
286+
// AnnotationRefName is validated on the way into the store's index (AddIndex
287+
// parses it), so a malformed ref name can never reach CopyCmd's walk. The
288+
// referrer destRef, however, is derived from the descriptor's raw Digest
289+
// field ("<repo>@<digest>"), which is never validated as a reference. Seeding
290+
// a referrer descriptor with a Digest containing a space reproduces the
291+
// derivation failure without needing an actually malformed AnnotationRefName.
292+
func TestCopy_UndeliverableArtifact_RespectsIgnoreErrors(t *testing.T) {
293+
ctx := newTestContext(t)
294+
295+
buildStore := func(t *testing.T) *store.Layout {
296+
s := newTestStore(t)
297+
desc := ocispec.Descriptor{
298+
MediaType: ocispec.MediaTypeImageManifest,
299+
Digest: digest.Digest("sha256:not a valid digest"),
300+
Size: 1,
301+
Annotations: map[string]string{
302+
ocispec.AnnotationRefName: "myorg/myimage",
303+
consts.ContainerdImageNameKey: "myorg/myimage",
304+
consts.KindAnnotationName: consts.KindAnnotationReferrers + "/" + strings.Repeat("a", 64),
305+
},
306+
}
307+
if err := s.OCI.AddIndex(desc); err != nil {
308+
t.Fatalf("AddIndex: %v", err)
309+
}
310+
return s
311+
}
312+
313+
dstHost, _ := newTestRegistry(t)
314+
315+
t.Run("default returns error", func(t *testing.T) {
316+
s := buildStore(t)
317+
o := &flags.CopyOpts{StoreRootOpts: defaultRootOpts(s.Root), PlainHTTP: true}
318+
if err := CopyCmd(ctx, o, s, "registry://"+dstHost, defaultCliOpts()); err == nil {
319+
t.Fatal("expected error for undeliverable artifact, got nil")
320+
}
321+
})
322+
323+
t.Run("ignore errors returns nil", func(t *testing.T) {
324+
s := buildStore(t)
325+
o := &flags.CopyOpts{StoreRootOpts: defaultRootOpts(s.Root), PlainHTTP: true}
326+
ro := defaultCliOpts()
327+
ro.IgnoreErrors = true
328+
if err := CopyCmd(ctx, o, s, "registry://"+dstHost, ro); err != nil {
329+
t.Errorf("expected no error with IgnoreErrors=true, got: %v", err)
330+
}
331+
})
332+
}
333+
235334
// TestCopyCmd_Registry_InvalidFilenameSkipTest verifies that CopyCmd emits a
236335
// warning and skips file artifacts whose names begin with characters invalid
237336
// as OCI tag starts, rather than attempting to push them to the registry.

0 commit comments

Comments
 (0)