Skip to content

feat: make Gateway.Spec.Addresses.Value optional #3616

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

Merged
merged 10 commits into from
Mar 16, 2025
Merged

feat: make Gateway.Spec.Addresses.Value optional #3616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bcc53c1
feat: make `Gateway.Spec.Addresses.Value` optional
EyalPazz Feb 16, 2025
e3c5cb6
wip: test mock
EyalPazz Feb 19, 2025
9b4c8d1
feat: flag injection & change `GatewayAddress` -> `GatewaySpecAddress`
EyalPazz Feb 24, 2025
687de42
add `omitempty` to the value
EyalPazz Feb 27, 2025
cc7a71b
chore: update codegen
EyalPazz Mar 3, 2025
afb0490
fix: type validation and minLength
EyalPazz Mar 6, 2025
1d7df0f
chore: remove conformance test for now
EyalPazz Mar 6, 2025
35f890d
complete rename of `GatewayAddress` -> `GatewaySpecAddress`
EyalPazz Mar 6, 2025
eb95ca1
fix: improve docs for value field
EyalPazz Mar 15, 2025
d9c6ec1
fix: value field doc
EyalPazz Mar 15, 2025
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
17 changes: 10 additions & 7 deletions apis/v1/gateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ type GatewaySpec struct {
// +kubebuilder:validation:MaxItems=16
// +kubebuilder:validation:XValidation:message="IPAddress values must be unique",rule="self.all(a1, a1.type == 'IPAddress' ? self.exists_one(a2, a2.type == a1.type && a2.value == a1.value) : true )"
// +kubebuilder:validation:XValidation:message="Hostname values must be unique",rule="self.all(a1, a1.type == 'Hostname' ? self.exists_one(a2, a2.type == a1.type && a2.value == a1.value) : true )"
Addresses []GatewayAddress `json:"addresses,omitempty"`
Addresses []GatewaySpecAddress `json:"addresses,omitempty"`

// Infrastructure defines infrastructure level attributes about this Gateway instance.
//
Expand Down Expand Up @@ -724,24 +724,27 @@ type RouteGroupKind struct {
Kind Kind `json:"kind"`
}

// GatewayAddress describes an address that can be bound to a Gateway.
// GatewaySpecAddress describes an address that can be bound to a Gateway.
//
// +kubebuilder:validation:XValidation:message="Hostname value must only contain valid characters (matching ^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$)",rule="self.type == 'Hostname' ? self.value.matches(r\"\"\"^(\\*\\.)?[a-z0-9]([-a-z0-9]*[a-z0-9])?(\\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$\"\"\"): true"
type GatewayAddress struct {
type GatewaySpecAddress struct {
// Type of the address.
//
// +optional
// +kubebuilder:default=IPAddress
Type *AddressType `json:"type,omitempty"`

// Value of the address. The validity of the values will depend
// on the type and support by the controller.
// When a value is unspecified, an implementation SHOULD automatically
// assign an address matching the requested type if possible.
//
// If an implementation does not support an empty value, they MUST set the
// "Programmed" condition in status to False with a reason of "AddressNotAssigned".
//
// Examples: `1.2.3.4`, `128::1`, `my-ip-address`.
//
// +kubebuilder:validation:MinLength=1
// +optional
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs some more explanation:

  • what behavior is expected if an implementation supports an empty Value here?
  • what behavior is expected if an implementation does not support an empty Value here?

This should then guide you to adding some conformance tests for implementations to test against.

Copy link
Member Author

Choose a reason for hiding this comment

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

amazing thanks!

Copy link
Member

Choose a reason for hiding this comment

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

+1 to @youngnick's comments, a couple suggestions as a starting point:

  1. If an implementation does support an empty value, we expect them to assign an address matching the type that has been requested with a value of the implementation's choice.
  2. If an implementation does not support an empty value, we expect them to set the Programmed condition in status to False with a reason of AddressNotAssigned. Here's what that reason means as a reference:

gateway-api/apis/v1/gateway_types.go

Lines 874 to 886 in d936258

// This reason is used with the "Programmed" condition when the underlying
// implementation and network have yet to dynamically assign addresses for a
// Gateway.
//
// Some example situations where this reason can be used:
//
// * IPAM address exhaustion
// * Address not yet allocated
//
// When this reason is used the implementation SHOULD provide a clear
// message explaining the underlying problem, ideally with some hints as to
// what actions can be taken that might resolve the problem.
GatewayReasonAddressNotAssigned GatewayConditionReason = "AddressNotAssigned"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the extra doc requirement here got missed. I'd still like to see some description of what should happen in both cases on this field, if it's supported and not.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for catching this @youngnick! @EyalPazz any chance you can squeeze this in in the next day or two? Really hoping we can fit this into v1.3 if possible, and we'll need to cut a RC pretty soon.

// +kubebuilder:validation:MaxLength=253
Value string `json:"value"`
Value string `json:"value,omitempty"`
}

// GatewayStatusAddress describes a network address that is bound to a Gateway.
Expand Down
42 changes: 21 additions & 21 deletions apis/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 apis/v1beta1/gateway_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ type RouteGroupKind = v1.RouteGroupKind

// GatewayAddress describes an address that can be bound to a Gateway.
// +k8s:deepcopy-gen=false
type GatewayAddress = v1.GatewayAddress
type GatewaySpecAddress = v1.GatewaySpecAddress

// GatewayStatus defines the observed state of Gateway.
// +k8s:deepcopy-gen=false
Expand Down
4 changes: 2 additions & 2 deletions applyconfiguration/apis/v1/gatewayspec.go
View file
Open in desktop

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

52 changes: 52 additions & 0 deletions applyconfiguration/apis/v1/gatewayspecaddress.go
View file
Open in desktop

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

21 changes: 10 additions & 11 deletions applyconfiguration/internal/internal.go
View file
Open in desktop

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

4 changes: 2 additions & 2 deletions applyconfiguration/utils.go
View file
Open in desktop

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

28 changes: 14 additions & 14 deletions config/crd/experimental/gateway.networking.k8s.io_gateways.yaml
View file
Open in desktop

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

28 changes: 14 additions & 14 deletions config/crd/standard/gateway.networking.k8s.io_gateways.yaml
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 conformance/tests/gateway-static-addresses.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func extractStatusAddresses(addresses []v1.GatewayStatusAddress) []string {
// Private Helper Functions
// -----------------------------------------------------------------------------

func filterAddr(addrs []v1.GatewayAddress, filter v1.GatewayAddress) (newAddrs []v1.GatewayAddress) {
func filterAddr(addrs []v1.GatewaySpecAddress, filter v1.GatewaySpecAddress) (newAddrs []v1.GatewaySpecAddress) {
for _, addr := range addrs {
if addr.Value != filter.Value {
newAddrs = append(newAddrs, addr)
Expand Down
Loading