From 71bd170e6ba340e797bb85f75ba1df2bbb427a82 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 19 Oct 2023 11:02:07 -0700 Subject: [PATCH 1/3] Support configuring loadBalancerIP in envoy svc Fixes: https://github.com/envoyproxy/gateway/issues/1841 Signed-off-by: Arko Dasgupta --- api/v1alpha1/shared_types.go | 7 +++ .../validation/envoyproxy_validate.go | 10 ++++ .../validation/envoyproxy_validate_test.go | 46 ++++++++++++++++++- api/v1alpha1/zz_generated.deepcopy.go | 5 ++ .../gateway.envoyproxy.io_envoyproxies.yaml | 8 ++++ .../kubernetes/resource/resource.go | 3 ++ .../kubernetes/resource/resource_test.go | 17 ++++++- site/content/en/latest/api/extension_types.md | 1 + 8 files changed, 93 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/shared_types.go b/api/v1alpha1/shared_types.go index e197602986a..ea207b73891 100644 --- a/api/v1alpha1/shared_types.go +++ b/api/v1alpha1/shared_types.go @@ -191,6 +191,13 @@ type KubernetesServiceSpec struct { // +optional AllocateLoadBalancerNodePorts *bool `json:"allocateLoadBalancerNodePorts,omitempty"` + // LoadBalancerIP defines the IP Address of the underlying load balancer service. This field + // may be ignored if the load balancer provider does not support this feature. + // This field has been Deprecated, but it is still used for set the IP Address in some cloud + // providers such as GCP. + // +optional + LoadBalancerIP *string `json:"loadBalancerIP,omitempty"` + // TODO: Expose config as use cases are better understood, e.g. labels. } diff --git a/api/v1alpha1/validation/envoyproxy_validate.go b/api/v1alpha1/validation/envoyproxy_validate.go index b4fe30afee5..c5f8d6becbe 100644 --- a/api/v1alpha1/validation/envoyproxy_validate.go +++ b/api/v1alpha1/validation/envoyproxy_validate.go @@ -8,6 +8,7 @@ package validation import ( "errors" "fmt" + "net" "reflect" bootstrapv3 "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3" @@ -95,6 +96,15 @@ func validateService(spec *egv1a1.EnvoyProxySpec) []error { errs = append(errs, fmt.Errorf("allocateLoadBalancerNodePorts can only be set for %v type", egv1a1.ServiceTypeLoadBalancer)) } } + if serviceType, serviceLoadBalancerIP := spec.Provider.Kubernetes.EnvoyService.Type, spec.Provider.Kubernetes.EnvoyService.LoadBalancerIP; serviceType != nil && serviceLoadBalancerIP != nil { + if *serviceType != egv1a1.ServiceTypeLoadBalancer { + errs = append(errs, fmt.Errorf("loadBalancerIP can only be set for %v type", egv1a1.ServiceTypeLoadBalancer)) + } + + if net.ParseIP(*serviceLoadBalancerIP) == nil { + errs = append(errs, fmt.Errorf("loadBalancerIP:%s is an invalid IP address", *serviceLoadBalancerIP)) + } + } } return errs } diff --git a/api/v1alpha1/validation/envoyproxy_validate_test.go b/api/v1alpha1/validation/envoyproxy_validate_test.go index 3dc1e08d355..c2b3bde71e6 100644 --- a/api/v1alpha1/validation/envoyproxy_validate_test.go +++ b/api/v1alpha1/validation/envoyproxy_validate_test.go @@ -184,7 +184,7 @@ func TestValidateEnvoyProxy(t *testing.T) { Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ EnvoyService: &egv1a1.KubernetesServiceSpec{ Type: egv1a1.GetKubernetesServiceType(egv1a1.ServiceTypeLoadBalancer), - AllocateLoadBalancerNodePorts: ptr.To[bool](false), + AllocateLoadBalancerNodePorts: ptr.To(false), }, }, }, @@ -205,7 +205,49 @@ func TestValidateEnvoyProxy(t *testing.T) { Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ EnvoyService: &egv1a1.KubernetesServiceSpec{ Type: egv1a1.GetKubernetesServiceType(egv1a1.ServiceTypeClusterIP), - AllocateLoadBalancerNodePorts: ptr.To[bool](false), + AllocateLoadBalancerNodePorts: ptr.To(false), + }, + }, + }, + }, + }, + expected: false, + }, + { + name: "envoy service type 'LoadBalancer' with valid loadBalancerIP", + proxy: &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.EnvoyProxySpec{ + Provider: &egv1a1.EnvoyProxyProvider{ + Type: egv1a1.ProviderTypeKubernetes, + Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ + EnvoyService: &egv1a1.KubernetesServiceSpec{ + Type: egv1a1.GetKubernetesServiceType(egv1a1.ServiceTypeLoadBalancer), + LoadBalancerIP: ptr.To("10.11.12.13"), + }, + }, + }, + }, + }, + expected: true, + }, + { + name: "envoy service type 'LoadBalancer' with invalid loadBalancerIP", + proxy: &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.EnvoyProxySpec{ + Provider: &egv1a1.EnvoyProxyProvider{ + Type: egv1a1.ProviderTypeKubernetes, + Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ + EnvoyService: &egv1a1.KubernetesServiceSpec{ + Type: egv1a1.GetKubernetesServiceType(egv1a1.ServiceTypeLoadBalancer), + LoadBalancerIP: ptr.To("invalid-ip"), }, }, }, diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 22762b7e1f8..44e453e4fa8 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -1233,6 +1233,11 @@ func (in *KubernetesServiceSpec) DeepCopyInto(out *KubernetesServiceSpec) { *out = new(bool) **out = **in } + if in.LoadBalancerIP != nil { + in, out := &in.LoadBalancerIP, &out.LoadBalancerIP + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesServiceSpec. diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml index a92bfb35276..1e224b32a68 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml @@ -5258,6 +5258,14 @@ spec: if more than one are available or is otherwise expected to be specified type: string + loadBalancerIP: + description: LoadBalancerIP defines the IP Address of + the underlying load balancer service. This field may + be ignored if the load balancer provider does not support + this feature. This field has been Deprecated, but it + is still used for set the IP Address in some cloud providers + such as GCP. + type: string type: default: LoadBalancer description: Type determines how the Service is exposed. diff --git a/internal/infrastructure/kubernetes/resource/resource.go b/internal/infrastructure/kubernetes/resource/resource.go index 07bb458ee15..b001334898e 100644 --- a/internal/infrastructure/kubernetes/resource/resource.go +++ b/internal/infrastructure/kubernetes/resource/resource.go @@ -34,6 +34,9 @@ func ExpectedServiceSpec(service *egv1a1.KubernetesServiceSpec) corev1.ServiceSp if service.AllocateLoadBalancerNodePorts != nil { serviceSpec.AllocateLoadBalancerNodePorts = service.AllocateLoadBalancerNodePorts } + if service.LoadBalancerIP != nil { + serviceSpec.LoadBalancerIP = *service.LoadBalancerIP + } // Preserve the client source IP and avoid a second hop for LoadBalancer. serviceSpec.ExternalTrafficPolicy = corev1.ServiceExternalTrafficPolicyTypeLocal } diff --git a/internal/infrastructure/kubernetes/resource/resource_test.go b/internal/infrastructure/kubernetes/resource/resource_test.go index 04964a4eabd..f0ace1384ca 100644 --- a/internal/infrastructure/kubernetes/resource/resource_test.go +++ b/internal/infrastructure/kubernetes/resource/resource_test.go @@ -56,15 +56,28 @@ func TestExpectedServiceSpec(t *testing.T) { name: "LoadBalancerWithAllocateLoadBalancerNodePorts", args: args{service: &egv1a1.KubernetesServiceSpec{ Type: egv1a1.GetKubernetesServiceType(egv1a1.ServiceTypeLoadBalancer), - AllocateLoadBalancerNodePorts: ptr.To[bool](true), + AllocateLoadBalancerNodePorts: ptr.To(true), }}, want: corev1.ServiceSpec{ Type: corev1.ServiceTypeLoadBalancer, - AllocateLoadBalancerNodePorts: ptr.To[bool](true), + AllocateLoadBalancerNodePorts: ptr.To(true), SessionAffinity: corev1.ServiceAffinityNone, ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal, }, }, + { + name: "LoadBalancerWithLoadBalancerIP", + args: args{service: &egv1a1.KubernetesServiceSpec{ + Type: egv1a1.GetKubernetesServiceType(egv1a1.ServiceTypeLoadBalancer), + LoadBalancerIP: ptr.To("10.11.12.13"), + }}, + want: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + LoadBalancerIP: "10.11.12.13", + SessionAffinity: corev1.ServiceAffinityNone, + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal, + }, + }, { name: "ClusterIP", args: args{service: &egv1a1.KubernetesServiceSpec{ diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 64a8b0803ed..0b17c1a2435 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -847,6 +847,7 @@ _Appears in:_ | `type` _[ServiceType](#servicetype)_ | Type determines how the Service is exposed. Defaults to LoadBalancer. Valid options are ClusterIP, LoadBalancer and NodePort. "LoadBalancer" means a service will be exposed via an external load balancer (if the cloud provider supports it). "ClusterIP" means a service will only be accessible inside the cluster, via the cluster IP. "NodePort" means a service will be exposed on a static Port on all Nodes of the cluster. | | `loadBalancerClass` _string_ | LoadBalancerClass, when specified, allows for choosing the LoadBalancer provider implementation if more than one are available or is otherwise expected to be specified | | `allocateLoadBalancerNodePorts` _boolean_ | AllocateLoadBalancerNodePorts defines if NodePorts will be automatically allocated for services with type LoadBalancer. Default is "true". It may be set to "false" if the cluster load-balancer does not rely on NodePorts. If the caller requests specific NodePorts (by specifying a value), those requests will be respected, regardless of this field. This field may only be set for services with type LoadBalancer and will be cleared if the type is changed to any other type. | +| `loadBalancerIP` _string_ | LoadBalancerIP defines the IP Address of the underlying load balancer service. This field may be ignored if the load balancer provider does not support this feature. This field has been Deprecated, but it is still used for set the IP Address in some cloud providers such as GCP. | #### KubernetesWatchMode From 53d159e1a4a77928b7a733381ea1bdc3913696e8 Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Thu, 19 Oct 2023 13:05:46 -0700 Subject: [PATCH 2/3] typo Signed-off-by: Arko Dasgupta --- api/v1alpha1/shared_types.go | 2 +- .../crds/generated/gateway.envoyproxy.io_envoyproxies.yaml | 6 +++--- site/content/en/latest/api/extension_types.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/v1alpha1/shared_types.go b/api/v1alpha1/shared_types.go index ea207b73891..61dab908975 100644 --- a/api/v1alpha1/shared_types.go +++ b/api/v1alpha1/shared_types.go @@ -193,7 +193,7 @@ type KubernetesServiceSpec struct { // LoadBalancerIP defines the IP Address of the underlying load balancer service. This field // may be ignored if the load balancer provider does not support this feature. - // This field has been Deprecated, but it is still used for set the IP Address in some cloud + // This field has been deprecated in Kubernetes, but it is still used for setting the IP Address in some cloud // providers such as GCP. // +optional LoadBalancerIP *string `json:"loadBalancerIP,omitempty"` diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml index 1e224b32a68..f3af9f56801 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_envoyproxies.yaml @@ -5262,9 +5262,9 @@ spec: description: LoadBalancerIP defines the IP Address of the underlying load balancer service. This field may be ignored if the load balancer provider does not support - this feature. This field has been Deprecated, but it - is still used for set the IP Address in some cloud providers - such as GCP. + this feature. This field has been deprecated in Kubernetes, + but it is still used for setting the IP Address in some + cloud providers such as GCP. type: string type: default: LoadBalancer diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 0b17c1a2435..53bd49902c5 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -847,7 +847,7 @@ _Appears in:_ | `type` _[ServiceType](#servicetype)_ | Type determines how the Service is exposed. Defaults to LoadBalancer. Valid options are ClusterIP, LoadBalancer and NodePort. "LoadBalancer" means a service will be exposed via an external load balancer (if the cloud provider supports it). "ClusterIP" means a service will only be accessible inside the cluster, via the cluster IP. "NodePort" means a service will be exposed on a static Port on all Nodes of the cluster. | | `loadBalancerClass` _string_ | LoadBalancerClass, when specified, allows for choosing the LoadBalancer provider implementation if more than one are available or is otherwise expected to be specified | | `allocateLoadBalancerNodePorts` _boolean_ | AllocateLoadBalancerNodePorts defines if NodePorts will be automatically allocated for services with type LoadBalancer. Default is "true". It may be set to "false" if the cluster load-balancer does not rely on NodePorts. If the caller requests specific NodePorts (by specifying a value), those requests will be respected, regardless of this field. This field may only be set for services with type LoadBalancer and will be cleared if the type is changed to any other type. | -| `loadBalancerIP` _string_ | LoadBalancerIP defines the IP Address of the underlying load balancer service. This field may be ignored if the load balancer provider does not support this feature. This field has been Deprecated, but it is still used for set the IP Address in some cloud providers such as GCP. | +| `loadBalancerIP` _string_ | LoadBalancerIP defines the IP Address of the underlying load balancer service. This field may be ignored if the load balancer provider does not support this feature. This field has been deprecated in Kubernetes, but it is still used for setting the IP Address in some cloud providers such as GCP. | #### KubernetesWatchMode From 3834fe2b2d582f9071e1a1bcdd21444bb5312a0d Mon Sep 17 00:00:00 2001 From: Arko Dasgupta Date: Fri, 20 Oct 2023 12:27:31 -0700 Subject: [PATCH 3/3] reject ipv6 Signed-off-by: Arko Dasgupta --- .../validation/envoyproxy_validate.go | 4 ++-- .../validation/envoyproxy_validate_test.go | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/api/v1alpha1/validation/envoyproxy_validate.go b/api/v1alpha1/validation/envoyproxy_validate.go index c5f8d6becbe..73f8a5e7aaf 100644 --- a/api/v1alpha1/validation/envoyproxy_validate.go +++ b/api/v1alpha1/validation/envoyproxy_validate.go @@ -101,8 +101,8 @@ func validateService(spec *egv1a1.EnvoyProxySpec) []error { errs = append(errs, fmt.Errorf("loadBalancerIP can only be set for %v type", egv1a1.ServiceTypeLoadBalancer)) } - if net.ParseIP(*serviceLoadBalancerIP) == nil { - errs = append(errs, fmt.Errorf("loadBalancerIP:%s is an invalid IP address", *serviceLoadBalancerIP)) + if ip := net.ParseIP(*serviceLoadBalancerIP); ip == nil || ip.To4() == nil { + errs = append(errs, fmt.Errorf("loadBalancerIP:%s is an invalid IPv4 address", *serviceLoadBalancerIP)) } } } diff --git a/api/v1alpha1/validation/envoyproxy_validate_test.go b/api/v1alpha1/validation/envoyproxy_validate_test.go index c2b3bde71e6..a692c0caba0 100644 --- a/api/v1alpha1/validation/envoyproxy_validate_test.go +++ b/api/v1alpha1/validation/envoyproxy_validate_test.go @@ -255,6 +255,28 @@ func TestValidateEnvoyProxy(t *testing.T) { }, expected: false, }, + { + name: "envoy service type 'LoadBalancer' with ipv6 loadBalancerIP", + proxy: &egv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egv1a1.EnvoyProxySpec{ + Provider: &egv1a1.EnvoyProxyProvider{ + Type: egv1a1.ProviderTypeKubernetes, + Kubernetes: &egv1a1.EnvoyProxyKubernetesProvider{ + EnvoyService: &egv1a1.KubernetesServiceSpec{ + Type: egv1a1.GetKubernetesServiceType(egv1a1.ServiceTypeLoadBalancer), + LoadBalancerIP: ptr.To("2001:db8::68"), + }, + }, + }, + }, + }, + expected: false, + }, + { name: "valid user bootstrap replace type", proxy: &egv1a1.EnvoyProxy{