diff --git a/cmd/tink-cli/cmd/template/create.go b/cmd/tink-cli/cmd/template/create.go index 12e3c03e1..b19d825b8 100644 --- a/cmd/tink-cli/cmd/template/create.go +++ b/cmd/tink-cli/cmd/template/create.go @@ -69,7 +69,7 @@ func readAll(reader io.Reader) []byte { func addFlags() { flags := createCmd.PersistentFlags() flags.StringVarP(&filePath, "path", "p", "", "path to the template file") - flags.StringVarP(&templateName, "name", "n", "", "unique name for the template (alphanumeric)") + flags.StringVarP(&templateName, "name", "n", "", "unique name for the template (alphanumeric and case sensitive)") _ = createCmd.MarkPersistentFlagRequired(fName) } diff --git a/db/db.go b/db/db.go index 9eba92e0c..044f03ed0 100644 --- a/db/db.go +++ b/db/db.go @@ -36,7 +36,7 @@ type hardware interface { type template interface { CreateTemplate(ctx context.Context, name string, data string, id uuid.UUID) error - GetTemplate(ctx context.Context, fields map[string]string) (string, string, string, error) + GetTemplate(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) DeleteTemplate(ctx context.Context, name string) error ListTemplates(in string, fn func(id, n string, in, del *timestamp.Timestamp) error) error UpdateTemplate(ctx context.Context, name string, data string, id uuid.UUID) error @@ -114,7 +114,7 @@ func get(ctx context.Context, db *sql.DB, query string, args ...interface{}) (st func buildGetCondition(fields map[string]string) (string, error) { for column, field := range fields { if field != "" { - return fmt.Sprintf("%s = '%s' AND", column, field), nil + return fmt.Sprintf("%s = '%s'", column, field), nil } } return "", errors.New("one GetBy field must be set to build a get condition") diff --git a/db/migration/202012041103-remove-unique-constraints.go b/db/migration/202012041103-remove-unique-constraints.go new file mode 100644 index 000000000..1f7e9a4da --- /dev/null +++ b/db/migration/202012041103-remove-unique-constraints.go @@ -0,0 +1,12 @@ +package migration + +import migrate "github.com/rubenv/sql-migrate" + +func Get202012041103() *migrate.Migration { + return &migrate.Migration{ + Id: "202010221010-remove-unique-index", + Up: []string{` + ALTER TABLE template DROP CONSTRAINT template_name_key; + `}, + } +} diff --git a/db/migration/migration.go b/db/migration/migration.go index f8a9a35bb..bc461c876 100644 --- a/db/migration/migration.go +++ b/db/migration/migration.go @@ -7,6 +7,7 @@ func GetMigrations() *migrate.MemoryMigrationSource { Migrations: []*migrate.Migration{ Get202009171251(), Get202010221010(), + Get202012041103(), }, } } diff --git a/db/mock/mock.go b/db/mock/mock.go index ddd7867b4..9ec26e995 100644 --- a/db/mock/mock.go +++ b/db/mock/mock.go @@ -13,6 +13,7 @@ import ( type DB struct { // workflow CreateWorkflowFunc func(ctx context.Context, wf db.Workflow, data string, id uuid.UUID) error + GetWorkflowFunc func(ctx context.Context, id string) (db.Workflow, error) GetfromWfDataTableFunc func(ctx context.Context, req *pb.GetWorkflowDataRequest) ([]byte, error) InsertIntoWfDataTableFunc func(ctx context.Context, req *pb.UpdateWorkflowDataRequest) error GetWorkflowMetadataFunc func(ctx context.Context, req *pb.GetWorkflowDataRequest) ([]byte, error) @@ -24,5 +25,5 @@ type DB struct { InsertIntoWorkflowEventTableFunc func(ctx context.Context, wfEvent *pb.WorkflowActionStatus, time time.Time) error // template TemplateDB map[string]interface{} - GetTemplateFunc func(ctx context.Context, fields map[string]string) (string, string, string, error) + GetTemplateFunc func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) } diff --git a/db/mock/template.go b/db/mock/template.go index 1e128ed85..40e72a240 100644 --- a/db/mock/template.go +++ b/db/mock/template.go @@ -9,8 +9,9 @@ import ( ) type Template struct { - ID uuid.UUID - Data string + ID uuid.UUID + Data string + Deleted bool } // CreateTemplate creates a new workflow template @@ -20,20 +21,28 @@ func (d DB) CreateTemplate(ctx context.Context, name string, data string, id uui } if _, ok := d.TemplateDB[name]; ok { - return errors.New("Template name already exist in the database") + tmpl := d.TemplateDB[name] + switch stmpl := tmpl.(type) { + case Template: + if !stmpl.Deleted { + return errors.New("Template name already exist in the database") + } + default: + return errors.New("Not a Template type in the database") + } } - d.TemplateDB[name] = Template{ - ID: id, - Data: data, + ID: id, + Data: data, + Deleted: false, } return nil } // GetTemplate returns a workflow template -func (d DB) GetTemplate(ctx context.Context, fields map[string]string) (string, string, string, error) { - return d.GetTemplateFunc(ctx, fields) +func (d DB) GetTemplate(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { + return d.GetTemplateFunc(ctx, fields, deleted) } // DeleteTemplate deletes a workflow template diff --git a/db/mock/workflow.go b/db/mock/workflow.go index b49a875ce..4a845802f 100644 --- a/db/mock/workflow.go +++ b/db/mock/workflow.go @@ -41,7 +41,7 @@ func (d DB) GetWorkflowsForWorker(id string) ([]string, error) { // GetWorkflow returns a workflow func (d DB) GetWorkflow(ctx context.Context, id string) (db.Workflow, error) { - return db.Workflow{}, nil + return d.GetWorkflowFunc(ctx, id) } // DeleteWorkflow deletes a workflow diff --git a/db/template.go b/db/template.go index 47e0d1cc8..fde96c4ca 100644 --- a/db/template.go +++ b/db/template.go @@ -19,11 +19,17 @@ func (d TinkDB) CreateTemplate(ctx context.Context, name string, data string, id return err } + fields := map[string]string{ + "name": name, + } + _, _, _, err = d.GetTemplate(ctx, fields, false) + if err != sql.ErrNoRows { + return errors.New("Template with name '" + name + "' already exist") + } tx, err := d.instance.BeginTx(ctx, &sql.TxOptions{Isolation: sql.LevelSerializable}) if err != nil { return errors.Wrap(err, "BEGIN transaction") } - _, err = tx.Exec(` INSERT INTO template (created_at, updated_at, name, data, id) @@ -45,20 +51,31 @@ func (d TinkDB) CreateTemplate(ctx context.Context, name string, data string, id return nil } -// GetTemplate returns a workflow template -func (d TinkDB) GetTemplate(ctx context.Context, fields map[string]string) (string, string, string, error) { +// GetTemplate returns template which is not deleted +func (d TinkDB) GetTemplate(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { getCondition, err := buildGetCondition(fields) if err != nil { return "", "", "", errors.Wrap(err, "failed to get template") } - query := ` + var query string + if !deleted { + query = ` SELECT id, name, data FROM template WHERE - ` + getCondition + ` + ` + getCondition + ` AND deleted_at IS NULL ` + } else { + query = ` + SELECT id, name, data + FROM template + WHERE + ` + getCondition + ` + ` + } + row := d.instance.QueryRowContext(ctx, query) id := []byte{} name := []byte{} diff --git a/grpc-server/template.go b/grpc-server/template.go index 59b24fa51..90ab1eb87 100644 --- a/grpc-server/template.go +++ b/grpc-server/template.go @@ -62,7 +62,7 @@ func (s *server) GetTemplate(ctx context.Context, in *template.GetRequest) (*tem "id": in.GetId(), "name": in.GetName(), } - id, n, d, err := s.db.GetTemplate(ctx, fields) + id, n, d, err := s.db.GetTemplate(ctx, fields, false) logger.Info("done " + msg) if err != nil { metrics.CacheErrors.With(labels).Inc() diff --git a/grpc-server/template_test.go b/grpc-server/template_test.go index b2f059594..5bafe942b 100644 --- a/grpc-server/template_test.go +++ b/grpc-server/template_test.go @@ -83,7 +83,10 @@ func TestCreateTemplate(t *testing.T) { args: args{ db: mock.DB{ TemplateDB: map[string]interface{}{ - "template_1": template1, + "template_1": mock.Template{ + Data: template1, + Deleted: false, + }, }, }, name: "template_2", @@ -98,7 +101,10 @@ func TestCreateTemplate(t *testing.T) { args: args{ db: mock.DB{ TemplateDB: map[string]interface{}{ - "template_1": template1, + "template_1": mock.Template{ + Data: template1, + Deleted: false, + }, }, }, name: "template_1", @@ -108,6 +114,24 @@ func TestCreateTemplate(t *testing.T) { expectedError: true, }, }, + + "SuccessfulTemplateCreationAfterDeletingWithSameName": { + args: args{ + db: mock.DB{ + TemplateDB: map[string]interface{}{ + "template_1": mock.Template{ + Data: template1, + Deleted: true, + }, + }, + }, + name: "template_1", + template: template2, + }, + want: want{ + expectedError: false, + }, + }, } ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -124,6 +148,7 @@ func TestCreateTemplate(t *testing.T) { assert.Nil(t, err) assert.NotEmpty(t, res) } + tc.args.db.ClearTemplateDB() }) } } @@ -148,7 +173,7 @@ func TestGetTemplate(t *testing.T) { TemplateDB: map[string]interface{}{ templateName1: template1, }, - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { t.Log("in get template func") if fields["id"] == templateID1 || fields["name"] == templateName1 { @@ -171,7 +196,7 @@ func TestGetTemplate(t *testing.T) { TemplateDB: map[string]interface{}{ templateName1: template1, }, - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { t.Log("in get template func") if fields["id"] == templateID1 || fields["name"] == templateName1 { @@ -194,7 +219,7 @@ func TestGetTemplate(t *testing.T) { TemplateDB: map[string]interface{}{ templateName1: template1, }, - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { t.Log("in get template func") if fields["id"] == templateID1 || fields["name"] == templateName1 { @@ -217,7 +242,7 @@ func TestGetTemplate(t *testing.T) { TemplateDB: map[string]interface{}{ templateName1: template1, }, - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { t.Log("in get template func") if fields["id"] == templateID1 || fields["name"] == templateName1 { @@ -240,7 +265,7 @@ func TestGetTemplate(t *testing.T) { TemplateDB: map[string]interface{}{ templateName1: template1, }, - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { t.Log("in get template func") if fields["id"] == templateID1 || fields["name"] == templateName1 { @@ -263,7 +288,7 @@ func TestGetTemplate(t *testing.T) { TemplateDB: map[string]interface{}{ templateName1: template1, }, - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { t.Log("in get template func") if fields["id"] == templateID1 || fields["name"] == templateName1 { diff --git a/grpc-server/workflow.go b/grpc-server/workflow.go index d2523be6f..5fcffe2b1 100644 --- a/grpc-server/workflow.go +++ b/grpc-server/workflow.go @@ -48,8 +48,15 @@ func (s *server) CreateWorkflow(ctx context.Context, in *workflow.CreateRequest) defer timer.ObserveDuration() logger.Info(msg) + fields := map[string]string{ + "id": in.GetTemplate(), + } + _, _, templateData, err := s.db.GetTemplate(ctx, fields, false) + if err != nil { + return &workflow.CreateResponse{}, errors.Wrapf(err, errFailedToGetTemplate, in.GetTemplate()) + } + data, err := renderTemplate(in.GetTemplate(), templateData, []byte(in.Hardware)) - data, err := createYaml(ctx, s.db, in.Template, in.Hardware) if err != nil { metrics.CacheErrors.With(labels).Inc() logger.Error(err) @@ -102,7 +109,15 @@ func (s *server) GetWorkflow(ctx context.Context, in *workflow.GetRequest) (*wor } l.Error(err) } - yamlData, err := createYaml(ctx, s.db, w.Template, w.Hardware) + + fields := map[string]string{ + "id": w.Template, + } + _, _, templateData, err := s.db.GetTemplate(ctx, fields, true) + if err != nil { + return &workflow.Workflow{}, errors.Wrapf(err, errFailedToGetTemplate, w.Template) + } + data, err := renderTemplate(w.Template, templateData, []byte(w.Hardware)) if err != nil { return &workflow.Workflow{}, err } @@ -111,7 +126,7 @@ func (s *server) GetWorkflow(ctx context.Context, in *workflow.GetRequest) (*wor Template: w.Template, Hardware: w.Hardware, State: state[w.State], - Data: yamlData, + Data: data, } l := logger.With("workflowID", w.ID) l.Info("done " + msg) @@ -270,17 +285,6 @@ func (s *server) ShowWorkflowEvents(req *workflow.GetRequest, stream workflow.Wo return nil } -func createYaml(ctx context.Context, db db.Database, templateID string, devices string) (string, error) { - fields := map[string]string{ - "id": templateID, - } - _, _, templateData, err := db.GetTemplate(ctx, fields) - if err != nil { - return "", errors.Wrapf(err, errFailedToGetTemplate, templateID) - } - return renderTemplate(templateID, templateData, []byte(devices)) -} - func renderTemplate(templateID, templateData string, devices []byte) (string, error) { var hardware map[string]interface{} err := json.Unmarshal(devices, &hardware) diff --git a/grpc-server/workflow_test.go b/grpc-server/workflow_test.go index 6413566fe..ca32f01fd 100644 --- a/grpc-server/workflow_test.go +++ b/grpc-server/workflow_test.go @@ -24,7 +24,7 @@ tasks: actions: - name: "hello_world" image: hello-world - timeout: 60` + timeout: 60` ) func TestCreateWorkflow(t *testing.T) { @@ -44,7 +44,7 @@ func TestCreateWorkflow(t *testing.T) { "FailedToGetTemplate": { args: args{ db: mock.DB{ - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { return "", "", "", errors.New("failed to get template") }, }, @@ -58,7 +58,7 @@ func TestCreateWorkflow(t *testing.T) { "FailedCreatingWorkflow": { args: args{ db: mock.DB{ - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { return "", "", templateData, nil }, CreateWorkflowFunc: func(ctx context.Context, wf db.Workflow, data string, id uuid.UUID) error { @@ -75,7 +75,7 @@ func TestCreateWorkflow(t *testing.T) { "SuccessCreatingWorkflow": { args: args{ db: mock.DB{ - GetTemplateFunc: func(ctx context.Context, fields map[string]string) (string, string, string, error) { + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { return "", "", templateData, nil }, CreateWorkflowFunc: func(ctx context.Context, wf db.Workflow, data string, id uuid.UUID) error { @@ -112,3 +112,60 @@ func TestCreateWorkflow(t *testing.T) { }) } } + +func TestGetWorkflow(t *testing.T) { + type ( + args struct { + db mock.DB + wfTemplate, wfHardware string + } + want struct { + expectedError bool + } + ) + testCases := map[string]struct { + args args + want want + }{ + "SuccessGettingWorkflow": { + args: args{ + db: mock.DB{ + GetWorkflowFunc: func(ctx context.Context, workflowID string) (db.Workflow, error) { + return db.Workflow{ + ID: workflowID, + Template: templateID, + Hardware: hw}, nil + }, + GetTemplateFunc: func(ctx context.Context, fields map[string]string, deleted bool) (string, string, string, error) { + return "", "", templateData, nil + }, + }, + wfTemplate: templateID, + wfHardware: hw, + }, + want: want{ + expectedError: false, + }, + }, + } + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + s := testServer(tc.args.db) + res, err := s.GetWorkflow(ctx, &workflow.GetRequest{ + Id: workflowID, + }) + if err != nil { + assert.Error(t, err) + assert.Empty(t, res) + assert.True(t, tc.want.expectedError) + return + } + assert.NoError(t, err) + assert.NotEmpty(t, res) + assert.False(t, tc.want.expectedError) + }) + } +}