diff --git a/cmd/tink-worker/action.go b/cmd/tink-worker/action.go index bdb2460f9..f06ff2f26 100644 --- a/cmd/tink-worker/action.go +++ b/cmd/tink-worker/action.go @@ -43,15 +43,13 @@ func executeAction(ctx context.Context, action *pb.WorkflowAction, wfID string) return pb.ActionState_ACTION_IN_PROGRESS, errors.Wrap(err, "DOCKER CREATE") } l.With("containerID", id, "command", action.GetOnTimeout()).Info("container created") - - var timeCtx context.Context - var cancel context.CancelFunc + // Setting time context for action + timeCtx := ctx if action.Timeout > 0 { - timeCtx, cancel = context.WithTimeout(context.Background(), time.Duration(action.Timeout)*time.Second) - } else { - timeCtx, cancel = context.WithTimeout(context.Background(), 1*time.Hour) + var cancel context.CancelFunc + timeCtx, cancel = context.WithTimeout(ctx, time.Duration(action.Timeout)*time.Second) + defer cancel() } - defer cancel() err = startContainer(timeCtx, l, id) if err != nil { return pb.ActionState_ACTION_IN_PROGRESS, errors.Wrap(err, "DOCKER RUN") diff --git a/db/mock/mock.go b/db/mock/mock.go index 789efe88a..93dd10516 100644 --- a/db/mock/mock.go +++ b/db/mock/mock.go @@ -22,7 +22,7 @@ type DB struct { GetWorkflowActionsFunc func(ctx context.Context, wfID string) (*pb.WorkflowActionList, error) UpdateWorkflowStateFunc func(ctx context.Context, wfContext *pb.WorkflowContext) error InsertIntoWorkflowEventTableFunc func(ctx context.Context, wfEvent *pb.WorkflowActionStatus, time time.Time) error - // template + TemplateDB map[string]interface{} GetTemplateFunc func(ctx context.Context, id string) (string, string, error) } diff --git a/db/mock/template.go b/db/mock/template.go index 6e083c718..c09d13802 100644 --- a/db/mock/template.go +++ b/db/mock/template.go @@ -8,30 +8,26 @@ import ( "github.com/google/uuid" ) -type template struct { - id uuid.UUID - data string +type Template struct { + ID uuid.UUID + Data string } -var templateDB = map[string]interface{}{} - // CreateTemplate creates a new workflow template func (d DB) CreateTemplate(ctx context.Context, name string, data string, id uuid.UUID) error { - if len(templateDB) > 0 { - if _, ok := templateDB[name]; ok { - return errors.New("Template name already exist in the database") - } - templateDB[name] = template{ - id: id, - data: data, - } + if d.TemplateDB == nil { + d.TemplateDB = make(map[string]interface{}) + } + + if _, ok := d.TemplateDB[name]; ok { + return errors.New("Template name already exist in the database") + } - } else { - templateDB[name] = template{ - id: id, - data: data, - } + d.TemplateDB[name] = Template{ + ID: id, + Data: data, } + return nil } @@ -42,12 +38,10 @@ func (d DB) GetTemplate(ctx context.Context, id string) (string, string, error) // DeleteTemplate deletes a workflow template func (d DB) DeleteTemplate(ctx context.Context, name string) error { - if len(templateDB) > 0 { - if _, ok := templateDB[name]; !ok { - return errors.New("Template name does not exist") - } - delete(templateDB, name) + if d.TemplateDB != nil { + delete(d.TemplateDB, name) } + return nil } @@ -63,7 +57,5 @@ func (d DB) UpdateTemplate(ctx context.Context, name string, data string, id uui // ClearTemplateDB clear all the templates func (d DB) ClearTemplateDB() { - for name := range templateDB { - delete(templateDB, name) - } + d.TemplateDB = make(map[string]interface{}) } diff --git a/db/template.go b/db/template.go index 9e2f07acf..5fbd2d881 100644 --- a/db/template.go +++ b/db/template.go @@ -9,6 +9,7 @@ import ( "github.com/golang/protobuf/ptypes/timestamp" "github.com/google/uuid" "github.com/pkg/errors" + "github.com/tinkerbell/tink/pkg" ) // CreateTemplate creates a new workflow template @@ -18,6 +19,15 @@ func (d TinkDB) CreateTemplate(ctx context.Context, name string, data string, id return errors.Wrap(err, "BEGIN transaction") } + wf, err := pkg.ParseYAML([]byte(data)) + if err != nil { + return err + } + err = pkg.ValidateTemplate(wf) + if err != nil { + return err + } + _, err = tx.Exec(` INSERT INTO template (created_at, updated_at, name, data, id) diff --git a/go.mod b/go.mod index 4a6c2693b..156b3c1e1 100644 --- a/go.mod +++ b/go.mod @@ -32,6 +32,7 @@ require ( github.com/spf13/viper v1.4.0 github.com/stretchr/testify v1.3.0 go.mongodb.org/mongo-driver v1.1.2 // indirect + golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2 golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e // indirect golang.org/x/sys v0.0.0-20200331124033-c3d80250170d // indirect golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect diff --git a/grpc-server/template_test.go b/grpc-server/template_test.go index 36fe170dd..84fc6b315 100644 --- a/grpc-server/template_test.go +++ b/grpc-server/template_test.go @@ -22,13 +22,13 @@ tasks: timeout: 60` template2 = `version: "0.1" -name: hello_world_workflow +name: hello_world_again_workflow global_timeout: 600 tasks: - name: "hello world again" - worker: "{{.device_1}}" + worker: "{{.device_2}}" actions: - - name: "hello_world_again" + - name: "hello_world_again" image: hello-world timeout: 60` ) @@ -36,9 +36,9 @@ tasks: func TestCreateTemplate(t *testing.T) { type ( args struct { - db mock.DB - name []string - templates []string + db mock.DB + name string + template string } want struct { expectedError bool @@ -50,9 +50,11 @@ func TestCreateTemplate(t *testing.T) { }{ "SuccessfullTemplateCreation": { args: args{ - db: mock.DB{}, - name: []string{"template_1"}, - templates: []string{template1}, + db: mock.DB{ + TemplateDB: make(map[string]interface{}), + }, + name: "template_1", + template: template1, }, want: want{ expectedError: false, @@ -61,9 +63,13 @@ func TestCreateTemplate(t *testing.T) { "SuccessfullMultipleTemplateCreation": { args: args{ - db: mock.DB{}, - name: []string{"template_1", "template_2"}, - templates: []string{template1, template2}, + db: mock.DB{ + TemplateDB: map[string]interface{}{ + "template_1": template1, + }, + }, + name: "template_2", + template: template2, }, want: want{ expectedError: false, @@ -72,35 +78,31 @@ func TestCreateTemplate(t *testing.T) { "FailedMultipleTemplateCreationWithSameName": { args: args{ - db: mock.DB{}, - name: []string{"template_1", "template_1"}, - templates: []string{template1, template2}, + db: mock.DB{ + TemplateDB: map[string]interface{}{ + "template_1": template1, + }, + }, + name: "template_1", + template: template2, }, want: want{ expectedError: true, }, }, } - for name, tc := range testCases { + + for name := range testCases { + tc := testCases[name] t.Run(name, func(t *testing.T) { + t.Parallel() s := testServer(tc.args.db) - tc.args.db.ClearTemplateDB() - index := 0 - res, err := s.CreateTemplate(context.TODO(), &pb.WorkflowTemplate{Name: tc.args.name[index], Data: tc.args.templates[index]}) - assert.Nil(t, err) - assert.NotNil(t, res) - if err == nil && len(tc.args.templates) > 1 { - index++ - _, err = s.CreateTemplate(context.TODO(), &pb.WorkflowTemplate{Name: tc.args.name[index], Data: tc.args.templates[index]}) - } else { - return - } - if err != nil { + res, err := s.CreateTemplate(context.TODO(), &pb.WorkflowTemplate{Name: tc.args.name, Data: tc.args.template}) + if tc.want.expectedError { assert.Error(t, err) - assert.True(t, tc.want.expectedError) } else { assert.Nil(t, err) - assert.False(t, tc.want.expectedError) + assert.NotEmpty(t, res) } }) } diff --git a/pkg/template_validator.go b/pkg/template_validator.go index 51660842b..278d94ef3 100644 --- a/pkg/template_validator.go +++ b/pkg/template_validator.go @@ -1,18 +1,17 @@ package pkg import ( - "fmt" - "github.com/docker/distribution/reference" + "github.com/pkg/errors" "gopkg.in/yaml.v2" ) const ( - errEmptyName = "task/action name cannot be empty: %v" - errInvalidLength = "task/action name cannot have more than 200 characters: %v" - errDuplicateTaskName = "two tasks in a template cannot have same name: %v" - errInvalidActionImage = "invalid action image: %v" - errDuplicateActionName = "two actions in a task cannot have same name: %v" + errEmptyName = "task/action name cannot be empty: " + errInvalidLength = "task/action name cannot have more than 200 characters: " + errDuplicateTaskName = "two tasks in a template cannot have same name: " + errInvalidActionImage = "invalid action image: " + errDuplicateActionName = "two actions in a task cannot have same name: " ) // ParseYAML parses the template yaml content @@ -30,29 +29,34 @@ func ParseYAML(yamlContent []byte) (*Workflow, error) { func ValidateTemplate(wf *Workflow) error { taskNameMap := make(map[string]struct{}) for _, task := range wf.Tasks { - err := hasValidLength(task.Name) - if err != nil { - return err + if hasEmptyName(task.Name) { + return errors.New(errEmptyName + task.Name) + } + if !hasValidLength(task.Name) { + return errors.New(errInvalidLength + task.Name) } _, ok := taskNameMap[task.Name] if ok { - return fmt.Errorf(errDuplicateTaskName, task.Name) + return errors.New(errDuplicateTaskName + task.Name) } taskNameMap[task.Name] = struct{}{} actionNameMap := make(map[string]struct{}) for _, action := range task.Actions { - err := hasValidLength(action.Name) - if err != nil { - return err + if hasEmptyName(action.Name) { + return errors.New(errEmptyName + action.Name) + } + + if !hasValidLength(action.Name) { + return errors.New(errInvalidLength + action.Name) } - err = isValidImageName(action.Image) - if err != nil { - return fmt.Errorf(errInvalidActionImage, action.Image) + + if !hasValidImageName(action.Image) { + return errors.New(errInvalidActionImage + action.Image) } _, ok := actionNameMap[action.Name] if ok { - return fmt.Errorf(errDuplicateActionName, action.Name) + return errors.New(errDuplicateActionName + action.Name) } actionNameMap[action.Name] = struct{}{} } @@ -60,20 +64,14 @@ func ValidateTemplate(wf *Workflow) error { return nil } -func hasValidLength(name string) error { - if name == "" { - return fmt.Errorf(errEmptyName, name) - } - if len(name) > 200 { - return fmt.Errorf(errInvalidLength, name) - } - return nil +func hasEmptyName(name string) bool { + return name == "" +} +func hasValidLength(name string) bool { + return len(name) < 200 } -func isValidImageName(name string) error { +func hasValidImageName(name string) bool { _, err := reference.ParseNormalizedNamed(name) - if err != nil { - return err - } - return nil + return err == nil } diff --git a/pkg/template_validator_test.go b/pkg/template_validator_test.go index e7f3efe6d..a101b6065 100644 --- a/pkg/template_validator_test.go +++ b/pkg/template_validator_test.go @@ -97,6 +97,11 @@ func TestValidateTemplate(t *testing.T) { wf: workflow(withDuplicateActionName()), expectedError: true, }, + { + name: "long action name", + wf: workflow(withLongActionName()), + expectedError: true, + }, { name: "valid task name", wf: workflow(), @@ -122,6 +127,12 @@ func withLongTaskName() workflowModifier { } } +func withLongActionName() workflowModifier { + return func(wf *Workflow) { + wf.Tasks[0].Actions[0].Name = "this action has a very long name to test whether we recevice an error or not if an action has very long name, one that would probably go beyond the limit of not having an action name with more than two hundred characters" + } +} + func withInvalidTaskName() workflowModifier { return func(wf *Workflow) { wf.Tasks[0].Name = "" } }