set webhooks OwnerReferences to namespace

knative · Apr 20, 2021 · 1b3b751 · 1b3b751
1 parent b80a192
commit 1b3b751
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 40 deletions.
diff --git a/webhook/configmaps/configmaps.go b/webhook/configmaps/configmaps.go
@@ -137,9 +137,15 @@ func (ac *reconciler) reconcileValidatingWebhook(ctx context.Context, caCert []b
 
 	webhook := configuredWebhook.DeepCopy()
 
-	// Clear out any previous (bad) OwnerReferences.
-	// See: https://github.com/knative/serving/issues/5845
-	webhook.OwnerReferences = nil
+	// Set the owner to namespace.
+	if len(webhook.OwnerReferences) == 0 {
+		ns, err := ac.client.CoreV1().Namespaces().Get(ctx, system.Namespace(), metav1.GetOptions{})
+		if err != nil {
+			return fmt.Errorf("failed to fetch namespace: %v", err)
+		}
+		nsRef := *metav1.NewControllerRef(ns, corev1.SchemeGroupVersion.WithKind("Namespace"))
+		webhook.OwnerReferences = []metav1.OwnerReference{nsRef}
+	}
 
 	for i, wh := range webhook.Webhooks {
 		if wh.Name != webhook.Name {

diff --git a/webhook/configmaps/table_test.go b/webhook/configmaps/table_test.go
@@ -60,6 +60,14 @@ func TestReconcile(t *testing.T) {
 		},
 	}
 
+	ns := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: system.Namespace(),
+		},
+	}
+	nsRef := *metav1.NewControllerRef(ns, corev1.SchemeGroupVersion.WithKind("Namespace"))
+	expectedOwnerReferences := []metav1.OwnerReference{nsRef}
+
 	ruleScope := admissionregistrationv1.NamespacedScope
 
 	// These are the rules we expect given the context of "validations".
