Validate action for resource templates (#1346)

argoproj · May 10, 2019 · d7e74fe · d7e74fe
1 parent 810949d
commit d7e74fe
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 3 deletions.
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
@@ -531,7 +531,7 @@
       ],
       "properties": {
         "action": {
-          "description": "Action is the action to perform to the resource. Must be one of: get, create, apply, delete, replace",
+          "description": "Action is the action to perform to the resource. Must be one of: get, create, apply, delete, replace, patch",
           "type": "string"
         },
         "failureCondition": {

diff --git a/pkg/apis/workflow/v1alpha1/openapi_generated.go b/pkg/apis/workflow/v1alpha1/openapi_generated.go
diff --git a/pkg/apis/workflow/v1alpha1/types.go b/pkg/apis/workflow/v1alpha1/types.go
@@ -815,7 +815,7 @@ type ScriptTemplate struct {
 // ResourceTemplate is a template subtype to manipulate kubernetes resources
 type ResourceTemplate struct {
 	// Action is the action to perform to the resource.
-	// Must be one of: get, create, apply, delete, replace
+	// Must be one of: get, create, apply, delete, replace, patch
 	Action string `json:"action"`
 
 	// MergeStrategy is the strategy used to merge a patch. It defaults to "strategic"

diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go
@@ -1016,6 +1016,7 @@ spec:
   templates:
   - name: resource
     resource:
+      action: create
       manifest: |
         apiVersion: v1
         kind: ConfigMap
@@ -1074,6 +1075,7 @@ spec:
         template: resource-3
   - name: resource-1
     resource:
+      action: create
       manifest: |
         apiVersion: v1
         kind: ConfigMap
@@ -1087,6 +1089,7 @@ spec:
             uid: "manual-ref-uid"
   - name: resource-2
     resource:
+      action: create
       setOwnerReference: true
       manifest: |
         apiVersion: v1
@@ -1095,6 +1098,7 @@ spec:
           name: resource-cm-2
   - name: resource-3
     resource:
+      action: create
       setOwnerReference: true
       manifest: |
         apiVersion: v1

diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go
@@ -328,6 +328,12 @@ func validateLeaf(scope map[string]interface{}, tmpl *wfv1.Template) error {
 		}
 	}
 	if tmpl.Resource != nil {
+		switch tmpl.Resource.Action {
+		case "get", "create", "apply", "delete", "replace", "patch":
+			// OK
+		default:
+			return errors.Errorf(errors.CodeBadRequest, "templates.%s.resource.action must be either get, create, apply, delete or replace", tmpl.Name)
+		}
 		// Try to unmarshal the given manifest.
 		obj := unstructured.Unstructured{}
 		err := yaml.Unmarshal([]byte(tmpl.Resource.Manifest), &obj)

diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go
@@ -1367,6 +1367,7 @@ spec:
   templates:
   - name: whalesay
     resource:
+      action: create
       manifest: |
         apiVersion: v1
         kind: ConfigMap
@@ -1398,9 +1399,31 @@ spec:
           name: whalesay-cm
 `
 
+var invalidActionResourceWorkflow = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: invalid-resource-
+spec:
+  entrypoint: whalesay
+  templates:
+  - name: whalesay
+    resource:
+      action: foo
+      manifest: |
+      apiVersion: v1
+      kind: ConfigMap
+      metadata:
+        name: whalesay-cm
+`
+
 // TestInvalidResourceWorkflow verifies an error against a workflow of an invalid resource.
 func TestInvalidResourceWorkflow(t *testing.T) {
 	wf := unmarshalWf(invalidResourceWorkflow)
 	err := ValidateWorkflow(wf, ValidateOpts{})
 	assert.Error(t, err, "templates.whalesay.resource.manifest must be a valid yaml")
+
+	wf = unmarshalWf(invalidActionResourceWorkflow)
+	err = ValidateWorkflow(wf, ValidateOpts{})
+	assert.Error(t, err, "templates.whalesay.resource.action must be either get, create, apply, delete or replace")
 }