Can we please get this merged?

knative · Sep 11, 2023 · 6b0d948 · 6b0d948
1 parent acec746
commit 6b0d948
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 20 deletions.
diff --git a/pkg/reconciler/knativeserving/ingress/kourier.go b/pkg/reconciler/knativeserving/ingress/kourier.go
@@ -93,24 +93,29 @@ func configureGatewayService(instance *v1beta1.KnativeServing) mf.Transformer {
 				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:
 				return fmt.Errorf("unknown service type %q", serviceType)
 			}
 		}
 
-		// Then configure LoadBalancerIP if set.
+		// Configure LoadBalancerIP if set.
 		if instance.Spec.Ingress.Kourier.ServiceLoadBalancerIP != "" {
 			if svc.Spec.Type != v1.ServiceTypeLoadBalancer {
 				return fmt.Errorf("cannot configure LoadBalancerIP for service type %q", svc.Spec.Type)
 			}
 			svc.Spec.LoadBalancerIP = instance.Spec.Ingress.Kourier.ServiceLoadBalancerIP
 		}
 
+		// Configure HTTPPort/HTTPSPort if set.
+		if instance.Spec.Ingress.Kourier.HTTPPort > 0 || instance.Spec.Ingress.Kourier.HTTPSPort > 0 {
+			if svc.Spec.Type != v1.ServiceTypeNodePort {
+				return fmt.Errorf("cannot configure HTTP(S)Port for service type %q", svc.Spec.Type)
+			}
+			configureGatewayServiceTypeNodePort(instance, svc)
+		}
+
 		// Return any conversion error or nil
 		return scheme.Scheme.Convert(svc, u, nil)
 	}
@@ -150,7 +155,7 @@ func configureBootstrapConfigMap(instance *v1beta1.KnativeServing) mf.Transforme
 	}
 }
 
-func configureGWServiceTypeNodePort(instance *v1beta1.KnativeServing, svc *v1.Service) {
+func configureGatewayServiceTypeNodePort(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

diff --git a/pkg/reconciler/knativeserving/ingress/kourier_test.go b/pkg/reconciler/knativeserving/ingress/kourier_test.go
@@ -75,21 +75,21 @@ func servingInstanceNodePorts(ns string, bootstrapConfigmapName string, httpPort
 
 func TestTransformKourierManifest(t *testing.T) {
 	tests := []struct {
-		name              string
-		instance          *servingv1beta1.KnativeServing
-		expNamespace      string
-		expServiceType    string
-    expServiceLoadBalancerIP string
-		expConfigMapName  string
-		expNodePortsHTTP  int32
-		expNodePortsHTTPS int32
-		expError          error
+		name                     string
+		instance                 *servingv1beta1.KnativeServing
+		expNamespace             string
+		expServiceType           string
+		expServiceLoadBalancerIP string
+		expConfigMapName         string
+		expNodePortsHTTP         int32
+		expNodePortsHTTPS        int32
+		expError                 error
 	}{{
-		name:                     "Replaces Kourier Gateway Namespace, ServiceType and bootstrap cm",
-		instance:                 servingInstance(servingNamespace, "ClusterIP", "my-bootstrap", ""),
-		expNamespace:             servingNamespace,
-		expConfigMapName:         "my-bootstrap",
-		expServiceType:           "ClusterIP",
+		name:             "Replaces Kourier Gateway Namespace, ServiceType and bootstrap cm",
+		instance:         servingInstance(servingNamespace, "ClusterIP", "my-bootstrap", ""),
+		expNamespace:     servingNamespace,
+		expConfigMapName: "my-bootstrap",
+		expServiceType:   "ClusterIP",
 	}, {
 		name:                     "Use Kourier default service type",
 		instance:                 servingInstance(servingNamespace, "" /* empty service type */, "", ""),
@@ -114,7 +114,7 @@ func TestTransformKourierManifest(t *testing.T) {
 		expError:                 fmt.Errorf("cannot configure LoadBalancerIP for service type \"ClusterIP\""),
 	}, {
 		name:             "Use unknown service type",
-		instance:         servingInstance(servingNamespace, "Foo", ""),
+		instance:         servingInstance(servingNamespace, "Foo", "", ""),
 		expNamespace:     servingNamespace,
 		expServiceType:   "Foo",
 		expConfigMapName: kourierDefaultVolumeName,