From 99441515d4292aa766f386ce79fadef2413f9764 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Fri, 20 Jun 2025 10:53:43 -0400 Subject: [PATCH] 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. --- pkg/cli/admin/release/extract.go | 34 +++++++++------------- pkg/cli/admin/release/extract_tools.go | 40 ++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/pkg/cli/admin/release/extract.go b/pkg/cli/admin/release/extract.go index a3c0a471cc..eccf11dec1 100644 --- a/pkg/cli/admin/release/extract.go +++ b/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) diff --git a/pkg/cli/admin/release/extract_tools.go b/pkg/cli/admin/release/extract_tools.go index a1d12aa966..624698dd74 100644 --- a/pkg/cli/admin/release/extract_tools.go +++ b/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) +}