Skip to content

Commit

Permalink
the manifest size limit can be configured (open-cluster-management-io…
Browse files Browse the repository at this point in the history
…#186)

Signed-off-by: Zhiwei Yin <zyin@redhat.com>
  • Loading branch information
zhiweiyin318 authored Feb 28, 2023
1 parent ed501f7 commit 9a872eb
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 111 deletions.
10 changes: 7 additions & 3 deletions pkg/cmd/webhook/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ import "github.com/spf13/pflag"

// Config contains the server (the webhook) cert and key.
type Options struct {
Port int
CertDir string
Port int
CertDir string
ManifestLimit int
}

// NewOptions constructs a new set of default options for webhook.
func NewOptions() *Options {
return &Options{
Port: 9443,
Port: 9443,
ManifestLimit: 500 * 1024, // the default manifest limit is 500k.
}
}

Expand All @@ -20,4 +22,6 @@ func (c *Options) AddFlags(fs *pflag.FlagSet) {
"Port is the port that the webhook server serves at.")
fs.StringVar(&c.CertDir, "certdir", c.CertDir,
"CertDir is the directory that contains the server key and certificate. If not set, webhook server would look up the server key and certificate in {TempDir}/k8s-webhook-server/serving-certs")
fs.IntVar(&c.ManifestLimit, "manifestLimit", c.ManifestLimit,
"ManifestLimit is the max size of manifests in a manifestWork. If not set, the default is 500k.")
}
3 changes: 3 additions & 0 deletions pkg/cmd/webhook/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package webhook

