From fd6063146b370ba4ac2f5ba6b24630c02a3e59f7 Mon Sep 17 00:00:00 2001 From: omar Date: Mon, 8 Dec 2025 15:22:52 -0600 Subject: [PATCH 1/7] feat: set loadbalancerIP from Gateway.spec.addresses Signed-off-by: omar --- pkg/deployer/values.go | 1 + pkg/deployer/values_helpers.go | 27 +- pkg/kgateway/deployer/gateway_parameters.go | 42 ++- .../helm/agentgateway/templates/service.yaml | 3 + .../helm/envoy/templates/service.yaml | 3 + .../testdata/loadbalancer-static-ip-out.yaml | 338 ++++++++++++++++++ .../testdata/loadbalancer-static-ip.yaml | 26 ++ 7 files changed, 432 insertions(+), 8 deletions(-) create mode 100644 test/deployer/testdata/loadbalancer-static-ip-out.yaml create mode 100644 test/deployer/testdata/loadbalancer-static-ip.yaml diff --git a/pkg/deployer/values.go b/pkg/deployer/values.go index 797b5bf70d0..e6feaebcc72 100644 --- a/pkg/deployer/values.go +++ b/pkg/deployer/values.go @@ -113,6 +113,7 @@ type HelmImage struct { type HelmService struct { Type *string `json:"type,omitempty"` ClusterIP *string `json:"clusterIP,omitempty"` + LoadBalancerIP *string `json:"loadBalancerIP,omitempty"` ExtraAnnotations map[string]string `json:"extraAnnotations,omitempty"` ExtraLabels map[string]string `json:"extraLabels,omitempty"` ExternalTrafficPolicy *string `json:"externalTrafficPolicy,omitempty"` diff --git a/pkg/deployer/values_helpers.go b/pkg/deployer/values_helpers.go index f386aa0fd43..9533d025f6c 100644 --- a/pkg/deployer/values_helpers.go +++ b/pkg/deployer/values_helpers.go @@ -104,19 +104,32 @@ func AppendPortValue(gwPorts []HelmPort, port int32, name string, gwp *kgateway. } // Convert service values from GatewayParameters into helm values to be used by the deployer. -func GetServiceValues(svcConfig *kgateway.Service) *HelmService { +func GetServiceValues(svcConfig *kgateway.Service, loadBalancerIP *string) *HelmService { // convert the service type enum to its string representation; // if type is not set, it will default to 0 ("ClusterIP") var svcType *string - if svcConfig.GetType() != nil { - svcType = ptr.To(string(*svcConfig.GetType())) + var clusterIP *string + var extraAnnotations map[string]string + var extraLabels map[string]string + var externalTrafficPolicy *string + + if svcConfig != nil { + if svcConfig.GetType() != nil { + svcType = ptr.To(string(*svcConfig.GetType())) + } + clusterIP = svcConfig.GetClusterIP() + extraAnnotations = svcConfig.GetExtraAnnotations() + extraLabels = svcConfig.GetExtraLabels() + externalTrafficPolicy = svcConfig.GetExternalTrafficPolicy() } + return &HelmService{ Type: svcType, - ClusterIP: svcConfig.GetClusterIP(), - ExtraAnnotations: svcConfig.GetExtraAnnotations(), - ExtraLabels: svcConfig.GetExtraLabels(), - ExternalTrafficPolicy: svcConfig.GetExternalTrafficPolicy(), + ClusterIP: clusterIP, + LoadBalancerIP: loadBalancerIP, + ExtraAnnotations: extraAnnotations, + ExtraLabels: extraLabels, + ExternalTrafficPolicy: externalTrafficPolicy, } } diff --git a/pkg/kgateway/deployer/gateway_parameters.go b/pkg/kgateway/deployer/gateway_parameters.go index 617952d7237..454ebfb327b 100644 --- a/pkg/kgateway/deployer/gateway_parameters.go +++ b/pkg/kgateway/deployer/gateway_parameters.go @@ -5,11 +5,13 @@ import ( "errors" "fmt" "log/slog" + "net/netip" "os" "strings" "helm.sh/helm/v3/pkg/chart" "istio.io/istio/pkg/kube/kclient" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" "k8s.io/utils/ptr" @@ -268,6 +270,39 @@ func (k *kgatewayParameters) getGatewayParametersForGatewayClass(gwc *gwv1.Gatew return mergedGwp, nil } +// extractLoadBalancerIP extracts the first IP address from Gateway.spec.addresses +// where the address type is IPAddressType. Returns nil if no valid IP address is found. +// If multiple IP addresses are provided, uses the first one and logs a warning. +func extractLoadBalancerIP(gw *gwv1.Gateway) *string { + if len(gw.Spec.Addresses) == 0 { + return nil + } + + if len(gw.Spec.Addresses) > 1 { + slog.Warn("multiple IP addresses found in Gateway.spec.addresses, using first valid one", + "gateway", fmt.Sprintf("%s/%s", gw.Namespace, gw.Name), + "count", len(gw.Spec.Addresses), + ) + } + + for _, addr := range gw.Spec.Addresses { + // IPAddressType or nil (defaults to IPAddressType per Gateway API spec) + if addr.Type == nil || *addr.Type == gwv1.IPAddressType { + // Validate IP format + if parsedIP, err := netip.ParseAddr(addr.Value); err == nil && parsedIP.IsValid() { + return &addr.Value + } + // Log warning for invalid IP but continue searching + slog.Warn("invalid IP address in Gateway.spec.addresses, skipping", "value", addr.Value) + } + } + + slog.Error("no valid IP address found in Gateway.spec.addresses", + "gateway", fmt.Sprintf("%s/%s", gw.Namespace, gw.Name), + ) + return nil +} + func (k *kgatewayParameters) getValues(gw *gwv1.Gateway, gwParam *kgateway.GatewayParameters) (*deployer.HelmConfig, error) { irGW := deployer.GetGatewayIR(gw, k.inputs.CommonCollections) ports := deployer.GetPortsValues(irGW, gwParam, irGW.ControllerName == k.inputs.AgentgatewayControllerName) @@ -354,7 +389,12 @@ func (k *kgatewayParameters) getValues(gw *gwv1.Gateway, gwParam *kgateway.Gatew gateway.Strategy = deployConfig.GetStrategy() // service values - gateway.Service = deployer.GetServiceValues(svcConfig) + // Extract loadBalancerIP from Gateway.spec.addresses if service type is LoadBalancer + var loadBalancerIP *string + if svcConfig != nil && svcConfig.GetType() != nil && *svcConfig.GetType() == corev1.ServiceTypeLoadBalancer { + loadBalancerIP = extractLoadBalancerIP(gw) + } + gateway.Service = deployer.GetServiceValues(svcConfig, loadBalancerIP) // serviceaccount values gateway.ServiceAccount = deployer.GetServiceAccountValues(svcAccountConfig) // pod template values diff --git a/pkg/kgateway/helm/agentgateway/templates/service.yaml b/pkg/kgateway/helm/agentgateway/templates/service.yaml index 10e830fb4cb..2d790bf8255 100644 --- a/pkg/kgateway/helm/agentgateway/templates/service.yaml +++ b/pkg/kgateway/helm/agentgateway/templates/service.yaml @@ -21,6 +21,9 @@ spec: {{- with $gateway.service.clusterIP }} clusterIP: {{ . }} {{- end }} + {{- with $gateway.service.loadBalancerIP }} + loadBalancerIP: {{ . }} + {{- end }} ports: {{- range $p := $gateway.ports }} - name: {{ $p.name }} diff --git a/pkg/kgateway/helm/envoy/templates/service.yaml b/pkg/kgateway/helm/envoy/templates/service.yaml index 6f859d089cb..fd79847d432 100644 --- a/pkg/kgateway/helm/envoy/templates/service.yaml +++ b/pkg/kgateway/helm/envoy/templates/service.yaml @@ -21,6 +21,9 @@ spec: {{- with $gateway.service.clusterIP }} clusterIP: {{ . }} {{- end }} + {{- with $gateway.service.loadBalancerIP }} + loadBalancerIP: {{ . }} + {{- end }} ports: {{- range $p := $gateway.ports }} - name: {{ $p.name }} diff --git a/test/deployer/testdata/loadbalancer-static-ip-out.yaml b/test/deployer/testdata/loadbalancer-static-ip-out.yaml new file mode 100644 index 00000000000..8a230571f97 --- /dev/null +++ b/test/deployer/testdata/loadbalancer-static-ip-out.yaml @@ -0,0 +1,338 @@ +--- +# Source: kgateway-proxy/templates/serviceaccount.yaml +apiVersion: v1 +kind: ServiceAccount +metadata: + name: gw + labels: + app.kubernetes.io/instance: gw + app.kubernetes.io/managed-by: kgateway + app.kubernetes.io/name: gw + app.kubernetes.io/version: 1.0.0-ci1 + gateway.networking.k8s.io/gateway-class-name: kgateway + gateway.networking.k8s.io/gateway-name: gw + kgateway: kube-gateway +automountServiceAccountToken: false +--- +# Source: kgateway-proxy/templates/configmap.yaml +apiVersion: v1 +kind: ConfigMap +metadata: + name: gw + labels: + app.kubernetes.io/instance: gw + app.kubernetes.io/managed-by: kgateway + app.kubernetes.io/name: gw + app.kubernetes.io/version: 1.0.0-ci1 + gateway.networking.k8s.io/gateway-class-name: kgateway + gateway.networking.k8s.io/gateway-name: gw + kgateway: kube-gateway +data: + envoy.yaml: | + admin: + address: + socket_address: { address: 127.0.0.1, port_value: 19000 } + layered_runtime: + layers: + - name: static_layer + static_layer: + envoy.restart_features.use_eds_cache_for_ads: true + - name: admin_layer + admin_layer: {} + node: + cluster: gw.default + metadata: + role: kgateway-kube-gateway-api~default~gw + static_resources: + listeners: + - name: readiness_listener + address: + socket_address: { address: 0.0.0.0, port_value: 8082 } + filter_chains: + - filters: + - name: envoy.filters.network.http_connection_manager + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + stat_prefix: ingress_http + codec_type: AUTO + route_config: + name: main_route + virtual_hosts: + - name: local_service + domains: ["*"] + routes: + - match: + path: "/ready" + headers: + - name: ":method" + string_match: + exact: GET + route: + cluster: admin_port_cluster + http_filters: + - name: envoy.filters.http.health_check + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.health_check.v3.HealthCheck + pass_through_mode: false + headers: + - name: ":path" + string_match: + exact: "/envoy-hc" + - name: envoy.filters.http.router + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + - name: prometheus_listener + address: + socket_address: + address: 0.0.0.0 + port_value: 9091 + filter_chains: + - filters: + - name: envoy.filters.network.http_connection_manager + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + codec_type: AUTO + stat_prefix: prometheus + route_config: + name: prometheus_route + virtual_hosts: + - name: prometheus_host + domains: + - "*" + routes: + - match: + path: "/ready" + headers: + - name: ":method" + string_match: + exact: GET + route: + cluster: admin_port_cluster + - match: + prefix: "/metrics" + headers: + - name: ":method" + string_match: + exact: GET + route: + prefix_rewrite: /stats/prometheus?usedonly + cluster: admin_port_cluster + - match: + prefix: "/stats" + headers: + - name: ":method" + string_match: + exact: GET + route: + prefix_rewrite: /stats + cluster: admin_port_cluster + http_filters: + - name: envoy.filters.http.router + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + clusters: + - name: xds_cluster + alt_stat_name: xds_cluster + connect_timeout: 5.000s + load_assignment: + cluster_name: xds_cluster + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: xds.cluster.local + port_value: 9977 + typed_extension_protocol_options: + envoy.extensions.upstreams.http.v3.HttpProtocolOptions: + "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions + explicit_http_config: + http2_protocol_options: {} + http_filters: + - name: transform + typed_config: + "@type": type.googleapis.com/envoy.api.v2.filter.http.FilterTransformations + transformations: + - match: + prefix: "/" + route_transformations: + request_transformation: + transformation_template: + headers: + authorization: {"text": 'Bearer {{ "{{ trim(data_source(\"token\")) -}}" }}'} + passthrough: {} + data_sources: + token: + filename: "/var/run/secrets/tokens/xds-token" + watched_directory: + path: "/var/run/secrets/tokens" + - name: envoy.filters.http.upstream_codec + typed_config: + "@type": type.googleapis.com/envoy.extensions.filters.http.upstream_codec.v3.UpstreamCodec + upstream_connection_options: + tcp_keepalive: + keepalive_time: 10 + cluster_type: + name: envoy.cluster.strict_dns + typed_config: + "@type": type.googleapis.com/envoy.extensions.clusters.dns.v3.DnsCluster + respect_dns_ttl: true + - name: admin_port_cluster + connect_timeout: 5.000s + type: STATIC + lb_policy: ROUND_ROBIN + load_assignment: + cluster_name: admin_port_cluster + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: 127.0.0.1 + port_value: 19000 + dynamic_resources: + ads_config: + transport_api_version: V3 + api_type: GRPC + rate_limit_settings: {} + grpc_services: + - envoy_grpc: + cluster_name: xds_cluster + cds_config: + resource_api_version: V3 + ads: {} + lds_config: + resource_api_version: V3 + ads: {} +--- +# Source: kgateway-proxy/templates/service.yaml +apiVersion: v1 +kind: Service +metadata: + name: gw + labels: + app.kubernetes.io/instance: gw + app.kubernetes.io/managed-by: kgateway + app.kubernetes.io/name: gw + app.kubernetes.io/version: 1.0.0-ci1 + gateway.networking.k8s.io/gateway-class-name: kgateway + gateway.networking.k8s.io/gateway-name: gw + kgateway: kube-gateway +spec: + type: LoadBalancer + loadBalancerIP: 203.0.113.10 + ports: + - name: listener-8080 + protocol: TCP + targetPort: 8080 + port: 8080 + selector: + app.kubernetes.io/name: gw + app.kubernetes.io/instance: gw + gateway.networking.k8s.io/gateway-name: gw +--- +# Source: kgateway-proxy/templates/deployment.yaml +apiVersion: apps/v1 +kind: Deployment +metadata: + name: gw + labels: + app.kubernetes.io/instance: gw + app.kubernetes.io/managed-by: kgateway + app.kubernetes.io/name: gw + app.kubernetes.io/version: 1.0.0-ci1 + gateway.networking.k8s.io/gateway-class-name: kgateway + gateway.networking.k8s.io/gateway-name: gw + kgateway: kube-gateway +spec: + selector: + matchLabels: + app.kubernetes.io/name: gw + app.kubernetes.io/instance: gw + gateway.networking.k8s.io/gateway-name: gw + template: + metadata: + annotations: + prometheus.io/path: /metrics + prometheus.io/port: "9091" + prometheus.io/scrape: "true" + labels: + app.kubernetes.io/instance: gw + app.kubernetes.io/name: gw + gateway.networking.k8s.io/gateway-class-name: kgateway + gateway.networking.k8s.io/gateway-name: gw + kgateway: kube-gateway + spec: + serviceAccountName: gw + containers: + - name: kgateway-proxy + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + readOnlyRootFilesystem: true + runAsNonRoot: true + runAsUser: 10101 + args: + - "--disable-hot-restart" + - "--service-node" + - $(POD_NAME).$(POD_NAMESPACE) + - "--log-level" + - "info" + image: "ghcr.io/envoy-wrapper:v2.1.0-dev" + volumeMounts: + - mountPath: /etc/envoy + name: envoy-config + - name: xds-token + mountPath: /var/run/secrets/tokens + readOnly: true + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: ENVOY_UID + value: "0" + ports: + - name: listener-8080 + protocol: TCP + containerPort: 8080 + - name: http-monitoring + containerPort: 9091 + startupProbe: + failureThreshold: 60 + httpGet: + path: /ready + port: 8082 + periodSeconds: 1 + successThreshold: 1 + timeoutSeconds: 2 + readinessProbe: + httpGet: + path: /ready + port: 8082 + periodSeconds: 10 + lifecycle: + preStop: + exec: + command: + - /bin/sh + - -c + - wget --post-data "" -O /dev/null 127.0.0.1:19000/healthcheck/fail; sleep 10 + terminationGracePeriodSeconds: 60 + volumes: + - name: xds-token + projected: + sources: + - serviceAccountToken: + audience: kgateway + expirationSeconds: 43200 + path: xds-token + - configMap: + name: gw + name: envoy-config diff --git a/test/deployer/testdata/loadbalancer-static-ip.yaml b/test/deployer/testdata/loadbalancer-static-ip.yaml new file mode 100644 index 00000000000..e7e57e82053 --- /dev/null +++ b/test/deployer/testdata/loadbalancer-static-ip.yaml @@ -0,0 +1,26 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + name: kgateway +spec: + controllerName: kgateway.dev/kgateway + description: Standard class for managing Gateway API ingress traffic. +--- +kind: Gateway +apiVersion: gateway.networking.k8s.io/v1 +metadata: + name: gw + namespace: default +spec: + gatewayClassName: kgateway + addresses: + - type: IPAddress + value: 203.0.113.10 + listeners: + - protocol: HTTP + port: 8080 + name: http + allowedRoutes: + namespaces: + from: Same +--- From f80228b65ba507b3bbf745a95ff8b2932e38bca1 Mon Sep 17 00:00:00 2001 From: omar Date: Mon, 8 Dec 2025 15:49:57 -0600 Subject: [PATCH 2/7] put TODO for agw Signed-off-by: omar --- .../deployer/agentgateway_parameters.go | 3 +- pkg/kgateway/deployer/gateway_parameters.go | 2 +- .../deployer/gateway_parameters_test.go | 130 ++++++++++++++++++ test/deployer/internal_helm_test.go | 4 + 4 files changed, 137 insertions(+), 2 deletions(-) diff --git a/pkg/kgateway/deployer/agentgateway_parameters.go b/pkg/kgateway/deployer/agentgateway_parameters.go index 03fad1ce50a..6b7c9512a7c 100644 --- a/pkg/kgateway/deployer/agentgateway_parameters.go +++ b/pkg/kgateway/deployer/agentgateway_parameters.go @@ -479,7 +479,8 @@ func (g *agentgatewayParametersHelmValuesGenerator) applyGatewayParametersToHelm } svcConfig := gwp.Spec.Kube.GetService() - vals.Gateway.Service = deployer.GetServiceValues(svcConfig) + // TODO: extract loadBalancerIP from Gateway.spec.addresses if service type is LoadBalancer + vals.Gateway.Service = deployer.GetServiceValues(svcConfig, nil) svcAccountConfig := gwp.Spec.Kube.GetServiceAccount() vals.Gateway.ServiceAccount = deployer.GetServiceAccountValues(svcAccountConfig) diff --git a/pkg/kgateway/deployer/gateway_parameters.go b/pkg/kgateway/deployer/gateway_parameters.go index a2c40081c61..f0ee3c12850 100644 --- a/pkg/kgateway/deployer/gateway_parameters.go +++ b/pkg/kgateway/deployer/gateway_parameters.go @@ -361,7 +361,7 @@ func extractLoadBalancerIP(gw *gwv1.Gateway) *string { } if len(gw.Spec.Addresses) > 1 { - slog.Warn("multiple IP addresses found in Gateway.spec.addresses, using first valid one", + slog.Warn("multiple addresses found in Gateway.spec.addresses, using first valid IP address", "gateway", fmt.Sprintf("%s/%s", gw.Namespace, gw.Name), "count", len(gw.Spec.Addresses), ) diff --git a/pkg/kgateway/deployer/gateway_parameters_test.go b/pkg/kgateway/deployer/gateway_parameters_test.go index 82f9ec036d8..83748aa3202 100644 --- a/pkg/kgateway/deployer/gateway_parameters_test.go +++ b/pkg/kgateway/deployer/gateway_parameters_test.go @@ -332,3 +332,133 @@ func newCommonCols(t test.Failer, initObjs ...client.Object) *collections.Common gateways.Gateways.WaitUntilSynced(ctx.Done()) return commonCols } + +func TestExtractLoadBalancerIP(t *testing.T) { + tests := []struct { + name string + addresses []gwv1.GatewaySpecAddress + want *string + }{ + { + name: "single valid IPv4 address", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + want: ptr.To("203.0.113.10"), + }, + { + name: "single valid IPv6 address", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, + }, + want: ptr.To("2001:db8::1"), + }, + { + name: "nil address type defaults to IPAddressType", + addresses: []gwv1.GatewaySpecAddress{ + {Type: nil, Value: "192.0.2.1"}, + }, + want: ptr.To("192.0.2.1"), + }, + { + name: "empty addresses array", + addresses: []gwv1.GatewaySpecAddress{}, + want: nil, + }, + { + name: "multiple valid IP addresses uses first one", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, + }, + want: ptr.To("203.0.113.10"), + }, + { + name: "multiple IP addresses with invalid format skips invalid and uses first valid", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "invalid-ip"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + want: ptr.To("203.0.113.10"), + }, + { + name: "mixed address types uses first IP address", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + want: ptr.To("203.0.113.10"), + }, + { + name: "all hostname addresses returns nil", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, + {Type: ptr.To(gwv1.HostnameAddressType), Value: "test.example.com"}, + }, + want: nil, + }, + { + name: "all invalid IP addresses returns nil", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "not-an-ip"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "256.256.256.256"}, + }, + want: nil, + }, + { + name: "nil type with invalid IP skips and continues", + addresses: []gwv1.GatewaySpecAddress{ + {Type: nil, Value: "invalid"}, + {Type: nil, Value: "203.0.113.10"}, + }, + want: ptr.To("203.0.113.10"), + }, + { + name: "IPv4 and IPv6 mixed uses first valid", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, + }, + want: ptr.To("203.0.113.10"), + }, + { + name: "IPv6 first in multiple addresses", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + want: ptr.To("2001:db8::1"), + }, + { + name: "hostname before IP address skips hostname", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, + }, + want: ptr.To("203.0.113.10"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gw := &gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "default", + }, + Spec: gwv1.GatewaySpec{ + Addresses: tt.addresses, + }, + } + + got := extractLoadBalancerIP(gw) + if tt.want == nil { + assert.Nil(t, got, "expected nil but got %v", got) + } else { + assert.NotNil(t, got, "expected non-nil result") + assert.Equal(t, *tt.want, *got, "IP address mismatch") + } + }) + } +} diff --git a/test/deployer/internal_helm_test.go b/test/deployer/internal_helm_test.go index 17fb1aff68f..af5d499e480 100644 --- a/test/deployer/internal_helm_test.go +++ b/test/deployer/internal_helm_test.go @@ -135,6 +135,10 @@ wIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQBtestcertdata Name: "envoy-infrastructure", InputFile: "envoy-infrastructure", }, + { + Name: "gateway with static IP address", + InputFile: "loadbalancer-static-ip", + }, { Name: "agentgateway-params-primary", InputFile: "agentgateway-params-primary", From eae987173198d3f6a2dae4a81b2b2f0b9bbee647 Mon Sep 17 00:00:00 2001 From: omar Date: Tue, 9 Dec 2025 09:41:41 -0600 Subject: [PATCH 3/7] update test file Signed-off-by: omar --- .../testdata/loadbalancer-static-ip-out.yaml | 125 +++++++++--------- 1 file changed, 63 insertions(+), 62 deletions(-) diff --git a/test/deployer/testdata/loadbalancer-static-ip-out.yaml b/test/deployer/testdata/loadbalancer-static-ip-out.yaml index 8a230571f97..45ea6c3e4f6 100644 --- a/test/deployer/testdata/loadbalancer-static-ip-out.yaml +++ b/test/deployer/testdata/loadbalancer-static-ip-out.yaml @@ -1,9 +1,7 @@ ---- -# Source: kgateway-proxy/templates/serviceaccount.yaml apiVersion: v1 +automountServiceAccountToken: false kind: ServiceAccount metadata: - name: gw labels: app.kubernetes.io/instance: gw app.kubernetes.io/managed-by: kgateway @@ -12,21 +10,9 @@ metadata: gateway.networking.k8s.io/gateway-class-name: kgateway gateway.networking.k8s.io/gateway-name: gw kgateway: kube-gateway -automountServiceAccountToken: false + name: gw --- -# Source: kgateway-proxy/templates/configmap.yaml apiVersion: v1 -kind: ConfigMap -metadata: - name: gw - labels: - app.kubernetes.io/instance: gw - app.kubernetes.io/managed-by: kgateway - app.kubernetes.io/name: gw - app.kubernetes.io/version: 1.0.0-ci1 - gateway.networking.k8s.io/gateway-class-name: kgateway - gateway.networking.k8s.io/gateway-name: gw - kgateway: kube-gateway data: envoy.yaml: | admin: @@ -204,12 +190,21 @@ data: lds_config: resource_api_version: V3 ads: {} +kind: ConfigMap +metadata: + labels: + app.kubernetes.io/instance: gw + app.kubernetes.io/managed-by: kgateway + app.kubernetes.io/name: gw + app.kubernetes.io/version: 1.0.0-ci1 + gateway.networking.k8s.io/gateway-class-name: kgateway + gateway.networking.k8s.io/gateway-name: gw + kgateway: kube-gateway + name: gw --- -# Source: kgateway-proxy/templates/service.yaml apiVersion: v1 kind: Service metadata: - name: gw labels: app.kubernetes.io/instance: gw app.kubernetes.io/managed-by: kgateway @@ -218,24 +213,25 @@ metadata: gateway.networking.k8s.io/gateway-class-name: kgateway gateway.networking.k8s.io/gateway-name: gw kgateway: kube-gateway + name: gw spec: - type: LoadBalancer loadBalancerIP: 203.0.113.10 ports: - name: listener-8080 + port: 8080 protocol: TCP targetPort: 8080 - port: 8080 selector: - app.kubernetes.io/name: gw app.kubernetes.io/instance: gw + app.kubernetes.io/name: gw gateway.networking.k8s.io/gateway-name: gw + type: LoadBalancer +status: + loadBalancer: {} --- -# Source: kgateway-proxy/templates/deployment.yaml apiVersion: apps/v1 kind: Deployment metadata: - name: gw labels: app.kubernetes.io/instance: gw app.kubernetes.io/managed-by: kgateway @@ -244,12 +240,14 @@ metadata: gateway.networking.k8s.io/gateway-class-name: kgateway gateway.networking.k8s.io/gateway-name: gw kgateway: kube-gateway + name: gw spec: selector: matchLabels: - app.kubernetes.io/name: gw app.kubernetes.io/instance: gw + app.kubernetes.io/name: gw gateway.networking.k8s.io/gateway-name: gw + strategy: {} template: metadata: annotations: @@ -263,30 +261,13 @@ spec: gateway.networking.k8s.io/gateway-name: gw kgateway: kube-gateway spec: - serviceAccountName: gw containers: - - name: kgateway-proxy - securityContext: - allowPrivilegeEscalation: false - capabilities: - drop: - - ALL - readOnlyRootFilesystem: true - runAsNonRoot: true - runAsUser: 10101 - args: - - "--disable-hot-restart" - - "--service-node" + - args: + - --disable-hot-restart + - --service-node - $(POD_NAME).$(POD_NAMESPACE) - - "--log-level" - - "info" - image: "ghcr.io/envoy-wrapper:v2.1.0-dev" - volumeMounts: - - mountPath: /etc/envoy - name: envoy-config - - name: xds-token - mountPath: /var/run/secrets/tokens - readOnly: true + - --log-level + - info env: - name: POD_NAME valueFrom: @@ -298,12 +279,36 @@ spec: fieldPath: metadata.namespace - name: ENVOY_UID value: "0" + image: ghcr.io/envoy-wrapper:v2.1.0-dev + lifecycle: + preStop: + exec: + command: + - /bin/sh + - -c + - wget --post-data "" -O /dev/null 127.0.0.1:19000/healthcheck/fail; + sleep 10 + name: kgateway-proxy ports: - - name: listener-8080 + - containerPort: 8080 + name: listener-8080 protocol: TCP - containerPort: 8080 - - name: http-monitoring - containerPort: 9091 + - containerPort: 9091 + name: http-monitoring + readinessProbe: + httpGet: + path: /ready + port: 8082 + periodSeconds: 10 + resources: {} + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: + - ALL + readOnlyRootFilesystem: true + runAsNonRoot: true + runAsUser: 10101 startupProbe: failureThreshold: 60 httpGet: @@ -312,18 +317,13 @@ spec: periodSeconds: 1 successThreshold: 1 timeoutSeconds: 2 - readinessProbe: - httpGet: - path: /ready - port: 8082 - periodSeconds: 10 - lifecycle: - preStop: - exec: - command: - - /bin/sh - - -c - - wget --post-data "" -O /dev/null 127.0.0.1:19000/healthcheck/fail; sleep 10 + volumeMounts: + - mountPath: /etc/envoy + name: envoy-config + - mountPath: /var/run/secrets/tokens + name: xds-token + readOnly: true + serviceAccountName: gw terminationGracePeriodSeconds: 60 volumes: - name: xds-token @@ -336,3 +336,4 @@ spec: - configMap: name: gw name: envoy-config +status: {} From 4a81e393df3ea20f3ddaae9c5db731a0c0e6cce6 Mon Sep 17 00:00:00 2001 From: omar Date: Thu, 11 Dec 2025 10:26:04 -0600 Subject: [PATCH 4/7] only support one address Signed-off-by: omar --- pkg/kgateway/deployer/gateway_parameters.go | 47 +++++----- .../deployer/gateway_parameters_test.go | 89 ++++++++++--------- 2 files changed, 72 insertions(+), 64 deletions(-) diff --git a/pkg/kgateway/deployer/gateway_parameters.go b/pkg/kgateway/deployer/gateway_parameters.go index aabcbf94806..ed3327c2afc 100644 --- a/pkg/kgateway/deployer/gateway_parameters.go +++ b/pkg/kgateway/deployer/gateway_parameters.go @@ -31,6 +31,12 @@ var ( // ErrNotFound is returned when a requested resource is not found ErrNotFound = errors.New("resource not found") + + // ErrMultipleAddresses is returned when multiple addresses are specified in Gateway.spec.addresses + ErrMultipleAddresses = errors.New("multiple addresses given, only one address is supported") + + // ErrNoValidIPAddress is returned when no valid IP address is found in Gateway.spec.addresses + ErrNoValidIPAddress = errors.New("no valid IP address found in Gateway.spec.addresses") ) func NewGatewayParameters(cli apiclient.Client, inputs *deployer.Inputs) *GatewayParameters { @@ -351,37 +357,28 @@ func (k *kgatewayParameters) getGatewayParametersForGatewayClass(gwc *gwv1.Gatew return mergedGwp, nil } -// extractLoadBalancerIP extracts the first IP address from Gateway.spec.addresses -// where the address type is IPAddressType. Returns nil if no valid IP address is found. -// If multiple IP addresses are provided, uses the first one and logs a warning. -func extractLoadBalancerIP(gw *gwv1.Gateway) *string { +// extractLoadBalancerIP extracts the IP address from Gateway.spec.addresses +// where the address type is IPAddressType. +// Returns an error if more than one address is specified or no valid IP address is found. +func extractLoadBalancerIP(gw *gwv1.Gateway) (*string, error) { if len(gw.Spec.Addresses) == 0 { - return nil + return nil, nil } if len(gw.Spec.Addresses) > 1 { - slog.Warn("multiple addresses found in Gateway.spec.addresses, using first valid IP address", - "gateway", fmt.Sprintf("%s/%s", gw.Namespace, gw.Name), - "count", len(gw.Spec.Addresses), - ) + return nil, fmt.Errorf("%w: gateway %s/%s has %d addresses", ErrMultipleAddresses, gw.Namespace, gw.Name, len(gw.Spec.Addresses)) } - for _, addr := range gw.Spec.Addresses { - // IPAddressType or nil (defaults to IPAddressType per Gateway API spec) - if addr.Type == nil || *addr.Type == gwv1.IPAddressType { - // Validate IP format - if parsedIP, err := netip.ParseAddr(addr.Value); err == nil && parsedIP.IsValid() { - return &addr.Value - } - // Log warning for invalid IP but continue searching - slog.Warn("invalid IP address in Gateway.spec.addresses, skipping", "value", addr.Value) + addr := gw.Spec.Addresses[0] + // IPAddressType or nil (defaults to IPAddressType per Gateway API spec) + if addr.Type == nil || *addr.Type == gwv1.IPAddressType { + // Validate IP format + if parsedIP, err := netip.ParseAddr(addr.Value); err == nil && parsedIP.IsValid() { + return &addr.Value, nil } } - slog.Error("no valid IP address found in Gateway.spec.addresses", - "gateway", fmt.Sprintf("%s/%s", gw.Namespace, gw.Name), - ) - return nil + return nil, fmt.Errorf("%w: gateway %s/%s has no valid IP address", ErrNoValidIPAddress, gw.Namespace, gw.Name) } func (k *kgatewayParameters) getValues(gw *gwv1.Gateway, gwParam *kgateway.GatewayParameters) (*deployer.HelmConfig, error) { @@ -464,7 +461,11 @@ func (k *kgatewayParameters) getValues(gw *gwv1.Gateway, gwParam *kgateway.Gatew // Extract loadBalancerIP from Gateway.spec.addresses if service type is LoadBalancer var loadBalancerIP *string if svcConfig != nil && svcConfig.GetType() != nil && *svcConfig.GetType() == corev1.ServiceTypeLoadBalancer { - loadBalancerIP = extractLoadBalancerIP(gw) + var err error + loadBalancerIP, err = extractLoadBalancerIP(gw) + if err != nil { + return nil, err + } } gateway.Service = deployer.GetServiceValues(svcConfig, loadBalancerIP) // serviceaccount values diff --git a/pkg/kgateway/deployer/gateway_parameters_test.go b/pkg/kgateway/deployer/gateway_parameters_test.go index eaf72c82766..af158f4ec36 100644 --- a/pkg/kgateway/deployer/gateway_parameters_test.go +++ b/pkg/kgateway/deployer/gateway_parameters_test.go @@ -202,105 +202,105 @@ func TestExtractLoadBalancerIP(t *testing.T) { name string addresses []gwv1.GatewaySpecAddress want *string + wantErr error }{ { name: "single valid IPv4 address", addresses: []gwv1.GatewaySpecAddress{ {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, }, - want: ptr.To("203.0.113.10"), + want: ptr.To("203.0.113.10"), + wantErr: nil, }, { name: "single valid IPv6 address", addresses: []gwv1.GatewaySpecAddress{ {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, }, - want: ptr.To("2001:db8::1"), + want: ptr.To("2001:db8::1"), + wantErr: nil, }, { name: "nil address type defaults to IPAddressType", addresses: []gwv1.GatewaySpecAddress{ {Type: nil, Value: "192.0.2.1"}, }, - want: ptr.To("192.0.2.1"), + want: ptr.To("192.0.2.1"), + wantErr: nil, }, { name: "empty addresses array", addresses: []gwv1.GatewaySpecAddress{}, want: nil, + wantErr: nil, }, { - name: "multiple valid IP addresses uses first one", + name: "multiple valid IP addresses returns error", addresses: []gwv1.GatewaySpecAddress{ {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, }, - want: ptr.To("203.0.113.10"), + want: nil, + wantErr: ErrMultipleAddresses, }, { - name: "multiple IP addresses with invalid format skips invalid and uses first valid", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "invalid-ip"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, - }, - want: ptr.To("203.0.113.10"), - }, - { - name: "mixed address types uses first IP address", + name: "multiple addresses with mixed types returns error", addresses: []gwv1.GatewaySpecAddress{ {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, }, - want: ptr.To("203.0.113.10"), + want: nil, + wantErr: ErrMultipleAddresses, }, { - name: "all hostname addresses returns nil", + name: "single hostname address returns nil", addresses: []gwv1.GatewaySpecAddress{ {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, - {Type: ptr.To(gwv1.HostnameAddressType), Value: "test.example.com"}, }, - want: nil, + want: nil, + wantErr: nil, }, { - name: "all invalid IP addresses returns nil", + name: "single invalid IP address returns error", addresses: []gwv1.GatewaySpecAddress{ {Type: ptr.To(gwv1.IPAddressType), Value: "not-an-ip"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "256.256.256.256"}, }, - want: nil, + want: nil, + wantErr: ErrNoValidIPAddress, }, { - name: "nil type with invalid IP skips and continues", + name: "single invalid IP address format returns error", addresses: []gwv1.GatewaySpecAddress{ - {Type: nil, Value: "invalid"}, - {Type: nil, Value: "203.0.113.10"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "256.256.256.256"}, }, - want: ptr.To("203.0.113.10"), + want: nil, + wantErr: ErrNoValidIPAddress, }, { - name: "IPv4 and IPv6 mixed uses first valid", + name: "nil type with valid IP returns IP", addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, + {Type: nil, Value: "203.0.113.10"}, }, - want: ptr.To("203.0.113.10"), + want: ptr.To("203.0.113.10"), + wantErr: nil, }, { - name: "IPv6 first in multiple addresses", + name: "nil type with invalid IP returns error", addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + {Type: nil, Value: "invalid"}, }, - want: ptr.To("2001:db8::1"), + want: nil, + wantErr: ErrNoValidIPAddress, }, { - name: "hostname before IP address skips hostname", + name: "three addresses returns error", addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.12"}, }, - want: ptr.To("203.0.113.10"), + want: nil, + wantErr: ErrMultipleAddresses, }, } @@ -316,12 +316,19 @@ func TestExtractLoadBalancerIP(t *testing.T) { }, } - got := extractLoadBalancerIP(gw) - if tt.want == nil { - assert.Nil(t, got, "expected nil but got %v", got) + got, err := extractLoadBalancerIP(gw) + if tt.wantErr != nil { + assert.Error(t, err, "expected error") + assert.ErrorIs(t, err, tt.wantErr, "error should be ErrMultipleAddresses") + assert.Nil(t, got, "expected nil IP when error occurs") } else { - assert.NotNil(t, got, "expected non-nil result") - assert.Equal(t, *tt.want, *got, "IP address mismatch") + assert.NoError(t, err, "unexpected error") + if tt.want == nil { + assert.Nil(t, got, "expected nil but got %v", got) + } else { + assert.NotNil(t, got, "expected non-nil result") + assert.Equal(t, *tt.want, *got, "IP address mismatch") + } } }) } From e6ba24b1c4b4646f26cf60cdd3d303c715574d98 Mon Sep 17 00:00:00 2001 From: omar Date: Thu, 11 Dec 2025 10:49:57 -0600 Subject: [PATCH 5/7] tests Signed-off-by: omar --- pkg/kgateway/deployer/gateway_parameters_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kgateway/deployer/gateway_parameters_test.go b/pkg/kgateway/deployer/gateway_parameters_test.go index af158f4ec36..711bf71f7fe 100644 --- a/pkg/kgateway/deployer/gateway_parameters_test.go +++ b/pkg/kgateway/deployer/gateway_parameters_test.go @@ -253,12 +253,12 @@ func TestExtractLoadBalancerIP(t *testing.T) { wantErr: ErrMultipleAddresses, }, { - name: "single hostname address returns nil", + name: "single hostname address returns error", addresses: []gwv1.GatewaySpecAddress{ {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, }, want: nil, - wantErr: nil, + wantErr: ErrNoValidIPAddress, }, { name: "single invalid IP address returns error", From f06b86600aab091ae43f900311b139d5aab1cf99 Mon Sep 17 00:00:00 2001 From: omar Date: Thu, 11 Dec 2025 16:32:19 -0600 Subject: [PATCH 6/7] refactor : move to deployer pkg since has nothing to do with GWP Signed-off-by: omar --- pkg/deployer/values_helpers.go | 44 ++++- pkg/deployer/values_helpers_test.go | 176 ++++++++++++++++++ pkg/kgateway/deployer/gateway_parameters.go | 45 +---- .../deployer/gateway_parameters_test.go | 137 -------------- 4 files changed, 222 insertions(+), 180 deletions(-) diff --git a/pkg/deployer/values_helpers.go b/pkg/deployer/values_helpers.go index f2c35ccb155..ebb2a0bdb17 100644 --- a/pkg/deployer/values_helpers.go +++ b/pkg/deployer/values_helpers.go @@ -1,7 +1,9 @@ package deployer import ( + "errors" "fmt" + "net/netip" "regexp" "sort" "strings" @@ -18,6 +20,14 @@ import ( "github.com/kgateway-dev/kgateway/v2/pkg/pluginsdk/ir" ) +var ( + // ErrMultipleAddresses is returned when multiple addresses are specified in Gateway.spec.addresses + ErrMultipleAddresses = errors.New("multiple addresses given, only one address is supported") + + // ErrNoValidIPAddress is returned when no valid IP address is found in Gateway.spec.addresses + ErrNoValidIPAddress = errors.New("no valid IP address found in Gateway.spec.addresses") +) + // This file contains helper functions that generate helm values in the format needed // by the deployer. @@ -104,7 +114,7 @@ func AppendPortValue(gwPorts []HelmPort, port int32, name string, gwp *kgateway. } // Convert service values from GatewayParameters into helm values to be used by the deployer. -func GetServiceValues(svcConfig *kgateway.Service, loadBalancerIP *string) *HelmService { +func GetServiceValues(svcConfig *kgateway.Service) *HelmService { // convert the service type enum to its string representation; // if type is not set, it will default to 0 ("ClusterIP") var svcType *string @@ -126,13 +136,43 @@ func GetServiceValues(svcConfig *kgateway.Service, loadBalancerIP *string) *Helm return &HelmService{ Type: svcType, ClusterIP: clusterIP, - LoadBalancerIP: loadBalancerIP, ExtraAnnotations: extraAnnotations, ExtraLabels: extraLabels, ExternalTrafficPolicy: externalTrafficPolicy, } } +// SetLoadBalancerIPFromGateway extracts the IP address from Gateway.spec.addresses +// and sets it on the HelmService if the service type is LoadBalancer. +// Only sets the IP if exactly one valid IP address is found in Gateway.spec.addresses. +// Returns an error if more than one address is specified or no valid IP address is found. +func SetLoadBalancerIPFromGateway(gw *gwv1.Gateway, svc *HelmService) error { + // Only extract IP if service type is LoadBalancer + if svc.Type == nil || *svc.Type != string(corev1.ServiceTypeLoadBalancer) { + return nil + } + + if len(gw.Spec.Addresses) == 0 { + return nil + } + + if len(gw.Spec.Addresses) > 1 { + return fmt.Errorf("%w: gateway %s/%s has %d addresses", ErrMultipleAddresses, gw.Namespace, gw.Name, len(gw.Spec.Addresses)) + } + + addr := gw.Spec.Addresses[0] + // IPAddressType or nil (defaults to IPAddressType per Gateway API spec) + if addr.Type == nil || *addr.Type == gwv1.IPAddressType { + // Validate IP format + if parsedIP, err := netip.ParseAddr(addr.Value); err == nil && parsedIP.IsValid() { + svc.LoadBalancerIP = &addr.Value + return nil + } + } + + return fmt.Errorf("%w: gateway %s/%s has no valid IP address", ErrNoValidIPAddress, gw.Namespace, gw.Name) +} + // Convert service account values from GatewayParameters into helm values to be used by the deployer. func GetServiceAccountValues(svcAccountConfig *kgateway.ServiceAccount) *HelmServiceAccount { return &HelmServiceAccount{ diff --git a/pkg/deployer/values_helpers_test.go b/pkg/deployer/values_helpers_test.go index 9760136e154..262906b9fe8 100644 --- a/pkg/deployer/values_helpers_test.go +++ b/pkg/deployer/values_helpers_test.go @@ -4,6 +4,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) func TestComponentLogLevelsToString(t *testing.T) { @@ -56,3 +60,175 @@ func TestComponentLogLevelsToString(t *testing.T) { }) } } + +func TestSetLoadBalancerIPFromGateway(t *testing.T) { + tests := []struct { + name string + addresses []gwv1.GatewaySpecAddress + serviceType *string + wantIP *string + wantErr error + }{ + { + name: "single valid IPv4 address with LoadBalancer service", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: ptr.To("203.0.113.10"), + wantErr: nil, + }, + { + name: "single valid IPv6 address with LoadBalancer service", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: ptr.To("2001:db8::1"), + wantErr: nil, + }, + { + name: "nil address type defaults to IPAddressType", + addresses: []gwv1.GatewaySpecAddress{ + {Type: nil, Value: "192.0.2.1"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: ptr.To("192.0.2.1"), + wantErr: nil, + }, + { + name: "empty addresses array with LoadBalancer service", + addresses: []gwv1.GatewaySpecAddress{}, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: nil, + }, + { + name: "multiple valid IP addresses returns error", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: ErrMultipleAddresses, + }, + { + name: "multiple addresses with mixed types returns error", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: ErrMultipleAddresses, + }, + { + name: "single hostname address returns error", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: ErrNoValidIPAddress, + }, + { + name: "single invalid IP address returns error", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "not-an-ip"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: ErrNoValidIPAddress, + }, + { + name: "single invalid IP address format returns error", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "256.256.256.256"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: ErrNoValidIPAddress, + }, + { + name: "nil type with valid IP returns IP", + addresses: []gwv1.GatewaySpecAddress{ + {Type: nil, Value: "203.0.113.10"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: ptr.To("203.0.113.10"), + wantErr: nil, + }, + { + name: "nil type with invalid IP returns error", + addresses: []gwv1.GatewaySpecAddress{ + {Type: nil, Value: "invalid"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: ErrNoValidIPAddress, + }, + { + name: "three addresses returns error", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.12"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeLoadBalancer)), + wantIP: nil, + wantErr: ErrMultipleAddresses, + }, + { + name: "valid IP with ClusterIP service does not set IP", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + serviceType: ptr.To(string(corev1.ServiceTypeClusterIP)), + wantIP: nil, + wantErr: nil, + }, + { + name: "valid IP with nil service type does not set IP", + addresses: []gwv1.GatewaySpecAddress{ + {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, + }, + serviceType: nil, + wantIP: nil, + wantErr: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gw := &gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-gateway", + Namespace: "default", + }, + Spec: gwv1.GatewaySpec{ + Addresses: tt.addresses, + }, + } + + svc := &HelmService{ + Type: tt.serviceType, + } + + err := SetLoadBalancerIPFromGateway(gw, svc) + if tt.wantErr != nil { + assert.Error(t, err, "expected error") + assert.ErrorIs(t, err, tt.wantErr, "error type mismatch") + assert.Nil(t, svc.LoadBalancerIP, "expected nil IP when error occurs") + } else { + assert.NoError(t, err, "unexpected error") + if tt.wantIP == nil { + assert.Nil(t, svc.LoadBalancerIP, "expected nil but got %v", svc.LoadBalancerIP) + } else { + assert.NotNil(t, svc.LoadBalancerIP, "expected non-nil IP") + assert.Equal(t, *tt.wantIP, *svc.LoadBalancerIP, "IP address mismatch") + } + } + }) + } +} diff --git a/pkg/kgateway/deployer/gateway_parameters.go b/pkg/kgateway/deployer/gateway_parameters.go index 75dee302311..a93a6721402 100644 --- a/pkg/kgateway/deployer/gateway_parameters.go +++ b/pkg/kgateway/deployer/gateway_parameters.go @@ -5,12 +5,10 @@ import ( "errors" "fmt" "log/slog" - "net/netip" "strings" "helm.sh/helm/v3/pkg/chart" "istio.io/istio/pkg/kube/kclient" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" "k8s.io/utils/ptr" @@ -31,12 +29,6 @@ var ( // ErrNotFound is returned when a requested resource is not found ErrNotFound = errors.New("resource not found") - - // ErrMultipleAddresses is returned when multiple addresses are specified in Gateway.spec.addresses - ErrMultipleAddresses = errors.New("multiple addresses given, only one address is supported") - - // ErrNoValidIPAddress is returned when no valid IP address is found in Gateway.spec.addresses - ErrNoValidIPAddress = errors.New("no valid IP address found in Gateway.spec.addresses") ) func NewGatewayParameters(cli apiclient.Client, inputs *deployer.Inputs) *GatewayParameters { @@ -408,30 +400,6 @@ func (k *kgatewayParameters) getGatewayParametersForGatewayClass(gwc *gwv1.Gatew return mergedGwp, nil } -// extractLoadBalancerIP extracts the IP address from Gateway.spec.addresses -// where the address type is IPAddressType. -// Returns an error if more than one address is specified or no valid IP address is found. -func extractLoadBalancerIP(gw *gwv1.Gateway) (*string, error) { - if len(gw.Spec.Addresses) == 0 { - return nil, nil - } - - if len(gw.Spec.Addresses) > 1 { - return nil, fmt.Errorf("%w: gateway %s/%s has %d addresses", ErrMultipleAddresses, gw.Namespace, gw.Name, len(gw.Spec.Addresses)) - } - - addr := gw.Spec.Addresses[0] - // IPAddressType or nil (defaults to IPAddressType per Gateway API spec) - if addr.Type == nil || *addr.Type == gwv1.IPAddressType { - // Validate IP format - if parsedIP, err := netip.ParseAddr(addr.Value); err == nil && parsedIP.IsValid() { - return &addr.Value, nil - } - } - - return nil, fmt.Errorf("%w: gateway %s/%s has no valid IP address", ErrNoValidIPAddress, gw.Namespace, gw.Name) -} - func (k *kgatewayParameters) getValues(gw *gwv1.Gateway, gwParam *kgateway.GatewayParameters) (*deployer.HelmConfig, error) { irGW := deployer.GetGatewayIR(gw, k.inputs.CommonCollections) // kgatewayParameters is only used for envoy gateways (agentgateway uses agentgatewayParametersHelmValuesGenerator) @@ -509,16 +477,11 @@ func (k *kgatewayParameters) getValues(gw *gwv1.Gateway, gwParam *kgateway.Gatew gateway.Strategy = deployConfig.GetStrategy() // service values - // Extract loadBalancerIP from Gateway.spec.addresses if service type is LoadBalancer - var loadBalancerIP *string - if svcConfig != nil && svcConfig.GetType() != nil && *svcConfig.GetType() == corev1.ServiceTypeLoadBalancer { - var err error - loadBalancerIP, err = extractLoadBalancerIP(gw) - if err != nil { - return nil, err - } + gateway.Service = deployer.GetServiceValues(svcConfig) + // Extract loadBalancerIP from Gateway.spec.addresses and set it on the service if service type is LoadBalancer + if err := deployer.SetLoadBalancerIPFromGateway(gw, gateway.Service); err != nil { + return nil, err } - gateway.Service = deployer.GetServiceValues(svcConfig, loadBalancerIP) // serviceaccount values gateway.ServiceAccount = deployer.GetServiceAccountValues(svcAccountConfig) // pod template values diff --git a/pkg/kgateway/deployer/gateway_parameters_test.go b/pkg/kgateway/deployer/gateway_parameters_test.go index 0cba54b308a..0192c177c79 100644 --- a/pkg/kgateway/deployer/gateway_parameters_test.go +++ b/pkg/kgateway/deployer/gateway_parameters_test.go @@ -202,140 +202,3 @@ func newCommonCols(t test.Failer, initObjs ...client.Object) *collections.Common gateways.Gateways.WaitUntilSynced(ctx.Done()) return commonCols } - -func TestExtractLoadBalancerIP(t *testing.T) { - tests := []struct { - name string - addresses []gwv1.GatewaySpecAddress - want *string - wantErr error - }{ - { - name: "single valid IPv4 address", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, - }, - want: ptr.To("203.0.113.10"), - wantErr: nil, - }, - { - name: "single valid IPv6 address", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "2001:db8::1"}, - }, - want: ptr.To("2001:db8::1"), - wantErr: nil, - }, - { - name: "nil address type defaults to IPAddressType", - addresses: []gwv1.GatewaySpecAddress{ - {Type: nil, Value: "192.0.2.1"}, - }, - want: ptr.To("192.0.2.1"), - wantErr: nil, - }, - { - name: "empty addresses array", - addresses: []gwv1.GatewaySpecAddress{}, - want: nil, - wantErr: nil, - }, - { - name: "multiple valid IP addresses returns error", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, - }, - want: nil, - wantErr: ErrMultipleAddresses, - }, - { - name: "multiple addresses with mixed types returns error", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, - }, - want: nil, - wantErr: ErrMultipleAddresses, - }, - { - name: "single hostname address returns error", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.HostnameAddressType), Value: "example.com"}, - }, - want: nil, - wantErr: ErrNoValidIPAddress, - }, - { - name: "single invalid IP address returns error", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "not-an-ip"}, - }, - want: nil, - wantErr: ErrNoValidIPAddress, - }, - { - name: "single invalid IP address format returns error", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "256.256.256.256"}, - }, - want: nil, - wantErr: ErrNoValidIPAddress, - }, - { - name: "nil type with valid IP returns IP", - addresses: []gwv1.GatewaySpecAddress{ - {Type: nil, Value: "203.0.113.10"}, - }, - want: ptr.To("203.0.113.10"), - wantErr: nil, - }, - { - name: "nil type with invalid IP returns error", - addresses: []gwv1.GatewaySpecAddress{ - {Type: nil, Value: "invalid"}, - }, - want: nil, - wantErr: ErrNoValidIPAddress, - }, - { - name: "three addresses returns error", - addresses: []gwv1.GatewaySpecAddress{ - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.10"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.11"}, - {Type: ptr.To(gwv1.IPAddressType), Value: "203.0.113.12"}, - }, - want: nil, - wantErr: ErrMultipleAddresses, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - gw := &gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-gateway", - Namespace: "default", - }, - Spec: gwv1.GatewaySpec{ - Addresses: tt.addresses, - }, - } - - got, err := extractLoadBalancerIP(gw) - if tt.wantErr != nil { - assert.Error(t, err, "expected error") - assert.ErrorIs(t, err, tt.wantErr, "error should be ErrMultipleAddresses") - assert.Nil(t, got, "expected nil IP when error occurs") - } else { - assert.NoError(t, err, "unexpected error") - if tt.want == nil { - assert.Nil(t, got, "expected nil but got %v", got) - } else { - assert.NotNil(t, got, "expected non-nil result") - assert.Equal(t, *tt.want, *got, "IP address mismatch") - } - } - }) - } -} From 6f51567816ae7307351eac0e3c6f0083cc87ab50 Mon Sep 17 00:00:00 2001 From: omar Date: Fri, 12 Dec 2025 10:18:11 -0600 Subject: [PATCH 7/7] error phrasing Signed-off-by: omar --- pkg/deployer/values_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/deployer/values_helpers.go b/pkg/deployer/values_helpers.go index ebb2a0bdb17..dd07575b744 100644 --- a/pkg/deployer/values_helpers.go +++ b/pkg/deployer/values_helpers.go @@ -25,7 +25,7 @@ var ( ErrMultipleAddresses = errors.New("multiple addresses given, only one address is supported") // ErrNoValidIPAddress is returned when no valid IP address is found in Gateway.spec.addresses - ErrNoValidIPAddress = errors.New("no valid IP address found in Gateway.spec.addresses") + ErrNoValidIPAddress = errors.New("IP address in Gateway.spec.addresses not valid") ) // This file contains helper functions that generate helm values in the format needed