OTA-1010: extract included manifests with net-new capabilities

The `ManifestInclusionConfiguration` is used to
determine is 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 that calculates
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.
* 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 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` 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())
 				}