Feat: Add Name in ServiceOptions and allow specifying ingress service…

… name of dataplane (#966)

* Support specifying name of dataplane ingress service

* add integration test and changelog

CHANGELOG.md

@@ -25,6 +25,13 @@
 
 ## Unreleased
 
+### Added
+
+- Added `Name` field in `ServiceOptions` to allow specifying name of the
+  owning service. Currently specifying ingress service of `DataPlane` is
+  supported.
+  [#966](https://github.com/Kong/gateway-operator/pull/966)
+
 ### Changed
 
 - `KonnectExtension` does not require `spec.serverHostname` to be set by a user

api/v1beta1/dataplane_types.go

@@ -260,6 +260,10 @@ type ServiceOptions struct {
 	// +kubebuilder:validation:Enum=LoadBalancer;ClusterIP
 	Type corev1.ServiceType `json:"type,omitempty" protobuf:"bytes,4,opt,name=type,casttype=ServiceType"`
 
+	// Name defines the name of the service.
+	// If Name is empty, the controller will generate a service name from the owning object.
+	Name *string `json:"name,omitempty"`
+
 	// Annotations is an unstructured key value map stored with a resource that may be
 	// set by external tools to store and retrieve arbitrary metadata. They are not
 	// queryable and should be preserved when modifying objects.

config/crd/bases/gateway-operator.konghq.com_dataplanes.yaml

@@ -8989,6 +8989,11 @@ spec:
                             - Cluster
                             - Local
                             type: string
+                          name:
+                            description: |-
+                              Name defines the name of the service.
+                              If Name is empty, the controller will generate a service name from the owning object.
+                            type: string
                           ports:
                             description: |-
                               Ports defines the list of ports that are exposed by the service.

config/crd/bases/gateway-operator.konghq.com_gatewayconfigurations.yaml

@@ -17345,6 +17345,11 @@ spec:
                                 - Cluster
                                 - Local
                                 type: string
+                              name:
+                                description: |-
+                                  Name defines the name of the service.
+                                  If Name is empty, the controller will generate a service name from the owning object.
+                                type: string
                               type:
                                 default: LoadBalancer
                                 description: |-

config/crd/dataplane/gateway-operator.konghq.com_dataplanes.yaml

@@ -8989,6 +8989,11 @@ spec:
                             - Cluster
                             - Local
                             type: string
+                          name:
+                            description: |-
+                              Name defines the name of the service.
+                              If Name is empty, the controller will generate a service name from the owning object.
+                            type: string
                           ports:
                             description: |-
                               Ports defines the list of ports that are exposed by the service.

controller/dataplane/owned_resources.go

@@ -295,7 +295,14 @@ func ensureIngressServiceForDataPlane(
 	}
 
 	count := len(services)
-	if count > 1 {
+	if serviceName := k8sresources.GetDataPlaneIngressServiceName(dataPlane); serviceName != "" {
+		if count > 1 || (count == 1 && services[0].Name != serviceName) {
+			if err := k8sreduce.ReduceServicesByName(ctx, cl, services, serviceName, dataplane.OwnedObjectPreDeleteHook); err != nil {
+				return op.Noop, nil, err
+			}
+			return op.Noop, nil, errors.New("DataPlane ingress services with different names reduced")
+		}
+	} else if count > 1 {
 		if err := k8sreduce.ReduceServices(ctx, cl, services, dataplane.OwnedObjectPreDeleteHook); err != nil {
 			return op.Noop, nil, err
 		}

controller/dataplane/owned_resources_test.go

@@ -30,6 +30,7 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) {
 		existingServiceModifier  func(*testing.T, context.Context, client.Client, *corev1.Service)
 		expectedCreatedOrUpdated op.Result
 		expectedServiceType      corev1.ServiceType
+		expectedServiceName      string
 		expectedServicePorts     []corev1.ServicePort
 		expectedAnnotations      map[string]string
 		expectedLabels           map[string]string
@@ -264,6 +265,24 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) {
 			expectedServiceType:      corev1.ServiceTypeLoadBalancer,
 			expectedServicePorts:     k8sresources.DefaultDataPlaneIngressServicePorts,
 		},
+		{
+			name: "should create service with specified name if name is specified",
+			dataplane: builder.NewDataPlaneBuilder().
+				WithObjectMeta(metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "ingress-service-specified",
+				}).WithIngressServiceName("ingress-service-1").
+				WithIngressServiceType(corev1.ServiceTypeLoadBalancer).
+				Build(),
+			existingServiceModifier: func(t *testing.T, ctx context.Context, c client.Client, svc *corev1.Service) {
+				require.NoError(t, dataplane.OwnedObjectPreDeleteHook(ctx, c, svc))
+				require.NoError(t, c.Delete(ctx, svc))
+			},
+			expectedCreatedOrUpdated: op.Created,
+			expectedServiceType:      corev1.ServiceTypeLoadBalancer,
+			expectedServiceName:      "ingress-service-1",
+			expectedServicePorts:     k8sresources.DefaultDataPlaneIngressServicePorts,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -293,6 +312,10 @@ func TestEnsureIngressServiceForDataPlane(t *testing.T) {
 			)
 			require.NoError(t, err)
 			require.Equal(t, tc.expectedCreatedOrUpdated, res)
+			// check service name.
+			if tc.expectedServiceName != "" {
+				require.Equal(t, tc.expectedServiceName, svc.Name, "should have the same name")
+			}
 			// check service type.
 			require.Equal(t, tc.expectedServiceType, svc.Spec.Type, "should have the same service type")
 			// check service ports.

controller/pkg/builder/builder.go

@@ -57,6 +57,13 @@ func (b *testDataPlaneBuilder) WithIngressServiceType(typ corev1.ServiceType) *t
 	return b
 }
 
+// WithIngressServiceName sets the Name of the Ingress service.
+func (b *testDataPlaneBuilder) WithIngressServiceName(name string) *testDataPlaneBuilder {
+	b.initIngressServiceOptions()
+	b.dataplane.Spec.DataPlaneOptions.Network.Services.Ingress.Name = &name
+	return b
+}
+
 // WithIngressServiceExternalTrafficPolicy sets the ExternalTrafficPolicy of the Ingress service.
 func (b *testDataPlaneBuilder) WithIngressServiceExternalTrafficPolicy(typ corev1.ServiceExternalTrafficPolicyType) *testDataPlaneBuilder {
 	b.initIngressServiceOptions()

docs/api-reference.md

@@ -2616,6 +2616,7 @@ DataPlaneServiceOptions contains Services related DataPlane configuration.
 | --- | --- |
 | `ports` _[DataPlaneServicePort](#dataplaneserviceport) array_ | Ports defines the list of ports that are exposed by the service. The ports field allows defining the name, port and targetPort of the underlying service ports, while the protocol is defaulted to TCP, as it is the only protocol currently supported. |
 | `type` _[ServiceType](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#servicetype-v1-core)_ | Type determines how the Service is exposed. Defaults to `LoadBalancer`.<br /><br /> Valid options are `LoadBalancer` and `ClusterIP`.<br /><br /> `ClusterIP` allocates a cluster-internal IP address for load-balancing to endpoints.<br /><br /> `LoadBalancer` builds on NodePort and creates an external load-balancer (if supported in the current cloud) which routes to the same endpoints as the clusterIP.<br /><br /> More info: https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services-service-types |
+| `name` _string_ | Name defines the name of the service. If Name is empty, the controller will generate a service name from the owning object. |
 | `annotations` _object (keys:string, values:string)_ | Annotations is an unstructured key value map stored with a resource that may be set by external tools to store and retrieve arbitrary metadata. They are not queryable and should be preserved when modifying objects.<br /><br /> More info: http://kubernetes.io/docs/user-guide/annotations |
 | `externalTrafficPolicy` _[ServiceExternalTrafficPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#serviceexternaltrafficpolicy-v1-core)_ | ExternalTrafficPolicy describes how nodes distribute service traffic they receive on one of the Service's "externally-facing" addresses (NodePorts, ExternalIPs, and LoadBalancer IPs). If set to "Local", the proxy will configure the service in a way that assumes that external load balancers will take care of balancing the service traffic between nodes, and so each node will deliver traffic only to the node-local endpoints of the service, without masquerading the client source IP. (Traffic mistakenly sent to a node with no endpoints will be dropped.) The default value, "Cluster", uses the standard behavior of routing to all endpoints evenly (possibly modified by topology and other features). Note that traffic sent to an External IP or LoadBalancer IP from within the cluster will always get "Cluster" semantics, but clients sending to a NodePort from within the cluster may need to take traffic policy into account when picking a node.<br /><br /> More info: https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/#preserving-the-client-source-ip |
 
@@ -2776,6 +2777,7 @@ such as the annotations.
 | Field | Description |
 | --- | --- |
 | `type` _[ServiceType](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#servicetype-v1-core)_ | Type determines how the Service is exposed. Defaults to `LoadBalancer`.<br /><br /> Valid options are `LoadBalancer` and `ClusterIP`.<br /><br /> `ClusterIP` allocates a cluster-internal IP address for load-balancing to endpoints.<br /><br /> `LoadBalancer` builds on NodePort and creates an external load-balancer (if supported in the current cloud) which routes to the same endpoints as the clusterIP.<br /><br /> More info: https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services-service-types |
+| `name` _string_ | Name defines the name of the service. If Name is empty, the controller will generate a service name from the owning object. |
 | `annotations` _object (keys:string, values:string)_ | Annotations is an unstructured key value map stored with a resource that may be set by external tools to store and retrieve arbitrary metadata. They are not queryable and should be preserved when modifying objects.<br /><br /> More info: http://kubernetes.io/docs/user-guide/annotations |
 | `externalTrafficPolicy` _[ServiceExternalTrafficPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#serviceexternaltrafficpolicy-v1-core)_ | ExternalTrafficPolicy describes how nodes distribute service traffic they receive on one of the Service's "externally-facing" addresses (NodePorts, ExternalIPs, and LoadBalancer IPs). If set to "Local", the proxy will configure the service in a way that assumes that external load balancers will take care of balancing the service traffic between nodes, and so each node will deliver traffic only to the node-local endpoints of the service, without masquerading the client source IP. (Traffic mistakenly sent to a node with no endpoints will be dropped.) The default value, "Cluster", uses the standard behavior of routing to all endpoints evenly (possibly modified by topology and other features). Note that traffic sent to an External IP or LoadBalancer IP from within the cluster will always get "Cluster" semantics, but clients sending to a NodePort from within the cluster may need to take traffic policy into account when picking a node.<br /><br /> More info: https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/#preserving-the-client-source-ip |
 
@@ -3062,6 +3064,7 @@ such as the annotations.
 | Field | Description |
 | --- | --- |
 | `type` _[ServiceType](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#servicetype-v1-core)_ | Type determines how the Service is exposed. Defaults to `LoadBalancer`.<br /><br /> Valid options are `LoadBalancer` and `ClusterIP`.<br /><br /> `ClusterIP` allocates a cluster-internal IP address for load-balancing to endpoints.<br /><br /> `LoadBalancer` builds on NodePort and creates an external load-balancer (if supported in the current cloud) which routes to the same endpoints as the clusterIP.<br /><br /> More info: https://kubernetes.io/docs/concepts/services-networking/service/#publishing-services-service-types |
+| `name` _string_ | Name defines the name of the service. If Name is empty, the controller will generate a service name from the owning object. |
 | `annotations` _object (keys:string, values:string)_ | Annotations is an unstructured key value map stored with a resource that may be set by external tools to store and retrieve arbitrary metadata. They are not queryable and should be preserved when modifying objects.<br /><br /> More info: http://kubernetes.io/docs/user-guide/annotations |
 | `externalTrafficPolicy` _[ServiceExternalTrafficPolicy](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#serviceexternaltrafficpolicy-v1-core)_ | ExternalTrafficPolicy describes how nodes distribute service traffic they receive on one of the Service's "externally-facing" addresses (NodePorts, ExternalIPs, and LoadBalancer IPs). If set to "Local", the proxy will configure the service in a way that assumes that external load balancers will take care of balancing the service traffic between nodes, and so each node will deliver traffic only to the node-local endpoints of the service, without masquerading the client source IP. (Traffic mistakenly sent to a node with no endpoints will be dropped.) The default value, "Cluster", uses the standard behavior of routing to all endpoints evenly (possibly modified by topology and other features). Note that traffic sent to an External IP or LoadBalancer IP from within the cluster will always get "Cluster" semantics, but clients sending to a NodePort from within the cluster may need to take traffic policy into account when picking a node.<br /><br /> More info: https://kubernetes.io/docs/tasks/access-application-cluster/create-external-load-balancer/#preserving-the-client-source-ip |
 

pkg/utils/kubernetes/reduce/reduce.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/samber/lo"
 	admregv1 "k8s.io/api/admissionregistration/v1"
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
@@ -132,6 +133,25 @@ func ReduceServices(ctx context.Context, k8sClient client.Client, services []cor
 	return nil
 }
 
+// ReduceServicesByName deletes all service in the list except the one with specified name (if exists).
+// It accepts optional preDeleteHooks which are executed before every Service delete operation.
+func ReduceServicesByName(ctx context.Context, k8sClient client.Client, services []corev1.Service, name string, preDeleteHooks ...PreDeleteHook) error {
+	filteredServices := lo.Filter(services, func(svc corev1.Service, _ int) bool {
+		return svc.Name != name
+	})
+	for _, service := range filteredServices {
+		for _, hook := range preDeleteHooks {
+			if err := hook(ctx, k8sClient, &service); err != nil {
+				return fmt.Errorf("failed to execute pre delete hook: %w", err)
+			}
+		}
+		if err := k8sClient.Delete(ctx, &service); client.IgnoreNotFound(err) != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // +kubebuilder:rbac:groups=networking.k8s.io,resources=networkpolicies,verbs=delete
 
 // ReduceNetworkPolicies detects the best NetworkPolicy in the set and deletes all the others.

pkg/utils/kubernetes/resources/services.go

@@ -49,8 +49,8 @@ func GenerateNewServiceForCertificateConfig(namespace, name string) *corev1.Serv
 func GenerateNewIngressServiceForDataPlane(dataplane *operatorv1beta1.DataPlane, opts ...ServiceOpt) (*corev1.Service, error) {
 	svc := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace:    dataplane.Namespace,
-			GenerateName: k8sutils.TrimGenerateName(fmt.Sprintf("%s-ingress-%s-", consts.DataPlanePrefix, dataplane.Name)),
+			Namespace: dataplane.Namespace,
+
 			Labels: map[string]string{
 				"app":                            dataplane.Name,
 				consts.DataPlaneServiceTypeLabel: string(consts.DataPlaneIngressServiceLabelValue),
@@ -65,6 +65,14 @@ func GenerateNewIngressServiceForDataPlane(dataplane *operatorv1beta1.DataPlane,
 		},
 	}
 
+	// Assign the service name if the DataPlane specifies name of ingress service.
+	if serviceName := GetDataPlaneIngressServiceName(dataplane); serviceName != "" {
+		svc.Name = serviceName
+	} else {
+		// If the service name is not specified, use the generated name.
+		svc.GenerateName = k8sutils.TrimGenerateName(fmt.Sprintf("%s-ingress-%s-", consts.DataPlanePrefix, dataplane.Name))
+	}
+
 	setDataPlaneIngressServiceExternalTrafficPolicy(dataplane, svc)
 	LabelObjectAsDataPlaneManaged(svc)
 
@@ -288,3 +296,16 @@ func GenerateNewAdmissionWebhookServiceForControlPlane(cp *operatorv1beta1.Contr
 
 	return svc, nil
 }
+
+// GetDataPlaneIngressServiceName fetches the specified name of ingress service of dataplane.
+// If the service name is not specified, it returns an empty string.
+func GetDataPlaneIngressServiceName(dataPlane *operatorv1beta1.DataPlane) string {
+	if dataPlane == nil {
+		return ""
+	}
+	dpServices := dataPlane.Spec.DataPlaneOptions.Network.Services
+	if dpServices == nil || dpServices.Ingress == nil || dpServices.Ingress.Name == nil {
+		return ""
+	}
+	return *dpServices.Ingress.Name
+}

pkg/utils/test/predicates.go

@@ -470,6 +470,22 @@ func DataPlaneHasActiveService(t *testing.T, ctx context.Context, dataplaneName
 	}, clients.OperatorClient)
 }
 
+// DataPlaneHasActiveServiceWithName is a helper function for tests that returns a function
+// that can be used to check if a DataPlane has an active proxy service with the specified name.
+// Should be used in conjunction with require.Eventually or assert.Eventually.
+func DataPlaneHasActiveServiceWithName(t *testing.T, ctx context.Context, dataplaneName types.NamespacedName, ret *corev1.Service, clients K8sClients, matchingLabels client.MatchingLabels, name string) func() bool {
+	return DataPlanePredicate(t, ctx, dataplaneName, func(dataplane *operatorv1beta1.DataPlane) bool {
+		services := MustListDataPlaneServices(t, ctx, dataplane, clients.MgrClient, matchingLabels)
+		if len(services) == 1 && services[0].Name == name {
+			if ret != nil {
+				*ret = services[0]
+			}
+			return true
+		}
+		return false
+	}, clients.OperatorClient)
+}
+
 // DataPlaneServiceHasNActiveEndpoints is a helper function for tests that returns a function
 // that can be used to check if a Service has active endpoints.
 // Should be used in conjunction with require.Eventually or assert.Eventually.