Skip to content

Commit 37416f0

Browse files
Wei WengWei Weng
Wei Weng
authored and
Wei Weng
committed
check CA bundle injection for readiness
Signed-off-by: Wei Weng <[email protected]>
1 parent 5011109 commit 37416f0

File tree

4 files changed

+362
-9
lines changed

4 files changed

+362
-9
lines changed

‎cmd/hubagent/main.go‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"flag"
2121
"fmt"
2222
"math"
23+
"net/http"
2324
"os"
2425
"strings"
2526
"sync"
@@ -157,11 +158,25 @@ func main() {
157158

158159
if opts.EnableWebhook {
159160
whiteListedUsers := strings.Split(opts.WhiteListedUsers, ",")
160-
if err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers,
161-
opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload, opts.NetworkingAgentsEnabled, opts.UseCertManager, opts.WebhookCertDir, opts.WebhookCertName, opts.WebhookCertSecretName); err != nil {
161+
webhookConfig, err := SetupWebhook(mgr, options.WebhookClientConnectionType(opts.WebhookClientConnectionType), opts.WebhookServiceName, whiteListedUsers,
162+
opts.EnableGuardRail, opts.EnableV1Beta1APIs, opts.DenyModifyMemberClusterLabels, opts.EnableWorkload, opts.NetworkingAgentsEnabled, opts.UseCertManager, opts.WebhookCertDir, opts.WebhookCertName, opts.WebhookCertSecretName)
163+
if err != nil {
162164
klog.ErrorS(err, "unable to set up webhook")
163165
exitWithErrorFunc()
164166
}
167+
168+
// When using cert-manager, add a readiness check to ensure CA bundles are injected before marking ready.
169+
// This prevents the pod from accepting traffic before cert-manager has populated the webhook CA bundles,
170+
// which would cause webhook calls to fail.
171+
if opts.UseCertManager {
172+
if err := mgr.AddReadyzCheck("cert-manager-ca-injection", func(req *http.Request) error {
173+
return webhookConfig.CheckCAInjection(req.Context())
174+
}); err != nil {
175+
klog.ErrorS(err, "unable to set up cert-manager CA injection readiness check")
176+
exitWithErrorFunc()
177+
}
178+
klog.V(2).InfoS("Added cert-manager CA injection readiness check")
179+
}
165180
}
166181

167182
ctx := ctrl.SetupSignalHandler()
@@ -200,21 +215,22 @@ func main() {
200215
}
201216

202217
// SetupWebhook generates the webhook cert and then set up the webhook configurator.
218+
// Returns the webhook Config so it can be used for readiness checks.
203219
func SetupWebhook(mgr manager.Manager, webhookClientConnectionType options.WebhookClientConnectionType, webhookServiceName string,
204-
whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool, networkingAgentsEnabled bool, useCertManager bool, webhookCertDir string, webhookCertName string, webhookCertSecretName string) error {
220+
whiteListedUsers []string, enableGuardRail, isFleetV1Beta1API bool, denyModifyMemberClusterLabels bool, enableWorkload bool, networkingAgentsEnabled bool, useCertManager bool, webhookCertDir string, webhookCertName string, webhookCertSecretName string) (*webhook.Config, error) {
205221
// Generate self-signed key and crt files in webhookCertDir for the webhook server to start.
206222
w, err := webhook.NewWebhookConfig(mgr, webhookServiceName, FleetWebhookPort, &webhookClientConnectionType, webhookCertDir, enableGuardRail, denyModifyMemberClusterLabels, enableWorkload, useCertManager, webhookCertName, webhookCertSecretName)
207223
if err != nil {
208224
klog.ErrorS(err, "fail to generate WebhookConfig")
209-
return err
225+
return nil, err
210226
}
211227
if err = mgr.Add(w); err != nil {
212228
klog.ErrorS(err, "unable to add WebhookConfig")
213-
return err
229+
return nil, err
214230
}
215231
if err = webhook.AddToManager(mgr, whiteListedUsers, denyModifyMemberClusterLabels, networkingAgentsEnabled); err != nil {
216232
klog.ErrorS(err, "unable to register webhooks to the manager")
217-
return err
233+
return nil, err
218234
}
219-
return nil
235+
return w, nil
220236
}

‎pkg/webhook/webhook.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,56 @@ func (w *Config) Start(ctx context.Context) error {
219219
return nil
220220
}
221221

222+
// CheckCAInjection verifies that cert-manager has injected the CA bundle into all webhook configurations.
223+
// This is used as a readiness check when useCertManager is enabled.
224+
// Returns nil when CA bundles are injected, or an error if they are missing.
225+
func (w *Config) CheckCAInjection(ctx context.Context) error {
226+
if !w.useCertManager {
227+
// Not using cert-manager, no need to check
228+
return nil
229+
}
230+
231+
cl := w.mgr.GetClient()
232+
233+
// Check mutating webhook configuration
234+
var mutatingCfg admv1.MutatingWebhookConfiguration
235+
if err := cl.Get(ctx, client.ObjectKey{Name: fleetMutatingWebhookCfgName}, &mutatingCfg); err != nil {
236+
return fmt.Errorf("failed to get MutatingWebhookConfiguration %s: %w", fleetMutatingWebhookCfgName, err)
237+
}
238+
for _, webhook := range mutatingCfg.Webhooks {
239+
if len(webhook.ClientConfig.CABundle) == 0 {
240+
return fmt.Errorf("MutatingWebhookConfiguration %s webhook %s is missing CA bundle (cert-manager injection pending)", fleetMutatingWebhookCfgName, webhook.Name)
241+
}
242+
}
243+
244+
// Check validating webhook configuration
245+
var validatingCfg admv1.ValidatingWebhookConfiguration
246+
if err := cl.Get(ctx, client.ObjectKey{Name: fleetValidatingWebhookCfgName}, &validatingCfg); err != nil {
247+
return fmt.Errorf("failed to get ValidatingWebhookConfiguration %s: %w", fleetValidatingWebhookCfgName, err)
248+
}
249+
for _, webhook := range validatingCfg.Webhooks {
250+
if len(webhook.ClientConfig.CABundle) == 0 {
251+
return fmt.Errorf("ValidatingWebhookConfiguration %s webhook %s is missing CA bundle (cert-manager injection pending)", fleetValidatingWebhookCfgName, webhook.Name)
252+
}
253+
}
254+
255+
// Check guard rail webhook configuration if enabled
256+
if w.enableGuardRail {
257+
var guardRailCfg admv1.ValidatingWebhookConfiguration
258+
if err := cl.Get(ctx, client.ObjectKey{Name: fleetGuardRailWebhookCfgName}, &guardRailCfg); err != nil {
259+
return fmt.Errorf("failed to get ValidatingWebhookConfiguration %s: %w", fleetGuardRailWebhookCfgName, err)
260+
}
261+
for _, webhook := range guardRailCfg.Webhooks {
262+
if len(webhook.ClientConfig.CABundle) == 0 {
263+
return fmt.Errorf("ValidatingWebhookConfiguration %s webhook %s is missing CA bundle (cert-manager injection pending)", fleetGuardRailWebhookCfgName, webhook.Name)
264+
}
265+
}
266+
}
267+
268+
klog.V(2).InfoS("All webhook configurations have CA bundles injected by cert-manager")
269+
return nil
270+
}
271+
222272
// createFleetWebhookConfiguration creates the ValidatingWebhookConfiguration object for the webhook.
223273
func (w *Config) createFleetWebhookConfiguration(ctx context.Context) error {
224274
if err := w.createMutatingWebhookConfiguration(ctx, w.buildFleetMutatingWebhooks(), fleetMutatingWebhookCfgName); err != nil {

‎pkg/webhook/webhook_test.go‎

Lines changed: 254 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,25 @@
11
package webhook
22

33
import (
4+
"context"
45
"strings"
56
"testing"
67

78
"github.com/google/go-cmp/cmp"
89
"github.com/google/go-cmp/cmp/cmpopts"
910
"github.com/stretchr/testify/assert"
11+
admv1 "k8s.io/api/admissionregistration/v1"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/runtime"
14+
"sigs.k8s.io/controller-runtime/pkg/client"
15+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1016
"sigs.k8s.io/controller-runtime/pkg/manager"
1117

1218
"github.com/kubefleet-dev/kubefleet/cmd/hubagent/options"
1319
"github.com/kubefleet-dev/kubefleet/pkg/utils"
20+
testmanager "github.com/kubefleet-dev/kubefleet/test/utils/manager"
1421
)
1522

16-
const testWebhookServiceName = "test-webhook"
17-
1823
func TestBuildFleetMutatingWebhooks(t *testing.T) {
1924
url := options.WebhookClientConnectionType("url")
2025
testCases := map[string]struct {
@@ -340,3 +345,250 @@ func TestCreateClientConfig(t *testing.T) {
340345
})
341346
}
342347
}
348+
349+
func TestCheckCAInjection(t *testing.T) {
350+
testCases := map[string]struct {
351+
config Config
352+
existingObjects []client.Object
353+
expectError bool
354+
expectedErrorMsg string
355+
}{
356+
"useCertManager is false - returns nil without checking": {
357+
config: Config{
358+
useCertManager: false,
359+
enableGuardRail: false,
360+
},
361+
expectError: false,
362+
},
363+
"useCertManager is true, all CA bundles present": {
364+
config: Config{
365+
useCertManager: true,
366+
enableGuardRail: false,
367+
},
368+
existingObjects: []client.Object{
369+
&admv1.MutatingWebhookConfiguration{
370+
ObjectMeta: metav1.ObjectMeta{Name: fleetMutatingWebhookCfgName},
371+
Webhooks: []admv1.MutatingWebhook{
372+
{
373+
Name: "test-mutating-webhook",
374+
ClientConfig: admv1.WebhookClientConfig{
375+
CABundle: []byte("fake-ca-bundle"),
376+
},
377+
},
378+
},
379+
},
380+
&admv1.ValidatingWebhookConfiguration{
381+
ObjectMeta: metav1.ObjectMeta{Name: fleetValidatingWebhookCfgName},
382+
Webhooks: []admv1.ValidatingWebhook{
383+
{
384+
Name: "test-validating-webhook-1",
385+
ClientConfig: admv1.WebhookClientConfig{
386+
CABundle: []byte("fake-ca-bundle"),
387+
},
388+
},
389+
{
390+
Name: "test-validating-webhook-2",
391+
ClientConfig: admv1.WebhookClientConfig{
392+
CABundle: []byte("fake-ca-bundle"),
393+
},
394+
},
395+
},
396+
},
397+
},
398+
expectError: false,
399+
},
400+
"useCertManager is true, mutating webhook missing CA bundle": {
401+
config: Config{
402+
useCertManager: true,
403+
enableGuardRail: false,
404+
},
405+
existingObjects: []client.Object{
406+
&admv1.MutatingWebhookConfiguration{
407+
ObjectMeta: metav1.ObjectMeta{Name: fleetMutatingWebhookCfgName},
408+
Webhooks: []admv1.MutatingWebhook{
409+
{
410+
Name: "test-mutating-webhook",
411+
ClientConfig: admv1.WebhookClientConfig{
412+
CABundle: nil, // Missing CA bundle
413+
},
414+
},
415+
},
416+
},
417+
&admv1.ValidatingWebhookConfiguration{
418+
ObjectMeta: metav1.ObjectMeta{Name: fleetValidatingWebhookCfgName},
419+
Webhooks: []admv1.ValidatingWebhook{
420+
{
421+
Name: "test-validating-webhook",
422+
ClientConfig: admv1.WebhookClientConfig{
423+
CABundle: []byte("fake-ca-bundle"),
424+
},
425+
},
426+
},
427+
},
428+
},
429+
expectError: true,
430+
expectedErrorMsg: "test-mutating-webhook is missing CA bundle",
431+
},
432+
"useCertManager is true, validating webhook missing CA bundle": {
433+
config: Config{
434+
useCertManager: true,
435+
enableGuardRail: false,
436+
},
437+
existingObjects: []client.Object{
438+
&admv1.MutatingWebhookConfiguration{
439+
ObjectMeta: metav1.ObjectMeta{Name: fleetMutatingWebhookCfgName},
440+
Webhooks: []admv1.MutatingWebhook{
441+
{
442+
Name: "test-mutating-webhook",
443+
ClientConfig: admv1.WebhookClientConfig{
444+
CABundle: []byte("fake-ca-bundle"),
445+
},
446+
},
447+
},
448+
},
449+
&admv1.ValidatingWebhookConfiguration{
450+
ObjectMeta: metav1.ObjectMeta{Name: fleetValidatingWebhookCfgName},
451+
Webhooks: []admv1.ValidatingWebhook{
452+
{
453+
Name: "test-validating-webhook-1",
454+
ClientConfig: admv1.WebhookClientConfig{
455+
CABundle: []byte("fake-ca-bundle"),
456+
},
457+
},
458+
{
459+
Name: "test-validating-webhook-2",
460+
ClientConfig: admv1.WebhookClientConfig{
461+
CABundle: []byte{}, // Empty CA bundle
462+
},
463+
},
464+
},
465+
},
466+
},
467+
expectError: true,
468+
expectedErrorMsg: "test-validating-webhook-2 is missing CA bundle",
469+
},
470+
"useCertManager is true with guard rail, all CA bundles present": {
471+
config: Config{
472+
useCertManager: true,
473+
enableGuardRail: true,
474+
},
475+
existingObjects: []client.Object{
476+
&admv1.MutatingWebhookConfiguration{
477+
ObjectMeta: metav1.ObjectMeta{Name: fleetMutatingWebhookCfgName},
478+
Webhooks: []admv1.MutatingWebhook{
479+
{
480+
Name: "test-mutating-webhook",
481+
ClientConfig: admv1.WebhookClientConfig{
482+
CABundle: []byte("fake-ca-bundle"),
483+
},
484+
},
485+
},
486+
},
487+
&admv1.ValidatingWebhookConfiguration{
488+
ObjectMeta: metav1.ObjectMeta{Name: fleetValidatingWebhookCfgName},
489+
Webhooks: []admv1.ValidatingWebhook{
490+
{
491+
Name: "test-validating-webhook",
492+
ClientConfig: admv1.WebhookClientConfig{
493+
CABundle: []byte("fake-ca-bundle"),
494+
},
495+
},
496+
},
497+
},
498+
&admv1.ValidatingWebhookConfiguration{
499+
ObjectMeta: metav1.ObjectMeta{Name: fleetGuardRailWebhookCfgName},
500+
Webhooks: []admv1.ValidatingWebhook{
501+
{
502+
Name: "test-guard-rail-webhook",
503+
ClientConfig: admv1.WebhookClientConfig{
504+
CABundle: []byte("fake-ca-bundle"),
505+
},
506+
},
507+
},
508+
},
509+
},
510+
expectError: false,
511+
},
512+
"useCertManager is true with guard rail, guard rail webhook missing CA bundle": {
513+
config: Config{
514+
useCertManager: true,
515+
enableGuardRail: true,
516+
},
517+
existingObjects: []client.Object{
518+
&admv1.MutatingWebhookConfiguration{
519+
ObjectMeta: metav1.ObjectMeta{Name: fleetMutatingWebhookCfgName},
520+
Webhooks: []admv1.MutatingWebhook{
521+
{
522+
Name: "test-mutating-webhook",
523+
ClientConfig: admv1.WebhookClientConfig{
524+
CABundle: []byte("fake-ca-bundle"),
525+
},
526+
},
527+
},
528+
},
529+
&admv1.ValidatingWebhookConfiguration{
530+
ObjectMeta: metav1.ObjectMeta{Name: fleetValidatingWebhookCfgName},
531+
Webhooks: []admv1.ValidatingWebhook{
532+
{
533+
Name: "test-validating-webhook",
534+
ClientConfig: admv1.WebhookClientConfig{
535+
CABundle: []byte("fake-ca-bundle"),
536+
},
537+
},
538+
},
539+
},
540+
&admv1.ValidatingWebhookConfiguration{
541+
ObjectMeta: metav1.ObjectMeta{Name: fleetGuardRailWebhookCfgName},
542+
Webhooks: []admv1.ValidatingWebhook{
543+
{
544+
Name: "test-guard-rail-webhook",
545+
ClientConfig: admv1.WebhookClientConfig{
546+
CABundle: nil, // Missing CA bundle
547+
},
548+
},
549+
},
550+
},
551+
},
552+
expectError: true,
553+
expectedErrorMsg: "test-guard-rail-webhook is missing CA bundle",
554+
},
555+
"useCertManager is true, webhook configuration not found": {
556+
config: Config{
557+
useCertManager: true,
558+
enableGuardRail: false,
559+
},
560+
existingObjects: []client.Object{},
561+
expectError: true,
562+
expectedErrorMsg: "failed to get MutatingWebhookConfiguration",
563+
},
564+
}
565+
566+
for name, tc := range testCases {
567+
t.Run(name, func(t *testing.T) {
568+
// Create a fake client with a proper scheme
569+
scheme := runtime.NewScheme()
570+
_ = admv1.AddToScheme(scheme)
571+
fakeClient := fake.NewClientBuilder().
572+
WithScheme(scheme).
573+
WithObjects(tc.existingObjects...).
574+
Build()
575+
576+
// Create a fake manager that returns our fake client
577+
tc.config.mgr = &testmanager.FakeManager{Client: fakeClient}
578+
579+
err := tc.config.CheckCAInjection(context.Background())
580+
581+
if tc.expectError {
582+
if err == nil {
583+
t.Errorf("Expected error but got nil")
584+
} else if !strings.Contains(err.Error(), tc.expectedErrorMsg) {
585+
t.Errorf("Expected error message to contain %q, but got: %v", tc.expectedErrorMsg, err)
586+
}
587+
} else {
588+
if err != nil {
589+
t.Errorf("Expected no error but got: %v", err)
590+
}
591+
}
592+
})
593+
}
594+
}

0 commit comments

Comments
 (0)