From 995a331284673318d8452b90a39fd48193b34d68 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 9 Oct 2024 13:50:50 +0200 Subject: [PATCH 1/6] Make max request size configurable. --- pkg/cmd/server/start.go | 23 ++++++++++++----------- test/e2e/e2e_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index a3f8067d..0d1b93b9 100644 --- a/pkg/cmd/server/start.go +++ b/pkg/cmd/server/start.go @@ -49,15 +49,16 @@ const ( // PorchServerOptions contains state for master/api server type PorchServerOptions struct { - RecommendedOptions *genericoptions.RecommendedOptions - LocalStandaloneDebugging bool // Enables local standalone running/debugging of the apiserver. - CacheDirectory string - CoreAPIKubeconfigPath string - FunctionRunnerAddress string - DefaultImagePrefix string - RepoSyncFrequency time.Duration - UseGitCaBundle bool - DisableValidatingAdmissionPolicy bool + RecommendedOptions *genericoptions.RecommendedOptions + LocalStandaloneDebugging bool // Enables local standalone running/debugging of the apiserver. + CacheDirectory string + CoreAPIKubeconfigPath string + FunctionRunnerAddress string + DefaultImagePrefix string + RepoSyncFrequency time.Duration + UseGitCaBundle bool + DisableValidatingAdmissionPolicy bool + MaxRequestBodySize int64 SharedInformerFactory informers.SharedInformerFactory StdOut io.Writer @@ -172,11 +173,9 @@ func (o *PorchServerOptions) Config() (*apiserver.Config, error) { o.SharedInformerFactory = informerFactory return []admission.PluginInitializer{}, nil } - if o.DisableValidatingAdmissionPolicy { o.RecommendedOptions.Admission.DisablePlugins = []string{"ValidatingAdmissionPolicy"} } - serverConfig := genericapiserver.NewRecommendedConfig(apiserver.Codecs) serverConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(sampleopenapi.GetOpenAPIDefinitions, openapi.NewDefinitionNamer(apiserver.Scheme)) @@ -186,6 +185,7 @@ func (o *PorchServerOptions) Config() (*apiserver.Config, error) { serverConfig.OpenAPIV3Config = genericapiserver.DefaultOpenAPIV3Config(sampleopenapi.GetOpenAPIDefinitions, openapi.NewDefinitionNamer(apiserver.Scheme)) serverConfig.OpenAPIConfig.Info.Title = OpenAPITitle serverConfig.OpenAPIConfig.Info.Version = OpenAPIVersion + serverConfig.MaxRequestBodyBytes = o.MaxRequestBodySize if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil { return nil, err @@ -245,6 +245,7 @@ func (o *PorchServerOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.FunctionRunnerAddress, "function-runner", "", "Address of the function runner gRPC service.") fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", "gcr.io/kpt-fn/", "Default prefix for unqualified function names") fs.StringVar(&o.CacheDirectory, "cache-directory", "", "Directory where Porch server stores repository and package caches.") + fs.Int64Var(&o.MaxRequestBodySize, "max-request-body-size", 6*1024*1024, "Maximum size of the request body in bytes.") fs.BoolVar(&o.UseGitCaBundle, "use-git-cabundle", false, "Determine whether to use a user-defined CaBundle for TLS towards git.") fs.BoolVar(&o.DisableValidatingAdmissionPolicy, "disable-validating-admissions-policy", true, "Determine whether to (dis|en)able the Validating Admission Policy, which requires k8s version >= v1.30") fs.DurationVar(&o.RepoSyncFrequency, "repo-sync-frequency", 60*time.Second, "Frequency in seconds at which registered repositories will be synced.") diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 25c6156b..a904d91d 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -2427,3 +2427,43 @@ func (t *PorchSuite) TestPackageRevisionFieldselectors(ctx context.Context) { } } } + +func (t *PorchSuite) TestLargePackageRevision(ctx context.Context) { + const ( + repository = "large-pkg-rev" + ) + + t.registerMainGitRepositoryF(ctx, repository) + pr := porchapi.PackageRevision{ + TypeMeta: metav1.TypeMeta{ + Kind: porchapi.PackageRevisionGVR.Resource, + APIVersion: porchapi.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.namespace, + }, + Spec: porchapi.PackageRevisionSpec{ + PackageName: "new-package", + WorkspaceName: "workspace", + RepositoryName: repository, + Tasks: []porchapi.Task{ + { + Type: porchapi.TaskTypeInit, + Init: &porchapi.PackageInitTaskSpec{ + Description: "this is a test", + }, + }, + }, + }, + } + + t.CreateE(ctx, &pr) + + var prr porchapi.PackageRevisionResources + + t.GetE(ctx, client.ObjectKey{Name: pr.Name, Namespace: pr.Namespace}, &prr) + + prr.Spec.Resources["largefile.txt"] = strings.Repeat("a", 2*1024*1024) + + t.UpdateE(ctx, &prr) +} From 525aa8c21b6cca30c4032bc602aba2845c5aed43 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 9 Oct 2024 13:54:27 +0200 Subject: [PATCH 2/6] Bump the e2e test to be above the k8s default --- test/e2e/e2e_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index a904d91d..15b50f9b 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -2463,7 +2463,7 @@ func (t *PorchSuite) TestLargePackageRevision(ctx context.Context) { t.GetE(ctx, client.ObjectKey{Name: pr.Name, Namespace: pr.Namespace}, &prr) - prr.Spec.Resources["largefile.txt"] = strings.Repeat("a", 2*1024*1024) + prr.Spec.Resources["largefile.txt"] = strings.Repeat("a", 4*1024*1024) t.UpdateE(ctx, &prr) } From e6a7197472b67a28829411ca2007a2e8977e9691 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 9 Oct 2024 13:50:50 +0200 Subject: [PATCH 3/6] Make max request size configurable. --- pkg/cmd/server/start.go | 23 ++++++++++++----------- test/e2e/e2e_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/server/start.go b/pkg/cmd/server/start.go index a3f8067d..0d1b93b9 100644 --- a/pkg/cmd/server/start.go +++ b/pkg/cmd/server/start.go @@ -49,15 +49,16 @@ const ( // PorchServerOptions contains state for master/api server type PorchServerOptions struct { - RecommendedOptions *genericoptions.RecommendedOptions - LocalStandaloneDebugging bool // Enables local standalone running/debugging of the apiserver. - CacheDirectory string - CoreAPIKubeconfigPath string - FunctionRunnerAddress string - DefaultImagePrefix string - RepoSyncFrequency time.Duration - UseGitCaBundle bool - DisableValidatingAdmissionPolicy bool + RecommendedOptions *genericoptions.RecommendedOptions + LocalStandaloneDebugging bool // Enables local standalone running/debugging of the apiserver. + CacheDirectory string + CoreAPIKubeconfigPath string + FunctionRunnerAddress string + DefaultImagePrefix string + RepoSyncFrequency time.Duration + UseGitCaBundle bool + DisableValidatingAdmissionPolicy bool + MaxRequestBodySize int64 SharedInformerFactory informers.SharedInformerFactory StdOut io.Writer @@ -172,11 +173,9 @@ func (o *PorchServerOptions) Config() (*apiserver.Config, error) { o.SharedInformerFactory = informerFactory return []admission.PluginInitializer{}, nil } - if o.DisableValidatingAdmissionPolicy { o.RecommendedOptions.Admission.DisablePlugins = []string{"ValidatingAdmissionPolicy"} } - serverConfig := genericapiserver.NewRecommendedConfig(apiserver.Codecs) serverConfig.OpenAPIConfig = genericapiserver.DefaultOpenAPIConfig(sampleopenapi.GetOpenAPIDefinitions, openapi.NewDefinitionNamer(apiserver.Scheme)) @@ -186,6 +185,7 @@ func (o *PorchServerOptions) Config() (*apiserver.Config, error) { serverConfig.OpenAPIV3Config = genericapiserver.DefaultOpenAPIV3Config(sampleopenapi.GetOpenAPIDefinitions, openapi.NewDefinitionNamer(apiserver.Scheme)) serverConfig.OpenAPIConfig.Info.Title = OpenAPITitle serverConfig.OpenAPIConfig.Info.Version = OpenAPIVersion + serverConfig.MaxRequestBodyBytes = o.MaxRequestBodySize if err := o.RecommendedOptions.ApplyTo(serverConfig); err != nil { return nil, err @@ -245,6 +245,7 @@ func (o *PorchServerOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.FunctionRunnerAddress, "function-runner", "", "Address of the function runner gRPC service.") fs.StringVar(&o.DefaultImagePrefix, "default-image-prefix", "gcr.io/kpt-fn/", "Default prefix for unqualified function names") fs.StringVar(&o.CacheDirectory, "cache-directory", "", "Directory where Porch server stores repository and package caches.") + fs.Int64Var(&o.MaxRequestBodySize, "max-request-body-size", 6*1024*1024, "Maximum size of the request body in bytes.") fs.BoolVar(&o.UseGitCaBundle, "use-git-cabundle", false, "Determine whether to use a user-defined CaBundle for TLS towards git.") fs.BoolVar(&o.DisableValidatingAdmissionPolicy, "disable-validating-admissions-policy", true, "Determine whether to (dis|en)able the Validating Admission Policy, which requires k8s version >= v1.30") fs.DurationVar(&o.RepoSyncFrequency, "repo-sync-frequency", 60*time.Second, "Frequency in seconds at which registered repositories will be synced.") diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 5233073d..a8000a3d 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -2480,3 +2480,43 @@ func (t *PorchSuite) TestPackageRevisionFieldSelectors(ctx context.Context) { } } } + +func (t *PorchSuite) TestLargePackageRevision(ctx context.Context) { + const ( + repository = "large-pkg-rev" + ) + + t.registerMainGitRepositoryF(ctx, repository) + pr := porchapi.PackageRevision{ + TypeMeta: metav1.TypeMeta{ + Kind: porchapi.PackageRevisionGVR.Resource, + APIVersion: porchapi.SchemeGroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: t.namespace, + }, + Spec: porchapi.PackageRevisionSpec{ + PackageName: "new-package", + WorkspaceName: "workspace", + RepositoryName: repository, + Tasks: []porchapi.Task{ + { + Type: porchapi.TaskTypeInit, + Init: &porchapi.PackageInitTaskSpec{ + Description: "this is a test", + }, + }, + }, + }, + } + + t.CreateE(ctx, &pr) + + var prr porchapi.PackageRevisionResources + + t.GetE(ctx, client.ObjectKey{Name: pr.Name, Namespace: pr.Namespace}, &prr) + + prr.Spec.Resources["largefile.txt"] = strings.Repeat("a", 2*1024*1024) + + t.UpdateE(ctx, &prr) +} From e9642cb1a7fb683a776796c9a290bd364fa77732 Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 9 Oct 2024 13:54:27 +0200 Subject: [PATCH 4/6] Bump the e2e test to be above the k8s default --- test/e2e/e2e_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index a8000a3d..6f42c720 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -2516,7 +2516,7 @@ func (t *PorchSuite) TestLargePackageRevision(ctx context.Context) { t.GetE(ctx, client.ObjectKey{Name: pr.Name, Namespace: pr.Namespace}, &prr) - prr.Spec.Resources["largefile.txt"] = strings.Repeat("a", 2*1024*1024) + prr.Spec.Resources["largefile.txt"] = strings.Repeat("a", 4*1024*1024) t.UpdateE(ctx, &prr) } From 148071f246046697f5663708f9305d0cf789293c Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 9 Oct 2024 15:55:31 +0200 Subject: [PATCH 5/6] Fix update --- test/e2e/e2e_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 6f42c720..328dfa63 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -2486,14 +2486,14 @@ func (t *PorchSuite) TestLargePackageRevision(ctx context.Context) { repository = "large-pkg-rev" ) - t.registerMainGitRepositoryF(ctx, repository) + t.RegisterMainGitRepositoryF(ctx, repository) pr := porchapi.PackageRevision{ TypeMeta: metav1.TypeMeta{ Kind: porchapi.PackageRevisionGVR.Resource, APIVersion: porchapi.SchemeGroupVersion.String(), }, ObjectMeta: metav1.ObjectMeta{ - Namespace: t.namespace, + Namespace: t.Namespace, }, Spec: porchapi.PackageRevisionSpec{ PackageName: "new-package", From c6f299115d02238ee8eac5bc17c802b5e0e02e1e Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Wed, 9 Oct 2024 16:08:19 +0200 Subject: [PATCH 6/6] Fix faulty git merge --- test/e2e/e2e_test.go | 40 ---------------------------------------- 1 file changed, 40 deletions(-) diff --git a/test/e2e/e2e_test.go b/test/e2e/e2e_test.go index 2a9eb09d..328dfa63 100644 --- a/test/e2e/e2e_test.go +++ b/test/e2e/e2e_test.go @@ -2520,43 +2520,3 @@ func (t *PorchSuite) TestLargePackageRevision(ctx context.Context) { t.UpdateE(ctx, &prr) } - -func (t *PorchSuite) TestLargePackageRevision(ctx context.Context) { - const ( - repository = "large-pkg-rev" - ) - - t.registerMainGitRepositoryF(ctx, repository) - pr := porchapi.PackageRevision{ - TypeMeta: metav1.TypeMeta{ - Kind: porchapi.PackageRevisionGVR.Resource, - APIVersion: porchapi.SchemeGroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: t.namespace, - }, - Spec: porchapi.PackageRevisionSpec{ - PackageName: "new-package", - WorkspaceName: "workspace", - RepositoryName: repository, - Tasks: []porchapi.Task{ - { - Type: porchapi.TaskTypeInit, - Init: &porchapi.PackageInitTaskSpec{ - Description: "this is a test", - }, - }, - }, - }, - } - - t.CreateE(ctx, &pr) - - var prr porchapi.PackageRevisionResources - - t.GetE(ctx, client.ObjectKey{Name: pr.Name, Namespace: pr.Namespace}, &prr) - - prr.Spec.Resources["largefile.txt"] = strings.Repeat("a", 4*1024*1024) - - t.UpdateE(ctx, &prr) -}