From 1dda833ee011fffca79ea2e47ce350662b80adaf Mon Sep 17 00:00:00 2001 From: Federico Paolinelli Date: Tue, 22 Aug 2023 12:06:16 +0200 Subject: [PATCH 1/2] 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 --- pkg/rotator/rotator.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/pkg/rotator/rotator.go b/pkg/rotator/rotator.go index f51ae59..131b648 100644 --- a/pkg/rotator/rotator.go +++ b/pkg/rotator/rotator.go @@ -132,6 +132,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 @@ -173,14 +174,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 @@ -686,6 +689,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 @@ -780,7 +784,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 From e7a7ad53d56c2833d9099b5a9cd4e19cd3e35776 Mon Sep 17 00:00:00 2001 From: Federico Paolinelli Date: Tue, 22 Aug 2023 13:59:02 +0200 Subject: [PATCH 2/2] Unit tests: validate the fieldowner field Signed-off-by: Federico Paolinelli --- pkg/rotator/rotator_test.go | 38 +++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/pkg/rotator/rotator_test.go b/pkg/rotator/rotator_test.go index 8625c8b..55d4b9c 100644 --- a/pkg/rotator/rotator_test.go +++ b/pkg/rotator/rotator_test.go @@ -213,7 +213,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() @@ -235,13 +235,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() @@ -353,6 +353,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 @@ -375,6 +376,7 @@ func TestReconcileWebhook(t *testing.T) { Type: tt.webhookType, }, }, + FieldOwner: fieldOwner, } wh, ok := tt.webhookConfig.DeepCopyObject().(client.Object) if !ok { @@ -382,7 +384,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, fieldOwner) }) // this test does not start the rotator as a runnable instead it tests that @@ -414,7 +416,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, "") }) } } @@ -583,7 +585,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) @@ -595,6 +597,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...) @@ -606,6 +612,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{}