Repository: knative/serving
Pull request #13636 (2023-01-23 18:29:19)

Change HTTP01 test DNS entry to *.{custom-domain}
Part of https://github.com/knative-sandbox/net-certmanager/issues/214#issuecomment-1400611756

Was testing it via https://github.com/knative/serving/pull/13621

This allows `net-certmanager` to use `commonName` of the form `k.{custom-domain}`

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

This pull-request has been approved by: *<a href="https://github.com/knative/serving/pull/13636#" title="Author self-approved">dprotaso</a>*

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

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:

- ~~[test/OWNERS](https://github.com/knative/serving/blob/main/test/OWNERS)~~ [dprotaso]

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":[]} -->
dprotaso: /assign @KauzClay 
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/serving/pull/13636?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
Base: **86.17**% // Head: **86.23**% // Increases project coverage by **`+0.05%`** :tada:
> Coverage data is based on head [(`ff417c9`)](https://codecov.io/gh/knative/serving/pull/13636?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) compared to base [(`87f5b62`)](https://codecov.io/gh/knative/serving/commit/87f5b626a5814cf6741b0d95323a032b1955f670?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).
> Patch has no changes to coverable lines.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main   #13636      +/-   ##
==========================================
+ Coverage   86.17%   86.23%   +0.05%     
==========================================
  Files         197      197              
  Lines       14722    14722              
==========================================
+ Hits        12687    12695       +8     
+ Misses       1734     1727       -7     
+ Partials      301      300       -1     
```


| [Impacted Files](https://codecov.io/gh/knative/serving/pull/13636?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [pkg/autoscaler/scaling/multiscaler.go](https://codecov.io/gh/knative/serving/pull/13636?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL2F1dG9zY2FsZXIvc2NhbGluZy9tdWx0aXNjYWxlci5nbw==) | `88.59% <0.00%> (+1.34%)` | :arrow_up: |
| [pkg/reconciler/configuration/configuration.go](https://codecov.io/gh/knative/serving/pull/13636?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL3JlY29uY2lsZXIvY29uZmlndXJhdGlvbi9jb25maWd1cmF0aW9uLmdv) | `84.36% <0.00%> (+1.42%)` | :arrow_up: |
| [pkg/autoscaler/statserver/server.go](https://codecov.io/gh/knative/serving/pull/13636?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-cGtnL2F1dG9zY2FsZXIvc3RhdHNlcnZlci9zZXJ2ZXIuZ28=) | `78.10% <0.00%> (+2.18%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/knative/serving/pull/13636?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).

KauzClay: /lgtm
knative-prow[bot]: @dprotaso: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
upgrade-tests_serving_main | ff417c94fd3fb1da10bb890ede713259dde238fc | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_serving/13636/upgrade-tests_serving_main/1617590327504277504) | true | `/test upgrade-tests`

[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 -->
dprotaso: This change doesn't affect any of the tests

/retest

Patch:
diff --git a/pkg/reconciler/domainmapping/resources/ingress.go b/pkg/reconciler/domainmapping/resources/ingress.go
index d8903c9aa..f9f3b2874 100644
--- a/pkg/reconciler/domainmapping/resources/ingress.go
+++ b/pkg/reconciler/domainmapping/resources/ingress.go
@@ -19,6 +19,7 @@ package resources
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/apimachinery/pkg/util/sets"
 
 	netapi "knative.dev/networking/pkg/apis/networking"
 	netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
@@ -35,6 +36,7 @@ import (
 // KIngress).  The created ingress will contain a RewriteHost rule to cause the
 // given hostName to be used as the host.
 func MakeIngress(dm *servingv1alpha1.DomainMapping, backendServiceName, hostName, ingressClass string, httpOption netv1alpha1.HTTPOption, tls []netv1alpha1.IngressTLS, acmeChallenges ...netv1alpha1.HTTP01Challenge) *netv1alpha1.Ingress {
+	paths, hosts := routeresources.MakeACMEIngressPaths(acmeChallenges, sets.NewString(dm.GetName()))
 	return &netv1alpha1.Ingress{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      kmeta.ChildName(dm.GetName(), ""),
@@ -52,11 +54,11 @@ func MakeIngress(dm *servingv1alpha1.DomainMapping, backendServiceName, hostName
 			HTTPOption: httpOption,
 			TLS:        tls,
 			Rules: []netv1alpha1.IngressRule{{
-				Hosts:      []string{dm.Name},
+				Hosts:      append(hosts, dm.Name),
 				Visibility: netv1alpha1.IngressVisibilityExternalIP,
 				HTTP: &netv1alpha1.HTTPIngressRuleValue{
 					// The order of the paths is sensitive, always put tls challenge first
-					Paths: append(routeresources.MakeACMEIngressPaths(acmeChallenges, dm.GetName()),
+					Paths: append(paths,
 						[]netv1alpha1.HTTPIngressPath{{
 							RewriteHost: hostName,
 							Splits: []netv1alpha1.IngressBackendSplit{{
diff --git a/pkg/reconciler/route/resources/ingress.go b/pkg/reconciler/route/resources/ingress.go
index f6cf82618..5046b4861 100644
--- a/pkg/reconciler/route/resources/ingress.go
+++ b/pkg/reconciler/route/resources/ingress.go
@@ -185,8 +185,9 @@ func makeIngressSpec(
 			}
 			// If this is a public rule, we need to configure ACME challenge paths.
 			if visibility == netv1alpha1.IngressVisibilityExternalIP {
-				rule.HTTP.Paths = append(
-					MakeACMEIngressPaths(acmeChallenges, domains...), rule.HTTP.Paths...)
+				paths, hosts := MakeACMEIngressPaths(acmeChallenges, domains)
+				rule.Hosts = append(hosts, rule.Hosts...)
+				rule.HTTP.Paths = append(paths, rule.HTTP.Paths...)
 			}
 			rules = append(rules, rule)
 		}
@@ -214,7 +215,7 @@ func getChallengeHosts(challenges []netv1alpha1.HTTP01Challenge) map[string]netv
 	return c
 }
 
-func routeDomain(ctx context.Context, targetName string, r *servingv1.Route, visibility netv1alpha1.IngressVisibility) ([]string, error) {
+func routeDomain(ctx context.Context, targetName string, r *servingv1.Route, visibility netv1alpha1.IngressVisibility) (sets.String, error) {
 	hostname, err := domains.HostnameFromTemplate(ctx, r.Name, targetName)
 	if err != nil {
 		return nil, err
@@ -232,18 +233,20 @@ func routeDomain(ctx context.Context, targetName string, r *servingv1.Route, vis
 	if isClusterLocal {
 		domains = ingress.ExpandedHosts(sets.NewString(domains...)).List()
 	}
-	return domains, err
+	return sets.NewString(domains...), err
 }
 
 // MakeACMEIngressPaths returns a set of netv1alpha1.HTTPIngressPath
 // that can be used to perform ACME challenges.
-func MakeACMEIngressPaths(acmeChallenges []netv1alpha1.HTTP01Challenge, domains ...string) []netv1alpha1.HTTPIngressPath {
+func MakeACMEIngressPaths(acmeChallenges []netv1alpha1.HTTP01Challenge, domains sets.String) ([]netv1alpha1.HTTPIngressPath, []string) {
 	challenges := getChallengeHosts(acmeChallenges)
+
 	paths := make([]netv1alpha1.HTTPIngressPath, 0, len(challenges))
-	for _, domain := range domains {
-		challenge, ok := challenges[domain]
-		if !ok {
-			continue
+	var extraHosts []string
+
+	for _, challenge := range challenges {
+		if !domains.Has(challenge.URL.Host) {
+			extraHosts = append(extraHosts, challenge.URL.Host)
 		}
 
 		paths = append(paths, netv1alpha1.HTTPIngressPath{
@@ -258,16 +261,16 @@ func MakeACMEIngressPaths(acmeChallenges []netv1alpha1.HTTP01Challenge, domains
 			Path: challenge.URL.Path,
 		})
 	}
-	return paths
+	return paths, extraHosts
 }
 
-func makeIngressRule(domains []string, ns string,
+func makeIngressRule(domains sets.String, ns string,
 	visibility netv1alpha1.IngressVisibility,
 	targets traffic.RevisionTargets,
 	roCfgs []*traffic.ConfigurationRollout,
 	encryption bool) netv1alpha1.IngressRule {
 	return netv1alpha1.IngressRule{
-		Hosts:      domains,
+		Hosts:      domains.List(),
 		Visibility: visibility,
 		HTTP: &netv1alpha1.HTTPIngressRuleValue{
 			Paths: []netv1alpha1.HTTPIngressPath{


Test patch:
diff --git a/pkg/reconciler/route/resources/ingress_test.go b/pkg/reconciler/route/resources/ingress_test.go
index cc6516a3e..89dd75eac 100644
--- a/pkg/reconciler/route/resources/ingress_test.go
+++ b/pkg/reconciler/route/resources/ingress_test.go
@@ -26,6 +26,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/apimachinery/pkg/util/sets"
 
 	"knative.dev/networking/pkg/apis/networking"
 	netv1alpha1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
@@ -853,7 +854,7 @@ func TestMakeIngressSpecCorrectRulesWithTagBasedRouting(t *testing.T) {
 
 // One active target.
 func TestMakeIngressRuleVanilla(t *testing.T) {
-	domains := []string{"a.com", "b.org"}
+	domains := sets.NewString("a.com", "b.org")
 	targets := traffic.RevisionTargets{{
 		TrafficTarget: v1.TrafficTarget{
 			ConfigurationName: "config",
@@ -913,7 +914,7 @@ func TestMakeIngressRuleZeroPercentTarget(t *testing.T) {
 			Percent:           ptr.Int64(0),
 		},
 	}}
-	domains := []string{"test.org"}
+	domains := sets.NewString("test.org")
 	tc := &traffic.Config{
 		Targets: map[string]traffic.RevisionTargets{
 			traffic.DefaultTarget: targets,
@@ -969,7 +970,7 @@ func TestMakeIngressRuleTwoTargets(t *testing.T) {
 		},
 	}
 	ro := tc.BuildRollout()
-	domains := []string{"test.org"}
+	domains := sets.NewString("test.org")
 	rule := makeIngressRule(domains, ns, netv1alpha1.IngressVisibilityExternalIP,
 		targets, ro.RolloutsByTag("a-tag"), false /* internal encryption */)
 	expected := netv1alpha1.IngressRule{
diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go
index eff5454be..accca01c3 100644
--- a/pkg/reconciler/route/table_test.go
+++ b/pkg/reconciler/route/table_test.go
@@ -2758,6 +2758,15 @@ func TestReconcileEnableAutoTLS(t *testing.T) {
 						ServiceName:      "cm-solver",
 						ServicePort:      intstr.FromInt(8090),
 						ServiceNamespace: "default",
+					}, {
+						URL: &apis.URL{
+							Scheme: "http",
+							Host:   "k.example.com",
+							Path:   "/.well-known/acme-challenge/challengeToken2",
+						},
+						ServiceName:      "cm-solver",
+						ServicePort:      intstr.FromInt(8090),
+						ServiceNamespace: "default",
 					}},
 				},
 			},
@@ -2788,6 +2797,15 @@ func TestReconcileEnableAutoTLS(t *testing.T) {
 					ServiceName:      "cm-solver",
 					ServicePort:      intstr.FromInt(8090),
 					ServiceNamespace: "default",
+				}, {
+					URL: &apis.URL{
+						Scheme: "http",
+						Host:   "k.example.com",
+						Path:   "/.well-known/acme-challenge/challengeToken2",
+					},
+					ServiceName:      "cm-solver",
+					ServicePort:      intstr.FromInt(8090),
+					ServiceNamespace: "default",
 				}},
 			),
 			simpleK8sService(
diff --git a/test/e2e-auto-tls-tests.sh b/test/e2e-auto-tls-tests.sh
index 861921069..a0a1583cb 100755
--- a/test/e2e-auto-tls-tests.sh
+++ b/test/e2e-auto-tls-tests.sh
@@ -86,7 +86,7 @@ function setup_http01_auto_tls() {
   # Rely on the built-in naming (for logstream)
   unset TLS_SERVICE_NAME
   # The full host name of the Knative Service. This is used to configure the DNS record.
-  export AUTO_TLS_TEST_FULL_HOST_NAME="*.${TLS_TEST_NAMESPACE}.${CUSTOM_DOMAIN_SUFFIX}"
+  export AUTO_TLS_TEST_FULL_HOST_NAME="*.${CUSTOM_DOMAIN_SUFFIX}"
 
   kubectl delete kcert --all -n "${TLS_TEST_NAMESPACE}"