Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an optional "fieldOwner" field to set when injecting the caBundle #118

Merged
merged 5 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions pkg/rotator/rotator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
38 changes: 32 additions & 6 deletions pkg/rotator/rotator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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, "")
})
}
}
Expand Down Expand Up @@ -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)
Expand All @@ -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...)
Expand All @@ -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{}
Expand Down
Loading