From c4e2316e03d79c57d48daff9f735197e73f4ea6b Mon Sep 17 00:00:00 2001 From: Marc Ende <380468+eBeyond@users.noreply.github.com> Date: Mon, 28 Aug 2023 18:21:30 +0200 Subject: [PATCH] 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 --- .../operator.knative.dev_knativeservings.yaml | 4 + .../operator/base/ingressconfiguration.go | 6 ++ .../knativeserving/ingress/kourier.go | 18 +++- .../knativeserving/ingress/kourier_test.go | 92 +++++++++++++++++-- 4 files changed, 113 insertions(+), 7 deletions(-) diff --git a/config/crd/bases/operator.knative.dev_knativeservings.yaml b/config/crd/bases/operator.knative.dev_knativeservings.yaml index 45f6641cb8..e59e59f147 100644 --- 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 index c349b4e44b..7c98a8f399 100644 --- 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 index ca0ece70bb..a76bee20ef 100644 --- 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 index ef22eee661..9a38127a85 100644 --- 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{}