Repository: knative/pkg
Pull request #1404 (2020-06-13 04:50:59)

Normalize newlines in webhook example checksum.
Fixes https://github.com/knative/serving/issues/8313

kubeclient seems to normalize newlines, which causes issues when you patch via that library.

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

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1404#" title="Author self-approved">Cynocracy</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **mattmoor**
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:

- **[configmap/OWNERS](https://github.com/knative/pkg/blob/master/configmap/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"]} -->
knative-prow-robot: @Cynocracy: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | 535dfe6302fdadc7cac838e3871ade28e143f33a | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1404/pull-knative-pkg-unit-tests/1271666499777466370) | `/test pull-knative-pkg-unit-tests`

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

<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-robot: @Cynocracy: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | a0479fb8b3f18b14fc8d7185673543c2e7b099d6 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1404/pull-knative-pkg-unit-tests/1271686883704836098) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
googlebot: All (the pull request submitter and all commit authors) CLAs are signed, **but** one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project.  Please have them confirm that by leaving a comment that contains only `@googlebot I consent.` in this pull request.

*Note to project maintainer:* There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent.  In those cases, you can manually confirm consent of the commit author(s), and set the `cla` label to `yes` (if enabled on your project).

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fknative%2Fpkg%2Fpull%2F1404) for more info**.

<!-- need_author_consent -->
Cynocracy: Alright, updated the tests and fixed my newline mangling (somehow using the local pkg fixed my issue though? sigh)
Cynocracy: @googlebot I consent
knative-prow-robot: @Cynocracy: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-knative-pkg-unit-tests | d70f7a75593c9c20e18d2acbf053e5d053ea2967 | [link](https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_pkg/1404/pull-knative-pkg-unit-tests/1271706261087326208) | `/test pull-knative-pkg-unit-tests`

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

<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 -->
googlebot: CLAs look good, thanks!

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fknative%2Fpkg%2Fpull%2F1404) for more info**.

<!-- ok -->
Cynocracy: WDYT? It matches mixed CR and non-CR newlines, lmk if you'd prefer I be strict about only one style being present in a file (I don't see a real compelling reason to do so)
knative-prow-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/1404#" title="Author self-approved">Cynocracy</a>*, *<a href="https://github.com/knative/pkg/pull/1404#pullrequestreview-430257986" title="Approved">vagababov</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:

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

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/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)
+		}
 	}
 }
 


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/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
 	}