Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ test-unit:

test-integration:
mkdir -p tests/results-integration
$(GO) test $(GO_MOD_FLAGS) $(GO_BUILD_FLAGS) -coverprofile=tests/results-integration/cover-additional.out -race -count=1 ./internal/pkg/... -run TestIntegrationAdditional
$(GO) test $(GO_MOD_FLAGS) $(GO_BUILD_FLAGS) -coverprofile=tests/results-integration/cover-release.out -race -count=1 ./internal/pkg/... -run TestIntegrationRelease
$(GO) test $(GO_MOD_FLAGS) $(GO_BUILD_FLAGS) -coverprofile=tests/results-integration/cover-additional.out -race -count=1 ./internal/pkg/... -run 'TestIntegrationAdditional$$'
$(GO) test $(GO_MOD_FLAGS) $(GO_BUILD_FLAGS) -coverprofile=tests/results-integration/cover-release.out -race -count=1 ./internal/pkg/... -run 'TestIntegrationRelease$$'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoids running the M2M twice

$(GO) test $(GO_MOD_FLAGS) $(GO_BUILD_FLAGS) -coverprofile=tests/results-integration/cover-additional.out -race -count=1 ./internal/pkg/... -run TestIntegrationAdditionalM2M
$(GO) test $(GO_MOD_FLAGS) $(GO_BUILD_FLAGS) -coverprofile=tests/results-integration/cover-release.out -race -count=1 ./internal/pkg/... -run TestIntegrationReleaseM2M

Expand Down
124 changes: 123 additions & 1 deletion internal/testutils/testutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import (
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/empty"
"github.com/google/go-containerregistry/pkg/v1/layout"
"github.com/google/go-containerregistry/pkg/v1/mutate"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/google/go-containerregistry/pkg/v1/types"
"github.com/openshift/oc-mirror/v2/internal/pkg/image"
)

Expand Down Expand Up @@ -159,6 +161,45 @@ func buildAndPushFakeImage(content map[string][]byte, imgRef string, dir string)
return digest.String(), nil
}

