Repository: knative/pkg
Pull request #1918 (2020-11-19 05:22:35)

Add webhookname from env
Signed-off-by: Arghya Sadhu <arghya88@gmail.com>

Ref: https://github.com/knative/eventing/issues/4530

This is currently used here
https://github.com/knative/eventing/blob/52597d174b52a680756b52b377c81bfb96fb636b/pkg/logconfig/config.go#L37

Moving it to pkg to be used across all knative components

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1918#" title="Author self-approved">arghya88</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **mattmoor** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @mattmoor` in a comment when ready.

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:

- **[webhook/OWNERS](https://github.com/knative/pkg/blob/master/webhook/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":["mattmoor"]} -->
arghya88: /assign @mattmoor
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/1918?src=pr&el=h1) Report
> Merging [#1918](https://codecov.io/gh/knative/pkg/pull/1918?src=pr&el=desc) (8e41506) into [master](https://codecov.io/gh/knative/pkg/commit/ab1a398f669ce2d3d10d876e63ca4a7641f09fff?el=desc) (ab1a398) will **increase** coverage by `0.08%`.
> The diff coverage is `100.00%`.

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

```diff
@@            Coverage Diff             @@
##           master    #1918      +/-   ##
==========================================
+ Coverage   69.45%   69.53%   +0.08%     
==========================================
  Files         207      207              
  Lines        8752     8759       +7     
==========================================
+ Hits         6079     6091      +12     
+ Misses       2402     2398       -4     
+ Partials      271      270       -1     
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/1918?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [webhook/env.go](https://codecov.io/gh/knative/pkg/pull/1918/diff?src=pr&el=tree#diff-d2ViaG9vay9lbnYuZ28=) | `100.00% <100.00%> (ø)` | |
| [network/network.go](https://codecov.io/gh/knative/pkg/pull/1918/diff?src=pr&el=tree#diff-bmV0d29yay9uZXR3b3JrLmdv) | `100.00% <0.00%> (ø)` | |
| [test/upgrade/functions.go](https://codecov.io/gh/knative/pkg/pull/1918/diff?src=pr&el=tree#diff-dGVzdC91cGdyYWRlL2Z1bmN0aW9ucy5nbw==) | `100.00% <0.00%> (ø)` | |
| [test/gcs/mock/mock.go](https://codecov.io/gh/knative/pkg/pull/1918/diff?src=pr&el=tree#diff-dGVzdC9nY3MvbW9jay9tb2NrLmdv) | `92.47% <0.00%> (+1.07%)` | :arrow_up: |
| [network/h2c.go](https://codecov.io/gh/knative/pkg/pull/1918/diff?src=pr&el=tree#diff-bmV0d29yay9oMmMuZ28=) | `20.00% <0.00%> (+4.61%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative/pkg/pull/1918?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/1918?src=pr&el=footer). Last update [ab1a398...8e41506](https://codecov.io/gh/knative/pkg/pull/1918?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/1918#" title="Author self-approved">arghya88</a>*, *<a href="https://github.com/knative/pkg/pull/1918#pullrequestreview-535501825" title="LGTM">mattmoor</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:

- ~~[webhook/OWNERS](https://github.com/knative/pkg/blob/master/webhook/OWNERS)~~ [mattmoor]

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

Patch:
diff --git a/webhook/env.go b/webhook/env.go
index 9e59f7cfd..cd038b40e 100644
--- a/webhook/env.go
+++ b/webhook/env.go
@@ -24,6 +24,9 @@ import (
 
 const portEnvKey = "WEBHOOK_PORT"
 
+// Webhook is the name of the override key used inside of the logging config for Webhook Controller.
+const webhookNameEnv = "WEBHOOK_NAME"
+
 // PortFromEnv returns the webhook port set by portEnvKey, or default port if env var is not set.
 func PortFromEnv(defaultPort int) int {
 	if os.Getenv(portEnvKey) == "" {
@@ -37,3 +40,17 @@ func PortFromEnv(defaultPort int) int {
 	}
 	return port
 }
+
+func NameFromEnv() string {
+	if webhook := os.Getenv(webhookNameEnv); webhook != "" {
+		return webhook
+	}
+
+	panic(fmt.Sprintf(`The environment variable %q is not set.
+This should be unique for the webhooks in a namespace
+If this is a process running on Kubernetes, then initialize this variable via:
+  env:
+  - name: WEBHOOK_NAME
+    value: webhook
+`, webhookNameEnv))
+}


Test patch:
diff --git a/webhook/env_test.go b/webhook/env_test.go
index 4ffc8956c..c08eaba77 100644
--- a/webhook/env_test.go
+++ b/webhook/env_test.go
@@ -33,6 +33,13 @@ type portTest struct {
 	wantPanic bool
 }
 
+type webhookNameTest struct {
+	name      string
+	in        string
+	want      string
+	wantPanic bool
+}
+
 func TestPort(t *testing.T) {
 	tests := []portTest{{
 		name: testMissingInputName,
@@ -81,3 +88,37 @@ func TestPort(t *testing.T) {
 		})
 	}
 }
+
+func TestWebhookName(t *testing.T) {
+	tests := []webhookNameTest{{
+		name:      "EmptyInput",
+		in:        "",
+		wantPanic: true,
+	}, {
+		name: "ValidInput",
+		in:   "mywebhook",
+		want: "mywebhook",
+	}}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// webhookNameEnv is unset when testing missing input.
+			if tc.name != testMissingInputName {
+				os.Setenv(webhookNameEnv, tc.in)
+			}
+
+			defer func() {
+				if r := recover(); r == nil && tc.wantPanic {
+					t.Error("Did not panic")
+				} else if r != nil && !tc.wantPanic {
+					t.Error("Got unexpected panic")
+				}
+				os.Unsetenv(webhookNameEnv)
+			}()
+
+			if got := NameFromEnv(); got != tc.want {
+				t.Errorf("NameFromEnv = %s, want: %s", got, tc.want)
+			}
+		})
+	}
+}