feat(controlplane): unique workflow name and formatted project

app/controlplane/internal/biz/biz.go

@@ -78,7 +78,7 @@ func ValidateIsDNS1123(name string) error {
 	if len(err) > 0 {
 		errMsg := ""
 		for _, e := range err {
-			errMsg += e + "\n"
+			errMsg += fmt.Sprintf("%q: %s\n", name, e)
 		}
 
 		return errors.New(errMsg)

app/controlplane/internal/biz/casmapping_integration_test.go

@@ -355,7 +355,7 @@ func (s *casMappingIntegrationSuite) SetupTest() {
 	workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org1.ID})
 	assert.NoError(err)
 
-	publicWorkflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org1.ID, Public: true})
+	publicWorkflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow-public", OrgID: s.org1.ID, Public: true})
 	assert.NoError(err)
 
 	// Robot account

app/controlplane/internal/biz/workflow.go

@@ -77,17 +77,43 @@ func (uc *WorkflowUseCase) Create(ctx context.Context, opts *WorkflowCreateOpts)
 		return nil, errors.New("workflow name is required")
 	}
 
+	// validate format of the name and the project
+	if err := ValidateIsDNS1123(opts.Name); err != nil {
+		return nil, NewErrValidation(err)
+	}
+
+	if opts.Project != "" {
+		if err := ValidateIsDNS1123(opts.Project); err != nil {
+			return nil, NewErrValidation(err)
+		}
+	}
+
 	contract, err := uc.findOrCreateContract(ctx, opts.OrgID, opts.ContractID, opts.Project, opts.Name)
 	if err != nil {
 		return nil, err
+	} else if contract == nil {
+		return nil, NewErrNotFound("contract")
 	}
 
 	// Set the potential new schemaID
 	opts.ContractID = contract.ID.String()
-	return uc.wfRepo.Create(ctx, opts)
+	wf, err := uc.wfRepo.Create(ctx, opts)
+	if err != nil {
+		if errors.Is(err, ErrAlreadyExists) {
+			return nil, NewErrValidationStr("name already taken")
+		}
+
+		return nil, fmt.Errorf("failed to create workflow: %w", err)
+	}
+
+	return wf, nil
 }
 
 func (uc *WorkflowUseCase) Update(ctx context.Context, orgID, workflowID string, opts *WorkflowUpdateOpts) (*Workflow, error) {
+	if opts == nil {
+		return nil, NewErrValidationStr("no updates provided")
+	}
+
 	orgUUID, err := uuid.Parse(orgID)
 	if err != nil {
 		return nil, NewErrInvalidUUID(err)
@@ -98,6 +124,19 @@ func (uc *WorkflowUseCase) Update(ctx context.Context, orgID, workflowID string,
 		return nil, NewErrInvalidUUID(err)
 	}
 
+	if opts.Name != nil {
+		// validate format of the name and the project
+		if err := ValidateIsDNS1123(*opts.Name); err != nil {
+			return nil, NewErrValidation(err)
+		}
+	}
+
+	if opts.Project != nil && *opts.Project != "" {
+		if err := ValidateIsDNS1123(*opts.Project); err != nil {
+			return nil, NewErrValidation(err)
+		}
+	}
+
 	// make sure that the workflow is for the provided org
 	if wf, err := uc.wfRepo.GetOrgScoped(ctx, orgUUID, workflowUUID); err != nil {
 		return nil, err
@@ -107,6 +146,10 @@ func (uc *WorkflowUseCase) Update(ctx context.Context, orgID, workflowID string,
 
 	wf, err := uc.wfRepo.Update(ctx, workflowUUID, opts)
 	if err != nil {
+		if errors.Is(err, ErrAlreadyExists) {
+			return nil, NewErrValidationStr("name already taken")
+		}
+
 		return nil, fmt.Errorf("failed to update workflow: %w", err)
 	} else if wf == nil {
 		return nil, NewErrNotFound("workflow")

app/controlplane/internal/biz/workflow_integration_test.go

@@ -1,5 +1,5 @@
 //
-// Copyright 2023 The Chainloop Authors.
+// Copyright 2024 The Chainloop Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@ package biz_test
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -54,6 +55,72 @@ func (s *workflowIntegrationTestSuite) TestContractLatestAvailable() {
 	})
 }
 
+func (s *workflowIntegrationTestSuite) TestCreate() {
+	ctx := context.Background()
+	testCases := []struct {
+		name       string
+		opts       *biz.WorkflowCreateOpts
+		wantErrMsg string
+	}{
+		{
+			name:       "org missing",
+			opts:       &biz.WorkflowCreateOpts{Name: "name"},
+			wantErrMsg: "required",
+		},
+		{
+			name:       "name missing",
+			opts:       &biz.WorkflowCreateOpts{OrgID: s.org.ID},
+			wantErrMsg: "required",
+		},
+		{
+			name:       "invalid name",
+			opts:       &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "this/not/valid"},
+			wantErrMsg: "RFC 1123",
+		},
+		{
+			name:       "another invalid name",
+			opts:       &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "this-not Valid"},
+			wantErrMsg: "RFC 1123",
+		},
+		{
+			name:       " invalid project name",
+			opts:       &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "valid", Project: "this-not Valid"},
+			wantErrMsg: "RFC 1123",
+		},
+		{
+			name:       "non-existing contract",
+			opts:       &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name", ContractID: uuid.Generate().String()},
+			wantErrMsg: "not found",
+		},
+		{
+			name: "can create it with just the name and the org",
+			opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name"},
+		},
+		{
+			name: "with all items",
+			opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "another-name", Project: "project", Team: "team", Description: "description"},
+		},
+	}
+
+	for _, tc := range testCases {
+		s.Run(tc.name, func() {
+			got, err := s.Workflow.Create(ctx, tc.opts)
+			if tc.wantErrMsg != "" {
+				s.ErrorContains(err, tc.wantErrMsg)
+				return
+			}
+
+			require.NoError(s.T(), err)
+			s.NotEmpty(got.ID)
+			s.NotEmpty(got.CreatedAt)
+			s.Equal(tc.opts.Name, got.Name)
+			s.Equal(tc.opts.Description, got.Description)
+			s.Equal(tc.opts.Team, got.Team)
+			s.Equal(tc.opts.Project, got.Project)
+		})
+	}
+}
+
 func (s *workflowIntegrationTestSuite) TestUpdate() {
 	ctx := context.Background()
 	const (
@@ -72,8 +139,15 @@ func (s *workflowIntegrationTestSuite) TestUpdate() {
 		s.False(workflow.Public)
 	})
 
-	s.Run("can't update a workflow in another org", func() {
+	s.Run("can't update if no changes are provided", func() {
 		got, err := s.Workflow.Update(ctx, org2.ID, workflow.ID.String(), nil)
+		s.True(biz.IsErrValidation(err))
+		s.Error(err)
+		s.Nil(got)
+	})
+
+	s.Run("can't update a workflow in another org", func() {
+		got, err := s.Workflow.Update(ctx, org2.ID, workflow.ID.String(), &biz.WorkflowUpdateOpts{Name: toPtrS("new-name")})
 		s.True(biz.IsNotFound(err))
 		s.Error(err)
 		s.Nil(got)
@@ -82,62 +156,77 @@ func (s *workflowIntegrationTestSuite) TestUpdate() {
 	testCases := []struct {
 		name string
 		// if not set, it will use the workflow we create on each run
-		id      string
-		updates *biz.WorkflowUpdateOpts
-		want    *biz.Workflow
-		wantErr bool
+		id         string
+		updates    *biz.WorkflowUpdateOpts
+		want       *biz.Workflow
+		wantErr    bool
+		wantErrMsg string
 	}{
 		{
 			name:    "non existing workflow",
 			id:      uuid.Generate().String(),
-			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new name")},
+			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new-name")},
 			wantErr: true,
 		},
 		{
 			name:    "invalid uuid",
 			id:      "deadbeef",
-			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new name")},
+			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new-name")},
 			wantErr: true,
 		},
 		{
-			name: "no updates",
-			want: &biz.Workflow{Name: name, Team: team, Project: project, Public: false, Description: description},
+			name:       "no updates",
+			wantErr:    true,
+			wantErrMsg: "no updates provided",
+		},
+		{
+			name:       "invalid name",
+			wantErr:    true,
+			wantErrMsg: "RFC 1123",
+			updates:    &biz.WorkflowUpdateOpts{Name: toPtrS(" no no ")},
+		},
+		{
+			name:       "invalid Project",
+			wantErr:    true,
+			wantErrMsg: "RFC 1123",
+			updates:    &biz.WorkflowUpdateOpts{Project: toPtrS(" no no ")},
 		},
 		{
 			name:    "update name",
-			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new name")},
-			want:    &biz.Workflow{Name: "new name", Description: description, Team: team, Project: project, Public: false},
+			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new-name")},
+			want:    &biz.Workflow{Name: "new-name", Description: description, Team: team, Project: project, Public: false},
 		},
 		{
 			name:    "update description",
 			updates: &biz.WorkflowUpdateOpts{Description: toPtrS("new description")},
-			want:    &biz.Workflow{Name: name, Description: "new description", Team: team, Project: project, Public: false},
+			want:    &biz.Workflow{Description: "new description", Team: team, Project: project, Public: false},
 		},
 		{
 			name:    "update visibility",
 			updates: &biz.WorkflowUpdateOpts{Public: toPtrBool(true)},
-			want:    &biz.Workflow{Name: name, Description: description, Team: team, Project: project, Public: true},
+			want:    &biz.Workflow{Description: description, Team: team, Project: project, Public: true},
 		},
 		{
 			name:    "update all options",
-			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new name"), Project: toPtrS("new project"), Team: toPtrS("new team"), Public: toPtrBool(true)},
-			want:    &biz.Workflow{Name: "new name", Description: description, Team: "new team", Project: "new project", Public: true},
+			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("new-name-2"), Project: toPtrS("new-project"), Team: toPtrS("new team"), Public: toPtrBool(true)},
+			want:    &biz.Workflow{Name: "new-name-2", Description: description, Team: "new team", Project: "new-project", Public: true},
 		},
 		{
 			name:    "name can't be emptied",
 			updates: &biz.WorkflowUpdateOpts{Name: toPtrS("")},
-			want:    &biz.Workflow{Name: name, Team: team, Project: project, Description: description},
+			wantErr: true,
 		},
 		{
 			name:    "but other opts can",
 			updates: &biz.WorkflowUpdateOpts{Team: toPtrS(""), Project: toPtrS(""), Description: toPtrS("")},
-			want:    &biz.Workflow{Name: name, Team: "", Project: "", Description: ""},
+			want:    &biz.Workflow{Team: "", Project: "", Description: ""},
 		},
 	}
 
-	for _, tc := range testCases {
+	for i, tc := range testCases {
 		s.Run(tc.name, func() {
-			workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Description: description, Name: name, Team: team, Project: project, OrgID: s.org.ID})
+			wfName := fmt.Sprintf("%s-%d", name, i)
+			workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Description: description, Name: wfName, Team: team, Project: project, OrgID: s.org.ID})
 			require.NoError(s.T(), err)
 
 			workflowID := tc.id
@@ -148,15 +237,25 @@ func (s *workflowIntegrationTestSuite) TestUpdate() {
 			got, err := s.Workflow.Update(ctx, s.org.ID, workflowID, tc.updates)
 			if tc.wantErr {
 				s.Error(err)
+				if tc.wantErrMsg != "" {
+					s.Contains(err.Error(), tc.wantErrMsg)
+				}
+
 				return
 			}
 			s.NoError(err)
 
 			if diff := cmp.Diff(tc.want, got,
-				cmpopts.IgnoreFields(biz.Workflow{}, "CreatedAt", "ID", "OrgID", "ContractID", "ContractRevisionLatest"),
+				cmpopts.IgnoreFields(biz.Workflow{}, "Name", "CreatedAt", "ID", "OrgID", "ContractID", "ContractRevisionLatest"),
 			); diff != "" {
 				s.Failf("mismatch (-want +got):\n%s", diff)
 			}
+
+			if tc.want.Name != "" {
+				s.Equal(tc.want.Name, got.Name)
+			} else {
+				s.Equal(wfName, got.Name)
+			}
 		})
 	}
 }

app/controlplane/internal/biz/workflowcontract.go

@@ -155,7 +155,7 @@ func (uc *WorkflowContractUseCase) Create(ctx context.Context, opts *WorkflowCon
 
 	if err != nil {
 		if errors.Is(err, ErrAlreadyExists) {
-			return nil, NewErrValidationStr("that name is already taken")
+			return nil, NewErrValidationStr("name already taken")
 		}
 
 		return nil, fmt.Errorf("failed to create contract: %w", err)
@@ -189,7 +189,7 @@ func (uc *WorkflowContractUseCase) createWithUniqueName(ctx context.Context, opt
 		return c, nil
 	}
 
-	return nil, NewErrValidationStr("that name is already taken")
+	return nil, NewErrValidationStr("name already taken")
 }
 
 func (uc *WorkflowContractUseCase) Describe(ctx context.Context, orgID, contractID string, revision int) (*WorkflowContractWithVersion, error) {
@@ -247,7 +247,7 @@ func (uc *WorkflowContractUseCase) Update(ctx context.Context, orgID, contractID
 	c, err := uc.repo.Update(ctx, opts)
 	if err != nil {
 		if errors.Is(err, ErrAlreadyExists) {
-			return nil, NewErrValidationStr("that name is already taken")
+			return nil, NewErrValidationStr("name already taken")
 		}
 
 		return nil, fmt.Errorf("failed to update contract: %w", err)

app/controlplane/internal/data/ent/migrate/migrations/20240312141340.sql

@@ -0,0 +1,47 @@
+-- Update existing data in "workflows" table
+-- Make the name and project RFC 1123 compliant
+UPDATE workflows
+SET name = regexp_replace(
+             lower(name), 
+             '[^a-z0-9-]', 
+             '-', 
+             'g'
+         );
+
+-- and project
+UPDATE workflows
+SET project = regexp_replace(
+             lower(name), 
+             '[^a-z0-9-]', 
+             '-', 
+             'g'
+         );
+
+-- Append suffixes to duplicates
+WITH numbered_names AS (
+    SELECT 
+        id,
+        name,
+        ROW_NUMBER() OVER (PARTITION BY name ORDER BY id) AS rn
+    FROM workflows
+)
+UPDATE workflows AS o
+SET name = CONCAT(o.name, '-', nn.rn - 1)
+FROM numbered_names AS nn
+WHERE o.id = nn.id AND nn.rn > 1;
+
+
+WITH numbered_projects AS (
+    SELECT 
+        id,
+        project,
+        ROW_NUMBER() OVER (PARTITION BY name ORDER BY id) AS rn
+    FROM workflows
+)
+UPDATE workflows AS o
+SET name = CONCAT(o.project, '-', np.rn - 1)
+FROM numbered_projects AS np
+WHERE o.id = np.id AND np.rn > 1;
+
+-- Create index "workflow_name_organization_id" to table: "workflows"
+CREATE UNIQUE INDEX "workflow_name_organization_id" ON "workflows" ("name", "organization_id");