OTA-1010: extract manifests with new capabilities

The `ManifestInclusionConfiguration` determines if a manifest is
included on a cluster. Its `Capabilities` field takes the implicitly
enabled capabilities into account.

This change removes the workaround that handles the net-new capabilities
introduced by a cluster upgrade. E.g. if a cluster is currently with
4.13, then it assumes that the capabilities "build", "deploymentConfig",
and "ImageRegistry" are enabled. This is because the components
underlying those capabilities are installed by default on 4.13, or
earlier and cannot be disabled once installed. Those capabilities will
become enabled after upgrade from 4.13 to 4.14: either explicitly or
implicitly depending on the current value of
`cv.spec.capabilities.baselineCapabilitySet`.

https://github.com/openshift/oc/blob/e005223acd7c478bac070134c16f5533a258be12/pkg/cli/admin/release/extract_tools.go#L1241-L1252

CVO has already defined the function `GetImplicitlyEnabledCapabilities`
to calculate the implicitly enabled capabilities of a cluster after a
cluster upgrade. For this function to work, we have to provide

* the manifests that are currently included on the cluster, and
* the manifests from the payload in the upgrade image.

The existing `ManifestReceiver` is enhanced in a way that it can provide
enabled capabilities, including both explicit and implicit ones, when
the callback to downstream is called. It is implemented by adding a
cache to collect manifests from the upstream and calls downstream only
when all manifests are collected and the capabilities are calculated
with them using the function `GetImplicitlyEnabledCapabilities` that is
mentioned earlier.

This enhancement can be opted in by setting up the
`needEnabledCapabilities` field of `ManifestReceiver`. Otherwise, its
behaviours stays the same as before.

In case that the inclusion configuration is taken from the cluster,
i.e., `--install-config` is not set, `needEnabledCapabilities` is set to
`true`.

Author: hongkailiu

pkg/cli/admin/release/extract.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"path"
 	"path/filepath"
 	"sync"
 	"time"
@@ -17,6 +18,7 @@ import (
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/yaml"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -25,7 +27,9 @@ import (
 	kcmdutil "k8s.io/kubectl/pkg/cmd/util"
 	"k8s.io/kubectl/pkg/util/templates"
 
+	configv1 "github.com/openshift/api/config/v1"
 	imagev1 "github.com/openshift/api/image/v1"
+	configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	"github.com/openshift/library-go/pkg/image/dockerv1client"
 	"github.com/openshift/library-go/pkg/manifest"
 	"github.com/openshift/oc/pkg/cli/image/extract"
@@ -371,17 +375,23 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 		versionInImageConfig = v
 	}
 
-	manifestReceiver := ManifestReceiver{skipNames: sets.New[string]("image-references", "release-metadata")}
+	needEnabledCapabilities := o.ExtractManifests && o.Included && o.InstallConfig == ""
+	var inclusionConfig manifestInclusionConfiguration
+	manifestReceiver := ManifestReceiver{skipNames: sets.New[string]("image-references", "release-metadata"),
+		needEnabledCapabilities: needEnabledCapabilities}
 	// o.ExtractManifests implies o.File == ""
 	if o.ExtractManifests {
 		expectedProviderSpecKind := credRequestCloudProviderSpecKindMapping[o.Cloud]
-
-		include := func(m *manifest.Manifest) error { return nil } // default to including everything
 		if o.Included {
 			context := "connected cluster"
-			inclusionConfig := manifestInclusionConfiguration{}
 			if o.InstallConfig == "" {
 				inclusionConfig, err = findClusterIncludeConfig(ctx, o.RESTConfig, versionInImageConfig)
+				currentPayloadManifests, err := currentPayloadManifests(ctx, opts, o.RESTConfig, inclusionConfig)
+				if err != nil {
+					err = fmt.Errorf("failed to get the current payload manifests: %w", err)
+				} else {
+					manifestReceiver.currentPayloadManifests = currentPayloadManifests
+				}
 			} else {
 				inclusionConfig, err = findClusterIncludeConfigFromInstallConfig(ctx, o.InstallConfig, versionInImageConfig)
 				context = o.InstallConfig
@@ -398,10 +408,10 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 					return fmt.Errorf("unrecognized platform for CredentialsRequests: %q", *inclusionConfig.Platform)
 				}
 			}
-			include = newIncluder(inclusionConfig)
+			manifestReceiver.inclusionConfiguration = inclusionConfig
 		}
 
