Repository: knative/pkg
Pull request #2605 (2022-10-05 07:49:02)

Preserve webhook namespaceSelector.matchLabels
I have a webhook with this definition and the reconciler is removing the `namespaceSelector.matchLabels` field:

Current resource:
```
				  namespaceSelector:
				    matchExpressions:
				    - key: webhooks.knative.dev/exclude
				      operator: DoesNotExist
				  objectSelector:
				    matchLabels:
				      app.kubernetes.io/component: kafka-dispatcher

```

Applied resource:
```
				    namespaceSelector:
				      matchExpressions: [ ]
				      matchLabels:
				        app.kubernetes.io/name: knative-eventing
				    objectSelector:
				      matchLabels:
				        app.kubernetes.io/component: kafka-dispatcher
```

Signed-off-by: Pierangelo Di Pilato <pierdipi@redhat.com>

<!-- Thanks for sending a pull request! -->

# Changes

<!-- 
Describe your changes here- ideally you can get that description straight from
your descriptive commit message(s)! 

- :gift: Add new feature
- :bug: Fix bug
- :broom: Update or clean up current behavior
- :wastebasket: Remove feature or internal logic
-->

- Preserve webhook namespaceSelector.matchLabels

<!--
In addition, categorize the changes you're making using the "/kind" Prow command, example:

/kind bug

Supported kinds are: api-change, bug, cleanup, deprecation, removal, documentation, enhancement, performance

-->
/kind bug

<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*

Fixes #
-->

<!-- Please include the 'why' behind your changes if no issue exists -->

**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
Preserve webhook namespaceSelector.matchLabels
```


pierDipi: /cc @dprotaso @evankanderson 
knative-prow[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative/pkg/pull/2605#" title="Author self-approved">pierDipi</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign dprotaso for approval by writing `/assign @dprotaso` in a comment. For more information see:[The Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

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/main/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":["dprotaso"]} -->
codecov[bot]: # [Codecov](https://codecov.io/gh/knative/pkg/pull/2605?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) Report
Base: **81.29**% // Head: **81.28**% // Decreases project coverage by **`-0.00%`** :warning:
> Coverage data is based on head [(`578f53c`)](https://codecov.io/gh/knative/pkg/pull/2605?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) compared to base [(`158538c`)](https://codecov.io/gh/knative/pkg/commit/158538cc46ec27038b7271a53a26e4e298ee326d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative).
> Patch coverage: 100.00% of modified lines in pull request are covered.

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


```diff
@@            Coverage Diff             @@
##             main    #2605      +/-   ##
==========================================
- Coverage   81.29%   81.28%   -0.01%     
==========================================
  Files         162      162              
  Lines        9820     9818       -2     
==========================================
- Hits         7983     7981       -2     
  Misses       1597     1597              
  Partials      240      240              
