From cfb9b372de9bc1329ef35eae498561c75edbf6b0 Mon Sep 17 00:00:00 2001
From: Paul Maidment <pmaidmen@redhat.com>
Date: Tue, 13 Jun 2023 15:55:55 +0300
Subject: [PATCH] MGMT-14730: Validate that manifest file size does not exceed
 1MB (#5281)

etcd resources in k8s are limited to 1.5 MiB as indicated here https://etcd.io/docs/v3.5/dev-guide/limit/#request-size-limit
however, one the the resource types that can be created from a manifest is a ConfigMap
which has a size limit of 1MiB as cited here https://kubernetes.io/docs/concepts/configuration/configmap
so this limit has been chosen based on the lowest permitted resource size (the size of the ConfigMap)

Presently we do not validate the maximum size during upload or edit.
This pull request addresses that by validating that the file does not exceed this limit.
---
 internal/manifests/manifests.go      |  8 +++++
 internal/manifests/manifests_test.go | 48 ++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/internal/manifests/manifests.go b/internal/manifests/manifests.go
index 6d0f604741..9e48644c3b 100644
--- a/internal/manifests/manifests.go
+++ b/internal/manifests/manifests.go
@@ -353,6 +353,14 @@ func (m *Manifests) validateAllowedToModifyManifests(ctx context.Context, cluste
 }
 
 func (m *Manifests) validateUserSuppliedManifest(ctx context.Context, clusterID strfmt.UUID, manifestContent []byte, fileName string) error {
+	// etcd resources in k8s are limited to 1.5 MiB as indicated here https://etcd.io/docs/v3.5/dev-guide/limit/#request-size-limit
+	// however, one the the resource types that can be created from a manifest is a ConfigMap
+	// which has a size limit of 1MiB as cited here https://kubernetes.io/docs/concepts/configuration/configmap
+	// so this limit has been chosen based on the lowest permitted resource size (the size of the ConfigMap)
+	maxFileSizeBytes := 1024 * 1024
+	if len(manifestContent) > maxFileSizeBytes {
+		return m.prepareAndLogError(ctx, http.StatusBadRequest, errors.Errorf("Manifest content of file %s for cluster ID %s exceeds the maximum file size of 1MiB", fileName, string(clusterID)))
+	}
 	extension := filepath.Ext(fileName)
 	if extension == ".yaml" || extension == ".yml" {
 		var s map[interface{}]interface{}
diff --git a/internal/manifests/manifests_test.go b/internal/manifests/manifests_test.go
index 402b572174..38e6ac520f 100644
--- a/internal/manifests/manifests_test.go
+++ b/internal/manifests/manifests_test.go
@@ -29,6 +29,7 @@ import (
 
 func TestValidator(t *testing.T) {
 	RegisterFailHandler(Fail)
+
 	common.InitializeDBTest()
 	defer common.TerminateDBTest()
 	RunSpecs(t, "manifests_test")
@@ -238,6 +239,16 @@ var _ = Describe("ClusterManifestTests", func() {
 		})
 
 		Context("File validation and format", func() {
+
+			generateLargeJSON := func(length int) string {
+				var largeElementBuilder strings.Builder
+				largeElementBuilder.Grow(length)
+				for i := 0; i <= length; i++ {
+					largeElementBuilder.WriteString("A")
+				}
+				return fmt.Sprintf("{\"data\":\"%s\"}", largeElementBuilder.String())
+			}
+
 			It("accepts manifest in json format and .json extension", func() {
 				clusterID := registerCluster().ID
 				jsonContent := encodeToBase64(contentAsJSON)
@@ -403,6 +414,43 @@ spec:
 				Expect(err.StatusCode()).To(Equal(int32(http.StatusBadRequest)))
 				Expect(err.Error()).To(ContainSubstring("Cluster manifest openshift/99-test.yaml for cluster " + clusterID.String() + " should not include a directory in its name."))
 			})
+
+			It("Creation fails for a manifest file that exceeds the maximum upload size", func() {
+				clusterID := registerCluster().ID
+				fileName := "99-test.json"
+				maxFileSizeBytes := 1024*1024 + 1
+				largeJSONContent := encodeToBase64(generateLargeJSON(maxFileSizeBytes))
+				response := manifestsAPI.V2CreateClusterManifest(ctx, operations.V2CreateClusterManifestParams{
+					ClusterID: *clusterID,
+					CreateManifestParams: &models.CreateManifestParams{
+						Content:  &largeJSONContent,
+						FileName: &fileName,
+					},
+				})
+				Expect(response).Should(BeAssignableToTypeOf(common.NewApiError(http.StatusBadRequest, errors.New(""))))
+				err := response.(*common.ApiErrorResponse)
+				Expect(err.StatusCode()).To(Equal(int32(http.StatusBadRequest)))
+				Expect(err.Error()).To(ContainSubstring("Manifest content of file manifests/99-test.json for cluster ID " + clusterID.String() + " exceeds the maximum file size of 1MiB"))
+			})
+
+			It("Update fails for a manifest file that exceeds the maximum upload size", func() {
+				clusterID := registerCluster().ID
+				maxFileSizeBytes := 1024*1024 + 1
+				largeJSONContent := encodeToBase64(generateLargeJSON(maxFileSizeBytes))
+				response := manifestsAPI.V2UpdateClusterManifest(ctx, operations.V2UpdateClusterManifestParams{
+					ClusterID: *clusterID,
+					UpdateManifestParams: &models.UpdateManifestParams{
+						UpdatedContent: &largeJSONContent,
+						FileName:       fileNameJson,
+						Folder:         defaultFolder,
+					},
+				})
+				Expect(response).Should(BeAssignableToTypeOf(common.NewApiError(http.StatusBadRequest, errors.New(""))))
+				err := response.(*common.ApiErrorResponse)
+				Expect(err.StatusCode()).To(Equal(int32(http.StatusBadRequest)))
+				Expect(err.Error()).To(ContainSubstring("Manifest content of file 99-openshift-machineconfig-master-kargs.json for cluster ID " + clusterID.String() + " exceeds the maximum file size of 1MiB"))
+			})
+
 		})
 	})