-		manifestReceiver.manifestsCallback = func(filename string, ms []manifest.Manifest, r io.Reader) (bool, error) {
+		manifestReceiver.manifestsCallback = func(filename string, ms []manifest.Manifest, r io.Reader, enabled []configv1.ClusterVersionCapability) (bool, error) {
 			if filename == "image-references" && !o.CredentialsRequests {
 				buf := &bytes.Buffer{}
 				if _, err := io.Copy(buf, r); err != nil {
@@ -445,12 +455,20 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 				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())
-				} else if err := include(&ms[i]); err != nil {
-					klog.V(4).Infof("Excluding %s: %s", ms[i].String(), err)
-					ms = append(ms[:i], ms[i+1:]...)
+			if o.Included {
+				for i := len(ms) - 1; i >= 0; i-- {
+					if 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())
+					} else {
+						clusterVersionCapabilitiesStatus := &configv1.ClusterVersionCapabilitiesStatus{
+							KnownCapabilities:   sets.New[configv1.ClusterVersionCapability](append(inclusionConfig.Capabilities.KnownCapabilities, configv1.KnownClusterVersionCapabilities...)...).UnsortedList(),
+							EnabledCapabilities: sets.New[configv1.ClusterVersionCapability](append(inclusionConfig.Capabilities.EnabledCapabilities, enabled...)...).UnsortedList(),
+						}
+						if err := ms[i].Include(inclusionConfig.ExcludeIdentifier, inclusionConfig.RequiredFeatureSet, inclusionConfig.Profile, clusterVersionCapabilitiesStatus, inclusionConfig.Overrides); err != nil {
+							klog.V(4).Infof("Excluding %s: %s", ms[i].String(), err)
+							ms = append(ms[:i], ms[i+1:]...)
+						}
+					}
 				}
 			}
 
@@ -501,6 +519,7 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 			return true, nil
 		}
 		opts.TarEntryCallback = manifestReceiver.TarEntryCallback
+		opts.TarEntryCallbackDoneCallback = manifestReceiver.TarEntryCallbackDoneCallback
 	}
 
 	fileFound := false
@@ -550,6 +569,70 @@ func (o *ExtractOptions) Run(ctx context.Context) error {
 
 }
 