```


| [Impacted Files](https://codecov.io/gh/knative/pkg/pull/2605?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative) | Coverage Δ | |
|---|---|---|
| [webhook/helper.go](https://codecov.io/gh/knative/pkg/pull/2605/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative#diff-d2ViaG9vay9oZWxwZXIuZ28=) | `100.00% <100.00%> (ø)` | |

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/pkg/pull/2605?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).


Patch:
diff --git a/.github/workflows/knative-releasability.yaml b/.github/workflows/knative-releasability.yaml
index 930ad8e62..9c20538e7 100644
--- a/.github/workflows/knative-releasability.yaml
+++ b/.github/workflows/knative-releasability.yaml
@@ -25,8 +25,8 @@ jobs:
   releasability:
     uses: knative/actions/.github/workflows/releasability.yaml@main
     with:
-      releaseFamily: ${{ github.event.inputs.releaseFamily || 'v1.7' }}
-      moduleReleaseFamily: ${{ github.event.inputs.moduleReleaseFamily || 'v0.34' }}
+      releaseFamily: ${{ github.event.inputs.releaseFamily || 'v1.8' }}
+      moduleReleaseFamily: ${{ github.event.inputs.moduleReleaseFamily || 'v0.35' }}
       slackChannel: ${{ github.event.inputs.slackChannel || 'release' }}
     secrets:
       SLACK_WEBHOOK: ${{ secrets.SLACK_WEBHOOK }}
diff --git a/OWNERS_ALIASES b/OWNERS_ALIASES
index ac08d3523..a8d05da71 100644
--- a/OWNERS_ALIASES
+++ b/OWNERS_ALIASES
@@ -53,6 +53,18 @@ aliases:
   - matzew
   - odacremolbap
   - pierDipi
+  func-reviewers:
+  - jrangelramos
+  - nainaz
+  func-writers:
+  - lance
+  - lkingland
+  - matejvasek
+  - salaboy
+  - zroubalik
+  functions-wg-leads:
+  - lance
+  - salaboy
   knative-admin:
   - csantanapr
   - dprotaso
diff --git a/go.mod b/go.mod
index 07b99b498..b6527dea3 100644
--- a/go.mod
+++ b/go.mod
@@ -50,7 +50,7 @@ require (
 	k8s.io/gengo v0.0.0-20220613173612-397b4ae3bce7
 	k8s.io/klog/v2 v2.70.2-0.20220707122935-0990e81f1a8f
 	k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9
-	knative.dev/hack v0.0.0-20221003111221-b3c97904d3b5
+	knative.dev/hack v0.0.0-20221004153928-92a65f105c37
 	sigs.k8s.io/yaml v1.3.0
 )
 
diff --git a/go.sum b/go.sum
index dfd4a9e8a..9001c0db8 100644
--- a/go.sum
+++ b/go.sum
@@ -1180,8 +1180,8 @@ k8s.io/kube-openapi v0.0.0-20220328201542-3ee0da9b0b42/go.mod h1:Z/45zLw8lUo4wdi
 k8s.io/utils v0.0.0-20210802155522-efc7438f0176/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
 k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 h1:HNSDgDCrr/6Ly3WEGKZftiE7IY19Vz2GdbOCyI4qqhc=
 k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
-knative.dev/hack v0.0.0-20221003111221-b3c97904d3b5 h1:iVRcbOZrI6XBc/JZh/IJZ56858QdoEV3DSbjy7XbKBg=
-knative.dev/hack v0.0.0-20221003111221-b3c97904d3b5/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
+knative.dev/hack v0.0.0-20221004153928-92a65f105c37 h1:4xB0A2aWQtzUcFjpZf9ufxRsjt+E7tEL364VlPttI8s=
+knative.dev/hack v0.0.0-20221004153928-92a65f105c37/go.mod h1:yk2OjGDsbEnQjfxdm0/HJKS2WqTLEFg/N6nUs6Rqx3Q=
 pgregory.net/rapid v0.3.3 h1:jCjBsY4ln4Atz78QoBWxUEvAHaFyNDQg9+WU62aCn1U=
 pgregory.net/rapid v0.3.3/go.mod h1:UYpPVyjFHzYBGHIxLFoupi8vwk6rXNzRY9OMvVxFIOU=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
diff --git a/vendor/knative.dev/hack/release.sh b/vendor/knative.dev/hack/release.sh
index a32a8e519..8a424bf20 100644
--- a/vendor/knative.dev/hack/release.sh
+++ b/vendor/knative.dev/hack/release.sh
@@ -320,16 +320,17 @@ function sign_release() {
 
   # Notarizing mac binaries needs to be done before cosign as it changes the checksum values
   # of the darwin binaries
- if [ -n "${APPLE_CODESIGN_KEY}" ] && [ -n "${APPLE_CODESIGN_PASSWORD}" ] && [ -n "${APPLE_NOTARY_API_KEY}" ]; then
+ if [ -n "${APPLE_CODESIGN_KEY}" ] && [ -n "${APPLE_CODESIGN_PASSWORD_FILE}" ] && [ -n "${APPLE_NOTARY_API_KEY}" ]; then
+    banner "Notarizing macOS Binaries for the release"
     FILES=$(find -- * -type f -name "*darwin*")
     for file in $FILES; do
       rcodesign sign "${file}" --p12-file="${APPLE_CODESIGN_KEY}" \
         --code-signature-flags=runtime \
         --p12-password-file="${APPLE_CODESIGN_PASSWORD_FILE}"
     done
-    zip files.zip "$FILES"
+    zip files.zip ${FILES}
     rcodesign notary-submit files.zip --api-key-path="${APPLE_NOTARY_API_KEY}" --wait
-    sha256sum "${ARTIFACTS_TO_PUBLISH//checksums.txt/}" > checksums.txt
+    sha256sum ${ARTIFACTS_TO_PUBLISH//checksums.txt/} > checksums.txt
   fi
 
   ## Sign the images with cosign
diff --git a/vendor/modules.txt b/vendor/modules.txt
index dcf1eddcd..86e5daba4 100644
--- a/vendor/modules.txt
+++ b/vendor/modules.txt
@@ -1098,7 +1098,7 @@ k8s.io/utils/net
 k8s.io/utils/pointer
 k8s.io/utils/strings/slices
 k8s.io/utils/trace
-# knative.dev/hack v0.0.0-20221003111221-b3c97904d3b5
+# knative.dev/hack v0.0.0-20221004153928-92a65f105c37
 ## explicit; go 1.18
 knative.dev/hack
 # sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2
diff --git a/version/version.go b/version/version.go
index 39e34464b..32b818ecf 100644
--- a/version/version.go
+++ b/version/version.go
@@ -33,7 +33,7 @@ const (
 	// NOTE: If you are changing this line, please also update the minimum kubernetes
 	// version listed here:
 	// https://github.com/knative/docs/blob/mkdocs/docs/snippets/prerequisites.md
-	defaultMinimumVersion = "v1.22.0"
+	defaultMinimumVersion = "v1.23.0"
 )
 
 func getMinimumVersion() string {
diff --git a/webhook/helper.go b/webhook/helper.go
index cbcbb58c7..f9ff2daa9 100644
--- a/webhook/helper.go
+++ b/webhook/helper.go
@@ -35,7 +35,10 @@ func EnsureLabelSelectorExpressions(
 	}
 
 	if len(current.MatchExpressions) == 0 {
-		return want
+		return &metav1.LabelSelector{
+			MatchLabels:      current.MatchLabels,
+			MatchExpressions: want.MatchExpressions,
+		}
 	}
 
 	var wantExpressions []metav1.LabelSelectorRequirement
@@ -44,6 +47,7 @@ func EnsureLabelSelectorExpressions(
 	}
 
 	return &metav1.LabelSelector{
+		MatchLabels: current.MatchLabels,
 		MatchExpressions: ensureLabelSelectorRequirements(
 			current.MatchExpressions, wantExpressions),
 	}


Test patch:
diff --git a/version/version_test.go b/version/version_test.go
index 6fa6d09d8..1685bc553 100644
--- a/version/version_test.go
+++ b/version/version_test.go
@@ -40,16 +40,16 @@ func TestVersionCheck(t *testing.T) {
 		wantError       bool
 	}{{
 		name:          "greater version (patch)",
-		actualVersion: &testVersioner{version: "v1.22.1"},
+		actualVersion: &testVersioner{version: "v1.23.1"},
 	}, {
 		name:          "greater version (patch), no v",
-		actualVersion: &testVersioner{version: "1.22.1"},
+		actualVersion: &testVersioner{version: "1.23.1"},
 	}, {
 		name:          "greater version (patch), pre-release",
-		actualVersion: &testVersioner{version: "1.22.1-kpn-065dce"},
+		actualVersion: &testVersioner{version: "1.23.1-kpn-065dce"},
 	}, {
 		name:          "greater version (patch), pre-release with build",
-		actualVersion: &testVersioner{version: "1.22.1-1095+9689d22dc3121e-dirty"},
+		actualVersion: &testVersioner{version: "1.23.1-1095+9689d22dc3121e-dirty"},
 	}, {
 		name:            "greater version (patch), pre-release, envvar override",
 		actualVersion:   &testVersioner{version: "1.15.11-kpn-065dce"},
@@ -60,16 +60,16 @@ func TestVersionCheck(t *testing.T) {
 		versionOverride: "1.15.11-0",
 	}, {
 		name:          "greater version (minor)",
-		actualVersion: &testVersioner{version: "v1.23.0"},
+		actualVersion: &testVersioner{version: "v1.24.0"},
 	}, {
 		name:          "same version",
-		actualVersion: &testVersioner{version: "v1.22.0"},
+		actualVersion: &testVersioner{version: "v1.23.0"},
 	}, {
 		name:          "same version with build",
-		actualVersion: &testVersioner{version: "v1.22.0+k3s.1"},
+		actualVersion: &testVersioner{version: "v1.23.0+k3s.1"},
 	}, {
 		name:          "same version with pre-release",
-		actualVersion: &testVersioner{version: "v1.22.0-k3s.1"},
+		actualVersion: &testVersioner{version: "v1.23.0-k3s.1"},
 	}, {
 		name:          "smaller version",
 		actualVersion: &testVersioner{version: "v1.14.3"},
diff --git a/webhook/resourcesemantics/validation/reconcile_config_test.go b/webhook/resourcesemantics/validation/reconcile_config_test.go
index d4721a819..e027a1516 100644
--- a/webhook/resourcesemantics/validation/reconcile_config_test.go
+++ b/webhook/resourcesemantics/validation/reconcile_config_test.go
@@ -419,6 +419,139 @@ func TestReconcile(t *testing.T) {
 				}},
 			},
 		}},
+	}, {
+		Name: "secret and VWH exist, correcting namespaceSelector, preserving matchLabels",
+		Key:  key,
+		Objects: []runtime.Object{secret, ns,
+			&admissionregistrationv1.ValidatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
+					Name: name,
+					ClientConfig: admissionregistrationv1.WebhookClientConfig{
+						Service: &admissionregistrationv1.ServiceReference{
+							Namespace: system.Namespace(),
+							Name:      "webhook",
+							// Path is fine.
+							Path: ptr.String(path),
+						},
+						// CABundle is fine.
+						CABundle: []byte("present"),
+					},
+					// Rules are fine.
+					Rules: expectedRules,
+					// NamespaceSelector contains non-knative things.
+					NamespaceSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app.kubernetes.io/name": "knative-eventing",
+						},
+						MatchExpressions: []metav1.LabelSelectorRequirement{{
+							Key:      "foo.knative.dev/exclude",
+							Operator: metav1.LabelSelectorOpDoesNotExist,
+						}, {
+							Key:      "foo.bar/baz",
+							Operator: metav1.LabelSelectorOpDoesNotExist,
+						}},
+					},
+				}},
+			},
+		},
+		WantUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
+				},
+				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
+					Name: name,
+					ClientConfig: admissionregistrationv1.WebhookClientConfig{
+						Service: &admissionregistrationv1.ServiceReference{
+							Namespace: system.Namespace(),
+							Name:      "webhook",
+							Path:      ptr.String(path),
+						},
+						CABundle: []byte("present"),
+					},
+					Rules: expectedRules,
+					NamespaceSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app.kubernetes.io/name": "knative-eventing",
+						},
+						// The knative key is added while the non-knative key is kept.
+						// Old knative key is removed.
+						MatchExpressions: []metav1.LabelSelectorRequirement{{
+							Key:      "webhooks.knative.dev/exclude",
+							Operator: metav1.LabelSelectorOpDoesNotExist,
+						}, {
+							Key:      "foo.bar/baz",
+							Operator: metav1.LabelSelectorOpDoesNotExist,
+						}},
+					},
+				}},
+			},
+		}},
+	}, {
+		Name: "secret and VWH exist, correcting namespaceSelector without existing matchExpressions, preserving matchLabels",
+		Key:  key,
+		Objects: []runtime.Object{secret, ns,
+			&admissionregistrationv1.ValidatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: name,
+				},
+				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
+					Name: name,
+					ClientConfig: admissionregistrationv1.WebhookClientConfig{
+						Service: &admissionregistrationv1.ServiceReference{
+							Namespace: system.Namespace(),
+							Name:      "webhook",
+							// Path is fine.
+							Path: ptr.String(path),
+						},
+						// CABundle is fine.
+						CABundle: []byte("present"),
+					},
+					// Rules are fine.
+					Rules: expectedRules,
+					// NamespaceSelector contains non-knative things.
+					NamespaceSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app.kubernetes.io/name": "knative-eventing",
+						},
+					},
+				}},
+			},
+		},
+		WantUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
+				},
+				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
+					Name: name,
+					ClientConfig: admissionregistrationv1.WebhookClientConfig{
+						Service: &admissionregistrationv1.ServiceReference{
+							Namespace: system.Namespace(),
+							Name:      "webhook",
+							Path:      ptr.String(path),
+						},
+						CABundle: []byte("present"),
+					},
+					Rules: expectedRules,
+					NamespaceSelector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							"app.kubernetes.io/name": "knative-eventing",
+						},
+						// The knative key is added
+						MatchExpressions: []metav1.LabelSelectorRequirement{{
+							Key:      "webhooks.knative.dev/exclude",
+							Operator: metav1.LabelSelectorOpDoesNotExist,
+						}},
+					},
+				}},
+			},
+		}},
 	}}
 
 	table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {