From b98c1d1cc88686765128a0228f9acbed14e7c26f Mon Sep 17 00:00:00 2001 From: Den <6628668+den3tsou@users.noreply.github.com> Date: Tue, 19 Sep 2023 11:24:53 +1000 Subject: [PATCH] feat: add field allocateLoadBalancerNodePorts to KubernetesServiceSpec (#1893) * feat: add field allocateLoadBalancerNodePorts This commit adds a new field allocateLoadBalancerNodePorts to KubernetesServiceSpec. But this field can only be set when the service type is 'LoadBalancer' Signed-off-by: Den Tsou * fix: use the function from library Signed-off-by: Den Tsou * refactor: merge validateServiceType and validateServiceAllocateLoadBalancerNodePorts Signed-off-by: Den Tsou * refactor: use builtin library to get pointer instead Signed-off-by: Den Tsou --------- Signed-off-by: Den Tsou --- api/config/v1alpha1/shared_types.go | 8 ++++ api/config/v1alpha1/validation/validate.go | 14 ++++-- .../v1alpha1/validation/validate_test.go | 45 ++++++++++++++++++- api/config/v1alpha1/zz_generated.deepcopy.go | 5 +++ ...ig.gateway.envoyproxy.io_envoyproxies.yaml | 11 +++++ docs/latest/api/config_types.md | 1 + .../kubernetes/resource/resource.go | 3 ++ .../kubernetes/resource/resource_test.go | 14 ++++++ 8 files changed, 96 insertions(+), 5 deletions(-) diff --git a/api/config/v1alpha1/shared_types.go b/api/config/v1alpha1/shared_types.go index b67ca08b622..a6963813fdb 100644 --- a/api/config/v1alpha1/shared_types.go +++ b/api/config/v1alpha1/shared_types.go @@ -177,6 +177,14 @@ type KubernetesServiceSpec struct { // +optional LoadBalancerClass *string `json:"loadBalancerClass,omitempty"` + // 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. + // +optional + AllocateLoadBalancerNodePorts *bool `json:"allocateLoadBalancerNodePorts,omitempty"` + // TODO: Expose config as use cases are better understood, e.g. labels. } diff --git a/api/config/v1alpha1/validation/validate.go b/api/config/v1alpha1/validation/validate.go index 6a221376aa3..9fc73be1537 100644 --- a/api/config/v1alpha1/validation/validate.go +++ b/api/config/v1alpha1/validation/validate.go @@ -71,15 +71,15 @@ func validateProvider(spec *egcfgv1a1.EnvoyProxySpec) []error { if spec.Provider.Type != egcfgv1a1.ProviderTypeKubernetes { errs = append(errs, fmt.Errorf("unsupported provider type %v", spec.Provider.Type)) } - validateServiceTypeErrs := validateServiceType(spec) - if len(validateServiceTypeErrs) != 0 { - errs = append(errs, validateServiceTypeErrs...) + validateServiceErrs := validateService(spec) + if len(validateServiceErrs) != 0 { + errs = append(errs, validateServiceErrs...) } } return errs } -func validateServiceType(spec *egcfgv1a1.EnvoyProxySpec) []error { +func validateService(spec *egcfgv1a1.EnvoyProxySpec) []error { var errs []error if spec.Provider.Kubernetes != nil && spec.Provider.Kubernetes.EnvoyService != nil { if serviceType := spec.Provider.Kubernetes.EnvoyService.Type; serviceType != nil { @@ -89,6 +89,12 @@ func validateServiceType(spec *egcfgv1a1.EnvoyProxySpec) []error { errs = append(errs, fmt.Errorf("unsupported envoy service type %v", serviceType)) } } + if serviceType, serviceAllocateLoadBalancerNodePorts := + spec.Provider.Kubernetes.EnvoyService.Type, spec.Provider.Kubernetes.EnvoyService.AllocateLoadBalancerNodePorts; serviceType != nil && serviceAllocateLoadBalancerNodePorts != nil { + if *serviceType != egcfgv1a1.ServiceTypeLoadBalancer { + errs = append(errs, fmt.Errorf("allocateLoadBalancerNodePorts can only be set for %v type", egcfgv1a1.ServiceTypeLoadBalancer)) + } + } } return errs } diff --git a/api/config/v1alpha1/validation/validate_test.go b/api/config/v1alpha1/validation/validate_test.go index 754a8911d17..ccebf7d6357 100644 --- a/api/config/v1alpha1/validation/validate_test.go +++ b/api/config/v1alpha1/validation/validate_test.go @@ -18,6 +18,7 @@ import ( "k8s.io/utils/pointer" egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1" + "github.com/envoyproxy/gateway/internal/utils/ptr" ) var ( @@ -111,7 +112,7 @@ func TestValidateEnvoyProxy(t *testing.T) { expected: false, }, { - name: "unsupported envoy service type 'NodePort'", + name: "valid envoy service type 'NodePort'", proxy: &egcfgv1a1.EnvoyProxy{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", @@ -170,6 +171,48 @@ func TestValidateEnvoyProxy(t *testing.T) { }, expected: true, }, + { + name: "envoy service type 'LoadBalancer' with allocateLoadBalancerNodePorts", + proxy: &egcfgv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egcfgv1a1.EnvoyProxySpec{ + Provider: &egcfgv1a1.EnvoyProxyProvider{ + Type: egcfgv1a1.ProviderTypeKubernetes, + Kubernetes: &egcfgv1a1.EnvoyProxyKubernetesProvider{ + EnvoyService: &egcfgv1a1.KubernetesServiceSpec{ + Type: egcfgv1a1.GetKubernetesServiceType(egcfgv1a1.ServiceTypeLoadBalancer), + AllocateLoadBalancerNodePorts: ptr.To[bool](false), + }, + }, + }, + }, + }, + expected: true, + }, + { + name: "non envoy service type 'LoadBalancer' with allocateLoadBalancerNodePorts", + proxy: &egcfgv1a1.EnvoyProxy{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "test", + }, + Spec: egcfgv1a1.EnvoyProxySpec{ + Provider: &egcfgv1a1.EnvoyProxyProvider{ + Type: egcfgv1a1.ProviderTypeKubernetes, + Kubernetes: &egcfgv1a1.EnvoyProxyKubernetesProvider{ + EnvoyService: &egcfgv1a1.KubernetesServiceSpec{ + Type: egcfgv1a1.GetKubernetesServiceType(egcfgv1a1.ServiceTypeClusterIP), + AllocateLoadBalancerNodePorts: ptr.To[bool](false), + }, + }, + }, + }, + }, + expected: false, + }, { name: "valid user bootstrap replace type", proxy: &egcfgv1a1.EnvoyProxy{ diff --git a/api/config/v1alpha1/zz_generated.deepcopy.go b/api/config/v1alpha1/zz_generated.deepcopy.go index 670bd007b9d..76a6146adf8 100644 --- a/api/config/v1alpha1/zz_generated.deepcopy.go +++ b/api/config/v1alpha1/zz_generated.deepcopy.go @@ -811,6 +811,11 @@ func (in *KubernetesServiceSpec) DeepCopyInto(out *KubernetesServiceSpec) { *out = new(string) **out = **in } + if in.AllocateLoadBalancerNodePorts != nil { + in, out := &in.AllocateLoadBalancerNodePorts, &out.AllocateLoadBalancerNodePorts + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KubernetesServiceSpec. diff --git a/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml b/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml index 3538a7ca4e5..c759f858b69 100644 --- a/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml +++ b/charts/gateway-helm/crds/generated/config.gateway.envoyproxy.io_envoyproxies.yaml @@ -3793,6 +3793,17 @@ spec: Envoy service resource. If unspecified, default settings for the manged Envoy service resource are applied. properties: + allocateLoadBalancerNodePorts: + description: 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. + type: boolean annotations: additionalProperties: type: string diff --git a/docs/latest/api/config_types.md b/docs/latest/api/config_types.md index ed187e67b69..e62d2fa6137 100644 --- a/docs/latest/api/config_types.md +++ b/docs/latest/api/config_types.md @@ -551,6 +551,7 @@ _Appears in:_ | `annotations` _object (keys:string, values:string)_ | Annotations that should be appended to the service. By default, no annotations are appended. | | `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. | ## KubernetesWatchMode diff --git a/internal/infrastructure/kubernetes/resource/resource.go b/internal/infrastructure/kubernetes/resource/resource.go index c35dc0577f8..a837302be3d 100644 --- a/internal/infrastructure/kubernetes/resource/resource.go +++ b/internal/infrastructure/kubernetes/resource/resource.go @@ -31,6 +31,9 @@ func ExpectedServiceSpec(service *egcfgv1a1.KubernetesServiceSpec) corev1.Servic if service.LoadBalancerClass != nil { serviceSpec.LoadBalancerClass = service.LoadBalancerClass } + if service.AllocateLoadBalancerNodePorts != nil { + serviceSpec.AllocateLoadBalancerNodePorts = service.AllocateLoadBalancerNodePorts + } // 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 a604dbb7e1d..286f5406082 100644 --- a/internal/infrastructure/kubernetes/resource/resource_test.go +++ b/internal/infrastructure/kubernetes/resource/resource_test.go @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" egcfgv1a1 "github.com/envoyproxy/gateway/api/config/v1alpha1" + "github.com/envoyproxy/gateway/internal/utils/ptr" ) func TestExpectedServiceSpec(t *testing.T) { @@ -51,6 +52,19 @@ func TestExpectedServiceSpec(t *testing.T) { ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal, }, }, + { + name: "LoadBalancerWithAllocateLoadBalancerNodePorts", + args: args{service: &egcfgv1a1.KubernetesServiceSpec{ + Type: egcfgv1a1.GetKubernetesServiceType(egcfgv1a1.ServiceTypeLoadBalancer), + AllocateLoadBalancerNodePorts: ptr.To[bool](true), + }}, + want: corev1.ServiceSpec{ + Type: corev1.ServiceTypeLoadBalancer, + AllocateLoadBalancerNodePorts: ptr.To[bool](true), + SessionAffinity: corev1.ServiceAffinityNone, + ExternalTrafficPolicy: corev1.ServiceExternalTrafficPolicyTypeLocal, + }, + }, { name: "ClusterIP", args: args{service: &egcfgv1a1.KubernetesServiceSpec{