+func currentPayloadManifests(ctx context.Context, opts *extract.ExtractOptions, config *rest.Config, inclusionConfig manifestInclusionConfiguration) ([]manifest.Manifest, error) {
+	var currentPayloadManifests []manifest.Manifest
+	optsToGetCurrentPayloadManifests, err := getOptsToGetCurrentPayloadManifests(ctx, opts, config)
+	if err != nil {
+		return nil, fmt.Errorf("error getting opts to get current payload manifests: %w", err)
+	}
+	optsToGetCurrentPayloadManifests.TarEntryCallback = func(h *tar.Header, _ extract.LayerInfo, r io.Reader) (cont bool, err error) {
+		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 in the current release payload", h.Name)
+		ms, err := manifest.ParseManifests(r)
+		if err != nil {
+			return false, err
+		}
+		for i := len(ms) - 1; i >= 0; i-- {
+			if err := ms[i].Include(inclusionConfig.ExcludeIdentifier, inclusionConfig.RequiredFeatureSet, inclusionConfig.Profile, inclusionConfig.Capabilities, inclusionConfig.Overrides); err != nil {
+				klog.V(4).Infof("Excluding %s in the current release payload: %s", ms[i].String(), err)
+				ms = append(ms[:i], ms[i+1:]...)
+			}
+		}
+		currentPayloadManifests = append(currentPayloadManifests, ms...)
+		return true, nil
+	}
+	if err := optsToGetCurrentPayloadManifests.Run(); err != nil {
+		return nil, fmt.Errorf("error getting current payload manifests: %w", err)
+	}
+	return currentPayloadManifests, nil
+}
+
+func getOptsToGetCurrentPayloadManifests(ctx context.Context, source *extract.ExtractOptions, config *rest.Config) (*extract.ExtractOptions, error) {
+	client, err := configv1client.NewForConfig(config)
+	if err != nil {
+		return nil, err
+	}
+
+	clusterVersion, err := client.ClusterVersions().Get(ctx, "version", metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	src := clusterVersion.Status.Desired.Image
+	ref, err := imagesource.ParseReference(src)
+	if err != nil {
+		return nil, err
+	}
+	klog.V(4).Infof("The outgoing release payload from %s is running on the cluster: %s", src, config.Host)
+	opts := extract.NewExtractOptions(genericiooptions.IOStreams{Out: source.Out, ErrOut: source.ErrOut})
+	opts.ParallelOptions = source.ParallelOptions
+	opts.SecurityOptions = source.SecurityOptions
+	opts.FilterOptions = source.FilterOptions
+	opts.FileDir = source.FileDir
+	opts.OnlyFiles = true
+	opts.ICSPFile = source.ICSPFile
+	opts.IDMSFile = source.IDMSFile
+	opts.Mappings = []extract.Mapping{
+		{
+			ImageRef: ref,
+			From:     "release-manifests/",
+		},
+	}
+	return opts, nil
+}
+
 func (o *ExtractOptions) extractGit(dir string) error {
 	switch o.Output {
 	case "commit", "":

pkg/cli/admin/release/extract_tools.go

@@ -15,7 +15,6 @@ import (
 	"os"
 	"path"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"sort"
 	"strings"
@@ -28,6 +27,7 @@ import (
 	terminal "golang.org/x/term"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/cli-runtime/pkg/genericiooptions"
 	appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1"
@@ -1248,20 +1248,17 @@ func findClusterIncludeConfig(ctx context.Context, restConfig *rest.Config, vers
 		return config, err
 	} else {
 		config.Overrides = clusterVersion.Spec.Overrides
-		config.Capabilities = &clusterVersion.Status.Capabilities
-
-		// FIXME: eventually pull in GetImplicitlyEnabledCapabilities from https://github.com/openshift/cluster-version-operator/blob/86e24d66119a73f50282b66a8d6f2e3518aa0e15/pkg/payload/payload.go#L237-L240 for cases where a minor update would implicitly enable some additional capabilities.  For now, 4.13 to 4.14 will always enable MachineAPI, ImageRegistry, etc..
-		currentVersion := clusterVersion.Status.Desired.Version
-		matches := regexp.MustCompile(`^(\d+[.]\d+)[.].*`).FindStringSubmatch(currentVersion)
-		if len(matches) < 2 {
-			return config, fmt.Errorf("failed to parse major.minor version from ClusterVersion status.desired.version %q", currentVersion)
-		} else if matches[1] == "4.13" {
-			build := configv1.ClusterVersionCapability("Build")
-			deploymentConfig := configv1.ClusterVersionCapability("DeploymentConfig")
-			imageRegistry := configv1.ClusterVersionCapability("ImageRegistry")
-			config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry)
-			config.Capabilities.KnownCapabilities = append(config.Capabilities.KnownCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry)
+
+		// known and baseline may grow from the current cluster version to the oc version
+		capSet := configv1.ClusterVersionCapabilitySetCurrent
+		if capabilitiesSpec := clusterVersion.Spec.Capabilities; capabilitiesSpec != nil &&
+			len(capabilitiesSpec.BaselineCapabilitySet) > 0 {
+			capSet = capabilitiesSpec.BaselineCapabilitySet
 		}
+		deepCopy := clusterVersion.Status.Capabilities.DeepCopy()
+		deepCopy.EnabledCapabilities = append(deepCopy.EnabledCapabilities, configv1.ClusterVersionCapabilitySets[capSet]...)
+		deepCopy.KnownCapabilities = configv1.KnownClusterVersionCapabilities
+		config.Capabilities = deepCopy
 
 		if err := logCapabilitySetMayDiffer(clusterVersion.Spec.Capabilities, versionInImageConfig); err != nil {
 			return config, err
@@ -1308,16 +1305,36 @@ func newIncluder(config manifestInclusionConfiguration) includer {
 // * 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.
+// If needEnabledCapabilities is not set, manifestsCallback with the argument
+// enabledCapabilities always nil is called right after each TarEntryCallback call from the upstream.
+// Otherwise, manifestsCallback is called only after TarEntryCallbackDoneCallback is invoked by the upstream. By then,
+// all the manifests have been handled. Those manifests, as the payloads of a release to update a cluster, together with
+// the given currentPayloadManifests and manifestInclusionConfiguration, are used to calculate the enabled capabilities
+// after the update. Those enabled capabilities is passed to manifestsCallback.
 type ManifestReceiver struct {
-	manifestsCallback func(filename string, manifests []manifest.Manifest, reader io.Reader) (cont bool, err error)
-	skipNames         sets.Set[string]
+	manifestsCallback       func(filename string, manifests []manifest.Manifest, reader io.Reader, enabledCapabilities []configv1.ClusterVersionCapability) (cont bool, err error)
+	skipNames               sets.Set[string]
+	needEnabledCapabilities bool
+	currentPayloadManifests []manifest.Manifest
+	inclusionConfiguration  manifestInclusionConfiguration
+
+	cache           []cacheData
+	manifests       []manifest.Manifest
+	enabledResolved bool
+	enabled         []configv1.ClusterVersionCapability
 
 	ManifestErrs []error
 }
 
+type cacheData struct {
+	name   string
+	ms     []manifest.Manifest
+	reader io.Reader
+}
+
 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)
+		return mr.manifestsCallback(h.Name, nil, r, nil)
 	}
 
 	if ext := path.Ext(h.Name); len(ext) == 0 || !(ext == ".yaml" || ext == ".yml" || ext == ".json") {
@@ -1329,5 +1346,107 @@ func (mr *ManifestReceiver) TarEntryCallback(h *tar.Header, _ extract.LayerInfo,
 		mr.ManifestErrs = append(mr.ManifestErrs, errors.Wrapf(err, "error parsing %s", h.Name))
 		return true, nil
 	}
-	return mr.manifestsCallback(h.Name, ms, nil)
+	mr.manifests = append(mr.manifests, ms...)
+	if mr.needEnabledCapabilities {
+		mr.cache = append(mr.cache, cacheData{name: h.Name, ms: ms})
+		return true, nil
+	}
+
+	return mr.manifestsCallback(h.Name, ms, nil, nil)
+}
+
+func (mr *ManifestReceiver) TarEntryCallbackDoneCallback() error {
+	defer func() {
+		mr.cache = []cacheData{}
+	}()
+
+	if mr.needEnabledCapabilities && !mr.enabledResolved {
+		enabled := GetImplicitlyEnabledCapabilities(
+			mr.manifests,
+			mr.currentPayloadManifests,
+			mr.inclusionConfiguration,
+			sets.New[configv1.ClusterVersionCapability](),
+		)
+
+		delta := enabled.Clone()
+		if mr.inclusionConfiguration.Capabilities != nil {
+			delta = delta.Difference(sets.New[configv1.ClusterVersionCapability](mr.inclusionConfiguration.Capabilities.EnabledCapabilities...))
+			enabled.Insert(mr.inclusionConfiguration.Capabilities.EnabledCapabilities...)
+		}
+		if delta.Len() > 0 {
+			klog.Infof("Those capabilities become implicitly enabled for the incoming release %s", sets.List(delta))
+		}
+
+		mr.enabled = enabled.UnsortedList()
+		mr.enabledResolved = true
+	}
+
+	for _, c := range mr.cache {
+		cont, err := mr.manifestsCallback(c.name, c.ms, c.reader, mr.enabled)
+		if err != nil {
+			mr.ManifestErrs = append(mr.ManifestErrs, errors.Wrapf(err, "error parsing %s", c.name))
+		}
+		if !cont {
+			break
+		}
+	}
+	return kerrors.NewAggregate(mr.ManifestErrs)
+}
+
+// GetImplicitlyEnabledCapabilities returns a set of capabilities that are implicitly enabled after a cluster update.
+// The arguments are two sets of manifests, manifest inclusion configuration, and
+// a set of capabilities that are implicitly enabled on the cluster, i.e., the capabilities
+// that are NOT specified in the cluster version but has to considered enabled on the cluster.
+// The manifest inclusion configuration is used to determine if a manifest should be included.
+// In other words, whether, or not the cluster version operator reconcile that manifest on the cluster.
+// The two sets of manifests are respectively from the release that is currently running on the cluster and
+// from the release that the cluster is updated to.
+// TODO lift this function to library-go
+func GetImplicitlyEnabledCapabilities(
+	updatePayloadManifests []manifest.Manifest,
+	currentPayloadManifests []manifest.Manifest,
+	manifestInclusionConfiguration manifestInclusionConfiguration,
+	currentImplicitlyEnabled sets.Set[configv1.ClusterVersionCapability],
+) sets.Set[configv1.ClusterVersionCapability] {
+	ret := currentImplicitlyEnabled.Clone()
+	for _, updateManifest := range updatePayloadManifests {
+		updateManErr := updateManifest.IncludeAllowUnknownCapabilities(
+			manifestInclusionConfiguration.ExcludeIdentifier,
+			manifestInclusionConfiguration.RequiredFeatureSet,
+			manifestInclusionConfiguration.Profile,
+			manifestInclusionConfiguration.Capabilities,
+			manifestInclusionConfiguration.Overrides,
+			true,
+		)
+		// update manifest is enabled, no need to check
+		if updateManErr == nil {
+			continue
+		}
+		for _, currentManifest := range currentPayloadManifests {
+			if !updateManifest.SameResourceID(currentManifest) {
+				continue
+			}
+			// current manifest is disabled, no need to check
+			if err := currentManifest.IncludeAllowUnknownCapabilities(
+				manifestInclusionConfiguration.ExcludeIdentifier,
+				manifestInclusionConfiguration.RequiredFeatureSet,
+				manifestInclusionConfiguration.Profile,
+				manifestInclusionConfiguration.Capabilities,
+				manifestInclusionConfiguration.Overrides,
+				true,
+			); err != nil {
+				continue
+			}
+			newImplicitlyEnabled := sets.New[configv1.ClusterVersionCapability](updateManifest.GetManifestCapabilities()...).
+				Difference(sets.New[configv1.ClusterVersionCapability](currentManifest.GetManifestCapabilities()...)).
+				Difference(currentImplicitlyEnabled).
+				Difference(sets.New[configv1.ClusterVersionCapability](manifestInclusionConfiguration.Capabilities.EnabledCapabilities...))
+			ret = ret.Union(newImplicitlyEnabled)
+			if newImplicitlyEnabled.Len() > 0 {
+				klog.V(2).Infof("%s has changed and is now part of one or more disabled capabilities. The following capabilities will be implicitly enabled: %s",
+					updateManifest.String(), sets.List(newImplicitlyEnabled))
+			}
+		}
+	}
+	return ret
 }

pkg/cli/image/extract/extract.go

@@ -150,6 +150,9 @@ type ExtractOptions struct {
 	// by name and only the entry in the highest layer will be passed to the callback. Returning false
 	// will halt processing of the image.
 	TarEntryCallback TarEntryFunc
+	// TarEntryCallbackDoneCallback, if set, is called when all layers image have been handled, i.e., no more
+	// TarEntryCallback is going to be passed. It has no effect if TarEntryCallback is not set.
+	TarEntryCallbackDoneCallback func() error
 	// AllLayers ensures the TarEntryCallback is invoked for all files, and will cause the callback
 	// order to start at the lowest layer and work outwards.
 	AllLayers bool
@@ -549,6 +552,12 @@ func (o *ExtractOptions) Run() error {
 					}
 				}
 
+				if o.TarEntryCallback != nil && o.TarEntryCallbackDoneCallback != nil {
+					if err := o.TarEntryCallbackDoneCallback(); err != nil {
+						return err
+					}
+				}
+
 				if o.ImageMetadataCallback != nil {
 					o.ImageMetadataCallback(&mapping, location.Manifest, contentDigest, imageConfig, location.ManifestListDigest())
 				}