Repository: knative/pkg
Pull request #2092 (2021-04-09 19:45:42)

resolve cluster domain using env CLUSTER_DOMAIN as a fallback
# Changes

- :gift: Resolve cluster domain with `CLUSTER_DOMAIN` as a fallback.

/kind enhancement

This will help with tests that are attempting to resolve cluster local urls from outside the cluster, that do not have access to `/etc/resolv.conf` directly. 

**Release Note**

<!--
:page_facing_up: If this change has user-visible impact, write a release note in the block
below. Include the string "action required" if additional action is required of
users switching to the new release, for example in case of a breaking change.

Write as if you are speaking to users, not other Knative contributors. If this
change has no user-visible impact, no release-note is needed.
-->
```release-note
network.GetServiceHostname and network.GetClusterDomainName now respect the env var CLUSTER_DOMAIN.
```


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

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please ask for approval from **n3wscott** after the PR has been reviewed.

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

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

- **[network/OWNERS](https://github.com/knative/pkg/blob/main/network/OWNERS)**

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":["n3wscott"]} -->
knative-prow-robot: @n3wscott: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 02fbc44469dd2e01652b04cd1a3390d8c542c6c1 | [link](https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_pkg/2092/pull-knative-pkg-unit-tests/1380607889692954624) | `/test pull-knative-pkg-unit-tests`

[Full PR test history](https://gubernator.knative.dev/pr/knative_pkg/2092). [Your PR dashboard](https://gubernator.knative.dev/pr/n3wscott).

<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 -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/2092?src=pr&el=h1) Report
> Merging [#2092](https://codecov.io/gh/knative/pkg/pull/2092?src=pr&el=desc) (02fbc44) into [main](https://codecov.io/gh/knative/pkg/commit/4302f33ad42cc8fb62257aec1b16f6942401a214?el=desc) (4302f33) will **increase** coverage by `0.05%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/knative/pkg/pull/2092/graphs/tree.svg?width=650&height=150&src=pr&token=rlqpDnfxbO)](https://codecov.io/gh/knative/pkg/pull/2092?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##             main    #2092      +/-   ##
==========================================
+ Coverage   67.30%   67.35%   +0.05%     
==========================================
  Files         215      215              
  Lines        9090     9092       +2     
==========================================
+ Hits         6118     6124       +6     
+ Misses       2697     2695       -2     
+ Partials      275      273       -2     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/2092?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [network/domain.go](https://codecov.io/gh/knative/pkg/pull/2092/diff?src=pr&el=tree#diff-bmV0d29yay9kb21haW4uZ28=) | `50.00% <100.00%> (+7.14%)` | :arrow_up: |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/2092/diff?src=pr&el=tree#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `92.47% <0.00%> (+2.15%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/2092?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative/pkg/pull/2092?src=pr&el=footer). Last update [4302f33...02fbc44](https://codecov.io/gh/knative/pkg/pull/2092?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2092#pullrequestreview-632752730" title="LGTM">vaikas</a>*

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

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:

- ~~[network/OWNERS](https://github.com/knative/pkg/blob/main/network/OWNERS)~~ [vaikas]

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":[]} -->
n3wscott: /retest

Patch:
diff --git a/network/domain.go b/network/domain.go
index a205b4eec..ee571e0bc 100644
--- a/network/domain.go
+++ b/network/domain.go
@@ -26,8 +26,9 @@ import (
 )
 
 const (
-	resolverFileName  = "/etc/resolv.conf"
-	defaultDomainName = "cluster.local"
+	resolverFileName    = "/etc/resolv.conf"
+	clusterDomainEnvKey = "CLUSTER_DOMAIN"
+	defaultDomainName   = "cluster.local"
 )
 
 var (
@@ -55,6 +56,7 @@ func GetClusterDomainName() string {
 }
 
 func getClusterDomainName(r io.Reader) string {
+	// First look in the conf file.
 	for scanner := bufio.NewScanner(r); scanner.Scan(); {
 		elements := strings.Split(scanner.Text(), " ")
 		if elements[0] != "search" {
@@ -66,6 +68,12 @@ func getClusterDomainName(r io.Reader) string {
 			}
 		}
 	}
+
+	// Then look in the ENV.
+	if domain := os.Getenv(clusterDomainEnvKey); len(domain) > 0 {
+		return domain
+	}
+
 	// For all abnormal cases return default domain name.
 	return defaultDomainName
 }


Test patch:
diff --git a/network/domain_test.go b/network/domain_test.go
index 6ce3ba3e6..e6a61d235 100644
--- a/network/domain_test.go
+++ b/network/domain_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package network
 
 import (
+	"os"
 	"strings"
 	"testing"
 )
@@ -24,46 +25,71 @@ import (
 func TestGetDomainName(t *testing.T) {
 	tests := []struct {
 		name       string
+		env        string
 		resolvConf string
 		want       string
-	}{
-		{
-			name: "all good",
-			resolvConf: `
+	}{{
+		name: "all good",
+		resolvConf: `
 nameserver 1.1.1.1
 search default.svc.abc.com svc.abc.com abc.com
 options ndots:5
 `,
-			want: "abc.com",
-		},
-		{
-			name: "all good with trailing dot",
-			resolvConf: `
+		want: "abc.com",
+	}, {
+		name: "all good with env set but ignored",
+		resolvConf: `
+nameserver 1.1.1.1
+search default.svc.abc.com svc.abc.com abc.com
+options ndots:5
+`,
+		env:  "ignored.com",
+		want: "abc.com",
+	}, {
+		name:       "all good from env",
+		resolvConf: ``,
+		env:        "abc.com",
+		want:       "abc.com",
+	}, {
+		name: "all good with trailing dot",
+		resolvConf: `
 nameserver 1.1.1.1
 search default.svc.abc.com. svc.abc.com. abc.com.
 options ndots:5
 `,
-			want: "abc.com",
-		},
-		{
-			name: "missing search line",
-			resolvConf: `
+		want: "abc.com",
+	}, {
+		name: "missing search line",
+		resolvConf: `
 nameserver 1.1.1.1
 options ndots:5
 `,
-			want: defaultDomainName,
-		},
-		{
-			name: "non k8s resolv.conf format",
-			resolvConf: `
+		want: defaultDomainName,
+	}, {
+		name: "non k8s resolv.conf format",
+		resolvConf: `
 nameserver 1.1.1.1
 search  abc.com xyz.com
 options ndots:5
 `,
-			want: defaultDomainName,
-		},
-	}
+		want: defaultDomainName,
+	}}
+
+	domainWas := os.Getenv(clusterDomainEnvKey)
+	t.Cleanup(func() {
+		if len(domainWas) > 0 {
+			_ = os.Setenv(clusterDomainEnvKey, domainWas)
+		} else {
+			_ = os.Unsetenv(clusterDomainEnvKey)
+		}
+	})
+
 	for _, tt := range tests {
+		if len(tt.env) > 0 {
+			_ = os.Setenv(clusterDomainEnvKey, tt.env)
+		} else {
+			_ = os.Unsetenv(clusterDomainEnvKey)
+		}
 		got := getClusterDomainName(strings.NewReader(tt.resolvConf))
 		if got != tt.want {
 			t.Errorf("Test %s failed expected: %s but got: %s", tt.name, tt.want, got)