From cb92c2a7e0c52552f46013345684c4e24b2a2ef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20So=C5=82tys?= <74361703+Sawthis@users.noreply.github.com> Date: Thu, 12 Dec 2024 11:29:21 +0100 Subject: [PATCH] Revert "Fix digest format for Signify (#12033)" (#12425) * Revert "quick fix (#12033)" This reverts commit ea6bb32ea389bfb739493be29828c34c41a86673. * Revert "Revert image digest to manifest digest, Switch to use digest.Hex instead of digest.String() (#12040)" This reverts commit 709bacbd8dd3f0cafe5c55521eaefddfb49c72e4. * Fix unit tests --- pkg/sign/notary.go | 38 +++++++++++++++++++++++++++++------- pkg/sign/notary_mocks.go | 16 +++++++-------- pkg/sign/notary_test.go | 42 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 79 insertions(+), 17 deletions(-) diff --git a/pkg/sign/notary.go b/pkg/sign/notary.go index 4285d72a439c..cc26fad596b3 100644 --- a/pkg/sign/notary.go +++ b/pkg/sign/notary.go @@ -42,6 +42,10 @@ type ReferenceInterface interface { type ImageInterface interface { // Manifest retrieves the manifest of the image. Manifest() (ManifestInterface, error) + // GetDigest returns the digest of the image manifest. + GetDigest() (string, error) + // GetSize returns the size of the image. + GetSize() (int64, error) } // ManifestInterface abstracts the functionality of v1.Manifest. @@ -119,6 +123,20 @@ type ImageWrapper struct { img v1.Image } +// GetDigest returns the digest of the image manifest. +func (iw *ImageWrapper) GetDigest() (string, error) { + digest, err := iw.img.Digest() + if err != nil { + return "", err + } + return digest.Hex, nil +} + +// GetSize returns the size of the image. +func (iw *ImageWrapper) GetSize() (int64, error) { + return iw.img.Size() +} + // Manifest retrieves the manifest of the image. func (iw *ImageWrapper) Manifest() (ManifestInterface, error) { manifest, err := iw.img.Manifest() @@ -138,9 +156,9 @@ func (mw *ManifestWrapper) GetConfigSize() int64 { return mw.manifest.Config.Size } -// GetConfigDigest returns the digest of the image config. +// GetConfigDigest returns the digest of the image config without the algorithm prefix. func (mw *ManifestWrapper) GetConfigDigest() string { - return mw.manifest.Config.Digest.Hex + return mw.manifest.Config.Digest.String() } // PayloadBuilderInterface defines the method for constructing the signing payload. @@ -177,17 +195,23 @@ func (pb *PayloadBuilder) BuildPayload(images []string) (SigningPayload, error) return SigningPayload{}, fmt.Errorf("failed to fetch image: %w", err) } - // Retrieve the image manifest. - manifest, err := img.Manifest() + // Retrieve the image manifest digest. + imageDigest, err := img.GetDigest() + if err != nil { + return SigningPayload{}, fmt.Errorf("failed to get image digest: %w", err) + } + + // Retrieve the image size. + imageSize, err := img.GetSize() if err != nil { - return SigningPayload{}, fmt.Errorf("failed to get image manifest: %w", err) + return SigningPayload{}, fmt.Errorf("failed to get image size: %w", err) } // Build the target information. target := Target{ Name: tag, - ByteSize: manifest.GetConfigSize(), - Digest: manifest.GetConfigDigest(), + ByteSize: imageSize, + Digest: imageDigest, } // Build the GUN (Global Unique Name) targets. diff --git a/pkg/sign/notary_mocks.go b/pkg/sign/notary_mocks.go index f335137c7af9..0ecc0b78e776 100644 --- a/pkg/sign/notary_mocks.go +++ b/pkg/sign/notary_mocks.go @@ -113,13 +113,6 @@ type MockImage struct { MockGetSize func() (int64, error) } -func (mi *MockImage) Manifest() (ManifestInterface, error) { - if mi.MockManifest != nil { - return mi.MockManifest() - } - return nil, fmt.Errorf("MockManifest not implemented") -} - func (mi *MockImage) GetDigest() (string, error) { if mi.MockGetDigest != nil { return mi.MockGetDigest() @@ -131,7 +124,14 @@ func (mi *MockImage) GetSize() (int64, error) { if mi.MockGetSize != nil { return mi.MockGetSize() } - return 0, fmt.Errorf("MockGetSize not implemented") + return 0, fmt.Errorf("MockGetDigest not implemented") +} + +func (mi *MockImage) Manifest() (ManifestInterface, error) { + if mi.MockManifest != nil { + return mi.MockManifest() + } + return nil, fmt.Errorf("MockManifest not implemented") } // MockManifest implements ManifestInterface diff --git a/pkg/sign/notary_test.go b/pkg/sign/notary_test.go index 87101e921266..fc03aa281940 100644 --- a/pkg/sign/notary_test.go +++ b/pkg/sign/notary_test.go @@ -127,6 +127,12 @@ func TestImageService_GetImage_Valid(t *testing.T) { }, }, nil }, + MockGetDigest: func() (string, error) { + return "dummy-digest", nil + }, + MockGetSize: func() (int64, error) { + return 1024, nil + }, }, nil }, } @@ -153,6 +159,23 @@ func TestImageService_GetImage_Valid(t *testing.T) { if manifest.GetConfigDigest() != "sha256:dummy-digest" { t.Errorf("Expected config digest to be 'sha256:dummy-digest', got '%s'", manifest.GetConfigDigest()) } + + // Additional checks for new methods + digest, err := img.GetDigest() + if err != nil { + t.Errorf("Expected no error getting digest, got %v", err) + } + if digest != "dummy-digest" { + t.Errorf("Expected digest to be 'dummy-digest', got '%s'", digest) + } + + size, err := img.GetSize() + if err != nil { + t.Errorf("Expected no error getting size, got %v", err) + } + if size != 1024 { + t.Errorf("Expected size to be 1024, got %d", size) + } } // TestImageService_GetImage_Invalid checks fetching an invalid image. @@ -195,7 +218,7 @@ func TestPayloadBuilder_BuildPayload_Valid(t *testing.T) { return 1024 }, MockGetConfigDigest: func() string { - return "sha256:dummy-digest" + return "sha256:dummy-config-digest" }, } @@ -203,6 +226,12 @@ func TestPayloadBuilder_BuildPayload_Valid(t *testing.T) { MockManifest: func() (ManifestInterface, error) { return mockManifest, nil }, + MockGetDigest: func() (string, error) { + return "dummy-manifest-digest", nil + }, + MockGetSize: func() (int64, error) { + return 2048, nil + }, } mockImageRepository := &MockImageRepository{ @@ -225,6 +254,15 @@ func TestPayloadBuilder_BuildPayload_Valid(t *testing.T) { if len(payload.GunTargets) == 0 { t.Errorf("Expected GunTargets to be populated") } + + // Additional checks + target := payload.GunTargets[0].Targets[0] + if target.Digest != "dummy-manifest-digest" { + t.Errorf("Expected digest to be 'dummy-manifest-digest', got '%s'", target.Digest) + } + if target.ByteSize != 2048 { + t.Errorf("Expected byteSize to be 2048, got %d", target.ByteSize) + } } // TestPayloadBuilder_BuildPayload_Invalid checks building a payload for invalid images. @@ -345,7 +383,7 @@ func TestNotarySigner_Sign_Valid(t *testing.T) { { Name: "latest", ByteSize: 1024, - Digest: "sha256:dummy-digest", + Digest: "dummy-manifest-digest", }, }, },