Isolate manifest handling in release-extraction

This pull adds a new type `ManifestReceiver` that works as a middleware
between the upstream `extract.ExtractOptions`'s `TarEntryCallback` and
the downstream `manifestsCallback`.

There are two files `image-references` and `release-metadata` that are
handled differently from manifest files. When those files come, their
readers from the upstream are sent to the downstream callback right
away.

Other files contain manifests. They are parsed out and then sent to the
downstream. We will embed more changes into this part, e.g., collecting
all manifests in the image and then use them to calculate the enabled
capabilities which are sent as an argument to the downstream callback.
Those changes are coming in other pulls.

Author: hongkailiu

pkg/cli/admin/release/extract.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"path"
 	"path/filepath"
 	"sync"
 	"time"
@@ -20,6 +19,7 @@ import (
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/cli-runtime/pkg/genericiooptions"
 	"k8s.io/client-go/rest"
 	kcmdutil "k8s.io/kubectl/pkg/cmd/util"
@@ -349,7 +349,7 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 		}
 	}
 
-	var manifestErrs []error
+	manifestReceiver := ManifestReceiver{skipNames: sets.New[string]("image-references", "release-metadata")}
 	// o.ExtractManifests implies o.File == ""
 	if o.ExtractManifests {
 		expectedProviderSpecKind := credRequestCloudProviderSpecKindMapping[o.Cloud]
@@ -379,8 +379,8 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 			include = newIncluder(inclusionConfig)
 		}
 
