From 4cd2c52720eca9cee916bc663bb7a1c0a45f61fc Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Fri, 20 Jun 2025 10:53:43 -0400 Subject: [PATCH 1/3] Isolate manifest handling in release-extraction 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 is 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) +} From bd40eabdcf9dac8f55b8878a0f0a4c17cbe12135 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Mon, 23 Jun 2025 10:15:20 -0400 Subject: [PATCH 2/3] NO-JIRA: Include the target version in the logging Before this full, the logging was only for the case that `findClusterIncludeConfigFromInstallConfig` is called, i.e., the path from an install-config file is provided. This pull extends it to the case where the configuration is taken from the current cluster. Another change from the pull is that the logging messages include the target version that is determined by inspecting the release image. The implementation for this is adding a new callback `ImageConfigCallback`. --- pkg/cli/admin/release/extract.go | 26 ++++++++++++++++++-- pkg/cli/admin/release/extract_tools.go | 33 +++++++++++++++++++------- pkg/cli/image/extract/extract.go | 5 ++++ 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/pkg/cli/admin/release/extract.go b/pkg/cli/admin/release/extract.go index eccf11dec1..93e531e358 100644 --- a/pkg/cli/admin/release/extract.go +++ b/pkg/cli/admin/release/extract.go @@ -349,6 +349,28 @@ func (o *ExtractOptions) Run(ctx context.Context) error { } } + // versionInImageConfig stores the version from the label of the image + // At the moment it is used later only for the logging purpose and thus is not blocking if failures occur upon getting its value + var versionInImageConfig string + opts.ImageConfigCallback = func(imageConfig *dockerv1client.DockerImageConfig) { + if imageConfig == nil { + // This should never happen + klog.Error("Cannot retrieve the version because no image configuration is provided in the image to extract") + return + } + if imageConfig.Config == nil { + klog.Error("Cannot retrieve the version from image configuration in the image to extract because it has no configuration") + return + } + v, ok := imageConfig.Config.Labels["io.openshift.release"] + if !ok { + klog.Error("Cannot retrieve the version from image configuration in the image to extract because it does not have the required label 'io.openshift.release'") + return + } + klog.V(2).Infof("Retrieved the version from image configuration in the image to extract: %s", v) + versionInImageConfig = v + } + manifestReceiver := ManifestReceiver{skipNames: sets.New[string]("image-references", "release-metadata")} // o.ExtractManifests implies o.File == "" if o.ExtractManifests { @@ -359,9 +381,9 @@ func (o *ExtractOptions) Run(ctx context.Context) error { context := "connected cluster" inclusionConfig := manifestInclusionConfiguration{} if o.InstallConfig == "" { - inclusionConfig, err = findClusterIncludeConfig(ctx, o.RESTConfig) + inclusionConfig, err = findClusterIncludeConfig(ctx, o.RESTConfig, versionInImageConfig) } else { - inclusionConfig, err = findClusterIncludeConfigFromInstallConfig(ctx, o.InstallConfig) + inclusionConfig, err = findClusterIncludeConfigFromInstallConfig(ctx, o.InstallConfig, versionInImageConfig) context = o.InstallConfig } if err != nil { diff --git a/pkg/cli/admin/release/extract_tools.go b/pkg/cli/admin/release/extract_tools.go index 624698dd74..2912d42a0a 100644 --- a/pkg/cli/admin/release/extract_tools.go +++ b/pkg/cli/admin/release/extract_tools.go @@ -1168,16 +1168,29 @@ func copyAndReplace(errorOutput io.Writer, w io.Writer, r io.Reader, bufferSize } -func findClusterIncludeConfigFromInstallConfig(ctx context.Context, installConfigPath string) (manifestInclusionConfiguration, error) { - config := manifestInclusionConfiguration{} - +// logCapabilitySetMayDiffer logs the messages if the oc-cli version is not the same as the target version +// * the enabled capability set may differ if the baseline is vCurrent, and +// * the known capability set may differ +// It raises an error if it fails to determine the oc-cli version. +func logCapabilitySetMayDiffer(capabilitiesSpec *configv1.ClusterVersionCapabilitiesSpec, targetVersion string) error { clientVersion, reportedVersion, err := version.ExtractVersion() if err != nil { - return config, err + return fmt.Errorf("failed to determine the version of 'oc': %w", err) } if reportedVersion == "" { reportedVersion = clientVersion.String() } + if reportedVersion != targetVersion { + if capabilitiesSpec != nil && capabilitiesSpec.BaselineCapabilitySet == configv1.ClusterVersionCapabilitySetCurrent { + klog.Infof("The eventual cluster %s will not be the same minor version as this %s 'oc', the actual %s capability set may differ.", targetVersion, reportedVersion, capabilitiesSpec.BaselineCapabilitySet) + } + klog.Infof("The eventual cluster %s will not be the same minor version as this %s 'oc', the known capability set may differ.", targetVersion, reportedVersion) + } + return nil +} + +func findClusterIncludeConfigFromInstallConfig(ctx context.Context, installConfigPath string, versionInImageConfig string) (manifestInclusionConfiguration, error) { + config := manifestInclusionConfiguration{} installConfigBytes, err := os.ReadFile(installConfigPath) if err != nil { @@ -1205,21 +1218,19 @@ func findClusterIncludeConfigFromInstallConfig(ctx context.Context, installConfi if enabled, ok := configv1.ClusterVersionCapabilitySets[data.Capabilities.BaselineCapabilitySet]; !ok { return config, fmt.Errorf("unrecognized baselineCapabilitySet %q", data.Capabilities.BaselineCapabilitySet) } else { - if data.Capabilities.BaselineCapabilitySet == configv1.ClusterVersionCapabilitySetCurrent { - klog.Infof("If the eventual cluster will not be the same minor version as this %s 'oc', the actual %s capability set may differ.", reportedVersion, data.Capabilities.BaselineCapabilitySet) + if err := logCapabilitySetMayDiffer(data.Capabilities, versionInImageConfig); err != nil { + return config, err } config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, enabled...) } config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, data.Capabilities.AdditionalEnabledCapabilities...) - - klog.Infof("If the eventual cluster will not be the same minor version as this %s 'oc', the known capability sets may differ.", reportedVersion) config.Capabilities.KnownCapabilities = configv1.KnownClusterVersionCapabilities } return config, nil } -func findClusterIncludeConfig(ctx context.Context, restConfig *rest.Config) (manifestInclusionConfiguration, error) { +func findClusterIncludeConfig(ctx context.Context, restConfig *rest.Config, versionInImageConfig string) (manifestInclusionConfiguration, error) { config := manifestInclusionConfiguration{} client, err := configv1client.NewForConfig(restConfig) @@ -1251,6 +1262,10 @@ func findClusterIncludeConfig(ctx context.Context, restConfig *rest.Config) (man config.Capabilities.EnabledCapabilities = append(config.Capabilities.EnabledCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry) config.Capabilities.KnownCapabilities = append(config.Capabilities.KnownCapabilities, configv1.ClusterVersionCapabilityMachineAPI, build, deploymentConfig, imageRegistry) } + + if err := logCapabilitySetMayDiffer(clusterVersion.Spec.Capabilities, versionInImageConfig); err != nil { + return config, err + } } if infrastructure, err := client.Infrastructures().Get(ctx, "cluster", metav1.GetOptions{}); err != nil { diff --git a/pkg/cli/image/extract/extract.go b/pkg/cli/image/extract/extract.go index ae4ac5042d..b1e9d3c745 100644 --- a/pkg/cli/image/extract/extract.go +++ b/pkg/cli/image/extract/extract.go @@ -141,6 +141,8 @@ type ExtractOptions struct { genericiooptions.IOStreams + // ImageConfigCallback is invoked once image config retrieved + ImageConfigCallback func(imageConfig *dockerv1client.DockerImageConfig) // ImageMetadataCallback is invoked once per image retrieved, and may be called in parallel if // MaxPerRegistry is set higher than 1. ImageMetadataCallback ImageMetadataFunc @@ -421,6 +423,9 @@ func (o *ExtractOptions) Run() error { if err != nil { return fmt.Errorf("unable to parse image %s: %v", from, err) } + if o.ImageConfigCallback != nil { + o.ImageConfigCallback(imageConfig) + } if mapping.ConditionFn != nil { ok, err := mapping.ConditionFn(&mapping, location.Manifest, imageConfig) From 91537c7c3df467c60e8ce3f03ef8019d806afcb6 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Wed, 25 Jun 2025 12:14:31 -0400 Subject: [PATCH 3/3] 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`. --- pkg/cli/admin/release/extract.go | 107 +++++++++++++++-- pkg/cli/admin/release/extract_tools.go | 155 ++++++++++++++++++++++--- pkg/cli/image/extract/extract.go | 9 ++ 3 files changed, 241 insertions(+), 30 deletions(-) diff --git a/pkg/cli/admin/release/extract.go b/pkg/cli/admin/release/extract.go index 93e531e358..f4aaf5f3c7 100644 --- a/pkg/cli/admin/release/extract.go +++ b/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", "": diff --git a/pkg/cli/admin/release/extract_tools.go b/pkg/cli/admin/release/extract_tools.go index 2912d42a0a..c275ad1c90 100644 --- a/pkg/cli/admin/release/extract_tools.go +++ b/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 } diff --git a/pkg/cli/image/extract/extract.go b/pkg/cli/image/extract/extract.go index b1e9d3c745..9e31c6e6c9 100644 --- a/pkg/cli/image/extract/extract.go +++ b/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()) }