Added Ports to NodePort service-types in knative-serving (#1541)

* Added Ports to NodePort service-types in knative-serving * fix: Linter requirements for uppercase HTTP and HTTPS. Added Fields to CRD. * feat: Added check if field is set before assigning the ports. * test: Added tests for 0 Http/Https NodePorts
knative · Aug 28, 2023 · c4e2316 · c4e2316
1 parent 14d0f77
commit c4e2316
Show file tree

Hide file tree

Showing 4 changed files with 113 additions and 7 deletions.
diff --git a/config/crd/bases/operator.knative.dev_knativeservings.yaml b/config/crd/bases/operator.knative.dev_knativeservings.yaml
@@ -2312,6 +2312,10 @@ spec:
                         type: string
                       bootstrap-configmap:
                         type: string
+                      http-port:
+                        type: integer
+                      https-port:
+                        type: integer
                     type: object
                 type: object
               security:

diff --git a/pkg/apis/operator/base/ingressconfiguration.go b/pkg/apis/operator/base/ingressconfiguration.go
@@ -41,6 +41,12 @@ type KourierIngressConfiguration struct {
 	// ServiceType specifies the service type for kourier gateway.
 	ServiceType v1.ServiceType `json:"service-type,omitempty"`
 
+	// HTTPPort specifies the port used in case of ServiceType = "NodePort" for http traffic
+	HTTPPort int32 `json:"http-port,omitempty"`
+
+	// HTTPSPort specifies the port used in case of ServiceType = "NodePort" for https (encrypted) traffic
+	HTTPSPort int32 `json:"https-port,omitempty"`
+
 	// BootstrapConfigmapName specifies the ConfigMap name which contains envoy bootstrap.
 	BootstrapConfigmapName string `json:"bootstrap-configmap,omitempty"`
 }

diff --git a/pkg/reconciler/knativeserving/ingress/kourier.go b/pkg/reconciler/knativeserving/ingress/kourier.go
@@ -89,8 +89,13 @@ func configureGWServiceType(instance *v1beta1.KnativeServing) mf.Transformer {
 
 			serviceType := instance.Spec.Ingress.Kourier.ServiceType
 			switch serviceType {
-			case v1.ServiceTypeClusterIP, v1.ServiceTypeNodePort, v1.ServiceTypeLoadBalancer:
+			case v1.ServiceTypeClusterIP, v1.ServiceTypeLoadBalancer:
 				svc.Spec.Type = serviceType
+			case v1.ServiceTypeNodePort:
+				svc.Spec.Type = serviceType
+				if instance.Spec.Ingress.Kourier.HTTPPort > 0 || instance.Spec.Ingress.Kourier.HTTPSPort > 0 {
+					configureGWServiceTypeNodePort(instance, svc)
+				}
 			case v1.ServiceTypeExternalName:
 				return fmt.Errorf("unsupported service type %q", serviceType)
 			default:
@@ -138,3 +143,14 @@ func configureBootstrapConfigMap(instance *v1beta1.KnativeServing) mf.Transforme
 		return nil
 	}
 }
+
+func configureGWServiceTypeNodePort(instance *v1beta1.KnativeServing, svc *v1.Service) {
+	for i, v := range svc.Spec.Ports {
+		if v.Name != "https" && instance.Spec.Ingress.Kourier.HTTPPort > 0 {
+			v.NodePort = instance.Spec.Ingress.Kourier.HTTPPort
+		} else if v.Name == "https" && instance.Spec.Ingress.Kourier.HTTPSPort > 0 {
+			v.NodePort = instance.Spec.Ingress.Kourier.HTTPSPort
+		}
+		svc.Spec.Ports[i] = v
+	}
+}
diff --git a/pkg/reconciler/knativeserving/ingress/kourier_test.go b/pkg/reconciler/knativeserving/ingress/kourier_test.go
@@ -52,14 +52,36 @@ func servingInstance(ns string, serviceType v1.ServiceType, bootstrapConfigmapNa
 	}
 }
 
