OCPBUGS-44655: Fix multiarch validations using MetadataProvider

Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
openshift · Nov 22, 2024 · 7e1e1e5 · 7e1e1e5
1 parent d419048
commit 7e1e1e5
Show file tree

Hide file tree

Showing 9 changed files with 281 additions and 40 deletions.
diff --git a/cmd/cluster/core/create.go b/cmd/cluster/core/create.go
@@ -642,7 +642,7 @@ func (opts *RawCreateOptions) Validate(ctx context.Context) (*ValidatedCreateOpt
 		if err != nil {
 			return nil, fmt.Errorf("could not retrieve kube clientset: %w", err)
 		}
-		if err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, opts, kc); err != nil {
+		if err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, opts, kc, &hyperutil.RegistryClientImageMetadataProvider{}); err != nil {
 			return nil, err
 		}
 	}
@@ -1006,7 +1006,7 @@ func parseTolerationString(str string) (*corev1.Toleration, error) {
 // validateMgmtClusterAndNodePoolCPUArchitectures checks if a multi-arch release image or release stream was provided.
 // If none were provided, checks to make sure the NodePool CPU arch and the management cluster CPU arch match; if they
 // do not, the CLI will return an error since the NodePool will fail to complete during runtime.
-func validateMgmtClusterAndNodePoolCPUArchitectures(ctx context.Context, opts *RawCreateOptions, kc kubeclient.Interface) error {
+func validateMgmtClusterAndNodePoolCPUArchitectures(ctx context.Context, opts *RawCreateOptions, kc kubeclient.Interface, imageMetadataProvider hyperutil.ImageMetadataProvider) error {
 	validMultiArchImage := false
 
 	// Check if the release image is multi-arch
@@ -1015,8 +1015,7 @@ func validateMgmtClusterAndNodePoolCPUArchitectures(ctx context.Context, opts *R
 		if err != nil {
 			return fmt.Errorf("failed to read pull secret file: %w", err)
 		}
-
-		validMultiArchImage, err = registryclient.IsMultiArchManifestList(ctx, opts.ReleaseImage, pullSecret)
+		validMultiArchImage, err = registryclient.IsMultiArchManifestList(ctx, opts.ReleaseImage, pullSecret, imageMetadataProvider)
 		if err != nil {
 			return err
 		}

diff --git a/cmd/cluster/core/create_test.go b/cmd/cluster/core/create_test.go
@@ -6,6 +6,8 @@ import (
 
 	. "github.com/onsi/gomega"
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
+	"github.com/openshift/hypershift/support/thirdparty/library-go/pkg/image/dockerv1client"
+	"github.com/openshift/hypershift/support/util/fakeimagemetadataprovider"
 	crclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	corev1 "k8s.io/api/core/v1"
@@ -19,11 +21,14 @@ func TestValidateMgmtClusterAndNodePoolCPUArchitectures(t *testing.T) {
 
 	fakeKubeClient := fakekubeclient.NewSimpleClientset()
 	fakeDiscovery, ok := fakeKubeClient.Discovery().(*fakediscovery.FakeDiscovery)
-
 	if !ok {
 		t.Fatalf("failed to convert FakeDiscovery")
 	}
 
+	fakeMetadataProvider := &fakeimagemetadataprovider.FakeImageMetadataProvider{
+		Result: &dockerv1client.DockerImageConfig{},
+	}
+
 	// if you want to fake a specific version
 	fakeDiscovery.FakedServerVersion = &apiversion.Info{
 		Platform: "linux/amd64",
@@ -80,7 +85,7 @@ func TestValidateMgmtClusterAndNodePoolCPUArchitectures(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewWithT(t)
-			err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, tc.opts, fakeKubeClient)
+			err := validateMgmtClusterAndNodePoolCPUArchitectures(ctx, tc.opts, fakeKubeClient, fakeMetadataProvider)
 			if tc.expectError {
 				g.Expect(err).To(HaveOccurred())
 			} else {

diff --git a/ignition-server/cmd/run_local_ignitionprovider.go b/ignition-server/cmd/run_local_ignitionprovider.go
@@ -92,15 +92,20 @@ func (o *RunLocalIgnitionProviderOptions) Run(ctx context.Context) error {
 		return fmt.Errorf("unable to create image file cache: %w", err)
 	}
 
+	imageMetaDataProvider := &util.RegistryClientImageMetadataProvider{
+		OpenShiftImageRegistryOverrides: map[string][]string{},
+	}
+
 	p := &controllers.LocalIgnitionProvider{
-		Client:              cl,
-		ReleaseProvider:     &releaseinfo.ProviderWithOpenShiftImageRegistryOverridesDecorator{},
-		CloudProvider:       "",
-		Namespace:           o.Namespace,
-		WorkDir:             o.WorkDir,
-		PreserveOutput:      true,
-		ImageFileCache:      imageFileCache,
-		FeatureGateManifest: o.FeatureGateManifest,
+		Client:                cl,
+		ReleaseProvider:       &releaseinfo.ProviderWithOpenShiftImageRegistryOverridesDecorator{},
+		ImageMetadataProvider: imageMetaDataProvider,
+		CloudProvider:         "",
+		Namespace:             o.Namespace,
+		WorkDir:               o.WorkDir,
+		PreserveOutput:        true,
+		ImageFileCache:        imageFileCache,
+		FeatureGateManifest:   o.FeatureGateManifest,
 	}
 
 	payload, err := p.GetPayload(ctx, o.Image, config.String(), "", "", "")

diff --git a/ignition-server/controllers/local_ignitionprovider.go b/ignition-server/controllers/local_ignitionprovider.go
@@ -72,6 +72,10 @@ type LocalIgnitionProvider struct {
 	// to render the ignition payload.
 	FeatureGateManifest string
 
+	// ImageMetaDataProvider is used to get the image metadata for the images
+	// used in the ignition payload.
+	ImageMetadataProvider *util.RegistryClientImageMetadataProvider
+
 	ImageFileCache *imageFileCache
 
 	lock sync.Mutex
@@ -191,7 +195,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu
 		return nil, fmt.Errorf("release image does not contain machine-config-operator (images: %v)", imageProvider.ComponentImages())
 	}
 
-	mcoImage, err = registryclient.GetCorrectArchImage(ctx, component, mcoImage, pullSecret)
+	mcoImage, err = registryclient.GetCorrectArchImage(ctx, component, mcoImage, pullSecret, p.ImageMetadataProvider)
 	if err != nil {
 		return nil, err
 	}
@@ -329,7 +333,7 @@ func (p *LocalIgnitionProvider) GetPayload(ctx context.Context, releaseImage, cu
 			return fmt.Errorf("release image does not contain $%s (images: %v)", clusterConfigComponent, imageProvider.ComponentImages())
 		}
 
-		clusterConfigImage, err = registryclient.GetCorrectArchImage(ctx, clusterConfigComponent, clusterConfigImage, pullSecret)
+		clusterConfigImage, err = registryclient.GetCorrectArchImage(ctx, clusterConfigComponent, clusterConfigImage, pullSecret, p.ImageMetadataProvider)
 		if err != nil {
 			return err
 		}
@@ -750,7 +754,7 @@ EOF
    --asset-input-dir %[2]s/input \
    --asset-output-dir %[2]s/output \
    --rendered-manifest-files=%[2]s/manifests \
-   --payload-version=%[4]s 
+   --payload-version=%[4]s
 cp %[2]s/manifests/99_feature-gate.yaml %[3]s/99_feature-gate.yaml
 `
 	}

diff --git a/support/releaseinfo/registryclient/client.go b/support/releaseinfo/registryclient/client.go
@@ -35,6 +35,14 @@ const (
 	ArchitectureARM64   = "arm64"
 )
 
+var (
+	deserializeFunc = deserializeManifest
+)
+
+type ManifestProvider interface {
+	GetManifest(ctx context.Context, imageRef string, pullSecret []byte) (distribution.Manifest, error)
+}
+
 // ExtractImageFiles extracts a list of files from a registry image given the image reference, pull secret and the
 // list of files to extract. It returns a map with file contents or an error.
 func ExtractImageFiles(ctx context.Context, imageRef string, pullSecret []byte, files ...string) (map[string][]byte, error) {
@@ -315,8 +323,8 @@ func GetManifest(ctx context.Context, imageRef string, pullSecret []byte) (distr
 }
 
 // IsMultiArchManifestList determines whether an image is a manifest listed image and contains manifests the following processor architectures: amd64, arm64, s390x, ppc64le
-func IsMultiArchManifestList(ctx context.Context, imageRef string, pullSecret []byte) (bool, error) {
-	srcManifest, err := GetManifest(ctx, imageRef, pullSecret)
+func IsMultiArchManifestList(ctx context.Context, imageRef string, pullSecret []byte, imageMetadataProvider ManifestProvider) (bool, error) {
+	srcManifest, err := imageMetadataProvider.GetManifest(ctx, imageRef, pullSecret)
 	if err != nil {
 		return false, fmt.Errorf("failed to retrieve manifest %s: %w", imageRef, err)
 	}
@@ -332,13 +340,13 @@ func IsMultiArchManifestList(ctx context.Context, imageRef string, pullSecret []
 		return false, nil
 	}
 
-	deserializedManifestList := new(manifestlist.DeserializedManifestList)
-	if err = deserializedManifestList.UnmarshalJSON(payload); err != nil {
+	manifestList, err := deserializeFunc(payload)
+	if err != nil {
 		return false, fmt.Errorf("failed to get unmarshalled manifest list: %w", err)
 	}
 
 	count := 0
-	for _, arch := range deserializedManifestList.ManifestList.Manifests {
+	for _, arch := range manifestList.ManifestList.Manifests {
 		switch arch.Platform.Architecture {
 		case ArchitectureAMD64, ArchitectureS390X, ArchitecturePPC64LE, ArchitectureARM64:
 			count = count + 1
@@ -351,9 +359,18 @@ func IsMultiArchManifestList(ctx context.Context, imageRef string, pullSecret []
 	return false, nil
 }
 
+func deserializeManifest(b []byte) (*manifestlist.DeserializedManifestList, error) {
+	deserializedManifestList := new(manifestlist.DeserializedManifestList)
+	if err := deserializedManifestList.UnmarshalJSON(b); err != nil {
+		return nil, fmt.Errorf("failed to get unmarshalled manifest list: %w", err)
+	}
+
+	return deserializedManifestList, nil
+}
+
 // findImageRefByArch finds the appropriate image reference in a multi-arch manifest image based on the current platform's OS and processor architecture
-func findImageRefByArch(ctx context.Context, imageRef string, pullSecret []byte, osToFind string, archToFind string) (manifestImageRef string, err error) {
-	manifestList, err := GetManifest(ctx, imageRef, pullSecret)
+func findImageRefByArch(ctx context.Context, imageRef string, pullSecret []byte, osToFind string, archToFind string, imageMetadataPorvider ManifestProvider) (manifestImageRef string, err error) {
+	manifestList, err := imageMetadataPorvider.GetManifest(ctx, imageRef, pullSecret)
 	if err != nil {
 		return "", fmt.Errorf("failed to retrieve manifest from image ref, %s: %w", imageRef, err)
 	}
@@ -422,10 +439,10 @@ func findMatchingManifest(ctx context.Context, imageRef string, deserializedMani
 
 // GetCorrectArchImage returns the appropriate image related to the system os/arch if the image reference is manifest
 // listed, else returns the original image reference
-func GetCorrectArchImage(ctx context.Context, component string, imageRef string, pullSecret []byte) (manifestImageRef string, err error) {
+func GetCorrectArchImage(ctx context.Context, component string, imageRef string, pullSecret []byte, imageMetadataProvider ManifestProvider) (manifestImageRef string, err error) {
 	log := ctrl.LoggerFrom(ctx)
 
-	isMultiArchImage, err := IsMultiArchManifestList(ctx, imageRef, pullSecret)
+	isMultiArchImage, err := IsMultiArchManifestList(ctx, imageRef, pullSecret, imageMetadataProvider)
 	if err != nil {
 		return "", fmt.Errorf("failed to determine if image is manifest listed: %w", err)
 	}
@@ -436,7 +453,7 @@ func GetCorrectArchImage(ctx context.Context, component string, imageRef string,
 		log.Info(component + " image is a manifest listed image; extracting manifest for os/arch: " + operatingSystem + "/" + arch)
 
 		// Verify MF Image has the right os/arch image
-		imageRef, err = findImageRefByArch(ctx, imageRef, pullSecret, operatingSystem, arch)
+		imageRef, err = findImageRefByArch(ctx, imageRef, pullSecret, operatingSystem, arch, imageMetadataProvider)
 		if err != nil {
 			return "", fmt.Errorf("failed to extract appropriate os/arch manifest from %s: %w", imageRef, err)
 		}

diff --git a/support/releaseinfo/registryclient/client_test.go b/support/releaseinfo/registryclient/client_test.go
@@ -2,6 +2,7 @@ package registryclient
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	"github.com/docker/distribution"
@@ -10,12 +11,38 @@ import (
 )
 
 const (
-	ReleaseImage1     = "quay.io/openshift-release-dev/ocp-release@sha256:1a101ef5215da468cea8bd2eb47114e85b2b64a6b230d5882f845701f55d057f"
-	ReleaseImage2     = "quay.io/openshift-release-dev/ocp-release:4.11.0-0.nightly-multi-2022-07-12-131716"
-	ManifestMediaType = "application/vnd.docker.distribution.manifest.v2+json"
-	LinuxOS           = "linux"
+	ReleaseImage1         = "quay.io/openshift-release-dev/ocp-release@sha256:1a101ef5215da468cea8bd2eb47114e85b2b64a6b230d5882f845701f55d057f"
+	ReleaseImage2         = "quay.io/openshift-release-dev/ocp-release:4.11.0-0.nightly-multi-2022-07-12-131716"
+	ManifestMediaType     = "application/vnd.docker.distribution.manifest.v2+json"
+	ManifestListMediaType = "application/vnd.docker.distribution.manifest.list.v2+json"
+	ImageIndexMediaType   = "application/vnd.oci.image.index.v1+json"
+	LinuxOS               = "linux"
 )
 
+type fakeMetadataProvider interface {
+	GetManifest(ctx context.Context, imageRef string, pullSecret []byte) (distribution.Manifest, error)
+}
+
+type fakeRegistryClientImageMetadataProvider struct {
+	mediaType string
+}
+type fakeManifest struct {
+	mediaType string
+}
+
+func (f *fakeRegistryClientImageMetadataProvider) GetManifest(ctx context.Context, imageRef string, pullSecret []byte) (distribution.Manifest, error) {
+	_, _, err := GetRepoSetup(ctx, imageRef, pullSecret)
+	if err != nil {
+		return nil, fmt.Errorf("failed to retrieve manifest %s: %w", imageRef, err)
+	}
+	return &fakeManifest{
+		f.mediaType,
+	}, nil
+}
+
+func (f *fakeManifest) References() []distribution.Descriptor { return []distribution.Descriptor{} }
+func (f *fakeManifest) Payload() (string, []byte, error)      { return f.mediaType, []byte{}, nil }
+
 func TestFindMatchingManifest(t *testing.T) {
 	deserializedManifestList1 := &manifestlist.DeserializedManifestList{
 		ManifestList: manifestlist.ManifestList{
@@ -198,48 +225,121 @@ func TestFindMatchingManifest(t *testing.T) {
 
 func TestIsMultiArchManifestList(t *testing.T) {
 	pullSecretBytes := []byte("{\"auths\":{\"quay.io\":{\"auth\":\"\",\"email\":\"\"}}}")
-
 	testCases := []struct {
 		name                   string
 		image                  string
 		pullSecretBytes        []byte
+		mediaType              string
+		manifests              []manifestlist.ManifestDescriptor
 		expectedMultiArchImage bool
 		expectErr              bool
 	}{
 		{
 			name:                   "Check an amd64 image; no err",
 			image:                  "quay.io/openshift-release-dev/ocp-release:4.16.10-x86_64",
+			mediaType:              ManifestMediaType,
 			pullSecretBytes:        pullSecretBytes,
 			expectedMultiArchImage: false,
 			expectErr:              false,
+			manifests: []manifestlist.ManifestDescriptor{
+				{
+					Descriptor: distribution.Descriptor{
+						MediaType: ManifestMediaType,
+						Digest:    "sha256:70fb4524d21e1b6c08477eb5d1ca2cf282b3270b1d008f70dd7e1cf13d8ba4ce",
+					},
+					Platform: manifestlist.PlatformSpec{
+						Architecture: ArchitectureAMD64,
+						OS:           LinuxOS,
+					},
+				},
+			},
 		},
 		{
 			name:                   "Check a ppc64le image; no err",
 			image:                  "quay.io/openshift-release-dev/ocp-release:4.16.11-ppc64le",
+			mediaType:              ManifestMediaType,
 			pullSecretBytes:        pullSecretBytes,
 			expectedMultiArchImage: false,
 			expectErr:              false,
+			manifests: []manifestlist.ManifestDescriptor{
+				{
+					Descriptor: distribution.Descriptor{
+						MediaType: ManifestMediaType,
+						Digest:    "sha256:70fb4524d21e1b6c08477eb5d1ca2cf282b3270b1d008f70dd7e1cf13d8ba4ce",
+					},
+					Platform: manifestlist.PlatformSpec{
+						Architecture: ArchitecturePPC64LE,
+						OS:           LinuxOS,
+					},
+				},
+			},
 		},
 		{
 			name:                   "Check a multi-arch image; no err",
 			image:                  "quay.io/openshift-release-dev/ocp-release:4.16.11-multi",
+			mediaType:              ManifestListMediaType,
 			pullSecretBytes:        pullSecretBytes,
 			expectedMultiArchImage: true,
 			expectErr:              false,
+			manifests: []manifestlist.ManifestDescriptor{
+				{
+					Descriptor: distribution.Descriptor{
+						MediaType: ManifestListMediaType,
+						Digest:    "sha256:70fb4524d21e1b6c08477eb5d1ca2cf282b3270b1d008f70dd7e1cf13d8ba4ce",
+					},
+					Platform: manifestlist.PlatformSpec{
+						Architecture: ArchitectureAMD64,
+						OS:           LinuxOS,
+					},
+				},
+				{
+					Descriptor: distribution.Descriptor{
+						MediaType: ManifestListMediaType,
+						Digest:    "sha256:70fb4524d21e1b6c08477eb5d1ca2cf282b3270b1d008f70dd7e1cf13d8ba4ce",
+					},
+					Platform: manifestlist.PlatformSpec{
+						Architecture: ArchitecturePPC64LE,
+						OS:           LinuxOS,
+					},
+				},
+			},
 		},
 		{
 			name:                   "Bad pull secret; err",
 			image:                  "quay.io/openshift-release-dev/ocp-release:4.16.11-ppc64le",
+			mediaType:              ManifestMediaType,
 			pullSecretBytes:        []byte(""),
 			expectedMultiArchImage: false,
 			expectErr:              true,
+			manifests: []manifestlist.ManifestDescriptor{
+				{
+					Descriptor: distribution.Descriptor{
+						MediaType: ManifestMediaType,
+						Digest:    "sha256:70fb4524d21e1b6c08477eb5d1ca2cf282b3270b1d008f70dd7e1cf13d8ba4ce",
+					},
+					Platform: manifestlist.PlatformSpec{
+						Architecture: ArchitecturePPC64LE,
+						OS:           LinuxOS,
+					},
+				},
+			},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			g := NewWithT(t)
-			isMultiArchImage, err := IsMultiArchManifestList(context.TODO(), tc.image, tc.pullSecretBytes)
+			imageMetadataProvider := &fakeRegistryClientImageMetadataProvider{
+				mediaType: tc.mediaType,
+			}
+			deserializeFunc = func(payload []byte) (*manifestlist.DeserializedManifestList, error) {
+				return &manifestlist.DeserializedManifestList{
+					ManifestList: manifestlist.ManifestList{
+						Manifests: tc.manifests,
+					},
+				}, nil
+			}
+			isMultiArchImage, err := IsMultiArchManifestList(context.TODO(), tc.image, tc.pullSecretBytes, imageMetadataProvider)
 			if tc.expectErr {
 				g.Expect(err).To(HaveOccurred())
 			} else {