-		opts.TarEntryCallback = func(hdr *tar.Header, _ extract.LayerInfo, r io.Reader) (bool, error) {
-			if hdr.Name == "image-references" && !o.CredentialsRequests {
+		manifestReceiver.manifestsCallback = func(filename string, ms []manifest.Manifest, r io.Reader) (bool, error) {
+			if filename == "image-references" && !o.CredentialsRequests {
 				buf := &bytes.Buffer{}
 				if _, err := io.Copy(buf, r); err != nil {
 					return false, fmt.Errorf("unable to load image-references from release payload: %w", err)
@@ -398,7 +398,7 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 
 				out := o.Out
 				if o.Directory != "" {
-					out, err = os.Create(filepath.Join(o.Directory, hdr.Name))
+					out, err = os.Create(filepath.Join(o.Directory, filename))
 					if err != nil {
 						return false, err
 					}
@@ -408,10 +408,10 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 					return true, err
 				}
 				return true, nil
-			} else if hdr.Name == "release-metadata" && !o.CredentialsRequests {
+			} else if filename == "release-metadata" && !o.CredentialsRequests {
 				out := o.Out
 				if o.Directory != "" {
-					out, err = os.Create(filepath.Join(o.Directory, hdr.Name))
+					out, err = os.Create(filepath.Join(o.Directory, filename))
 					if err != nil {
 						return false, err
 					}
@@ -423,16 +423,6 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 				return true, nil
 			}
 
-			if ext := path.Ext(hdr.Name); len(ext) == 0 || !(ext == ".yaml" || ext == ".yml" || ext == ".json") {
-				return true, nil
-			}
-			klog.V(4).Infof("Found manifest %s", hdr.Name)
-			ms, err := manifest.ParseManifests(r)
-			if err != nil {
-				manifestErrs = append(manifestErrs, errors.Wrapf(err, "error parsing %s", hdr.Name))
-				return true, nil
-			}
-
 			for i := len(ms) - 1; i >= 0; i-- {
 				if o.Included && o.CredentialsRequests && ms[i].GVK == credentialsRequestGVK && len(ms[i].Obj.GetAnnotations()) == 0 {
 					klog.V(4).Infof("Including %s for manual CredentialsRequests, despite lack of annotations", ms[i].String())
@@ -469,25 +459,26 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 
 			out := o.Out
 			if o.Directory != "" {
-				out, err = os.Create(filepath.Join(o.Directory, hdr.Name))
+				out, err = os.Create(filepath.Join(o.Directory, filename))
 				if err != nil {
-					return false, errors.Wrapf(err, "error creating manifest in %s", hdr.Name)
+					return false, errors.Wrapf(err, "error creating manifest in %s", filename)
 				}
 			}
 			if out != nil {
 				for _, m := range manifestsToWrite {
 					yamlBytes, err := yaml.JSONToYAML(m.Raw)
 					if err != nil {
-						return false, errors.Wrapf(err, "error serializing manifest in %s", hdr.Name)
+						return false, errors.Wrapf(err, "error serializing manifest in %s", filename)
 					}
 					fmt.Fprintf(out, "---\n")
 					if _, err := out.Write(yamlBytes); err != nil {
-						return false, errors.Wrapf(err, "error writing manifest in %s", hdr.Name)
+						return false, errors.Wrapf(err, "error writing manifest in %s", filename)
 					}
 				}
 			}
 			return true, nil
 		}
+		opts.TarEntryCallback = manifestReceiver.TarEntryCallback
 	}
 
 	fileFound := false
@@ -506,6 +497,7 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 		return err
 	}
 
+	manifestErrs := manifestReceiver.ManifestErrs
 	if metadataVerifyMsg != "" {
 		if o.File == "" && o.Out != nil {
 			fmt.Fprintf(o.Out, "%s\n", metadataVerifyMsg)

pkg/cli/admin/release/extract_tools.go

@@ -13,6 +13,7 @@ import (
 	"hash"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
 	"regexp"
 	"runtime"
@@ -21,25 +22,25 @@ import (
 	"sync"
 	"syscall"
 
-	"k8s.io/utils/ptr"
-
+	"github.com/MakeNowJust/heredoc"
+	"github.com/pkg/errors"
 	"golang.org/x/crypto/openpgp"
 	terminal "golang.org/x/term"
 
-	"k8s.io/klog/v2"
-
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/cli-runtime/pkg/genericiooptions"
 	appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1"
 	"k8s.io/client-go/rest"
+	"k8s.io/klog/v2"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/yaml"
 
-	"github.com/MakeNowJust/heredoc"
 	configv1 "github.com/openshift/api/config/v1"
 	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	imagereference "github.com/openshift/library-go/pkg/image/reference"
 	"github.com/openshift/library-go/pkg/manifest"
+
 	"github.com/openshift/oc/pkg/cli/admin/internal/codesign"
 	"github.com/openshift/oc/pkg/cli/image/extract"
 	"github.com/openshift/oc/pkg/cli/image/imagesource"
@@ -1286,3 +1287,32 @@ func newIncluder(config manifestInclusionConfiguration) includer {
 		return m.Include(config.ExcludeIdentifier, config.RequiredFeatureSet, config.Profile, config.Capabilities, config.Overrides)
 	}
 }
+
+// ManifestReceiver has a TarEntryCallback function which can be used to as a callback to ExtractOptions.TarEntryCallback.
+// It feeds the downstream manifestsCallback
+// * with the manifests from every file whose name is not in skipNames, OR
+// * with the reader that contains the content of each file whose name is in skipNames.
+// All the errors encountered when parsing the manifests are collected in ManifestErrs.
+type ManifestReceiver struct {
+	manifestsCallback func(filename string, manifests []manifest.Manifest, reader io.Reader) (cont bool, err error)
+	skipNames         sets.Set[string]
+
+	ManifestErrs []error
+}
+
+func (mr *ManifestReceiver) TarEntryCallback(h *tar.Header, _ extract.LayerInfo, r io.Reader) (cont bool, err error) {
+	if mr.skipNames.Has(h.Name) {
+		return mr.manifestsCallback(h.Name, nil, r)
+	}
+
+	if ext := path.Ext(h.Name); len(ext) == 0 || !(ext == ".yaml" || ext == ".yml" || ext == ".json") {
+		return true, nil
+	}
+	klog.V(4).Infof("Found manifest %s", h.Name)
+	ms, err := manifest.ParseManifests(r)
+	if err != nil {
+		mr.ManifestErrs = append(mr.ManifestErrs, errors.Wrapf(err, "error parsing %s", h.Name))
+		return true, nil
+	}
+	return mr.manifestsCallback(h.Name, ms, nil)
+}