Skip to content

🐛 (fix) Priority field to use pointer type with correct markers #2413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
camilamacedo86 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🐛 (fix) Priority field to use pointer type with correct markers #2413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions api/v1/clustercatalog_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,11 @@ type ClusterCatalogSpec struct {
// The highest possible value is 2147483647.
//
// +kubebuilder:default:=0
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

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

The +kubebuilder:default:=0 marker combined with the pointer type *int32 creates a contradiction. When this default is applied by the API server, the Go code will receive a non-nil pointer to 0 whether the user explicitly set priority to 0 or omitted it entirely. This defeats the purpose of using a pointer type, which is typically to distinguish between "not set" (nil) and "set to zero value" (non-nil pointer to 0). Consider removing the default marker if the goal is to detect user-specified vs omitted values, or changing back to a non-pointer type if a default value should always be present.

Copilot uses AI. Check for mistakes.
// +kubebuilder:validation:minimum:=-2147483648
// +kubebuilder:validation:maximum:=2147483647
Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Dec 24, 2025

Choose a reason for hiding this comment

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

The above ones are invalid.
Therefore, we are fixing the validation here.

// +kubebuilder:validation:Minimum:=-2147483648
// +kubebuilder:validation:Maximum:=2147483647
// +kubebuilder:validation:Format:=int32
// +optional
Priority int32 `json:"priority"`
Priority *int32 `json:"priority,omitempty"`
Copy link
Contributor Author

@camilamacedo86 camilamacedo86 Dec 24, 2025

Choose a reason for hiding this comment

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

  • Pointer = distinguish between "not set" (nil) vs "set to 0"
  • omitempty = don't show field in YAML when not set (cleaner)
  • kubebuilder default = API server fills in 0 when user omits it


