Preserve webhook namespaceSelector.matchLabels (#2605)

* Preserve webhook namespaceSelector.matchLabels I have a webhook with this definition and the reconciler is removing the 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 <[email protected]> * Optimize cases that don't need ensureLabelSelectorRequirements Signed-off-by: Pierangelo Di Pilato <[email protected]> Signed-off-by: Pierangelo Di Pilato <[email protected]>
knative · Oct 6, 2022 · fb2e4fb · fb2e4fb
1 parent 5c5da28
commit fb2e4fb
Show file tree

Hide file tree

Showing 2 changed files with 138 additions and 1 deletion.
diff --git 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),
 	}

diff --git 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 {