MGMT-17775: Ensure that patch manifest filenames are consistent and correct.

docs/user-guide/install-customization.md

@@ -9,7 +9,8 @@ These will only take effect after the machine config operator is up and running
 Formats json, yaml, and multi-document yaml are accepted as manifests.
 Multi-document yaml manifests are split with unique names to avoid any collisions with existing files when they are added to their respective folders (either "manifests" or "openshift").
 
-Yaml patches are also accepted and should be in the yaml-patch format, patches should contain `.yaml.patch.` or `.yml.patch.` as a part of their filename.
+Yaml patches are also accepted and should be in the yaml-patch format, patches should end with `.yml.patch`, `yaml.patch`.
+Extensions may have a suffix of an underscore followed by some alphanumeric characters, for example `.yml.patch_ocp_manifests` or `.yaml.patch_ocp_manifests`
 Here is an example of the format of a Yaml patch file, for more information on the format, please see https://github.com/krishicks/yaml-patch
 
 ```

internal/manifests/manifests.go

@@ -541,7 +541,7 @@ func (m *Manifests) validateUserSuppliedManifest(ctx context.Context, clusterID
 		if !json.Valid(manifestContent) {
 			return m.prepareAndLogError(ctx, http.StatusBadRequest, errors.Errorf("Manifest content of file %s for cluster ID %s has an illegal JSON format", fileName, string(clusterID)))
 		}
-	} else if strings.Contains(fileName, ".yaml.patch") || strings.Contains(fileName, ".yml.patch") {
+	} else if strings.HasPrefix(extension, ".patch") {
 		if _, err := yamlpatch.DecodePatch(manifestContent); err != nil {
 			return m.prepareAndLogError(ctx, http.StatusBadRequest, errors.Errorf("Patch content of file %s for cluster ID %s is invalid: %s", fileName, string(clusterID), err))
 		}

internal/manifests/manifests_test.go

@@ -73,8 +73,8 @@ var _ = Describe("ClusterManifestTests", func() {
 		mockS3Client      *s3wrapper.MockAPI
 		dbName            string
 		fileNameYaml      = "99-openshift-machineconfig-master-kargs.yaml"
-		fileNameYamlPatch = "99-openshift-machineconfig-master-kargs.yaml.patch.foobar"
-		fileNameYmlPatch  = "99-openshift-machineconfig-master-kargs.yml.patch.foobar"
+		fileNameYamlPatch = "99-openshift-machineconfig-master-kargs.yaml.patch_foobar"
+		fileNameYmlPatch  = "99-openshift-machineconfig-master-kargs.yml.patch_foobar"
 		fileNameJson      = "99-openshift-machineconfig-master-kargs.json"
 		validFolder       = "openshift"
 		defaultFolder     = "manifests"
@@ -506,6 +506,49 @@ spec:
 				Expect(response).Should(BeAssignableToTypeOf(operations.NewV2CreateClusterManifestCreated()))
 			})
 
+			It("manifest creation will succeed for patch files if filename is valid", func() {
+				clusterID := registerCluster().ID
+				validFileNames := []string{
+					"something.patch",
+					"something.patch_something_else",
+					"something.patch.patch_something",
+				}
+				for i, fileName := range validFileNames {
+					mockUpload(1)
+					expectUsageCalls()
+					mockS3Client.EXPECT().DoesObjectExist(ctx, filepath.Join(clusterID.String(), constants.ManifestFolder, "openshift", fileName)).Return(false, nil).AnyTimes()
+					response := manifestsAPI.V2CreateClusterManifest(ctx, operations.V2CreateClusterManifestParams{
+						ClusterID: *clusterID,
+						CreateManifestParams: &models.CreateManifestParams{
+							Content:  &contentYamlPatch,
+							FileName: &validFileNames[i],
+						},
+					})
+					Expect(response).Should(BeAssignableToTypeOf(operations.NewV2CreateClusterManifestCreated()))
+				}
+			})
+
+			It("manifest creation will fail for patch files if filename is invalid", func() {
+				clusterID := registerCluster().ID
+				invalidFileNames := []string{
+					"something.patch.something",
+					"something.patch.something.else",
+				}
+				for i, fileName := range invalidFileNames {
+					response := manifestsAPI.V2CreateClusterManifest(ctx, operations.V2CreateClusterManifestParams{
+						ClusterID: *clusterID,
+						CreateManifestParams: &models.CreateManifestParams{
+							Content:  &contentYaml,
+							FileName: &invalidFileNames[i],
+						},
+					})
+					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 filename of file manifests/%s for cluster ID %s is invalid. Only json, yaml and yml or patch extensions are supported", fileName, clusterID.String()))
+				}
+			})
+
 			It("fails for filename that contains folder in the name", func() {
 				clusterID := registerCluster().ID
 				fileNameWithFolder := "openshift/99-test.yaml"

swagger.yaml

@@ -6901,7 +6901,7 @@ definitions:
       file_name:
         description: The name of the manifest to customize the installed OCP cluster.
         type: string
-        pattern: '^[^/]*\.(yaml|yml|json|yaml.patch.*|yml.patch.*)$'
+        pattern: '^[^\/]*\.(json|ya?ml(\.patch_?[a-zA-Z0-9_]*)?)$'
       content:
         description: base64 encoded manifest content.
         type: string
@@ -6921,7 +6921,7 @@ definitions:
       file_name:
         description: The file name for the manifest to modify.
         type: string
-        pattern: '^[^/]*\.(yaml|yml|json|yaml.patch.*|yml.patch.*)$'
+        pattern: '^[^\/]*\.(json|ya?ml(\.patch_?[a-zA-Z0-9_]*)?)$'
         x-nullable: false
       updated_folder:
         description: The new folder for the manifest. Manifests can be placed in 'manifests' or 'openshift' directories.
@@ -6932,7 +6932,7 @@ definitions:
       updated_file_name:
         description: The new file name for the manifest.
         type: string
-        pattern: '^[^/]*\.(yaml|yml|json|yaml.patch.*|yml.patch.*)$'
+        pattern: '^[^\/]*\.(json|ya?ml(\.patch_?[a-zA-Z0-9_]*)?)$'
         x-nullable: true
       updated_content:
         description: The new base64 encoded manifest content.