Skip to content

Commit

Permalink
MGMT-14730: Validate that manifest file size does not exceed 1MB (#5281)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
paul-maidment committed Jun 13, 2023
1 parent 188e96f commit cfb9b37
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 0 deletions.
8 changes: 8 additions & 0 deletions internal/manifests/manifests.go
Expand Up @@ -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{}
Expand Down
48 changes: 48 additions & 0 deletions internal/manifests/manifests_test.go
Expand Up @@ -29,6 +29,7 @@ import (

func TestValidator(t *testing.T) {
RegisterFailHandler(Fail)

common.InitializeDBTest()
defer common.TerminateDBTest()
RunSpecs(t, "manifests_test")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"))
})

})
})

Expand Down

0 comments on commit cfb9b37

Please sign in to comment.