Add an optional "fieldOwner" field to set when injecting the caBundle (…

…#118) * Add an optional "fieldOwner" field to set when injecting the caBundle When using the cert-rotator with CI/CD systems such as argoCD, the injection of the caBundle makes the cd system to reconcile it again. By setting the field owner, we instruct the cd system that the field is managed by an external component so it will ignore those updates. Signed-off-by: Federico Paolinelli <fpaoline@redhat.com> * Unit tests: validate the fieldowner field Signed-off-by: Federico Paolinelli <fpaoline@redhat.com> --------- Signed-off-by: Federico Paolinelli <fpaoline@redhat.com> Co-authored-by: Sertaç Özercan <852750+sozercan@users.noreply.github.com> Co-authored-by: Max Smythe <smythe@google.com>
open-policy-agent · Sep 27, 2023 · 01a9f14 · 01a9f14
1 parent c2f5ff7
commit 01a9f14
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 15 deletions.
diff --git a/pkg/rotator/rotator.go b/pkg/rotator/rotator.go
@@ -161,6 +161,7 @@ func AddRotator(mgr manager.Manager, cr *CertRotator) error {
 		webhooks:                    cr.Webhooks,
 		needLeaderElection:          cr.RequireLeaderElection,
 		refreshCertIfNeededDelegate: cr.refreshCertIfNeeded,
+		fieldOwner:                  cr.FieldOwner,
 	}
 	if err := addController(mgr, reconciler); err != nil {
 		return err
@@ -207,14 +208,16 @@ type CertRotator struct {
 	reader SyncingReader
 	writer client.Writer
 
-	SecretKey              types.NamespacedName
-	CertDir                string
-	CAName                 string
-	CAOrganization         string
-	DNSName                string
-	ExtraDNSNames          []string
-	IsReady                chan struct{}
-	Webhooks               []WebhookInfo
+	SecretKey      types.NamespacedName
+	CertDir        string
+	CAName         string
+	CAOrganization string
+	DNSName        string
+	ExtraDNSNames  []string
+	IsReady        chan struct{}
+	Webhooks       []WebhookInfo
+	// FieldOwner is the optional fieldmanager of the webhook updated fields.
+	FieldOwner             string
 	RestartOnSecretRefresh bool
 	ExtKeyUsages           *[]x509.ExtKeyUsage
 	// RequireLeaderElection should be set to true if the CertRotator needs to
@@ -723,6 +726,7 @@ type ReconcileWH struct {
 	wasCAInjected               *atomic.Bool
 	needLeaderElection          bool
 	refreshCertIfNeededDelegate func() (bool, error)
+	fieldOwner                  string
 }
 
 // Reconcile reads that state of the cluster for a validatingwebhookconfiguration
@@ -817,7 +821,11 @@ func (r *ReconcileWH) ensureCerts(certPem []byte) error {
 			anyError = err
 			continue
 		}
-		if err := r.writer.Update(r.ctx, updatedResource); err != nil {
+		opts := []client.UpdateOption{}
+		if r.fieldOwner != "" {
+			opts = append(opts, client.FieldOwner(r.fieldOwner))
+		}
+		if err := r.writer.Update(r.ctx, updatedResource, opts...); err != nil {
 			log.Error(err, "Error updating webhook with certificate")
 			anyError = err
 			continue

diff --git a/pkg/rotator/rotator_test.go b/pkg/rotator/rotator_test.go
@@ -216,7 +216,7 @@ func setupManager(g *gomega.GomegaWithT) manager.Manager {
 	return mgr
 }
 
-func testWebhook(t *testing.T, secretKey types.NamespacedName, rotator *CertRotator, wh client.Object, webhooksField, caBundleField []string) {
+func testWebhook(t *testing.T, secretKey types.NamespacedName, rotator *CertRotator, wh client.Object, webhooksField, caBundleField []string, fieldOwner string) {
 	ctx, cancelFunc := context.WithCancel(context.Background())
 	defer cancelFunc()
 
@@ -238,13 +238,13 @@ func testWebhook(t *testing.T, secretKey types.NamespacedName, rotator *CertRota
 	ensureCertWasGenerated(ctx, g, c, secretKey)
 
 	// Wait for certificates to populated in managed webhookConfigurations
-	ensureWebhookPopulated(ctx, g, c, wh, webhooksField, caBundleField)
+	ensureWebhookPopulated(ctx, g, c, wh, webhooksField, caBundleField, fieldOwner)
 
 	// Zero out the certificates, ensure they are repopulated
 	resetWebhook(ctx, g, c, wh, webhooksField, caBundleField)
 
 	// Verify certificates are regenerated
-	ensureWebhookPopulated(ctx, g, c, wh, webhooksField, caBundleField)
+	ensureWebhookPopulated(ctx, g, c, wh, webhooksField, caBundleField, fieldOwner)
 
 	cancelFunc()
 	wg.Wait()
@@ -356,6 +356,7 @@ func TestReconcileWebhook(t *testing.T) {
 		var (
 			secretName = "test-secret"
 			whName     = fmt.Sprintf("test-webhook-%s", tt.name)
+			fieldOwner = "foo"
 		)
 
 		// this test relies on the rotator to generate/ rotate the CA
@@ -378,14 +379,15 @@ func TestReconcileWebhook(t *testing.T) {
 						Type: tt.webhookType,
 					},
 				},
+				FieldOwner: fieldOwner,
 			}
 			wh, ok := tt.webhookConfig.DeepCopyObject().(client.Object)
 			if !ok {
 				t.Fatalf("could not deep copy wh object")
 			}
 			wh.SetName(whName)
 
-			testWebhook(t, key, rotator, wh, tt.webhooksField, tt.caBundleField)
+			testWebhook(t, key, rotator, wh, tt.webhooksField, tt.caBundleField, fieldOwner)
 		})
 
 		// this test does not start the rotator as a runnable instead it tests that
@@ -417,7 +419,7 @@ func TestReconcileWebhook(t *testing.T) {
 			}
 			wh.SetName(whName)
 
-			testWebhook(t, key, rotator, wh, tt.webhooksField, tt.caBundleField)
+			testWebhook(t, key, rotator, wh, tt.webhooksField, tt.caBundleField, "")
 		})
 	}
 }
@@ -586,7 +588,7 @@ func extractWebhooks(g *gomega.WithT, u *unstructured.Unstructured, webhooksFiel
 	return webhooks
 }
 
-func ensureWebhookPopulated(ctx context.Context, g *gomega.WithT, c client.Client, wh interface{}, webhooksField, caBundleField []string) {
+func ensureWebhookPopulated(ctx context.Context, g *gomega.WithT, c client.Client, wh interface{}, webhooksField, caBundleField []string, fieldOwner string) {
 	// convert to unstructured object to accept either ValidatingWebhookConfiguration or MutatingWebhookConfiguration
 	whu := &unstructured.Unstructured{}
 	err := c.Scheme().Convert(wh, whu, nil)
@@ -598,6 +600,10 @@ func ensureWebhookPopulated(ctx context.Context, g *gomega.WithT, c client.Clien
 			return false
 		}
 
+		if !checkFieldOwner(whu.Object, fieldOwner) {
+			return false
+		}
+
 		webhooks := extractWebhooks(g, whu, webhooksField)
 		for _, w := range webhooks {
 			caBundle, found, err := unstructured.NestedFieldNoCopy(w.(map[string]interface{}), caBundleField...)
@@ -609,6 +615,26 @@ func ensureWebhookPopulated(ctx context.Context, g *gomega.WithT, c client.Clien
 	}, gEventuallyTimeout, gEventuallyInterval).Should(gomega.BeTrue(), "waiting for webhook reconciliation")
 }
 
+func checkFieldOwner(w map[string]interface{}, fieldOwner string) bool {
+	if fieldOwner == "" {
+		return true
+	}
+	managedFields, found, err := unstructured.NestedSlice(w, "metadata", "managedFields")
+	if !found || err != nil {
+		return false
+	}
+	for _, m := range managedFields {
+		manager, found, err := unstructured.NestedString(m.(map[string]interface{}), "manager")
+		if !found || err != nil {
+			continue
+		}
+		if manager == fieldOwner {
+			return true
+		}
+	}
+	return false
+}
+
 func resetWebhook(ctx context.Context, g *gomega.WithT, c client.Client, wh interface{}, webhooksField, caBundleField []string) {
 	// convert to unstructured object to accept either ValidatingWebhookConfiguration or MutatingWebhookConfiguration
 	whu := &unstructured.Unstructured{}