diff --git a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go index ccd8970c50..0034235fb2 100644 --- a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go +++ b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s.go @@ -50,7 +50,8 @@ func (k8s *k8s) PatchObjects(ctx context.Context, options PatchOptions) error { return fmt.Errorf("failurePolicy specified, but no webhook will be patched") } - if options.MutatingWebhookConfigurationName != options.ValidatingWebhookConfigurationName { + if patchMutating && patchValidating && + options.MutatingWebhookConfigurationName != options.ValidatingWebhookConfigurationName { return fmt.Errorf("webhook names must be the same") } @@ -64,8 +65,13 @@ func (k8s *k8s) PatchObjects(ctx context.Context, options PatchOptions) error { } } + webhookName := options.ValidatingWebhookConfigurationName + if webhookName == "" { + webhookName = options.MutatingWebhookConfigurationName + } + if patchMutating || patchValidating { - return k8s.patchWebhookConfigurations(ctx, options.ValidatingWebhookConfigurationName, options.CABundle, options.FailurePolicyType, patchMutating, patchValidating) + return k8s.patchWebhookConfigurations(ctx, webhookName, options.CABundle, options.FailurePolicyType, patchMutating, patchValidating) } return nil diff --git a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go index 0cddcb99e3..deaeb3540a 100644 --- a/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go +++ b/images/kube-webhook-certgen/rootfs/pkg/k8s/k8s_test.go @@ -6,17 +6,22 @@ import ( "errors" "math/rand" "testing" + "time" admissionv1 "k8s.io/api/admissionregistration/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" + apiregistrationv1 "k8s.io/kube-aggregator/pkg/apis/apiregistration/v1" + aggregatorfake "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake" ) const ( - testWebhookName = "c7c95710-d8c3-4cc3-a2a8-8d2b46909c76" - testSecretName = "15906410-af2a-4f9b-8a2d-c08ffdd5e129" - testNamespace = "7cad5f92-c0d5-4bc9-87a3-6f44d5a5619d" + testWebhookName = "c7c95710-d8c3-4cc3-a2a8-8d2b46909c76" + testSecretName = "15906410-af2a-4f9b-8a2d-c08ffdd5e129" + testAPIServiceName = "37f6a2d1-b401-4275-833b-9ff5004f0301" + testNamespace = "7cad5f92-c0d5-4bc9-87a3-6f44d5a5619d" ) var ( @@ -33,27 +38,27 @@ func genSecretData() (ca, cert, key []byte) { return } -func newTestSimpleK8s() *k8s { +func newTestSimpleK8s(objects ...runtime.Object) *k8s { return &k8s{ - clientset: fake.NewSimpleClientset(), + clientset: fake.NewSimpleClientset(objects...), + aggregatorClientset: aggregatorfake.NewSimpleClientset(), } } func TestGetCaFromCertificate(t *testing.T) { - k := newTestSimpleK8s() - ca, cert, key := genSecretData() secret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: testSecretName, + Name: testSecretName, + Namespace: testNamespace, }, Data: map[string][]byte{"ca": ca, "cert": cert, "key": key}, } - k.clientset.CoreV1().Secrets(testNamespace).Create(context.Background(), secret, metav1.CreateOptions{}) + k := newTestSimpleK8s(secret) - retrievedCa := k.GetCaFromSecret(context.TODO(), testSecretName, testNamespace) + retrievedCa := k.GetCaFromSecret(contextWithDeadline(t), testSecretName, testNamespace) if !bytes.Equal(retrievedCa, ca) { t.Error("Was not able to retrieve CA information that was saved") } @@ -64,9 +69,11 @@ func TestSaveCertsToSecret(t *testing.T) { ca, cert, key := genSecretData() - k.SaveCertsToSecret(context.TODO(), testSecretName, testNamespace, "cert", "key", ca, cert, key) + ctx := contextWithDeadline(t) + + k.SaveCertsToSecret(ctx, testSecretName, testNamespace, "cert", "key", ca, cert, key) - secret, _ := k.clientset.CoreV1().Secrets(testNamespace).Get(context.Background(), testSecretName, metav1.GetOptions{}) + secret, _ := k.clientset.CoreV1().Secrets(testNamespace).Get(ctx, testSecretName, metav1.GetOptions{}) if !bytes.Equal(secret.Data["cert"], cert) { t.Error("'cert' saved data does not match retrieved") @@ -80,7 +87,7 @@ func TestSaveCertsToSecret(t *testing.T) { func TestSaveThenLoadSecret(t *testing.T) { k := newTestSimpleK8s() ca, cert, key := genSecretData() - ctx := context.TODO() + ctx := contextWithDeadline(t) k.SaveCertsToSecret(ctx, testSecretName, testNamespace, "cert", "key", ca, cert, key) retrievedCert := k.GetCaFromSecret(ctx, testSecretName, testNamespace) if !bytes.Equal(retrievedCert, ca) { @@ -89,40 +96,35 @@ func TestSaveThenLoadSecret(t *testing.T) { } func TestPatchWebhookConfigurations(t *testing.T) { - k := newTestSimpleK8s() - ca, _, _ := genSecretData() - k.clientset. - AdmissionregistrationV1(). - MutatingWebhookConfigurations(). - Create(context.Background(), &admissionv1.MutatingWebhookConfiguration{ + k := newTestSimpleK8s( + &admissionv1.MutatingWebhookConfiguration{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: testWebhookName, }, Webhooks: []admissionv1.MutatingWebhook{{Name: "m1"}, {Name: "m2"}}, - }, metav1.CreateOptions{}) - - k.clientset. - AdmissionregistrationV1(). - ValidatingWebhookConfigurations(). - Create(context.Background(), &admissionv1.ValidatingWebhookConfiguration{ + }, + &admissionv1.ValidatingWebhookConfiguration{ TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{ Name: testWebhookName, }, Webhooks: []admissionv1.ValidatingWebhook{{Name: "v1"}, {Name: "v2"}}, - }, metav1.CreateOptions{}) + }, + ) + + ctx := contextWithDeadline(t) - if err := k.patchWebhookConfigurations(context.TODO(), testWebhookName, ca, fail, true, true); err != nil { + if err := k.patchWebhookConfigurations(ctx, testWebhookName, ca, fail, true, true); err != nil { t.Fatalf("Unexpected error patching webhooks: %s: %v", err.Error(), errors.Unwrap(err)) } whmut, err := k.clientset. AdmissionregistrationV1(). MutatingWebhookConfigurations(). - Get(context.Background(), testWebhookName, metav1.GetOptions{}) + Get(ctx, testWebhookName, metav1.GetOptions{}) if err != nil { t.Error(err) } @@ -130,7 +132,7 @@ func TestPatchWebhookConfigurations(t *testing.T) { whval, err := k.clientset. AdmissionregistrationV1(). MutatingWebhookConfigurations(). - Get(context.Background(), testWebhookName, metav1.GetOptions{}) + Get(ctx, testWebhookName, metav1.GetOptions{}) if err != nil { t.Error(err) } @@ -164,37 +166,194 @@ func TestPatchWebhookConfigurations(t *testing.T) { func Test_Patching_objects(t *testing.T) { t.Parallel() + ctx := contextWithDeadline(t) + t.Run("returns_error_when", func(t *testing.T) { t.Parallel() t.Run("failure_policy_is_defined_but_no_webhooks_will_be_patched", func(t *testing.T) { t.Parallel() + + k := testK8sWithUnpatchedObjects() + + o := PatchOptions{ + FailurePolicyType: admissionv1.Fail, + } + + if err := k.PatchObjects(ctx, o); err == nil { + t.Fatalf("Expected error while patching") + } }) // This is to preserve old behavior and log format, it could be improved. - t.Run("diffent_names_are_specified_for_validating_and_mutating_webhook", func(t *testing.T) { + t.Run("diffent_non_empty_names_are_specified_for_validating_and_mutating_webhook", func(t *testing.T) { t.Parallel() + + k := testK8sWithUnpatchedObjects() + + o := PatchOptions{ + ValidatingWebhookConfigurationName: "foo", + MutatingWebhookConfigurationName: "bar", + } + + if err := k.PatchObjects(ctx, o); err == nil { + t.Fatalf("Expected error while patching") + } }) - t.Run("patching_APIService_is_requested_and_it_does_not_exist", func(t *testing.T) { + t.Run("patching_webhook_is_requested_and_it_does_not_exist", func(t *testing.T) { t.Parallel() + + k := newTestSimpleK8s() + + o := PatchOptions{ + ValidatingWebhookConfigurationName: "foo", + } + + if err := k.PatchObjects(ctx, o); err == nil { + t.Fatalf("Expected error while patching") + } }) - t.Run("patching_webhook_is_requested_and_it_does_not_exist", func(t *testing.T) { + t.Run("patching_APIService_is_requested_and_it_does_not_exist", func(t *testing.T) { t.Parallel() + + k := newTestSimpleK8s() + + o := PatchOptions{ + APIServiceName: "foo", + } + + if err := k.PatchObjects(ctx, o); err == nil { + t.Fatalf("Expected error while patching") + } }) }) t.Run("when_patching_APIService_object", func(t *testing.T) { t.Parallel() + k := testK8sWithUnpatchedObjects() + + o := PatchOptions{ + APIServiceName: testAPIServiceName, + CABundle: []byte("foo"), + } + + if err := k.PatchObjects(ctx, o); err != nil { + t.Fatalf("Unexpected error while patching objects: %v", err) + } + + c := k.aggregatorClientset.ApiregistrationV1().APIServices() + apiService, err := c.Get(ctx, testAPIServiceName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Unexpected error while getting APIService object %q: %v", testAPIServiceName, err) + } + // This is required when CABundle field is populated. t.Run("sets_insecure_skip_tls_verity_to_false", func(t *testing.T) { t.Parallel() + + if apiService.Spec.InsecureSkipTLSVerify { + t.Fatalf("Expected insecureSkipTLSVerify of APIService to be false") + } }) t.Run("sets_ca_bundle_with_ca_certificate_from_created_secret", func(t *testing.T) { t.Parallel() + + if len(apiService.Spec.CABundle) == 0 { + t.Fatalf("Expected CABundle of APIService to be not empty") + } + + if !bytes.Equal(o.CABundle, apiService.Spec.CABundle) { + t.Fatalf("CABundle content of APIService does not match requested bundle") + } }) }) + + t.Run("allows_patching_only_validating_webhook", func(t *testing.T) { + t.Parallel() + + k := testK8sWithUnpatchedObjects() + + o := PatchOptions{ + ValidatingWebhookConfigurationName: testWebhookName, + } + + if err := k.PatchObjects(ctx, o); err != nil { + t.Fatalf("Unexpected error patching objects: %v", err) + } + }) + + t.Run("allows_patching_only_mutating_webhook", func(t *testing.T) { + t.Parallel() + + k := testK8sWithUnpatchedObjects() + + o := PatchOptions{ + MutatingWebhookConfigurationName: testWebhookName, + } + + if err := k.PatchObjects(ctx, o); err != nil { + t.Fatalf("Unexpected error patching objects: %v", err) + } + }) +} + +const ( + // Arbitrary amount of time to let tests exit cleanly before main process terminates. + timeoutGracePeriod = 10 * time.Second +) + +// contextWithDeadline returns context with will timeout before t.Deadline(). +func contextWithDeadline(t *testing.T) context.Context { + t.Helper() + + deadline, ok := t.Deadline() + if !ok { + return context.Background() + } + + ctx, cancel := context.WithDeadline(context.Background(), deadline.Truncate(timeoutGracePeriod)) + + t.Cleanup(cancel) + + return ctx +} + +func testK8sWithUnpatchedObjects() *k8s { + ca, cert, key := genSecretData() + + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: testSecretName, + Namespace: testNamespace, + }, + Data: map[string][]byte{"ca": ca, "cert": cert, "key": key}, + } + + validatingWebhook := &admissionv1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: testWebhookName, + }, + Webhooks: []admissionv1.ValidatingWebhook{{Name: "v1"}, {Name: "v2"}}, + } + mutatingWebhook := &admissionv1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{ + Name: testWebhookName, + }, + Webhooks: []admissionv1.MutatingWebhook{{Name: "m1"}, {Name: "m2"}}, + } + + apiService := &apiregistrationv1.APIService{ + ObjectMeta: metav1.ObjectMeta{ + Name: testAPIServiceName, + }, + } + + return &k8s{ + clientset: fake.NewSimpleClientset(secret, validatingWebhook, mutatingWebhook), + aggregatorClientset: aggregatorfake.NewSimpleClientset(apiService), + } }