import (
"k8s.io/klog/v2"
"open-cluster-management.io/work/pkg/webhook/common"

"k8s.io/apimachinery/pkg/runtime"

Expand Down Expand Up @@ -52,6 +53,8 @@ func (c *Options) RunWebhookServer() error {
return err
}

common.ManifestValidator.WithLimit(c.ManifestLimit)

if err = (&webhookv1.ManifestWorkWebhook{}).Init(mgr); err != nil {
klog.Error(err, "unable to create ManagedCluster webhook")
return err
Expand Down
62 changes: 62 additions & 0 deletions pkg/webhook/common/validator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package common

import (
"fmt"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
workv1 "open-cluster-management.io/api/work/v1"
)

type Validator struct {
limit int
}

var ManifestValidator = &Validator{limit: 500 * 1024} // the default manifest limit is 500k.

func (m *Validator) WithLimit(limit int) {
m.limit = limit
}

func (m *Validator) ValidateManifests(manifests []workv1.Manifest) error {
if len(manifests) == 0 {
return apierrors.NewBadRequest("Workload manifests should not be empty")
}

totalSize := 0
for _, manifest := range manifests {
totalSize = totalSize + manifest.Size()
}

if totalSize > m.limit {
return fmt.Errorf("the size of manifests is %v bytes which exceeds the %v limit", totalSize, m.limit)
}

for _, manifest := range manifests {
err := validateManifest(manifest.Raw)
if err != nil {
return err
}
}

return nil
}

func validateManifest(manifest []byte) error {
// If the manifest cannot be decoded, return err
unstructuredObj := &unstructured.Unstructured{}
err := unstructuredObj.UnmarshalJSON(manifest)
if err != nil {
return err
}

// The object must have name specified, generateName is not allowed in manifestwork
if unstructuredObj.GetName() == "" {
return fmt.Errorf("name must be set in manifest")
}

if unstructuredObj.GetGenerateName() != "" {
return fmt.Errorf("generateName must not be set in manifest")
}

return nil
}
59 changes: 59 additions & 0 deletions pkg/webhook/common/validator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package common

import (
"fmt"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
workv1 "open-cluster-management.io/api/work/v1"
"reflect"
"testing"
)

func newManifest(size int) workv1.Manifest {
data := ""
for i := 0; i < size; i++ {
data += "a"
}

obj := &unstructured.Unstructured{
Object: map[string]interface{}{
"apiVersion": "v1",
"kind": "Secret",
"metadata": map[string]interface{}{
"namespace": "test",
"name": "test",
},
"data": data,
},
}
objectStr, _ := obj.MarshalJSON()
manifest := workv1.Manifest{}
manifest.Raw = objectStr
return manifest
}
func Test_Validator(t *testing.T) {
cases := []struct {
name string
manifests []workv1.Manifest
expectedError error
}{
{
name: "not exceed the limit",
manifests: []workv1.Manifest{newManifest(100 * 1024), newManifest(100 * 1024)},
expectedError: nil,
},
{
name: "exceed the limit",
manifests: []workv1.Manifest{newManifest(300 * 1024), newManifest(200 * 1024)},
expectedError: fmt.Errorf("the size of manifests is 512192 bytes which exceeds the 512000 limit"),
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
err := ManifestValidator.ValidateManifests(c.manifests)
if !reflect.DeepEqual(err, c.expectedError) {
t.Errorf("expected %#v but got: %#v", c.expectedError, err)
}
})
}
}
47 changes: 2 additions & 45 deletions pkg/webhook/manifestwork_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"net/http"
"open-cluster-management.io/work/pkg/webhook/common"
"reflect"

ocmfeature "open-cluster-management.io/api/feature"
Expand All @@ -14,7 +15,6 @@ import (
authenticationv1 "k8s.io/api/authentication/v1"
authorizationv1 "k8s.io/api/authorization/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"

utilfeature "k8s.io/apiserver/pkg/util/feature"
Expand All @@ -23,9 +23,6 @@ import (
"k8s.io/klog/v2"
)

// 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 {
kubeClient kubernetes.Interface
Expand Down Expand Up @@ -102,7 +99,7 @@ func (a *ManifestWorkAdmissionHook) validateManifestWorkObj(request *admissionv1
return fmt.Errorf("manifests should not be empty")
}

if err := a.validateManifests(work); err != nil {
if err := common.ManifestValidator.ValidateManifests(work.Spec.Workload.Manifests); err != nil {
return err
}

Expand All @@ -125,46 +122,6 @@ func (a *ManifestWorkAdmissionHook) validateManifestWorkObj(request *admissionv1
return a.validateExecutor(work, request.UserInfo)
}

func (a *ManifestWorkAdmissionHook) validateManifests(work *workv1.ManifestWork) error {
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 {
err := a.validateManifest(manifest.Raw)
if err != nil {
return err
}
}

return nil
}

func (a *ManifestWorkAdmissionHook) validateManifest(manifest []byte) error {
// If the manifest cannot be decoded, return err
unstructuredObj := &unstructured.Unstructured{}
err := unstructuredObj.UnmarshalJSON(manifest)
if err != nil {
return err
}

// The object must have name specified, generateName is not allowed in manifestwork
if unstructuredObj.GetName() == "" {
return fmt.Errorf("name must be set in manifest")
}

if unstructuredObj.GetGenerateName() != "" {
return fmt.Errorf("generateName must not be set in manifest")
}

return nil
}

func (a *ManifestWorkAdmissionHook) validateExecutor(work *workv1.ManifestWork, userInfo authenticationv1.UserInfo) error {
executor := work.Spec.Executor

Expand Down
12 changes: 6 additions & 6 deletions pkg/webhook/manifestwork_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,17 @@ func TestManifestWorkValidate(t *testing.T) {
UserInfo: authenticationv1.UserInfo{Username: "tester"},
},
manifests: []*unstructured.Unstructured{
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),
spoketesting.NewUnstructuredSecretBySize("test1", "testns", 100*1024),
spoketesting.NewUnstructuredSecretBySize("test2", "testns", 100*1024),
spoketesting.NewUnstructuredSecretBySize("test3", "testns", 100*1024),
spoketesting.NewUnstructuredSecretBySize("test4", "testns", 100*1024),
spoketesting.NewUnstructuredSecretBySize("test5", "testns", 100*1024),
},
expectedResponse: &admissionv1beta1.AdmissionResponse{
Allowed: false,
Result: &metav1.Status{
Status: metav1.StatusFailure, Code: http.StatusBadRequest, Reason: metav1.StatusReasonBadRequest,
Message: "the size of manifests is 51685 bytes which exceeds the 50k limit",
Message: "the size of manifests is 512490 bytes which exceeds the 512000 limit",
},
},
},
Expand Down
51 changes: 2 additions & 49 deletions pkg/webhook/v1/manifestwork_validating.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ package v1
import (
"context"
"fmt"
"open-cluster-management.io/work/pkg/webhook/common"
"reflect"

authenticationv1 "k8s.io/api/authentication/v1"
authorizationv1 "k8s.io/api/authorization/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/kubernetes"
Expand All @@ -20,9 +20,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)

// ManifestLimit is the max size of manifests data which is 50k bytes.
const ManifestLimit = 50 * 1024

var _ webhook.CustomValidator = &ManifestWorkWebhook{}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
Expand Down Expand Up @@ -59,7 +56,7 @@ func (r *ManifestWorkWebhook) validateRequest(newWork, oldWork *workv1.ManifestW
return apierrors.NewBadRequest("manifests should not be empty")
}

if err := ValidateManifests(newWork.Spec.Workload.Manifests); err != nil {
if err := common.ManifestValidator.ValidateManifests(newWork.Spec.Workload.Manifests); err != nil {
return apierrors.NewBadRequest(err.Error())
}

Expand All @@ -75,50 +72,6 @@ func (r *ManifestWorkWebhook) validateRequest(newWork, oldWork *workv1.ManifestW
return validateExecutor(r.kubeClient, newWork, req.UserInfo)
}

func ValidateManifests(manifests []workv1.Manifest) error {
if len(manifests) == 0 {
return apierrors.NewBadRequest("Workload manifests should not be empty")
}

totalSize := 0
for _, manifest := range manifests {
totalSize = totalSize + manifest.Size()
}

if totalSize > ManifestLimit {
return apierrors.NewBadRequest(fmt.Sprintf("the size of manifests is %v bytes which exceeds the 50k limit", totalSize))
}

for _, manifest := range manifests {
err := validateManifest(manifest.Raw)
if err != nil {
return err
}
}

return nil
}

func validateManifest(manifest []byte) error {
// If the manifest cannot be decoded, return err
unstructuredObj := &unstructured.Unstructured{}
err := unstructuredObj.UnmarshalJSON(manifest)
if err != nil {
return err
}

// The object must have name specified, generateName is not allowed in manifestwork
if unstructuredObj.GetName() == "" {
return fmt.Errorf("name must be set in manifest")
}

if unstructuredObj.GetGenerateName() != "" {
return fmt.Errorf("generateName must not be set in manifest")
}

return nil
}

func validateExecutor(kubeClient kubernetes.Interface, work *workv1.ManifestWork, userInfo authenticationv1.UserInfo) error {
executor := work.Spec.Executor
if !utilfeature.DefaultMutableFeatureGate.Enabled(ocmfeature.NilExecutorValidating) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/webhook/v1/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func (r *ManifestWorkWebhook) Init(mgr ctrl.Manager) error {
return err
}

// SetExternalKubeClientSet is function to enable the webhook injecting to kube admssion
// SetExternalKubeClientSet is function to enable the webhook injecting to kube admission
func (r *ManifestWorkWebhook) SetExternalKubeClientSet(client kubernetes.Interface) {
r.kubeClient = client
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/webhook/v1alpha1/placemanifestwork_validating.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package v1alpha1

import (
"context"
"open-cluster-management.io/work/pkg/webhook/common"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
workv1alpha1 "open-cluster-management.io/api/work/v1alpha1"
webhookv1 "open-cluster-management.io/work/pkg/webhook/v1"
"sigs.k8s.io/controller-runtime/pkg/webhook"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
)
Expand Down Expand Up @@ -56,5 +56,5 @@ func (r *PlaceManifestWorkWebhook) validateRequest(newPlaceWork *workv1alpha1.Pl
}

func validatePlaceManifests(placeWork *workv1alpha1.PlaceManifestWork) error {
return webhookv1.ValidateManifests(placeWork.Spec.ManifestWorkTemplate.Workload.Manifests)
return common.ManifestValidator.ValidateManifests(placeWork.Spec.ManifestWorkTemplate.Workload.Manifests)
}
Loading

0 comments on commit 9a872eb

Please sign in to comment.