From 30b03bec3252a16546f70077abbb363f217ceeb7 Mon Sep 17 00:00:00 2001 From: Zhiwei Yin Date: Fri, 16 Apr 2021 15:00:22 +0800 Subject: [PATCH] vaidate manifests size Signed-off-by: Zhiwei Yin --- .../spoketesting/manifestwork_helpers.go | 19 ++++++++ pkg/webhook/manifestwork_webhook.go | 12 +++-- pkg/webhook/manifestwork_webhook_test.go | 12 ++--- test/e2e/webhook_test.go | 44 +++++++++++++++++++ test/e2e/work_agent_test.go | 21 +++++++++ 5 files changed, 99 insertions(+), 9 deletions(-) diff --git a/pkg/spoke/spoketesting/manifestwork_helpers.go b/pkg/spoke/spoketesting/manifestwork_helpers.go index d66483348..fa5f558d8 100644 --- a/pkg/spoke/spoketesting/manifestwork_helpers.go +++ b/pkg/spoke/spoketesting/manifestwork_helpers.go @@ -51,6 +51,25 @@ func NewSecret(name, namespace string, content string) *corev1.Secret { } } +func NewUnstructuredSecretBySize(namespace, name string, size int32) *unstructured.Unstructured { + data := "" + for i := int32(0); i < size; i++ { + data += "a" + } + + return &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "Secret", + "metadata": map[string]interface{}{ + "namespace": namespace, + "name": name, + }, + "data": data, + }, + } +} + func NewUnstructuredSecret(namespace, name string, terminated bool, uid string) *unstructured.Unstructured { u := NewUnstructured("v1", "Secret", namespace, name) if terminated { diff --git a/pkg/webhook/manifestwork_webhook.go b/pkg/webhook/manifestwork_webhook.go index 21175d2bb..786b465f5 100644 --- a/pkg/webhook/manifestwork_webhook.go +++ b/pkg/webhook/manifestwork_webhook.go @@ -16,7 +16,8 @@ import ( "k8s.io/klog/v2" ) -var manifestLimit = 10 +// ManifestLimit is the max size of manifests data which is 50k bytes. +const ManifestLimit = 50 * 1024 // ManifestWorkAdmissionHook will validate the creating/updating manifestwork request. type ManifestWorkAdmissionHook struct{} @@ -90,8 +91,13 @@ func (a *ManifestWorkAdmissionHook) validateManifestWorkObj(requestObj runtime.R return fmt.Errorf("manifests should not be empty") } - if len(work.Spec.Workload.Manifests) > manifestLimit { - return fmt.Errorf("number of manifests should not be larger than %d", manifestLimit) + totalSize := 0 + for _, manifest := range work.Spec.Workload.Manifests { + totalSize = totalSize + manifest.Size() + } + + if totalSize > ManifestLimit { + return fmt.Errorf("the size of manifests is %v bytes which exceeds the 50k limit", totalSize) } for _, manifest := range work.Spec.Workload.Manifests { diff --git a/pkg/webhook/manifestwork_webhook_test.go b/pkg/webhook/manifestwork_webhook_test.go index 1d8af0c5b..6a8016f42 100644 --- a/pkg/webhook/manifestwork_webhook_test.go +++ b/pkg/webhook/manifestwork_webhook_test.go @@ -21,7 +21,6 @@ var manifestWorkSchema = metav1.GroupVersionResource{ } func TestManifestWorkValidate(t *testing.T) { - manifestLimit = 3 cases := []struct { name string request *admissionv1beta1.AdmissionRequest @@ -153,16 +152,17 @@ func TestManifestWorkValidate(t *testing.T) { UserInfo: authenticationv1.UserInfo{Username: "tester"}, }, manifests: []*unstructured.Unstructured{ - spoketesting.NewUnstructured("v1", "Kind", "testns", "test"), - spoketesting.NewUnstructured("v1", "Kind", "testns", "test1"), - spoketesting.NewUnstructured("v1", "Kind", "testns", "test2"), - spoketesting.NewUnstructured("v1", "Kind", "testns", "test3"), + spoketesting.NewUnstructuredSecretBySize("test1", "testns", 10*1024), + spoketesting.NewUnstructuredSecretBySize("test2", "testns", 10*1024), + spoketesting.NewUnstructuredSecretBySize("test3", "testns", 10*1024), + spoketesting.NewUnstructuredSecretBySize("test4", "testns", 10*1024), + spoketesting.NewUnstructuredSecretBySize("test5", "testns", 10*1024), }, expectedResponse: &admissionv1beta1.AdmissionResponse{ Allowed: false, Result: &metav1.Status{ Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest, - Message: "number of manifests should not be larger than 3", + Message: "the size of manifests is 51685 bytes which exceeds the 50k limit", }, }, }, diff --git a/test/e2e/webhook_test.go b/test/e2e/webhook_test.go index dca6ba1bc..aacb0b988 100644 --- a/test/e2e/webhook_test.go +++ b/test/e2e/webhook_test.go @@ -91,5 +91,49 @@ var _ = ginkgo.Describe("ManifestWork admission webhook", func() { admissionName, ))) }) + + ginkgo.It("Should respond bad request when the size of manifests is more than the limit", func() { + manifests := []workapiv1.Manifest{ + { + RawExtension: runtime.RawExtension{ + Object: newSecretBySize("default", "test1", 10*1024), + }, + }, + { + RawExtension: runtime.RawExtension{ + Object: newSecretBySize("default", "test2", 10*1024), + }, + }, + { + RawExtension: runtime.RawExtension{ + Object: newSecretBySize("default", "test3", 10*1024), + }, + }, + { + RawExtension: runtime.RawExtension{ + Object: newSecretBySize("default", "test4", 10*1024), + }, + }, + { + RawExtension: runtime.RawExtension{ + Object: newSecretBySize("default", "test5", 10*1024), + }, + }, + } + + err := retry.RetryOnConflict(retry.DefaultRetry, func() error { + work, err = hubWorkClient.WorkV1().ManifestWorks(clusterName).Get(context.Background(), work.Name, metav1.GetOptions{}) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + work.Spec.Workload.Manifests = append(work.Spec.Workload.Manifests, manifests...) + _, err = hubWorkClient.WorkV1().ManifestWorks(clusterName).Update(context.Background(), work, metav1.UpdateOptions{}) + return err + }) + + gomega.Expect(err).To(gomega.HaveOccurred()) + gomega.Expect(errors.IsBadRequest(err)).Should(gomega.BeTrue()) + gomega.Expect(err.Error()).Should(gomega.HavePrefix(fmt.Sprintf( + "admission webhook \"%s\" denied the request: the size of manifests is", admissionName))) + }) }) }) diff --git a/test/e2e/work_agent_test.go b/test/e2e/work_agent_test.go index 44518de4a..b81432df1 100644 --- a/test/e2e/work_agent_test.go +++ b/test/e2e/work_agent_test.go @@ -541,6 +541,27 @@ func newConfigmap(namespace, name string, data map[string]string, finalizers []s return cm } +func newSecretBySize(namespace, name string, size int32) *corev1.Secret { + data := "" + for i := int32(0); i < size; i++ { + data += "a" + } + + return &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: map[string][]byte{ + "test": []byte(data), + }, + } +} + func haveManifestCondition(conditions []workapiv1.ManifestCondition, expectedType string, expectedStatuses []metav1.ConditionStatus) bool { if len(conditions) != len(expectedStatuses) { return false