feat: resolution: implemented task start-over (#308)

Once a resolution is executed, we can now start-over a task to its initial state if the resolution were stuck in BAD_REQUEST, CANCELLED or PAUSED state. It grants admins a way to amend the execution of a resolution. This feature can also be granted to resolvers of a resolution, so they can start-over themselves, using the boolean `allow_task_start_over` in the template. Signed-off-by: Romain Beuque <[email protected]>
ovh · Feb 9, 2022 · 0f44e94 · 0f44e94
1 parent 66b3ba6
commit 0f44e94
Show file tree

Hide file tree

Showing 9 changed files with 308 additions and 34 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -10,31 +10,34 @@ run:
 
 linters:
   enable:
-    - govet
-    - megacheck
-    - staticcheck
-    - deadcode
-    - structcheck
-    - typecheck
-    - gosimple
-    - varcheck
-    - ineffassign
-    - bodyclose
     - asciicheck
+    - bodyclose
+    - deadcode
+    - errcheck
     - exportloopref
     - goconst
-    - misspell
     - gofmt
     - goimports
-    - predeclared
     - gosec
-    - golint
+    - gosimple
+    - govet
+    - ineffassign
+    - megacheck
+    - misspell
     - nolintlint
+    - prealloc
+    - predeclared
+    - revive
+    - staticcheck
+    - structcheck
+    - stylecheck
+    - typecheck
     - unconvert
-    - errcheck
-    - exportloopref
     - unparam
     - unused
+    - varcheck
+    - wastedassign
+    - whitespace
 
   disable:
     - maligned

diff --git a/api/api_test.go b/api/api_test.go
@@ -26,7 +26,10 @@ import (
 	"github.com/ovh/utask/engine"
 	"github.com/ovh/utask/engine/input"
 	"github.com/ovh/utask/engine/step"
+	"github.com/ovh/utask/engine/step/condition"
 	"github.com/ovh/utask/engine/step/executor"
+	"github.com/ovh/utask/engine/values"
+	"github.com/ovh/utask/models/resolution"
 	"github.com/ovh/utask/models/task"
 	"github.com/ovh/utask/models/tasktemplate"
 	"github.com/ovh/utask/pkg/auth"
@@ -56,11 +59,17 @@ func TestMain(m *testing.M) {
 	step.RegisterRunner(echo.Plugin.PluginName(), echo.Plugin)
 	step.RegisterRunner(script.Plugin.PluginName(), script.Plugin)
 
-	db.Init(store)
+	if err := db.Init(store); err != nil {
+		panic(err)
+	}
 
-	now.Init()
+	if err := now.Init(); err != nil {
+		panic(err)
+	}
 
-	auth.Init(store)
+	if err := auth.Init(store); err != nil {
+		panic(err)
+	}
 
 	ctx := context.Background()
 	var wg sync.WaitGroup
@@ -276,6 +285,105 @@ func TestPasswordInput(t *testing.T) {
 	tester.Run()
 }
 
+func TestStartOver(t *testing.T) {
+	tester := iffy.NewTester(t, hdl)
+
+	dbp, err := zesty.NewDBProvider(utask.DBName)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tmpl := resolverInputTemplate()
+
+	_, err = tasktemplate.LoadFromName(dbp, tmpl.Name)
+	if err != nil {
+		if !errors.IsNotFound(err) {
+			t.Fatal(err)
+		}
+		if err := dbp.DB().Insert(&tmpl); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	tester.AddCall("getTemplate", http.MethodGet, "/template/"+tmpl.Name, "").
+		Headers(regularHeaders).
+		Checkers(
+			iffy.ExpectStatus(200),
+		)
+
+	tester.AddCall("newTask", http.MethodPost, "/task", `{"template_name":"{{.getTemplate.name}}","input":{"id":"yesssss!"}}`).
+		Headers(regularHeaders).
+		Checkers(iffy.ExpectStatus(201))
+
+	tester.AddCall("createResolutionMissingResolverInput", http.MethodPost, "/resolution", `{"task_id":"{{.newTask.id}}"}`).
+		Headers(adminHeaders).
+		Checkers(
+			iffy.ExpectStatus(400),
+			iffy.ExpectJSONBranch("error", `Failed to create Resolution: Missing input 'ri1'`),
+		)
+
+	tester.AddCall("createResolutionStartOverFailed", http.MethodPost, "/resolution", `{"task_id":"{{.newTask.id}}","resolver_inputs":{"ri1":"foo"}, "start_over":true}`).
+		Headers(adminHeaders).
+		Checkers(
+			iffy.ExpectStatus(400),
+			iffy.ExpectJSONBranch("error", `can't start over a task that hasn't been resolved at least once`),
+		)
+
+	tester.AddCall("createResolution", http.MethodPost, "/resolution", `{"task_id":"{{.newTask.id}}","resolver_inputs":{"ri1":"foo"}}`).
+		Headers(adminHeaders).
+		Checkers(iffy.ExpectStatus(201))
+
+	tester.AddCall("runResolution", http.MethodPost, "/resolution/{{.createResolution.id}}/run", "").
+		Headers(adminHeaders).
+		Checkers(
+			iffy.ExpectStatus(204),
+			waitChecker(time.Second), // fugly... need to give resolution manager some time to asynchronously finish running
+		)
+
+	tester.AddCall("getResolution", http.MethodGet, "/resolution/{{.createResolution.id}}", "").
+		Headers(adminHeaders).
+		Checkers(
+			//iffy.DumpResponse(t),
+			iffy.ExpectStatus(200),
+			iffy.ExpectJSONBranch("state", resolution.StateBlockedBadRequest),
+		)
+
+	tester.AddCall("createResolutionStartOver", http.MethodPost, "/resolution", `{"task_id":"{{.newTask.id}}","resolver_inputs":{"ri1":"bar"}, "start_over":true}`).
+		Headers(adminHeaders).
+		Checkers(iffy.ExpectStatus(201))
+
+	tester.AddCall("runResolutionStartOver", http.MethodPost, "/resolution/{{.createResolutionStartOver.id}}/run", "").
+		Headers(adminHeaders).
+		Checkers(
+			iffy.ExpectStatus(204),
+			waitChecker(time.Second), // fugly... need to give resolution manager some time to asynchronously finish running
+		)
+
+	tester.AddCall("getResolutionStartOver", http.MethodGet, "/resolution/{{.createResolutionStartOver.id}}", "").
+		Headers(adminHeaders).
+		Checkers(
+			//iffy.DumpResponse(t),
+			iffy.ExpectStatus(200),
+			iffy.ExpectJSONBranch("state", resolution.StateDone),
+		)
+
+	tester.AddCall("getResolutionAfterStartOver", http.MethodGet, "/resolution/{{.createResolution.id}}", "").
+		Headers(adminHeaders).
+		Checkers(
+			//iffy.DumpResponse(t),
+			iffy.ExpectStatus(404),
+		)
+
+	tester.AddCall("createResolutionStartOverFailed", http.MethodPost, "/resolution", `{"task_id":"{{.newTask.id}}","resolver_inputs":{"ri1":"foo"}, "start_over":true}`).
+		Headers(adminHeaders).
+		Checkers(
+			iffy.ExpectStatus(400),
+			iffy.ExpectJSONBranch("error", `can't start over a task that isn't in status "PAUSED", "BLOCKED_BADREQUEST" or "CANCELLED"`),
+		)
+
+	tester.Run()
+}
+
 func TestPagination(t *testing.T) {
 	tester := iffy.NewTester(t, hdl)
 
@@ -511,6 +619,70 @@ func templateWithPasswordInput() tasktemplate.TaskTemplate {
 	}
 }
 
+func resolverInputTemplate() tasktemplate.TaskTemplate {
+	return tasktemplate.TaskTemplate{
+		Name:        "resolver-input-template",
+		Description: "does nothing",
+		TitleFormat: "this task does nothing at all",
+		Inputs: []input.Input{
+			{
+				Name: "id",
+			},
+		},
+		Variables: []values.Variable{
+			{
+				Name:  "var1",
+				Value: "hello id {{.input.id }} for {{ .step.step1.output.foo }} and {{ .step.this.state | default \"BROKEN_TEMPLATING\" }}",
+			},
+			{
+				Name:       "var2",
+				Expression: "var a = 3+2; a;",
+			},
+		},
+		ResolverInputs: []input.Input{
+			{
+				Name:        "ri1",
+				Description: "resolver input 1",
+				LegalValues: []interface{}{"foo", "bar"},
+			},
+		},
+		Steps: map[string]*step.Step{
+			"step1": {
+				Action: executor.Executor{
+					Type: "echo",
+					Configuration: json.RawMessage(`{
+						"output": {"foo":"bar"}
+					}`),
+				},
+			},
+			"step2": {
+				Action: executor.Executor{
+					Type: "echo",
+					Configuration: json.RawMessage(`{
+						"output": {"foo":"bar"}
+					}`),
+				},
+				Dependencies: []string{"step1"},
+				Conditions: []*condition.Condition{
+					{
+						If: []*condition.Assert{
+							{
+								Expected: "foo",
+								Value:    "{{.resolver_input.ri1}}",
+								Operator: "EQ",
+							},
+						},
+						Then: map[string]string{
+							"this": "CLIENT_ERROR",
+						},
+						Type: "skip",
+					},
+				},
+			},
+		},
+	}
+}
+
 func dummyTemplate() tasktemplate.TaskTemplate {
 	return tasktemplate.TaskTemplate{
 		Name:        "dummy-template",

diff --git a/api/handler/resolution.go b/api/handler/resolution.go
@@ -22,6 +22,7 @@ import (
 type createResolutionIn struct {
 	TaskID         string                 `json:"task_id" binding:"required"`
 	ResolverInputs map[string]interface{} `json:"resolver_inputs"`
+	StartOver      bool                   `json:"start_over"`
 }
 
 // CreateResolution handles the creation of a resolution for a given task
@@ -45,17 +46,22 @@ func CreateResolution(c *gin.Context, in *createResolutionIn) (*resolution.Resol
 		return nil, err
 	}
 
+	if in.StartOver && t.Resolution == nil {
+		_ = dbp.Rollback()
+		return nil, errors.BadRequestf("can't start over a task that hasn't been resolved at least once")
+	}
+
 	tt, err := tasktemplate.LoadFromID(dbp, t.TemplateID)
 	if err != nil {
-		dbp.Rollback()
+		_ = dbp.Rollback()
 		return nil, err
 	}
 
 	admin := auth.IsAdmin(c) == nil
 	resolutionManager := auth.IsResolutionManager(c, tt, t, nil) == nil
 
 	if !admin && !resolutionManager {
-		dbp.Rollback()
+		_ = dbp.Rollback()
 		return nil, errors.Forbiddenf("You are not allowed to resolve this task")
 	} else if !resolutionManager {
 		metadata.SetSUDO(c)
@@ -73,6 +79,31 @@ func CreateResolution(c *gin.Context, in *createResolutionIn) (*resolution.Resol
 		}
 	}
 
+	if in.StartOver {
+		if !admin && resolutionManager && !tt.AllowTaskStartOver {
+			_ = dbp.Rollback()
+			return nil, errors.Forbiddenf("You are not allowed to start over this task")
+		}
+
+		res, err := resolution.LoadLockedFromPublicID(dbp, *t.Resolution)
+		if err != nil {
+			_ = dbp.Rollback()
+			return nil, err
+		}
+
+		if res.State != resolution.StatePaused && res.State != resolution.StateBlockedBadRequest && res.State != resolution.StateCancelled {
+			_ = dbp.Rollback()
+			return nil, errors.BadRequestf("can't start over a task that isn't in status %q, %q or %q", resolution.StatePaused, resolution.StateBlockedBadRequest, resolution.StateCancelled)
+		}
+
+		logrus.WithFields(logrus.Fields{"resolution_id": res.PublicID, "task_id": t.PublicID}).Debugf("Handler CreateResolution: start-over the resolution, deleting old resolution %s", res.PublicID)
+
+		if err := res.Delete(dbp); err != nil {
+			_ = dbp.Rollback()
+			return nil, err
+		}
+	}
+
 	r, err := resolution.Create(dbp, t, in.ResolverInputs, resUser, true, nil) // TODO accept delay in handler
 	if err != nil {
 		dbp.Rollback()