Skip to content

ListenerSet adjust PortNumber kubebuilder validations #3750

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.

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
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions apis/v1/gateway_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ type Listener struct {
// same port, subject to the Listener compatibility rules.
//
// Support: Core
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
Port PortNumber `json:"port"`

// Protocol specifies the network protocol this listener expects to receive.
Expand Down
3 changes: 3 additions & 0 deletions apis/v1/httproute_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,9 @@ type HTTPRequestRedirectFilter struct {
// Support: Extended
//
// +optional
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
Port *PortNumber `json:"port,omitempty"`

// StatusCode is the HTTP status code to be used in response.
Expand Down
2 changes: 2 additions & 0 deletions apis/v1/object_reference_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ type BackendObjectReference struct {
// resource or this field.
//
// +optional
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
Port *PortNumber `json:"port,omitempty"`
}

Expand Down
6 changes: 3 additions & 3 deletions apis/v1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ type ParentReference struct {
// Support: Extended
//
// +optional
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
Port *PortNumber `json:"port,omitempty"`
}

Expand Down Expand Up @@ -227,9 +230,6 @@ type CommonRouteSpec struct {
}

// PortNumber defines a network port.
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
type PortNumber int32

// BackendRef defines how a Route should forward a request to a Kubernetes
Expand Down
3 changes: 0 additions & 3 deletions apis/v1alpha2/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ type ParentReference = v1.ParentReference
type CommonRouteSpec = v1.CommonRouteSpec

// PortNumber defines a network port.
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
type PortNumber = v1.PortNumber

// BackendRef defines how a Route should forward a request to a Kubernetes
Expand Down
3 changes: 0 additions & 3 deletions apis/v1beta1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ type ParentReference = v1.ParentReference
type CommonRouteSpec = v1.CommonRouteSpec

// PortNumber defines a network port.
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
type PortNumber = v1.PortNumber

// BackendRef defines how a Route should forward a request to a Kubernetes
Expand Down
16 changes: 15 additions & 1 deletion apisx/v1alpha1/xlistenerset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,18 @@ type ListenerEntry struct {

// Port is the network port. Multiple listeners may use the
// same port, subject to the Listener compatibility rules.
Port PortNumber `json:"port"`
//
// If the port is not set or specified as zero, the implementation will assign
// a unique port. If the implementation does not support dynamic port
// assignment, it MUST set `Accepted` condition to `False` with the
// `UnsupportedPort` reason.
//
// +optional
//
// +kubebuilder:default=0
Copy link
Contributor

Choose a reason for hiding this comment

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

If the default is now 0, is this not a backward compatible change once it goes to standard? Formerly the port was optional, so there could be widespread omitted-port cases that were already properly handled. Now there could be a completely different way that missing ports are handled. Do we need to worry about that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the default is now 0, is this not a backward compatible change once it goes to standard?

I don't think so the ListenerSet is an alpha API so I believe we can make these changes.

Formerly the port was optional

It wasn't actually - maybe that's the confusion here

Copy link
Contributor

Choose a reason for hiding this comment

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

This is only non-breaking because we're going from required to optional. But as @dprotaso says, it would be okay to make a breaking change because this is alpha.

// +kubebuilder:validation:Minimum=0
// +kubebuilder:validation:Maximum=65535
Port PortNumber `json:"port,omitempty"`

// Protocol specifies the network protocol this listener expects to receive.
Protocol ProtocolType `json:"protocol"`
Expand Down Expand Up @@ -233,6 +244,9 @@ type ListenerEntryStatus struct {
Name SectionName `json:"name"`

// Port is the network port the listener is configured to listen on.
//
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=65535
Port PortNumber `json:"port"`

// SupportedKinds is the list indicating the Kinds supported by this
Expand Down
1 change: 0 additions & 1 deletion applyconfiguration/internal/internal.go

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

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

17 changes: 16 additions & 1 deletion geps/gep-1713/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,19 @@ type ListenerEntry struct {

// Port is the network port. Multiple listeners may use the
// same port, subject to the Listener compatibility rules.
//
//
// If the port is not set or specified as zero, the implementation will assign
// a unique port. If the implementation does not support dynamic port
// assignment, it MUST set `Accepted` condition to `False` with the
// `UnsupportedPort` reason.
//
// Support: Core
//
// +optional
//
// +kubebuilder:default=0
// +kubebuilder:validation:Minimum=0
// +kubebuilder:validation:Maximum=65535
Port PortNumber `json:"port,omitempty"`

// Protocol specifies the network protocol this listener expects to receive.
Expand Down Expand Up @@ -380,6 +391,10 @@ spec:

`ListenerEntry` is currently a copy of the `Listener` struct with some changes noted in the below sections

#### Port

`Port` is now optional to allow for dynamic port assignment. If the port is unspecified or set to zero, the implementation will assign a unique port. If the implementation does not support dynamic port assignment, it MUST set `Accepted` condition to `False` with the `UnsupportedPort` reason.

## Semantics

### Gateway Changes
Expand Down
5 changes: 2 additions & 3 deletions pkg/generated/openapi/zz_generated.openapi.go

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

2 changes: 2 additions & 0 deletions pkg/generator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ func main() {
log.Fatalf("failed to register markers: %s", err)
}

registerMarkerOverrides(parser.Collector.Registry)

crd.AddKnownTypes(parser)
for _, r := range roots {
parser.NeedPackage(r)
Expand Down
66 changes: 66 additions & 0 deletions pkg/generator/markers.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
Copyright 2025 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package main

import (
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"sigs.k8s.io/controller-tools/pkg/markers"
)

type Minimum float64

func (m Minimum) Value() float64 {
return float64(m)
}

//nolint:unparam
func (m Minimum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
val := m.Value()
schema.Minimum = &val
return nil
}

type Maximum float64

func (m Maximum) Value() float64 {
return float64(m)
}

//nolint:unparam
func (m Maximum) ApplyToSchema(schema *apiext.JSONSchemaProps) error {
val := m.Value()
schema.Maximum = &val
return nil
}

// kubebuilder Min Max markers are broken with type aliases
func registerMarkerOverrides(into *markers.Registry) {
minMarker, _ := markers.MakeDefinition(
"kubebuilder:validation:Minimum",
markers.DescribesField,
Minimum(0),
)

maxMarker, _ := markers.MakeDefinition(
"kubebuilder:validation:Maximum",
markers.DescribesField,
Maximum(0),
)

into.Register(minMarker) //nolint:errcheck
into.Register(maxMarker) //nolint:errcheck
}