Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 82 additions & 7 deletions pkg/manifest/manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"os"
"path/filepath"
"reflect"
"strconv"
"strings"

"k8s.io/apimachinery/pkg/util/sets"
Expand All @@ -23,10 +24,11 @@ import (
)

const (
CapabilityAnnotation = "capability.openshift.io/name"
DefaultClusterProfile = "self-managed-high-availability"
featureSetAnnotation = "release.openshift.io/feature-set"
featureGateAnnotation = "release.openshift.io/feature-gate"
CapabilityAnnotation = "capability.openshift.io/name"
DefaultClusterProfile = "self-managed-high-availability"
featureSetAnnotation = "release.openshift.io/feature-set"
featureGateAnnotation = "release.openshift.io/feature-gate"
majorVersionAnnotation = "release.openshift.io/major-version"
)

var knownFeatureSets = sets.Set[string]{}
Expand Down Expand Up @@ -232,12 +234,69 @@ func checkFeatureGates(enabledGates sets.Set[string], annotations map[string]str
return nil
}

// checkMajorVersion validates if manifest should be included based on major version requirements
func checkMajorVersion(gvk schema.GroupVersionKind, clusterMajorVersion uint64, annotations map[string]string) error {
if annotations == nil {
return nil // No annotations, include by default
}
majorVersionRequirements, ok := annotations[majorVersionAnnotation]
if !ok {
return nil // No requirements, include by default
}

if !isFeatureGate(gvk) && !isCustomResource(gvk) {
// Has a requirement, but is not of the expected kind
return fmt.Errorf("major version filtering is only supported for feature gates and custom resources")
}

requirements := strings.Split(majorVersionRequirements, ",")
includedVersions := sets.New[uint64]()
excludedVersions := sets.New[uint64]()

for _, req := range requirements {
req = strings.TrimSpace(req)
if req == "" {
continue
}

if strings.HasPrefix(req, "-") {
excludedVersionStr := req[1:]
excludedVersion, err := strconv.ParseUint(excludedVersionStr, 10, 64)
if err != nil {
return fmt.Errorf("invalid excluded major version %q in annotation: %v", excludedVersionStr, err)
}
excludedVersions.Insert(excludedVersion)
} else {
includedVersionStr := req
includedVersion, err := strconv.ParseUint(includedVersionStr, 10, 64)
if err != nil {
return fmt.Errorf("invalid included major version %q in annotation: %v", includedVersionStr, err)
}
includedVersions.Insert(includedVersion)
}
}

switch {
case includedVersions.Intersection(excludedVersions).Len() > 0:
return fmt.Errorf("inclusion and exclusion requirements overlap: %v", includedVersions.Intersection(excludedVersions).UnsortedList())
case includedVersions.Has(clusterMajorVersion):
return nil // Found matching version, include this manifest
case excludedVersions.Has(clusterMajorVersion):
return fmt.Errorf("major version %d matches excluded version %d", clusterMajorVersion, clusterMajorVersion)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the second variable in the error string should be excludedVersions.

I got logs:

2026-03-19T03:56:10.602708782Z I0319 03:56:10.602688       1 payload.go:217] excluding Filename: "0000_03_version-4-5-6-false.yaml" Group: "apiextensions.k8s.io" Kind: "CustomResourceDefinition" Name: "version-4-5-6.test.io": major version 4 matches excluded version 4

@JoelSpeed please help condifrm.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is actually correct. From the case condition we know that the clusterMajorVersion is in the excludedVersions set, so even if we tried to change this to print out the excluded versions, it would just show us a list of excluded versions rather than a specific excluded version that matched.

The error message that is printed out is always correct, the 4 here was an excluded version

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

um, it looks strange, but anyway from the message we can know the version is excluded

case len(includedVersions) > 0:
return fmt.Errorf("major version %d does not match any required versions", clusterMajorVersion)
default:
// No positive requirement and did not match any excluded versions
return nil
}
}

// Include returns an error if the manifest fails an inclusion filter and should be excluded from further
// processing by cluster version operator. Pointer arguments can be set nil to avoid excluding based on that
// filter. For example, setting profile non-nil and capabilities nil will return an error if the manifest's
// profile does not match, but will never return an error about capability issues.
func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string, profile *string, capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, enabledFeatureGates sets.Set[string]) error {
return m.IncludeAllowUnknownCapabilities(excludeIdentifier, requiredFeatureSet, profile, capabilities, overrides, enabledFeatureGates, false)
func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string, profile *string, capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, enabledFeatureGates sets.Set[string], majorVersion *uint64) error {
return m.IncludeAllowUnknownCapabilities(excludeIdentifier, requiredFeatureSet, profile, capabilities, overrides, enabledFeatureGates, majorVersion, false)
}

// IncludeAllowUnknownCapabilities returns an error if the manifest fails an inclusion filter and should be excluded from
Expand All @@ -247,7 +306,7 @@ func (m *Manifest) Include(excludeIdentifier *string, requiredFeatureSet *string
// to capabilities filtering. When set to true a manifest will not be excluded simply because it contains an unknown
// capability. This is necessary to allow updates to an OCP version containing newly defined capabilities.
func (m *Manifest) IncludeAllowUnknownCapabilities(excludeIdentifier *string, requiredFeatureSet *string, profile *string,
capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, enabledFeatureGates sets.Set[string], allowUnknownCapabilities bool) error {
capabilities *configv1.ClusterVersionCapabilitiesStatus, overrides []configv1.ComponentOverride, enabledFeatureGates sets.Set[string], majorVersion *uint64, allowUnknownCapabilities bool) error {

annotations := m.Obj.GetAnnotations()
if annotations == nil {
Expand Down Expand Up @@ -282,6 +341,14 @@ func (m *Manifest) IncludeAllowUnknownCapabilities(excludeIdentifier *string, re
}
}

// Major version filtering
if majorVersion != nil {
err := checkMajorVersion(m.GVK, *majorVersion, annotations)
if err != nil {
return err
}
}

if profile != nil {
profileAnnotation := fmt.Sprintf("include.release.openshift.io/%s", *profile)
if val, ok := annotations[profileAnnotation]; ok && val != "true" {
Expand Down Expand Up @@ -481,3 +548,11 @@ func addIfNotDuplicateResource(manifest Manifest, resourceIds map[resourceId]boo
}
return fmt.Errorf("duplicate resource: (%s)", manifest.id)
}

func isFeatureGate(gvk schema.GroupVersionKind) bool {
return gvk.Group == "config.openshift.io" && gvk.Kind == "FeatureGate"
}

func isCustomResource(gvk schema.GroupVersionKind) bool {
return gvk.Group == "apiextensions.k8s.io" && gvk.Kind == "CustomResourceDefinition"
}
Loading