Skip to content

Commit f488c36

Browse files
committed
squash! copy: add option to strip sparse manifest lists
Add StripOnlyListSignatures option to strip manifest list signatures while preserving per-instance signatures when using CopySpecificImages with StripSparseManifestList. This implements "Option 2" from PR podman-container-tools#655. Previously, users had to use RemoveSignatures which removed all signatures, including per-instance ones that would still be valid after stripping the manifest list. The option requires explicit validation: - Only valid with CopySpecificImages + StripSparseManifestList - Validates at multiple points in copy.Image() and copyMultipleImages() - When used with signed images, requires explicit signature handling Per-instance signatures are preserved because copySingleImage handles each instance independently. Signed-off-by: Alex Guidi <aguidi@redhat.com>
1 parent 6c8d7b7 commit f488c36

3 files changed

Lines changed: 238 additions & 3 deletions

File tree

image/copy/copy.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ type SparseManifestListAction int
9090
// Options allows supplying non-default configuration modifying the behavior of CopyImage.
9191
type Options struct {
9292
RemoveSignatures bool // Remove any pre-existing signatures. Signers and SignBy… will still add a new signature.
93+
// StripOnlyListSignatures strips the manifest list signature while preserving per-instance signatures.
94+
// Only valid with CopySpecificImages and SparseManifestListAction=StripSparseManifestList.
95+
// If RemoveSignatures is also true, RemoveSignatures takes precedence.
96+
StripOnlyListSignatures bool
9397
// Signers to use to add signatures during the copy.
9498
// Callers are still responsible for closing these Signer objects; they can be reused for multiple copy.Image operations in a row.
9599
Signers []*signer.Signer
@@ -334,6 +338,9 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
334338
if len(options.EnsureCompressionVariantsExist) > 0 {
335339
return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image")
336340
}
341+
if options.StripOnlyListSignatures {
342+
return nil, fmt.Errorf("StripOnlyListSignatures can only be used with manifest lists, not single images")
343+
}
337344
requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options)
338345
if err != nil {
339346
return nil, err
@@ -348,6 +355,9 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
348355
if len(options.EnsureCompressionVariantsExist) > 0 {
349356
return nil, fmt.Errorf("EnsureCompressionVariantsExist is not implemented when not creating a multi-architecture image")
350357
}
358+
if options.StripOnlyListSignatures {
359+
return nil, fmt.Errorf("StripOnlyListSignatures can only be used with CopySpecificImages and SparseManifestListAction=StripSparseManifestList, not with CopySystemImage")
360+
}
351361
requireCompressionFormatMatch, err := shouldRequireCompressionFormatMatch(options)
352362
if err != nil {
353363
return nil, err
@@ -378,6 +388,15 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
378388
if !supportsMultipleImages(c.dest) {
379389
return nil, fmt.Errorf("copying multiple images: destination transport %q does not support copying multiple images as a group", destRef.Transport().Name())
380390
}
391+
// Validate StripOnlyListSignatures usage
392+
if options.StripOnlyListSignatures {
393+
if c.options.ImageListSelection != CopySpecificImages {
394+
return nil, fmt.Errorf("StripOnlyListSignatures can only be used with CopySpecificImages, not CopyAllImages")
395+
}
396+
if options.SparseManifestListAction != StripSparseManifestList {
397+
return nil, fmt.Errorf("StripOnlyListSignatures requires SparseManifestListAction=StripSparseManifestList")
398+
}
399+
}
381400
// Copy some or all of the images.
382401
switch c.options.ImageListSelection {
383402
case CopyAllImages:

image/copy/multiple.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"go.podman.io/image/v5/internal/image"
1818
internalManifest "go.podman.io/image/v5/internal/manifest"
1919
"go.podman.io/image/v5/internal/set"
20+
internalsig "go.podman.io/image/v5/internal/signature"
2021
"go.podman.io/image/v5/manifest"
2122
"go.podman.io/image/v5/pkg/compression"
2223
)
@@ -219,7 +220,12 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
219220
// Compare, and perhaps keep in sync with, the version in copySingleImage.
220221
cannotModifyManifestListReason := ""
221222
if len(sigs) > 0 {
222-
cannotModifyManifestListReason = "Would invalidate signatures"
223+
// If StripOnlyListSignatures is set, we allow modification of the manifest list
224+
// by stripping only the list-level signature while preserving per-instance signatures.
225+
// This is handled below by setting sigs to empty (similar to RemoveSignatures).
226+
if !c.options.StripOnlyListSignatures {
227+
cannotModifyManifestListReason = "Would invalidate signatures"
228+
}
223229
}
224230
if destIsDigestedReference {
225231
cannotModifyManifestListReason = "Destination specifies a digest"
@@ -228,6 +234,22 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
228234
cannotModifyManifestListReason = "Instructed to preserve digests"
229235
}
230236

237+
// Validate that when using StripSparseManifestList with signed images, the user explicitly
238+
// acknowledges signature handling via either StripOnlyListSignatures or RemoveSignatures.
239+
// This is consistent with requiring explicit acknowledgment for other manifest edits.
240+
if c.options.SparseManifestListAction == StripSparseManifestList && len(sigs) > 0 &&
241+
!c.options.RemoveSignatures && !c.options.StripOnlyListSignatures {
242+
return nil, fmt.Errorf("SparseManifestListAction.StripSparseManifestList will modify the signed manifest list; use RemoveSignatures to remove all signatures, or StripOnlyListSignatures to strip only the list signature while preserving per-instance signatures")
243+
}
244+
245+
// If StripOnlyListSignatures is set, strip the list-level signatures.
246+
// Per-instance signatures will be preserved because copySingleImage handles each instance
247+
// independently and won't have RemoveSignatures set (unless the user also set it).
248+
if c.options.StripOnlyListSignatures && len(sigs) > 0 {
249+
logrus.Debugf("Stripping manifest list signatures while preserving per-instance signatures")
250+
sigs = []internalsig.Signature{}
251+
}
252+
231253
// Determine if we'll need to convert the manifest list to a different format.
232254
forceListMIMEType := c.options.ForceManifestMIMEType
233255
switch forceListMIMEType {

image/copy/multiple_test.go

Lines changed: 196 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,28 @@
11
package copy
22

33
import (
4+
"context"
45
"os"
56
"path/filepath"
67
"testing"
78

89
digest "github.com/opencontainers/go-digest"
910
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
12+
"go.podman.io/image/v5/directory"
1113
internalManifest "go.podman.io/image/v5/internal/manifest"
1214
"go.podman.io/image/v5/pkg/compression"
1315
)
1416

17+
const (
18+
// Test manifest files (relative to ../internal/manifest/testdata/)
19+
ociManifestFile = "ociv1.manifest.json"
20+
ociIndexZstdFile = "oci1.index.zstd-selection.json"
21+
)
22+
1523
// Test `instanceOpCopy` cases.
1624
func TestPrepareCopyInstancesforInstanceCopyCopy(t *testing.T) {
17-
validManifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", "oci1.index.zstd-selection.json"))
25+
validManifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", ociIndexZstdFile))
1826
require.NoError(t, err)
1927
list, err := internalManifest.ListFromBlob(validManifest, internalManifest.GuessMIMEType(validManifest))
2028
require.NoError(t, err)
@@ -77,7 +85,7 @@ func TestPrepareCopyInstancesforInstanceCopyCopy(t *testing.T) {
7785

7886
// Test `instanceOpClone` cases.
7987
func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) {
80-
validManifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", "oci1.index.zstd-selection.json"))
88+
validManifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", ociIndexZstdFile))
8189
require.NoError(t, err)
8290
list, err := internalManifest.ListFromBlob(validManifest, internalManifest.GuessMIMEType(validManifest))
8391
require.NoError(t, err)
@@ -194,3 +202,189 @@ func convertInstanceCopyToSimplerInstanceCopy(copies []instanceOp) []simplerInst
194202
}
195203
return res
196204
}
205+
206+
// TestStripOnlyListSignaturesValidation tests the validation logic for StripOnlyListSignatures
207+
// by actually calling copy.Image() with various option combinations.
208+
func TestStripOnlyListSignaturesValidation(t *testing.T) {
209+
tests := []struct {
210+
name string
211+
manifestFile string // Relative to testdata directory
212+
options *Options
213+
expectedError string
214+
}{
215+
{
216+
name: "Invalid: StripOnlyListSignatures with single image (not manifest list)",
217+
manifestFile: ociManifestFile,
218+
options: &Options{
219+
ImageListSelection: CopySpecificImages,
220+
SparseManifestListAction: StripSparseManifestList,
221+
StripOnlyListSignatures: true,
222+
},
223+
expectedError: "StripOnlyListSignatures can only be used with manifest lists, not single images",
224+
},
225+
{
226+
name: "Invalid: StripOnlyListSignatures with CopySystemImage",
227+
manifestFile: ociIndexZstdFile,
228+
options: &Options{
229+
ImageListSelection: CopySystemImage,
230+
SparseManifestListAction: StripSparseManifestList,
231+
StripOnlyListSignatures: true,
232+
},
233+
expectedError: "StripOnlyListSignatures can only be used with CopySpecificImages and SparseManifestListAction=StripSparseManifestList, not with CopySystemImage",
234+
},
235+
{
236+
name: "Invalid: StripOnlyListSignatures with CopyAllImages",
237+
manifestFile: ociIndexZstdFile,
238+
options: &Options{
239+
ImageListSelection: CopyAllImages,
240+
SparseManifestListAction: StripSparseManifestList,
241+
StripOnlyListSignatures: true,
242+
},
243+
expectedError: "StripOnlyListSignatures can only be used with CopySpecificImages, not CopyAllImages",
244+
},
245+
{
246+
name: "Invalid: StripOnlyListSignatures without StripSparseManifestList",
247+
manifestFile: ociIndexZstdFile,
248+
options: &Options{
249+
ImageListSelection: CopySpecificImages,
250+
SparseManifestListAction: KeepSparseManifestList,
251+
StripOnlyListSignatures: true,
252+
},
253+
expectedError: "StripOnlyListSignatures requires SparseManifestListAction=StripSparseManifestList",
254+
},
255+
}
256+
257+
for _, tt := range tests {
258+
t.Run(tt.name, func(t *testing.T) {
259+
// Load the appropriate manifest for this test case
260+
manifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", tt.manifestFile))
261+
require.NoError(t, err)
262+
263+
// Set up source directory with the manifest
264+
srcDir := t.TempDir()
265+
srcManifestPath := filepath.Join(srcDir, "manifest.json")
266+
require.NoError(t, os.WriteFile(srcManifestPath, manifest, 0644))
267+
268+
// Set up destination directory
269+
destDir := t.TempDir()
270+
271+
// Create source and destination references
272+
// Note: We use directory transport for simplicity, even though copy.Image
273+
// will fail later in the process. The validation we're testing happens
274+
// early in copy.Image() before it tries to actually copy data.
275+
srcRef, err := directory.NewReference(srcDir)
276+
require.NoError(t, err)
277+
destRef, err := directory.NewReference(destDir)
278+
require.NoError(t, err)
279+
280+
// Call the real copy.Image() function
281+
_, err = Image(context.Background(), nil, destRef, srcRef, tt.options)
282+
283+
// Verify the error matches expectations (all test cases in this function are invalid)
284+
require.Error(t, err, "Expected validation error from copy.Image()")
285+
assert.Equal(t, tt.expectedError, err.Error())
286+
})
287+
}
288+
}
289+
290+
// TestStripSparseManifestListRequiresSignatureHandling tests that when using
291+
// StripSparseManifestList with a signed manifest list, the user must explicitly
292+
// choose how to handle signatures via RemoveSignatures or StripOnlyListSignatures.
293+
func TestStripSparseManifestListRequiresSignatureHandling(t *testing.T) {
294+
// Load a manifest list
295+
manifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", ociIndexZstdFile))
296+
require.NoError(t, err)
297+
298+
tests := []struct {
299+
name string
300+
options *Options
301+
addSignature bool
302+
expectedError string
303+
}{
304+
{
305+
name: "Valid: StripSparseManifestList with signed manifest + RemoveSignatures",
306+
options: &Options{
307+
ImageListSelection: CopySpecificImages,
308+
Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")},
309+
SparseManifestListAction: StripSparseManifestList,
310+
RemoveSignatures: true,
311+
},
312+
addSignature: true,
313+
expectedError: "",
314+
},
315+
{
316+
name: "Valid: StripSparseManifestList with signed manifest + StripOnlyListSignatures",
317+
options: &Options{
318+
ImageListSelection: CopySpecificImages,
319+
Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")},
320+
SparseManifestListAction: StripSparseManifestList,
321+
StripOnlyListSignatures: true,
322+
},
323+
addSignature: true,
324+
expectedError: "",
325+
},
326+
{
327+
name: "Invalid: StripSparseManifestList with signed manifest without signature handling",
328+
options: &Options{
329+
ImageListSelection: CopySpecificImages,
330+
Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")},
331+
SparseManifestListAction: StripSparseManifestList,
332+
},
333+
addSignature: true,
334+
expectedError: "SparseManifestListAction.StripSparseManifestList will modify the signed manifest list; use RemoveSignatures to remove all signatures, or StripOnlyListSignatures to strip only the list signature while preserving per-instance signatures",
335+
},
336+
{
337+
name: "Valid: StripSparseManifestList with unsigned manifest (no signature handling needed)",
338+
options: &Options{
339+
ImageListSelection: CopySpecificImages,
340+
Instances: []digest.Digest{digest.Digest("sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")},
341+
SparseManifestListAction: StripSparseManifestList,
342+
},
343+
addSignature: false,
344+
expectedError: "",
345+
},
346+
}
347+
348+
for _, tt := range tests {
349+
t.Run(tt.name, func(t *testing.T) {
350+
// Set up source directory with the manifest
351+
srcDir := t.TempDir()
352+
srcManifestPath := filepath.Join(srcDir, "manifest.json")
353+
require.NoError(t, os.WriteFile(srcManifestPath, manifest, 0644))
354+
355+
// Add a signature file if requested
356+
if tt.addSignature {
357+
// For directory transport, signatures are stored as "signature-1", "signature-2", etc.
358+
// Copy an existing signature file from testdata
359+
existingSignature, err := os.ReadFile(filepath.Join("..", "internal", "signature", "testdata", "simple.signature"))
360+
require.NoError(t, err)
361+
signaturePath := filepath.Join(srcDir, "signature-1")
362+
require.NoError(t, os.WriteFile(signaturePath, existingSignature, 0644))
363+
}
364+
365+
// Set up destination directory
366+
destDir := t.TempDir()
367+
368+
// Create source and destination references
369+
srcRef, err := directory.NewReference(srcDir)
370+
require.NoError(t, err)
371+
destRef, err := directory.NewReference(destDir)
372+
require.NoError(t, err)
373+
374+
// Call the real copy.Image() function
375+
_, err = Image(context.Background(), nil, destRef, srcRef, tt.options)
376+
377+
// Verify the error matches expectations
378+
if tt.expectedError != "" {
379+
require.Error(t, err, "Expected validation error from copy.Image()")
380+
assert.Equal(t, tt.expectedError, err.Error())
381+
} else {
382+
// Note: The copy may fail for other reasons (missing blobs, etc.)
383+
// but should not fail with the signature handling error
384+
if err != nil {
385+
assert.NotContains(t, err.Error(), "will modify the signed manifest list")
386+
}
387+
}
388+
})
389+
}
390+
}

0 commit comments

Comments
 (0)