+func servingInstanceNodePorts(ns string, bootstrapConfigmapName string, httpPort int32, httpsPort int32) *servingv1beta1.KnativeServing {
+	return &servingv1beta1.KnativeServing{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-instance",
+			Namespace: ns,
+		},
+		Spec: servingv1beta1.KnativeServingSpec{
+			Ingress: &servingv1beta1.IngressConfigs{
+				Kourier: base.KourierIngressConfiguration{
+					Enabled:                true,
+					ServiceType:            "NodePort",
+					BootstrapConfigmapName: bootstrapConfigmapName,
+					HTTPPort:               httpPort,
+					HTTPSPort:              httpsPort,
+				},
+			},
+		},
+	}
+}
+
 func TestTransformKourierManifest(t *testing.T) {
 	tests := []struct {
-		name             string
-		instance         *servingv1beta1.KnativeServing
-		expNamespace     string
-		expServiceType   string
-		expConfigMapName string
-		expError         error
+		name              string
+		instance          *servingv1beta1.KnativeServing
+		expNamespace      string
+		expServiceType    string
+		expConfigMapName  string
+		expNodePortsHTTP  int32
+		expNodePortsHTTPS int32
+		expError          error
 	}{{
 		name:             "Replaces Kourier Gateway Namespace, ServiceType and bootstrap cm",
 		instance:         servingInstance(servingNamespace, "ClusterIP", "my-bootstrap"),
@@ -86,6 +108,30 @@ func TestTransformKourierManifest(t *testing.T) {
 		expServiceType:   "Foo",
 		expConfigMapName: kourierDefaultVolumeName,
 		expError:         fmt.Errorf("unknown service type \"Foo\""),
+	}, {
+		name:              "Use NodePort service type",
+		instance:          servingInstanceNodePorts(servingNamespace, "", 30001, 30002),
+		expNamespace:      servingNamespace,
+		expServiceType:    "NodePort",
+		expNodePortsHTTP:  30001,
+		expNodePortsHTTPS: 30002,
+		expConfigMapName:  kourierDefaultVolumeName,
+	}, {
+		name:              "Use NodePort service type with unset HTTP Port",
+		instance:          servingInstanceNodePorts(servingNamespace, "", 0, 30002),
+		expNamespace:      servingNamespace,
+		expServiceType:    "NodePort",
+		expNodePortsHTTP:  0,
+		expNodePortsHTTPS: 30002,
+		expConfigMapName:  kourierDefaultVolumeName,
+	}, {
+		name:              "Use NodePort service type with unset HTTPS Port",
+		instance:          servingInstanceNodePorts(servingNamespace, "", 30001, 0),
+		expNamespace:      servingNamespace,
+		expServiceType:    "NodePort",
+		expNodePortsHTTP:  30001,
+		expNodePortsHTTPS: 0,
+		expConfigMapName:  kourierDefaultVolumeName,
 	}}
 
 	for _, tt := range tests {
@@ -116,12 +162,46 @@ func TestTransformKourierManifest(t *testing.T) {
 			for _, u := range manifest.Resources() {
 				verifyControllerNamespace(t, &u, tt.expNamespace)
 				verifyGatewayServiceType(t, &u, tt.expServiceType)
+				verifyGatewayServiceTypeNodePortHTTP(t, &u, tt.expNodePortsHTTP)
+				verifyGatewayServiceTypeNodePortHTTPS(t, &u, tt.expNodePortsHTTPS)
 				verifyBootstrapVolumeName(t, &u, tt.expConfigMapName)
 			}
 		})
 	}
 }
 
+func verifyGatewayServiceTypeNodePortHTTP(t *testing.T, u *unstructured.Unstructured, expHTTPPort int32) {
+	if u.GetKind() == "Service" && u.GetName() == kourierGatewayServiceName {
+		svc := &v1.Service{}
+		err := scheme.Scheme.Convert(u, svc, nil)
+		util.AssertEqual(t, err, nil)
+		svcPorts := svc.Spec.Ports
+		var resultPort int32
+		for _, port := range svcPorts {
+			if port.Name != "https" {
+				resultPort = port.NodePort
+			}
+		}
+		util.AssertDeepEqual(t, resultPort, expHTTPPort)
+	}
+}
+
+func verifyGatewayServiceTypeNodePortHTTPS(t *testing.T, u *unstructured.Unstructured, expHTTPSPort int32) {
+	if u.GetKind() == "Service" && u.GetName() == kourierGatewayServiceName {
+		svc := &v1.Service{}
+		err := scheme.Scheme.Convert(u, svc, nil)
+		util.AssertEqual(t, err, nil)
+		svcPorts := svc.Spec.Ports
+		var resultPort int32
+		for _, port := range svcPorts {
+			if port.Name == "https" {
+				resultPort = port.NodePort
+			}
+		}
+		util.AssertDeepEqual(t, resultPort, expHTTPSPort)
+	}
+}
+
 func verifyControllerNamespace(t *testing.T, u *unstructured.Unstructured, expNamespace string) {
 	if u.GetKind() == "Deployment" && kourierControllerDeploymentNames.Has(u.GetName()) {
 		deployment := &appsv1.Deployment{}