feat(dataplane): move DataPlane ports validation to ValidationAdmissi…

…onPolicy and ValidationAdmissionPolicyBinding

CHANGELOG.md

@@ -46,6 +46,8 @@
   [#976](https://github.com/Kong/gateway-operator/pull/976)
 - Move `ControlPlane` `image` validation to CRD CEL rules.
   [#984](https://github.com/Kong/gateway-operator/pull/984)
+- Move `DataPlane` ports validation to `ValidationAdmissionPolicy` and `ValidationAdmissionPolicyBinding`.
+  [#1007](https://github.com/Kong/gateway-operator/pull/1007)
 
 ### Fixes
 

api/v1beta1/dataplane_types.go

@@ -203,6 +203,8 @@ type DataPlaneServiceOptions struct {
 	// 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.
+	//
+	// +kubebuilder:validation:MaxItems=4
 	Ports []DataPlaneServicePort `json:"ports,omitempty"`
 
 	// ServiceOptions is the struct containing service options shared with

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

@@ -9034,6 +9034,7 @@ spec:
                               required:
                               - port
                               type: object
+                            maxItems: 4
                             type: array
                           type:
                             default: LoadBalancer

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

@@ -9034,6 +9034,7 @@ spec:
                               required:
                               - port
                               type: object
+                            maxItems: 4
                             type: array
                           type:
                             default: LoadBalancer

config/default/kustomization.yaml

@@ -7,6 +7,7 @@ namePrefix: gateway-operator-
 resources:
 - ../rbac
 - ../manager
+- ./validation_policies/
 
 patches:
 - path: manager_auth_proxy_patch.yaml

config/default/validation_policies/dataplane_validation_policies.yaml

@@ -0,0 +1,73 @@
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicy
+metadata:
+  name: ports.dataplane.gateway-operator.konghq.com
+spec:
+  matchConstraints:
+    resourceRules:
+      - apiGroups:
+          - "gateway-operator.konghq.com"
+        apiVersions:
+          - "v1beta1"
+        operations:
+          - "CREATE"
+          - "UPDATE"
+        resources:
+          - "dataplanes"
+  variables:
+  - name: ingressPorts
+    expression: object.spec.network.services.ingress.ports
+  - name: podTemplateSpec
+    expression: object.spec.deployment.podTemplateSpec
+  - name: proxyContainer
+    expression: |
+      variables.podTemplateSpec.spec.containers.exists(c, c.name == 'proxy') ?
+        variables.podTemplateSpec.spec.containers.filter(c, c.name == 'proxy')[0] :
+        null
+  - name: envFilteredPortMaps
+    expression: |
+      variables.proxyContainer.env.filter(e, e.name == "KONG_PORT_MAPS")
+  - name: envFilteredProxyListen
+    expression: |
+      variables.proxyContainer.env.filter(e, e.name == "KONG_PROXY_LISTEN")
+  - name: envPortMaps
+    expression: |
+      variables.envFilteredPortMaps.size() > 0 ? variables.envFilteredPortMaps[0] : null
+  - name: envProxyListen
+    expression: |
+      variables.envFilteredProxyListen.size() > 0 ? variables.envFilteredProxyListen[0] : null
+  # Using string functions from: https://pkg.go.dev/github.com/google/cel-go/ext
+  validations:
+  - messageExpression: "'Each port from spec.network.services.ingress.ports has to have an accompanying port in KONG_PORT_MAPS env'"
+    expression: |
+        variables.ingressPorts == null ||
+        variables.envPortMaps == null ||
+        variables.ingressPorts.all(p, variables.envPortMaps.value.
+                  split(",").
+                  exists(pm,
+                      pm.split(":")[1].trim() == string(p.targetPort)
+                      )
+                  )
+    reason: Invalid
+  - messageExpression: "'Each port from spec.network.services.ingress.ports has to have an accompanying port in KONG_PROXY_LISTEN env'"
+    expression: |
+        variables.ingressPorts == null ||
+        variables.envProxyListen == null ||
+        variables.ingressPorts.all(p, variables.envProxyListen.value.
+                  split(",").
+                  exists(pm,
+                    pm.trim().split(" ")[0].split(":")[1].trim() == string(p.targetPort)
+                    )
+                  )
+    reason: Invalid
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: ValidatingAdmissionPolicyBinding
+metadata:
+  name: binding-ports.dataplane.gateway-operator.konghq.com
+spec:
+  policyName: ports.dataplane.gateway-operator.konghq.com
+  validationActions:
+  - Deny
+---

config/default/validation_policies/kustomization.yaml

@@ -0,0 +1,5 @@
+apiVersion: kustomize.config.k8s.io/v1beta1
+kind: Kustomization
+
+resources:
+- dataplane_validation_policies.yaml

config/samples/dataplane-with-custom-ports.yaml

@@ -44,4 +44,3 @@ spec:
           - name: https
             port: 8083
             targetPort: 9443
-

internal/validation/dataplane/validation.go

@@ -4,14 +4,9 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net"
-	"strconv"
-	"strings"
 
 	"github.com/google/go-cmp/cmp"
-	"github.com/samber/lo"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	operatorv1beta1 "github.com/kong/gateway-operator/api/v1beta1"
@@ -134,115 +129,5 @@ func (v *Validator) ValidateDataPlaneDeploymentOptions(namespace string, opts *o
 func (v *Validator) ValidateDataPlaneIngressServiceOptions(
 	namespace string, opts *operatorv1beta1.DataPlaneServiceOptions, proxyContainer *corev1.Container,
 ) error {
-	if len(opts.Ports) > 0 {
-		kongPortMaps, hasKongPortMaps, err := k8sutils.GetEnvValueFromContainer(context.Background(), proxyContainer, namespace, "KONG_PORT_MAPS", v.c)
-		if err != nil {
-			return err
-		}
-		kongProxyListen, hasProxyListen, err := k8sutils.GetEnvValueFromContainer(context.Background(), proxyContainer, namespace, "KONG_PROXY_LISTEN", v.c)
-		if err != nil {
-			return err
-		}
-
-		var portNumberMap map[int32]int32 = make(map[int32]int32, 0)
-		if hasKongPortMaps {
-			portNumberMap, err = parseKongPortMaps(kongPortMaps)
-			if err != nil {
-				return err
-			}
-
-		}
-
-		var listenPortNumbers []int32 = make([]int32, 0)
-		if hasProxyListen {
-			listenPortNumbers, err = parseKongProxyListenPortNumbers(kongProxyListen)
-			if err != nil {
-				return err
-			}
-
-		}
-
-		for _, port := range opts.Ports {
-			targetPortNumber, err := getTargetPortNumber(port.TargetPort, proxyContainer)
-			if err != nil {
-				return fmt.Errorf("failed to get target port of port %d (port name %s) of ingress service: %w",
-					port.Port, port.Name, err)
-			}
-			if hasKongPortMaps && portNumberMap[port.Port] != targetPortNumber {
-				return fmt.Errorf("KONG_PORT_MAPS specified but target port %s not properly set", port.TargetPort.String())
-			}
-			if hasProxyListen && !lo.Contains(listenPortNumbers, targetPortNumber) {
-				return fmt.Errorf("target port %s not included in KONG_PROXY_LISTEN", port.TargetPort.String())
-			}
-		}
-	}
-
 	return nil
 }
-
-func getTargetPortNumber(targetPort intstr.IntOrString, container *corev1.Container) (int32, error) {
-	switch targetPort.Type {
-	case intstr.Int:
-		return targetPort.IntVal, nil
-	case intstr.String:
-		for _, containerPort := range container.Ports {
-			if containerPort.Name == targetPort.StrVal {
-				return containerPort.ContainerPort, nil
-			}
-		}
-		return 0, fmt.Errorf("port %s not found in container", targetPort.StrVal)
-	}
-
-	return 0, fmt.Errorf("unknown targetPort Type: %v", targetPort.Type)
-}
-
-// parseKongPortMaps parses port maps specified in `proxy_maps` configuration.
-// and returns a map with expose port -> listening port.
-// For example, "80:8000,443:8443" will be parsed into map{80:8000,443:8443}.
-func parseKongPortMaps(kongPortMapEnv string) (map[int32]int32, error) {
-	portMaps := strings.Split(kongPortMapEnv, ",")
-	portNumberMap := map[int32]int32{}
-	for _, port := range portMaps {
-		parts := strings.SplitN(port, ":", 2)
-		if len(parts) != 2 {
-			return nil, fmt.Errorf("port map item %s cannot be parsed into 'port:port' format", port)
-		}
-		servicePort, err := strconv.ParseInt(parts[0], 10, 32)
-		if err != nil {
-			return nil, fmt.Errorf("port %s cannot be parsed into number: %w", parts[0], err)
-		}
-		targetPort, err := strconv.ParseInt(parts[1], 10, 32)
-		if err != nil {
-			return nil, fmt.Errorf("port %s cannot be parsed into number: %w", parts[1], err)
-		}
-		portNumberMap[int32(servicePort)] = int32(targetPort)
-	}
-	return portNumberMap, nil
-}
-
-// parseKongProxyListenPortNumbers parses `proxy_listen` configuration to listening ports.
-// It returns the list of listening port numbers.  For example,
-// `"0.0.0.0:8000 reuseport backlog=16384, 0.0.0.0:8443 http2 ssl reuseport backlog=16384`
-// will be parsed into []int32{8000,8443}.
-func parseKongProxyListenPortNumbers(kongProxyListenEnv string) ([]int32, error) {
-	listenAddresses := strings.Split(kongProxyListenEnv, ",")
-	retPorts := make([]int32, 0, len(listenAddresses))
-	for _, addr := range listenAddresses {
-		addr = strings.Trim(addr, " ")
-		// The splitted single listen address would be a list of strings starting with the host and port
-		// and following with options of listening separated by spaces, like `0.0.0.0:8000 reuseport backlog=16384`.
-		// So we extract the part before the first space as the host and port.
-		// It is possible that the listen port have only one part like `0.0.0.0:8000` so we do not check presence of space.
-		hostPort, _, _ := strings.Cut(addr, " ")
-		_, port, err := net.SplitHostPort(hostPort)
-		if err != nil {
-			return nil, fmt.Errorf("listening address %s cannot be parsed into host:port format: %w", hostPort, err)
-		}
-		portNum, err := strconv.ParseInt(port, 10, 32)
-		if err != nil {
-			return nil, fmt.Errorf("listening port %s cannot be parsed to number: %w", port, err)
-		}
-		retPorts = append(retPorts, int32(portNum))
-	}
-	return retPorts, nil
-}

internal/validation/dataplane/validation_test.go

@@ -412,135 +412,6 @@ func TestDataPlaneIngressServiceOptions(t *testing.T) {
 			},
 			hasError: false,
 		},
-		{
-			msg: "dataplane with ingress service options having target port name not found in proxy container should be invalid",
-			dataplane: &operatorv1beta1.DataPlane{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-db-off-in-secret",
-					Namespace: "default",
-				},
-				Spec: operatorv1beta1.DataPlaneSpec{
-					DataPlaneOptions: operatorv1beta1.DataPlaneOptions{
-						Deployment: operatorv1beta1.DataPlaneDeploymentOptions{
-							DeploymentOptions: operatorv1beta1.DeploymentOptions{
-								PodTemplateSpec: &corev1.PodTemplateSpec{
-									Spec: corev1.PodSpec{
-										Containers: []corev1.Container{
-											{
-												Name:  consts.DataPlaneProxyContainerName,
-												Image: consts.DefaultDataPlaneImage,
-												Ports: []corev1.ContainerPort{
-													{Name: "http", ContainerPort: int32(8080)},
-												},
-											},
-										},
-									},
-								},
-							},
-						},
-						Network: operatorv1beta1.DataPlaneNetworkOptions{
-							Services: &operatorv1beta1.DataPlaneServices{
-								Ingress: &operatorv1beta1.DataPlaneServiceOptions{
-									Ports: []operatorv1beta1.DataPlaneServicePort{
-										{Name: "http", Port: int32(80), TargetPort: intstr.FromString("http")},
-										{Name: "https", Port: int32(443), TargetPort: intstr.FromString("https")}, // container port name not found
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			hasError: true,
-			errMsg:   "failed to get target port of port 443 (port name https) of ingress service: port https not found in container",
-		},
-		{
-			msg: "dataplane with ingress service options having target port not in KONG_PORT_MAPS should be invalid",
-			dataplane: &operatorv1beta1.DataPlane{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-db-off-in-secret",
-					Namespace: "default",
-				},
-				Spec: operatorv1beta1.DataPlaneSpec{
-					DataPlaneOptions: operatorv1beta1.DataPlaneOptions{
-						Deployment: operatorv1beta1.DataPlaneDeploymentOptions{
-							DeploymentOptions: operatorv1beta1.DeploymentOptions{
-								PodTemplateSpec: &corev1.PodTemplateSpec{
-									Spec: corev1.PodSpec{
-										Containers: []corev1.Container{
-											{
-												Name: consts.DataPlaneProxyContainerName,
-												Env: []corev1.EnvVar{
-													{Name: "KONG_PORT_MAPS", Value: "80:8080"},
-													{Name: "KONG_PORT_LISTEN", Value: "0.0.0.0:8080"},
-												},
-												Image: consts.DefaultDataPlaneImage,
-											},
-										},
-									},
-								},
-							},
-						},
-						Network: operatorv1beta1.DataPlaneNetworkOptions{
-							Services: &operatorv1beta1.DataPlaneServices{
-								Ingress: &operatorv1beta1.DataPlaneServiceOptions{
-									Ports: []operatorv1beta1.DataPlaneServicePort{
-										{Name: "http", Port: int32(80), TargetPort: intstr.FromInt(8080)},
-										{Name: "https", Port: int32(443), TargetPort: intstr.FromInt(8443)},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			hasError: true,
-			errMsg:   "KONG_PORT_MAPS specified but target port 8443 not properly set",
-		},
-		{
-			msg: "dataplane with ingress service options having target port not in KONG_PROXY_LISTEN should be invalid",
-			dataplane: &operatorv1beta1.DataPlane{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-db-off-in-secret",
-					Namespace: "default",
-				},
-				Spec: operatorv1beta1.DataPlaneSpec{
-					DataPlaneOptions: operatorv1beta1.DataPlaneOptions{
-						Deployment: operatorv1beta1.DataPlaneDeploymentOptions{
-							DeploymentOptions: operatorv1beta1.DeploymentOptions{
-								PodTemplateSpec: &corev1.PodTemplateSpec{
-									Spec: corev1.PodSpec{
-										Containers: []corev1.Container{
-											{
-												Name: consts.DataPlaneProxyContainerName,
-												Env: []corev1.EnvVar{
-													{Name: "KONG_PORT_MAPS", Value: "80:8080,443:8443,8888:8888"},
-													{Name: "KONG_PROXY_LISTEN", Value: "0.0.0.0:8080 reuseport backlog=16384, 0.0.0.0:8443 http2 ssl reuseport backlog=16384"},
-												},
-												Image: consts.DefaultDataPlaneImage,
-											},
-										},
-									},
-								},
-							},
-						},
-						Network: operatorv1beta1.DataPlaneNetworkOptions{
-							Services: &operatorv1beta1.DataPlaneServices{
-								Ingress: &operatorv1beta1.DataPlaneServiceOptions{
-									Ports: []operatorv1beta1.DataPlaneServicePort{
-										{Name: "http", Port: int32(80), TargetPort: intstr.FromInt(8080)},
-										{Name: "https", Port: int32(443), TargetPort: intstr.FromInt(8443)},
-										{Name: "tcp", Port: int32(8888), TargetPort: intstr.FromInt(8888)},
-									},
-								},
-							},
-						},
-					},
-				},
-			},
-			hasError: true,
-			errMsg:   "target port 8888 not included in KONG_PROXY_LISTEN",
-		},
 	}
 
 	for _, tc := range testCases {