From 11e1f23579d5a282981e3a47c094acf260f25e46 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Mon, 29 Apr 2019 12:33:40 +0900 Subject: [PATCH 1/5] Validate action for resource templates --- workflow/validate/validate.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index b5714c9ba747..4b7937c9729a 100644 --- 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": + // 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) From 750e58a7ecc7fc310de00ca41919d3b0cf021864 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Mon, 29 Apr 2019 12:53:07 +0900 Subject: [PATCH 2/5] Add tests --- workflow/validate/validate_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go index b1875e91bd52..811527492153 100644 --- 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") } From 1b6172c97cb242e01a6a1c0e4e61ff26fd0e304a Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Wed, 8 May 2019 11:32:24 +0900 Subject: [PATCH 3/5] Allow patch action --- pkg/apis/workflow/v1alpha1/types.go | 2 +- workflow/validate/validate.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/workflow/v1alpha1/types.go b/pkg/apis/workflow/v1alpha1/types.go index 1d9d4308a0ba..6e0341f4fce1 100644 --- 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/validate/validate.go b/workflow/validate/validate.go index 4b7937c9729a..48853e957e88 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -329,7 +329,7 @@ func validateLeaf(scope map[string]interface{}, tmpl *wfv1.Template) error { } if tmpl.Resource != nil { switch tmpl.Resource.Action { - case "get", "create", "apply", "delete", "replace": + 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) From 3e1d395e28dfc5c8be7cfe5f7a3644ded3c7841f Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Fri, 10 May 2019 09:28:05 +0900 Subject: [PATCH 4/5] Update codegen --- api/openapi-spec/swagger.json | 2 +- pkg/apis/workflow/v1alpha1/openapi_generated.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index d9582c04bba4..dc762fb3d751 100644 --- 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 index dc8b401bf297..6aa073d0e19b 100644 --- a/pkg/apis/workflow/v1alpha1/openapi_generated.go +++ b/pkg/apis/workflow/v1alpha1/openapi_generated.go @@ -1007,7 +1007,7 @@ func schema_pkg_apis_workflow_v1alpha1_ResourceTemplate(ref common.ReferenceCall Properties: map[string]spec.Schema{ "action": { SchemaProps: spec.SchemaProps{ - 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{"string"}, Format: "", }, From c0a8e694f5039bfe661c64f754bb19b6f91731f7 Mon Sep 17 00:00:00 2001 From: Daisuke Taniwaki Date: Fri, 10 May 2019 09:56:52 +0900 Subject: [PATCH 5/5] Fix operator tests --- workflow/controller/operator_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index f92b4bbae502..0542f506d94b 100644 --- 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