Repository: knative-sandbox/net-kourier
Pull request #797 (2022-03-17 10:14:47)

Set `80` port if k8s service omits the port
k8s's ExternalName type service can omit the port as
https://kubernetes.io/docs/concepts/services-networking/service/#externalname.

But current net-kourier assumes that it is `0` port if port was
omitted, then prober fails to probe.
This patch uses `80` port instead of `0` when it was omitted.

__Note__, the k8s service generated by ksvc does not omit the port. This
happens only when users point to the k8s service with ExternalName by
DomainMapping. Please see https://github.com/knative/serving/issues/12722 as well.

Fix https://github.com/knative/serving/issues/12722

```release-note
The bug which DomainMapping is not working with Knative Channel was fixed.
```


knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/net-kourier/pull/797#" title="Author self-approved">nak3</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fnet-kourier).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/net-kourier/blob/main/OWNERS)~~ [nak3]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
nak3: /cc @ZhiminXiang @carlisia 
nak3: /cc @skonto @rhuss 
knative-prow[bot]: @nak3: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
unit-tests_net-kourier_main | f4e84003a7cbca13d57d082bcef2dfdceef3eefb | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative-sandbox_net-kourier/797/unit-tests_net-kourier_main/1514596379538755584) | unknown | `/test unit-tests_net-kourier_main`
build-tests_net-kourier_main | f4e84003a7cbca13d57d082bcef2dfdceef3eefb | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative-sandbox_net-kourier/797/build-tests_net-kourier_main/1514596379505201152) | unknown | `/test build-tests_net-kourier_main`

[Your PR dashboard](https://prow.knative.dev/pr).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
knative-prow[bot]: New changes are detected. LGTM label has been removed.
knative-prow[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/net-kourier/pull/797#" title="Author self-approved">nak3</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fnet-kourier).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/net-kourier/blob/main/OWNERS)~~ [nak3]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
nak3: Thank you @psschwei ! (just rebased.)
psschwei: Unit tests are passing fine locally, so will try rerunning...
/test unit-tests_net-kourier_main
/test build-tests_net-kourier_main

Patch:
diff --git a/pkg/generator/ingress_translator.go b/pkg/generator/ingress_translator.go
index f6d83639..730d04fc 100644
--- a/pkg/generator/ingress_translator.go
+++ b/pkg/generator/ingress_translator.go
@@ -143,9 +143,9 @@ func (translator *IngressTranslator) translateIngress(ctx context.Context, ingre
 				// Match the ingress' port with a port on the Service to find the target.
 				// Also find out if the target supports HTTP2.
 				var (
-					externalPort int32
-					targetPort   int32
-					http2        bool
+					externalPort = int32(80)
+					targetPort   = int32(80)
+					http2        = false
 				)
 				for _, port := range service.Spec.Ports {
 					if port.Port == split.ServicePort.IntVal || port.Name == split.ServicePort.StrVal {


Test patch:
diff --git a/pkg/generator/ingress_translator_test.go b/pkg/generator/ingress_translator_test.go
index 4019ea9b..21909723 100644
--- a/pkg/generator/ingress_translator_test.go
+++ b/pkg/generator/ingress_translator_test.go
@@ -527,6 +527,61 @@ func TestIngressTranslator(t *testing.T) {
 			}
 		}(),
 	}, {
+		name: "external service without service port",
+		in:   ing("testspace", "testname"),
+		state: []runtime.Object{
+			svc("servicens", "servicename", func(svc *corev1.Service) {
+				svc.Spec.Type = corev1.ServiceTypeExternalName
+				svc.Spec.ExternalName = "example.com"
+				svc.Spec.Ports = nil
+			}),
+		},
+		want: func() *translatedIngress {
+			vHosts := []*route.VirtualHost{
+				envoy.NewVirtualHost(
+					"(testspace/testname).Rules[0]",
+					[]string{"foo.example.com", "foo.example.com:*"},
+					[]*route.Route{envoy.NewRoute(
+						"(testspace/testname).Rules[0].Paths[/test]",
+						[]*route.HeaderMatcher{{
+							Name: "testheader",
+							HeaderMatchSpecifier: &route.HeaderMatcher_ExactMatch{
+								ExactMatch: "foo",
+							},
+						}},
+						"/test",
+						[]*route.WeightedCluster_ClusterWeight{
+							envoy.NewWeightedCluster("servicens/servicename", 100, map[string]string{"baz": "gna"}),
+						},
+						0,
+						map[string]string{"foo": "bar"},
+						"rewritten.example.com"),
+					},
+				),
+			}
+
+			return &translatedIngress{
+				name: types.NamespacedName{
+					Namespace: "testspace",
+					Name:      "testname",
+				},
+				sniMatches: []*envoy.SNIMatch{},
+				clusters: []*v3.Cluster{
+					envoy.NewCluster(
+						"servicens/servicename",
+						5*time.Second,
+						[]*endpoint.LbEndpoint{envoy.NewLBEndpoint("example.com", 80)},
+						false,
+						nil,
+						v3.Cluster_LOGICAL_DNS,
+					),
+				},
+				externalVirtualHosts:    vHosts,
+				externalTLSVirtualHosts: []*route.VirtualHost{},
+				internalVirtualHosts:    vHosts,
+			}
+		}(),
+	}, {
 		name: "missing service",
 		in:   ing("testspace", "testname"),
 	}, {