feat: add field allocateLoadBalancerNodePorts to KubernetesServiceSpec (

envoyproxy#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 <den3tsou@gmail.com> * fix: use the function from library Signed-off-by: Den Tsou <den3tsou@gmail.com> * refactor: merge validateServiceType and validateServiceAllocateLoadBalancerNodePorts Signed-off-by: Den Tsou <den3tsou@gmail.com> * refactor: use builtin library to get pointer instead Signed-off-by: Den Tsou <den3tsou@gmail.com> --------- Signed-off-by: Den Tsou <den3tsou@gmail.com>
arkodg · Sep 19, 2023 · b98c1d1 · b98c1d1
1 parent edaeefc
commit b98c1d1
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
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
@@ -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
@@ -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
@@ -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
@@ -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{