// availabilityMode is an optional field that defines how the ClusterCatalog is made available to clients on the cluster.
//
Expand Down
5 changes: 5 additions & 0 deletions api/v1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion docs/api-reference/olmv1-api-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ _Appears in:_
| Field | Description | Default | Validation |
| --- | --- | --- | --- |
| `source` _[CatalogSource](#catalogsource)_ | source is a required field that defines the source of a catalog.<br />A catalog contains information on content that can be installed on a cluster.<br />The catalog source makes catalog contents discoverable and usable by other on-cluster components.<br />These components can present the content in a GUI dashboard or install content from the catalog on the cluster.<br />The catalog source must contain catalog metadata in the File-Based Catalog (FBC) format.<br />For more information on FBC, see https://olm.operatorframework.io/docs/reference/file-based-catalogs/#docs.<br />Below is a minimal example of a ClusterCatalogSpec that sources a catalog from an image:<br /> source:<br /> type: Image<br /> image:<br /> ref: quay.io/operatorhubio/catalog:latest | | Required: \{\} <br /> |
| `priority` _integer_ | priority is an optional field that defines a priority for this ClusterCatalog.<br />Clients use the ClusterCatalog priority as a tie-breaker between ClusterCatalogs that meet their requirements.<br />Higher numbers mean higher priority.<br />Clients decide how to handle scenarios where multiple ClusterCatalogs with the same priority meet their requirements.<br />Clients should prompt users for additional input to break the tie.<br />When omitted, the default priority is 0.<br />Use negative numbers to specify a priority lower than the default.<br />Use positive numbers to specify a priority higher than the default.<br />The lowest possible value is -2147483648.<br />The highest possible value is 2147483647. | 0 | |
| `priority` _integer_ | priority is an optional field that defines a priority for this ClusterCatalog.<br />Clients use the ClusterCatalog priority as a tie-breaker between ClusterCatalogs that meet their requirements.<br />Higher numbers mean higher priority.<br />Clients decide how to handle scenarios where multiple ClusterCatalogs with the same priority meet their requirements.<br />Clients should prompt users for additional input to break the tie.<br />When omitted, the default priority is 0.<br />Use negative numbers to specify a priority lower than the default.<br />Use positive numbers to specify a priority higher than the default.<br />The lowest possible value is -2147483648.<br />The highest possible value is 2147483647. | 0 | Format: int32 <br />Maximum: 2.147483647e+09 <br />Minimum: -2.147483648e+09 <br /> |
| `availabilityMode` _[AvailabilityMode](#availabilitymode)_ | availabilityMode is an optional field that defines how the ClusterCatalog is made available to clients on the cluster.<br />Allowed values are "Available", "Unavailable", or omitted.<br />When omitted, the default value is "Available".<br />When set to "Available", the catalog contents are unpacked and served over the catalog content HTTP server.<br />Clients should consider this ClusterCatalog and its contents as usable.<br />When set to "Unavailable", the catalog contents are no longer served over the catalog content HTTP server.<br />Treat this the same as if the ClusterCatalog does not exist.<br />Use "Unavailable" when you want to keep the ClusterCatalog but treat it as if it doesn't exist. | Available | Enum: [Unavailable Available] <br /> |


Expand Down
2 changes: 2 additions & 0 deletions helm/olmv1/base/catalogd/crd/experimental/olm.operatorframework.io_clustercatalogs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ spec:
The lowest possible value is -2147483648.
The highest possible value is 2147483647.
format: int32
maximum: 2147483647
minimum: -2147483648
type: integer
source:
description: |-
Expand Down
2 changes: 2 additions & 0 deletions helm/olmv1/base/catalogd/crd/standard/olm.operatorframework.io_clustercatalogs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ spec:
The lowest possible value is -2147483648.
The highest possible value is 2147483647.
format: int32
maximum: 2147483647
minimum: -2147483648
type: integer
source:
description: |-
Expand Down
6 changes: 5 additions & 1 deletion internal/operator-controller/resolve/catalog.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,11 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio
}
// The current bundle shares deprecation status with prior bundles or
// there are no prior bundles. Add it to the list.
resolvedBundles = append(resolvedBundles, foundBundle{&thisBundle, cat.GetName(), cat.Spec.Priority})
priority := int32(0)
if cat.Spec.Priority != nil {
priority = *cat.Spec.Priority
}
resolvedBundles = append(resolvedBundles, foundBundle{&thisBundle, cat.GetName(), priority})
priorDeprecation = thisDeprecation
return nil
}, listOptions...); err != nil {
Expand Down
14 changes: 7 additions & 7 deletions internal/operator-controller/resolve/catalog_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -833,7 +833,7 @@ func TestUnequalPriority(t *testing.T) {
genBundle(pkgName, "1.0.0"),
},
Deprecations: []declcfg.Deprecation{},
}, &ocv1.ClusterCatalogSpec{Priority: 1}, nil
}, &ocv1.ClusterCatalogSpec{Priority: ptr.To[int32](1)}, nil
},
"b": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) {
return &declcfg.DeclarativeConfig{
Expand All @@ -847,7 +847,7 @@ func TestUnequalPriority(t *testing.T) {
genBundle(pkgName, "1.1.0"),
},
Deprecations: []declcfg.Deprecation{},
}, &ocv1.ClusterCatalogSpec{Priority: 0}, nil
}, &ocv1.ClusterCatalogSpec{Priority: ptr.To[int32](0)}, nil
},
}
r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs}
Expand All @@ -862,13 +862,13 @@ func TestMultiplePriority(t *testing.T) {
pkgName := randPkg()
w := staticCatalogWalker{
"a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) {
return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 1}, nil
return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: ptr.To[int32](1)}, nil
},
"b": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) {
return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 0}, nil
return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: ptr.To[int32](0)}, nil
},
"c": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) {
return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 1}, nil
return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: ptr.To[int32](1)}, nil
},
}
r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs}
Expand Down Expand Up @@ -944,7 +944,7 @@ func TestSomeCatalogsDisabled(t *testing.T) {
Name: "enabledCatalog",
},
Spec: ocv1.ClusterCatalogSpec{
Priority: 1, // Higher priority
Priority: ptr.To[int32](1), // Higher priority
AvailabilityMode: ocv1.AvailabilityModeAvailable,
},
},
Expand All @@ -953,7 +953,7 @@ func TestSomeCatalogsDisabled(t *testing.T) {
Name: "disabledCatalog",
},
Spec: ocv1.ClusterCatalogSpec{
Priority: 0, // Lower priority (but disabled)
Priority: ptr.To[int32](0), // Lower priority (but disabled)
AvailabilityMode: ocv1.AvailabilityModeUnavailable,
},
},
Expand Down
2 changes: 2 additions & 0 deletions manifests/experimental-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ spec:
The lowest possible value is -2147483648.
The highest possible value is 2147483647.
format: int32
maximum: 2147483647
minimum: -2147483648
type: integer
source:
description: |-
Expand Down
2 changes: 2 additions & 0 deletions manifests/experimental.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ spec:
The lowest possible value is -2147483648.
The highest possible value is 2147483647.
format: int32
maximum: 2147483647
minimum: -2147483648
type: integer
source:
description: |-
Expand Down
2 changes: 2 additions & 0 deletions manifests/standard-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ spec:
The lowest possible value is -2147483648.
The highest possible value is 2147483647.
format: int32
maximum: 2147483647
minimum: -2147483648
type: integer
source:
description: |-
Expand Down
2 changes: 2 additions & 0 deletions manifests/standard.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ spec:
The lowest possible value is -2147483648.
The highest possible value is 2147483647.
format: int32
maximum: 2147483647
minimum: -2147483648
type: integer
source:
description: |-
Expand Down
Loading