// GetAllManifestDigests retrieves all manifest digests from a pushed image
func GetAllManifestDigests(imgRef string) ([]string, error) {
var digests []string

ref, err := name.ParseReference(imgRef)
if err != nil {
return nil, err
}

// Get the image descriptor
desc, err := remote.Get(ref)
if err != nil {
return nil, err
}

// Add the main digest
digests = append(digests, desc.Digest.String())

// Check if it's an index/manifest list
if desc.MediaType.IsIndex() {
idx, err := desc.ImageIndex()
if err != nil {
return digests, nil // Return what we have
}

manifest, err := idx.IndexManifest()
if err != nil {
return digests, nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fail fast when index resolution breaks.

Returning digests, nil here masks manifest-list resolution failures and lets callers continue with only a partial signature set. For multi-arch images that means the top-level digest gets a .sig tag while instance digests are silently skipped.

Suggested fix
 	if desc.MediaType.IsIndex() {
 		idx, err := desc.ImageIndex()
 		if err != nil {
-			return digests, nil // Return what we have
+			return nil, fmt.Errorf("resolve image index for %s: %w", imgRef, err)
 		}

 		manifest, err := idx.IndexManifest()
 		if err != nil {
-			return digests, nil
+			return nil, fmt.Errorf("load index manifest for %s: %w", imgRef, err)
 		}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/testutils/testutils.go` around lines 184 - 191, The code currently
swallows errors from desc.ImageIndex() and idx.IndexManifest() by returning
digests, nil which masks manifest-list resolution failures; change both error
paths to fail fast by returning nil and the encountered error (propagate the
error) instead of returning the partial digests slice so callers see the failure
(update the branches that handle err after desc.ImageIndex() and after
idx.IndexManifest() to return nil, err).

}

// Add all manifest digests from the index
for _, m := range manifest.Manifests {
digests = append(digests, m.Digest.String())
}
}

return digests, nil
}

// GenerateFakeImage will use go-containerregistry to push a test image to
// an httptest.Server and will write the image to an OCI layout if dir is not "".
func GenerateFakeImage(content, imgRef string, tempFolder string) (string, error) {
Expand Down Expand Up @@ -251,6 +292,17 @@ func GenerateReleaseAndComponents(toRegistry, tempFolder string, templatePath st
contents.Ref1 = toRegistry + "/openshift-release-dev/ocp-v4.0-art-dev@" + digest1
relatedImages = append(relatedImages, contents.Ref1)

// Push signature images for all manifests in component1
digests1, err := GetAllManifestDigests(component1)
if err != nil {
return "", relatedImages, fmt.Errorf("failed to get manifests for component1: %w", err)
}
for _, d := range digests1 {
if err := PushSignatureImage(d, toRegistry, "openshift-release-dev/ocp-v4.0-art-dev"); err != nil {
return "", relatedImages, fmt.Errorf("failed to push signature for component1 manifest %s: %w", d, err)
}
}

component2 := toRegistry + "/openshift-release-dev/ocp-v4.0-art-dev:component2"
digest2, err := GenerateFakeImage("component2", component2, tempFolder)
if err != nil {
Expand All @@ -259,6 +311,17 @@ func GenerateReleaseAndComponents(toRegistry, tempFolder string, templatePath st
contents.Ref2 = toRegistry + "/openshift-release-dev/ocp-v4.0-art-dev@" + digest2
relatedImages = append(relatedImages, contents.Ref2)

// Push signature images for all manifests in component2
digests2, err := GetAllManifestDigests(component2)
if err != nil {
return "", relatedImages, fmt.Errorf("failed to get manifests for component2: %w", err)
}
for _, d := range digests2 {
if err := PushSignatureImage(d, toRegistry, "openshift-release-dev/ocp-v4.0-art-dev"); err != nil {
return "", relatedImages, fmt.Errorf("failed to push signature for component2 manifest %s: %w", d, err)
}
}

component3 := toRegistry + "/openshift-release-dev/ocp-v4.0-art-dev:component3"
digest3, err := GenerateFakeImage("component3", component3, tempFolder)
if err != nil {
Expand All @@ -267,11 +330,35 @@ func GenerateReleaseAndComponents(toRegistry, tempFolder string, templatePath st
contents.Ref3 = toRegistry + "/openshift-release-dev/ocp-v4.0-art-dev@" + digest3
relatedImages = append(relatedImages, contents.Ref3)

digest, err := GenerateFakeRelease(contents, toRegistry+"/openshift-release-dev/ocp-release:4.15.0-x86_64", tempFolder, templatePath)
// Push signature images for all manifests in component3
digests3, err := GetAllManifestDigests(component3)
if err != nil {
return "", relatedImages, fmt.Errorf("failed to get manifests for component3: %w", err)
}
for _, d := range digests3 {
if err := PushSignatureImage(d, toRegistry, "openshift-release-dev/ocp-v4.0-art-dev"); err != nil {
return "", relatedImages, fmt.Errorf("failed to push signature for component3 manifest %s: %w", d, err)
}
}

releaseImg := toRegistry + "/openshift-release-dev/ocp-release:4.15.0-x86_64"
digest, err := GenerateFakeRelease(contents, releaseImg, tempFolder, templatePath)
if err != nil {
return "", relatedImages, err
}
relatedImages = append(relatedImages, toRegistry+"/openshift-release-dev/ocp-release@"+digest)

// Push signature images for all manifests in the release image
releaseDigests, err := GetAllManifestDigests(releaseImg)
if err != nil {
return "", relatedImages, fmt.Errorf("failed to get manifests for release: %w", err)
}
for _, d := range releaseDigests {
if err := PushSignatureImage(d, toRegistry, "openshift-release-dev/ocp-release"); err != nil {
return "", relatedImages, fmt.Errorf("failed to push signature for release manifest %s: %w", d, err)
}
}

return digest, relatedImages, nil
}

Expand Down Expand Up @@ -355,3 +442,38 @@ func (c CincinnatiMock) CincinnatiHandler(w http.ResponseWriter, r *http.Request
return
}
}

// PushSignatureImage creates and pushes a sigstore-style signature image to the registry.
// The signature tag format is: sha256-<digest>.sig
func PushSignatureImage(imgDigest, registryHost, repository string) error {
// Parse the digest to create the signature tag
digestStr := strings.TrimPrefix(imgDigest, "sha256:")
sigTag := fmt.Sprintf("sha256-%s.sig", digestStr)

// Create a minimal signature image (just a config layer)
sigContent := map[string][]byte{
"/signature": []byte("fake signature content for " + imgDigest),
}

// Build the full image reference with signature tag
sigImgRef := fmt.Sprintf("%s/%s:%s", registryHost, repository, sigTag)

tag, err := name.NewTag(sigImgRef)
if err != nil {
return fmt.Errorf("failed to create signature tag: %w", err)
}

// Create the signature image
img, _ := crane.Image(sigContent)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

// Convert to OCI manifest format
ociImg := mutate.MediaType(img, types.OCIManifestSchema1)
ociImg = mutate.ConfigMediaType(ociImg, types.OCIConfigJSON)

// Push the image
if err := remote.Write(tag, ociImg); err != nil {
return fmt.Errorf("failed to push signature image: %w", err)
}

return nil
}