Repository: knative/pkg (go)

Issue:
allow for webhooks to exclude certain namespaces
**Problem**
Some webhooks such as: https://github.com/knative/eventing-contrib/blob/master/kafka/source/config/500-webhook-configuration.yaml#L18 do not limit themselves to just certain namespaces. This can be problematic if that webhook is acting up (e.g. the service is not responding), and the request that is being processed is not related to Knative at all but instead could be something critical such as being part of a system upgrade.

One options we discussed today is to define a label that can be added to any namespace such that the Knative webhooks could then be defined to exclude any namespace that has that label. For example, an admin could add the label to all Kube system namespaces to ensure that knative webhooks are never called for system related resource operations.

This technically isn't just an eventing issue, but since this is the first webhook I found with this problem I picked on it as a testcase and if we can find a solution that works for this one then we can consider applying it to all Knative webhooks.

( https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#resources-that-support-set-based-requirements )

/cc @mattmoor @mpetason 

Pull request description:
Allows for webhooks to exclude certain namespaces
Added a namespaces selector to the mutating webhook configuration which
allows for excluding namespaces from the webhook

Fixes #1379

Patch:
diff --git a/configmap/example.go b/configmap/example.go
index 1bbf3dc7e..f14a6e6ff 100644
--- a/configmap/example.go
+++ b/configmap/example.go
@@ -19,6 +19,7 @@ package configmap
 import (
 	"fmt"
 	"hash/crc32"
+	"regexp"
 	"strings"
 )
 
@@ -30,9 +31,14 @@ const (
 	ExampleChecksumAnnotation = "knative.dev/example-checksum"
 )
 
+var (
+	// Allows for normalizing by collapsing newlines.
+	sequentialNewlines = regexp.MustCompile("(?:\r?\n)+")
+)
+
 // Checksum generates a checksum for the example value to be compared against
 // a respective annotation.
 // Leading and trailing spaces are ignored.
 func Checksum(value string) string {
-	return fmt.Sprintf("%08x", crc32.ChecksumIEEE([]byte(strings.TrimSpace(value))))
+	return fmt.Sprintf("%08x", crc32.ChecksumIEEE([]byte(sequentialNewlines.ReplaceAllString(strings.TrimSpace(value), `\n`))))
 }
diff --git a/configmap/hash-gen/main.go b/configmap/hash-gen/main.go
index 7bafc5a56..72230b545 100644
--- a/configmap/hash-gen/main.go
+++ b/configmap/hash-gen/main.go
@@ -29,9 +29,10 @@ import (
 )
 
 func main() {
-	fileName := os.Args[1]
-	if err := processFile(fileName); err != nil {
-		log.Fatal(err)
+	for _, fileName := range os.Args[1:] {
+		if err := processFile(fileName); err != nil {
+			log.Fatal(err)
+		}
 	}
 }
 
diff --git a/webhook/resourcesemantics/defaulting/defaulting.go b/webhook/resourcesemantics/defaulting/defaulting.go
index 75bfa9fd6..d84e6ed9c 100644
--- a/webhook/resourcesemantics/defaulting/defaulting.go
+++ b/webhook/resourcesemantics/defaulting/defaulting.go
@@ -30,6 +30,7 @@ import (
 	jsonpatch "gomodules.xyz/jsonpatch/v2"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	admissionlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
@@ -172,6 +173,12 @@ func (ac *reconciler) reconcileMutatingWebhook(ctx context.Context, caCert []byt
 			continue
 		}
 		webhook.Webhooks[i].Rules = rules
+		webhook.Webhooks[i].NamespaceSelector = &metav1.LabelSelector{
+			MatchExpressions: []metav1.LabelSelectorRequirement{{
+				Key:      "webhooks.knative.dev/exclude",
+				Operator: metav1.LabelSelectorOpDoesNotExist,
+			}},
+		}
 		webhook.Webhooks[i].ClientConfig.CABundle = caCert
 		if webhook.Webhooks[i].ClientConfig.Service == nil {
 			return fmt.Errorf("missing service reference for webhook: %s", wh.Name)
diff --git a/webhook/resourcesemantics/validation/reconcile_config.go b/webhook/resourcesemantics/validation/reconcile_config.go
index e1b1b0e1e..c17db6281 100644
--- a/webhook/resourcesemantics/validation/reconcile_config.go
+++ b/webhook/resourcesemantics/validation/reconcile_config.go
@@ -25,6 +25,7 @@ import (
 	"github.com/markbates/inflect"
 	"go.uber.org/zap"
 	admissionregistrationv1beta1 "k8s.io/api/admissionregistration/v1beta1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	admissionlisters "k8s.io/client-go/listers/admissionregistration/v1beta1"
@@ -135,6 +136,12 @@ func (ac *reconciler) reconcileValidatingWebhook(ctx context.Context, caCert []b
 			continue
 		}
 		webhook.Webhooks[i].Rules = rules
+		webhook.Webhooks[i].NamespaceSelector = &metav1.LabelSelector{
+			MatchExpressions: []metav1.LabelSelectorRequirement{{
+				Key:      "webhooks.knative.dev/exclude",
+				Operator: metav1.LabelSelectorOpDoesNotExist,
+			}},
+		}
 		webhook.Webhooks[i].ClientConfig.CABundle = caCert
 		if webhook.Webhooks[i].ClientConfig.Service == nil {
 			return fmt.Errorf("missing service reference for webhook: %s", wh.Name)


Test patch:
diff --git a/configmap/example_test.go b/configmap/example_test.go
index d7b291e05..3a22538ea 100644
--- a/configmap/example_test.go
+++ b/configmap/example_test.go
@@ -25,6 +25,15 @@ func TestChecksum(t *testing.T) {
 	}{{
 		in:   "",
 		want: "00000000",
+	}, {
+		in:   "a somewhat\nlonger\ntext",
+		want: "2b4ed320",
+	}, {
+		in:   "a somewhat\n\n\nlonger\n\ntext",
+		want: "2b4ed320",
+	}, {
+		in:   "a somewhat\r\n\r\n\r\nlonger\r\n\r\ntext",
+		want: "2b4ed320",
 	}, {
 		in:   "1",
 		want: "83dcefb7",
diff --git a/configmap/hash-gen/testdata/add_want.yaml b/configmap/hash-gen/testdata/add_want.yaml
index 374280271..31ae5e63e 100644
--- a/configmap/hash-gen/testdata/add_want.yaml
+++ b/configmap/hash-gen/testdata/add_want.yaml
@@ -20,7 +20,7 @@ metadata:
   labels:
     serving.knative.dev/release: devel
   annotations:
-    knative.dev/example-checksum: 81e8e656
+    knative.dev/example-checksum: 05d48218
 data:
   leave-me: "alone"
   _example: |
diff --git a/configmap/hash-gen/testdata/update_want.yaml b/configmap/hash-gen/testdata/update_want.yaml
index 374280271..31ae5e63e 100644
--- a/configmap/hash-gen/testdata/update_want.yaml
+++ b/configmap/hash-gen/testdata/update_want.yaml
@@ -20,7 +20,7 @@ metadata:
   labels:
     serving.knative.dev/release: devel
   annotations:
-    knative.dev/example-checksum: 81e8e656
+    knative.dev/example-checksum: 05d48218
 data:
   leave-me: "alone"
   _example: |
diff --git a/test/ghutil/client.go b/test/ghutil/client.go
index 8a9872a47..8f6466eb7 100644
--- a/test/ghutil/client.go
+++ b/test/ghutil/client.go
@@ -61,6 +61,7 @@ type GithubOperations interface {
 	ListCommits(org, repo string, ID int) ([]*github.RepositoryCommit, error)
 	ListFiles(org, repo string, ID int) ([]*github.CommitFile, error)
 	CreatePullRequest(org, repo, head, base, title, body string) (*github.PullRequest, error)
+	ListBranches(org, repo string) ([]*github.Branch, error)
 }
 
 // GithubClient provides methods to perform github operations
diff --git a/test/ghutil/fakeghutil/fakeghutil.go b/test/ghutil/fakeghutil/fakeghutil.go
index 66a3d261c..7ac563580 100644
--- a/test/ghutil/fakeghutil/fakeghutil.go
+++ b/test/ghutil/fakeghutil/fakeghutil.go
@@ -36,6 +36,7 @@ type FakeGithubClient struct {
 	PullRequests map[string]map[int]*github.PullRequest // map of repo: map of PullRequest Number: pullrequests
 	PRCommits    map[int][]*github.RepositoryCommit     // map of PR number: slice of commits
 	CommitFiles  map[string][]*github.CommitFile        // map of commit SHA: slice of files
+	Branches     map[string][]*github.Branch            // map of repo: branches
 
 	NextNumber int    // number to be assigned to next newly created issue/comment
 	BaseURL    string // base URL of Github
@@ -330,6 +331,15 @@ func (fgc *FakeGithubClient) CreatePullRequest(org, repo, head, base, title, bod
 	return PR, nil
 }
 
+// ListBranches lists branchs for given repo
+func (fgc *FakeGithubClient) ListBranches(org, repo string) ([]*github.Branch, error) {
+	var branches []*github.Branch
+	for _, b := range fgc.Branches {
+		branches = append(branches, b...)
+	}
+	return branches, nil
+}
+
 // AddFileToCommit adds file to commit
 // This is complementary of mocking CreatePullRequest, so that newly created pull request can have files
 func (fgc *FakeGithubClient) AddFileToCommit(org, repo, SHA, filename, patch string) error {
diff --git a/test/ghutil/repo.go b/test/ghutil/repo.go
new file mode 100644
index 000000000..1f645856e
--- /dev/null
+++ b/test/ghutil/repo.go
@@ -0,0 +1,49 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// repo.go provides generic functions related to Repo
+
+package ghutil
+
+import (
+	"fmt"
+
+	"github.com/google/go-github/v27/github"
+)
+
+// ListBranches lists branchs for given repo
+func (gc *GithubClient) ListBranches(org, repo string) ([]*github.Branch, error) {
+	genericList, err := gc.depaginate(
+		fmt.Sprintf("listing Pull request from org %q and base %q", org, repo),
+		maxRetryCount,
+		&github.ListOptions{},
+		func() ([]interface{}, *github.Response, error) {
+			page, resp, err := gc.Client.Repositories.ListBranches(ctx, org, repo, nil)
+			var interfaceList []interface{}
+			if nil == err {
+				for _, PR := range page {
+					interfaceList = append(interfaceList, PR)
+				}
+			}
+			return interfaceList, resp, err
+		},
+	)
+	res := make([]*github.Branch, len(genericList))
+	for i, elem := range genericList {
+		res[i] = elem.(*github.Branch)
+	}
+	return res, err
+}
diff --git a/test/logstream/kubelogs.go b/test/logstream/kubelogs.go
index 84e4cf27e..cd137acdc 100644
--- a/test/logstream/kubelogs.go
+++ b/test/logstream/kubelogs.go
@@ -87,10 +87,12 @@ func (k *kubelogs) init(t test.TLegacy) {
 				}
 				defer stream.Close()
 				// Read this container's stream.
-				for scanner := bufio.NewScanner(stream); scanner.Scan(); {
-					k.handleLine(scanner.Text())
+				scanner := bufio.NewScanner(stream)
+				for scanner.Scan() {
+					k.handleLine(scanner.Bytes())
 				}
-				return fmt.Errorf("logstream completed prematurely for: %s/%s", pod.Name, container.Name)
+				return fmt.Errorf("logstream completed prematurely for %s/%s: %w",
+					pod.Name, container.Name, scanner.Err())
 			})
 		}
 	}
@@ -106,7 +108,7 @@ func (k *kubelogs) init(t test.TLegacy) {
 	}()
 }
 
-func (k *kubelogs) handleLine(l string) {
+func (k *kubelogs) handleLine(l []byte) {
 	// This holds the standard structure of our logs.
 	var line struct {
 		Level      string    `json:"level"`
@@ -119,7 +121,7 @@ func (k *kubelogs) handleLine(l string) {
 
 		// TODO(mattmoor): Parse out more context.
 	}
-	if err := json.Unmarshal([]byte(l), &line); err != nil {
+	if err := json.Unmarshal(l, &line); err != nil {
 		// Ignore malformed lines.
 		return
 	}
diff --git a/webhook/resourcesemantics/defaulting/table_test.go b/webhook/resourcesemantics/defaulting/table_test.go
index f32f2a8d8..433cba7e7 100644
--- a/webhook/resourcesemantics/defaulting/table_test.go
+++ b/webhook/resourcesemantics/defaulting/table_test.go
@@ -57,6 +57,14 @@ func TestReconcile(t *testing.T) {
 		},
 	}
 
+	// This is the namespace selector setup
+	namespaceSelector := &metav1.LabelSelector{
+		MatchExpressions: []metav1.LabelSelectorRequirement{{
+			Key:      "webhooks.knative.dev/exclude",
+			Operator: metav1.LabelSelectorOpDoesNotExist,
+		}},
+	}
+
 	// These are the rules we expect given the context of "handlers".
 	expectedRules := []admissionregistrationv1beta1.RuleWithOperations{{
 		Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE"},
@@ -167,7 +175,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are added.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		}},
@@ -221,7 +230,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are fixed.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		}},
@@ -279,7 +289,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are fixed.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		}},
@@ -304,7 +315,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are fine.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		},
diff --git a/webhook/resourcesemantics/validation/reconcile_config_test.go b/webhook/resourcesemantics/validation/reconcile_config_test.go
index b9e6e3af4..cbe903398 100644
--- a/webhook/resourcesemantics/validation/reconcile_config_test.go
+++ b/webhook/resourcesemantics/validation/reconcile_config_test.go
@@ -57,6 +57,14 @@ func TestReconcile(t *testing.T) {
 		},
 	}
 
+	// This is the namespace selector setup
+	namespaceSelector := &metav1.LabelSelector{
+		MatchExpressions: []metav1.LabelSelectorRequirement{{
+			Key:      "webhooks.knative.dev/exclude",
+			Operator: metav1.LabelSelectorOpDoesNotExist,
+		}},
+	}
+
 	// These are the rules we expect given the context of "handlers".
 	expectedRules := []admissionregistrationv1beta1.RuleWithOperations{{
 		Operations: []admissionregistrationv1beta1.OperationType{"CREATE", "UPDATE", "DELETE"},
@@ -167,7 +175,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are added.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		}},
@@ -221,7 +230,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are fixed.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		}},
@@ -279,7 +289,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are fixed.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		}},
@@ -304,7 +315,8 @@ func TestReconcile(t *testing.T) {
 						CABundle: []byte("present"),
 					},
 					// Rules are fine.
-					Rules: expectedRules,
+					Rules:             expectedRules,
+					NamespaceSelector: namespaceSelector,
 				}},
 			},
 		},