@@ -104,7 +112,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and VWH exist, missing service reference",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -118,7 +126,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and VWH exist, missing other stuff",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -137,7 +145,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,
@@ -159,7 +168,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and VWH exist, added fields are incorrect",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -191,7 +200,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,
@@ -217,7 +227,7 @@ func TestReconcile(t *testing.T) {
 		WithReactors: []clientgotesting.ReactionFunc{
 			InduceFailure("update", "validatingwebhookconfigurations"),
 		},
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -249,7 +259,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,
@@ -271,10 +282,11 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: ":fire: everything is fine :fire:",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,

diff --git a/webhook/resourcesemantics/defaulting/defaulting.go b/webhook/resourcesemantics/defaulting/defaulting.go
@@ -30,6 +30,7 @@ import (
 	jsonpatch "gomodules.xyz/jsonpatch/v2"
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -172,9 +173,14 @@ func (ac *reconciler) reconcileMutatingWebhook(ctx context.Context, caCert []byt
 
 	current := configuredWebhook.DeepCopy()
 
-	// Clear out any previous (bad) OwnerReferences.
-	// See: https://github.com/knative/serving/issues/5845
-	current.OwnerReferences = nil
+	if len(current.OwnerReferences) == 0 {
+		ns, err := ac.client.CoreV1().Namespaces().Get(ctx, system.Namespace(), metav1.GetOptions{})
+		if err != nil {
+			return fmt.Errorf("failed to fetch namespace: %v", err)
+		}
+		nsRef := *metav1.NewControllerRef(ns, corev1.SchemeGroupVersion.WithKind("Namespace"))
+		current.OwnerReferences = []metav1.OwnerReference{nsRef}
+	}
 
 	for i, wh := range current.Webhooks {
 		if wh.Name != current.Name {

diff --git a/webhook/resourcesemantics/defaulting/table_test.go b/webhook/resourcesemantics/defaulting/table_test.go
@@ -60,6 +60,13 @@ func TestReconcile(t *testing.T) {
 			certresources.CACert:     []byte("present"),
 		},
 	}
+	ns := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: system.Namespace(),
+		},
+	}
+	nsRef := *metav1.NewControllerRef(ns, corev1.SchemeGroupVersion.WithKind("Namespace"))
+	expectedOwnerReferences := []metav1.OwnerReference{nsRef}
 
 	// This is the namespace selector setup
 	namespaceSelector := &metav1.LabelSelector{
@@ -131,7 +138,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and MWH exist, missing service reference",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -145,7 +152,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and MWH exist, missing other stuff",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -164,7 +171,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.MutatingWebhook{{
 					Name: name,
@@ -187,7 +195,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and MWH exist, added fields are incorrect",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -219,7 +227,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.MutatingWebhook{{
 					Name: name,
@@ -246,7 +255,7 @@ func TestReconcile(t *testing.T) {
 		WithReactors: []clientgotesting.ReactionFunc{
 			InduceFailure("update", "mutatingwebhookconfigurations"),
 		},
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -278,7 +287,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.MutatingWebhook{{
 					Name: name,
@@ -301,10 +311,11 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: ":fire: everything is fine :fire:",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.MutatingWebhook{{
 					Name: name,
@@ -336,7 +347,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and MWH exist, correcting namespaceSelector",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -371,7 +382,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.MutatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.MutatingWebhook{{
 					Name: name,

diff --git a/webhook/resourcesemantics/validation/reconcile_config.go b/webhook/resourcesemantics/validation/reconcile_config.go
@@ -25,6 +25,7 @@ import (
 	"github.com/gobuffalo/flect"
 	"go.uber.org/zap"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
@@ -135,9 +136,15 @@ func (ac *reconciler) reconcileValidatingWebhook(ctx context.Context, caCert []b
 
 	current := configuredWebhook.DeepCopy()
 
-	// Clear out any previous (bad) OwnerReferences.
-	// See: https://github.com/knative/serving/issues/5845
-	current.OwnerReferences = nil
+	// Set the owner to namespace.
+	if len(current.OwnerReferences) == 0 {
+		ns, err := ac.client.CoreV1().Namespaces().Get(ctx, system.Namespace(), metav1.GetOptions{})
+		if err != nil {
+			return fmt.Errorf("failed to fetch namespace: %v", err)
+		}
+		nsRef := *metav1.NewControllerRef(ns, corev1.SchemeGroupVersion.WithKind("Namespace"))
+		current.OwnerReferences = []metav1.OwnerReference{nsRef}
+	}
 
 	for i, wh := range current.Webhooks {
 		if wh.Name != current.Name {

diff --git a/webhook/resourcesemantics/validation/reconcile_config_test.go b/webhook/resourcesemantics/validation/reconcile_config_test.go
@@ -61,6 +61,14 @@ func TestReconcile(t *testing.T) {
 		},
 	}
 
+	ns := &corev1.Namespace{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: system.Namespace(),
+		},
+	}
+	nsRef := *metav1.NewControllerRef(ns, corev1.SchemeGroupVersion.WithKind("Namespace"))
+	expectedOwnerReferences := []metav1.OwnerReference{nsRef}
+
 	// This is the namespace selector setup
 	namespaceSelector := &metav1.LabelSelector{
 		MatchExpressions: []metav1.LabelSelectorRequirement{{
@@ -131,7 +139,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and VWH exist, missing service reference",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -145,7 +153,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and VWH exist, missing other stuff",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -164,7 +172,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,
@@ -187,7 +196,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and VWH exist, added fields are incorrect",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -219,7 +228,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,
@@ -246,7 +256,7 @@ func TestReconcile(t *testing.T) {
 		WithReactors: []clientgotesting.ReactionFunc{
 			InduceFailure("update", "validatingwebhookconfigurations"),
 		},
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -278,7 +288,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,
@@ -301,10 +312,11 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: ":fire: everything is fine :fire:",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,
@@ -336,7 +348,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		Name: "secret and VWH exist, correcting namespaceSelector",
 		Key:  key,
-		Objects: []runtime.Object{secret,
+		Objects: []runtime.Object{secret, ns,
 			&admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: name,
@@ -371,7 +383,8 @@ func TestReconcile(t *testing.T) {
 		WantUpdates: []clientgotesting.UpdateActionImpl{{
 			Object: &admissionregistrationv1.ValidatingWebhookConfiguration{
 				ObjectMeta: metav1.ObjectMeta{
-					Name: name,
+					Name:            name,
+					OwnerReferences: expectedOwnerReferences,
 				},
 				Webhooks: []admissionregistrationv1.ValidatingWebhook{{
 					Name: name,