Skip to content

feat: remove workflow-name org scoping #1414

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 21, 2024
Merged

feat: remove workflow-name org scoping #1414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd95e9d
feat: remove workflow-name org scopping
migmartri Oct 19, 2024
61e11df
feat: remove workflow-name org scopping
migmartri Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/cli/cmd/attestation_init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ func newAttestationInitCmd() *cobra.Command {
cobra.CheckErr(cmd.Flags().MarkDeprecated("name", "please use --workflow instead"))
cmd.Flags().StringVar(&workflowName, "workflow-name", "", "name of the workflow to run the attestation")
cobra.CheckErr(cmd.Flags().MarkDeprecated("workflow-name", "please use --workflow instead"))

cmd.Flags().StringVar(&projectName, "project", "", "name of the project of this workflow")
cobra.CheckErr(cmd.MarkFlagRequired("project"))

return cmd
}
486 changes: 243 additions & 243 deletions app/controlplane/api/controlplane/v1/workflow_run.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 3 additions & 5 deletions app/controlplane/api/controlplane/v1/workflow_run.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ message AttestationServiceGetPolicyGroupResponse {
message AttestationServiceGetContractRequest {
int32 contract_revision = 1;
string workflow_name = 2 [(buf.validate.field).string.min_len = 1];
// optional for now for compatibility with the CLI
string project_name = 3;
string project_name = 3 [(buf.validate.field).string.min_len = 1];
}

message AttestationServiceGetContractResponse {
Expand All @@ -104,9 +103,8 @@ message AttestationServiceInitRequest {
int32 contract_revision = 1;
string job_url = 2;
workflowcontract.v1.CraftingSchema.Runner.RunnerType runner = 3;
string workflow_name = 4;
// optional for now for compatibility with the CLI
string project_name = 5;
string workflow_name = 4 [(buf.validate.field).string.min_len = 1];
string project_name = 5 [(buf.validate.field).string.min_len = 1];
}

message AttestationServiceInitResponse {
Expand Down
2 changes: 0 additions & 2 deletions app/controlplane/api/gen/frontend/controlplane/v1/workflow_run.ts
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions ...e/api/gen/jsonschema/controlplane.v1.AttestationServiceGetContractRequest.jsonschema.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions ...plane/api/gen/jsonschema/controlplane.v1.AttestationServiceGetContractRequest.schema.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions ...rolplane/api/gen/jsonschema/controlplane.v1.AttestationServiceInitRequest.jsonschema.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions ...controlplane/api/gen/jsonschema/controlplane.v1.AttestationServiceInitRequest.schema.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions app/controlplane/internal/dispatcher/dispatcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,11 @@ func (s *dispatcherTestSuite) SetupTest() {
assert.NoError(s.T(), err)

// Workflow
s.workflow, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID})
s.workflow, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID, Project: "test-project"})
assert.NoError(s.T(), err)

// Workflow
s.emptyWorkflow, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "empty-workflow", OrgID: s.org.ID})
s.emptyWorkflow, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "empty-workflow", OrgID: s.org.ID, Project: "test-project"})
assert.NoError(s.T(), err)

customImplementation := mockedSDK.NewFanOutPlugin(s.T())
Expand Down
4 changes: 2 additions & 2 deletions app/controlplane/pkg/biz/casmapping_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,10 @@ func (s *casMappingIntegrationSuite) SetupTest() {

// Create workflowRun in the database
// Workflow
workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org1.ID})
workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org1.ID, Project: "test-project"})
assert.NoError(err)

publicWorkflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow-public", OrgID: s.org1.ID, Public: true})
publicWorkflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow-public", OrgID: s.org1.ID, Public: true, Project: "test-project"})
assert.NoError(err)

// Find contract revision
Expand Down
2 changes: 1 addition & 1 deletion app/controlplane/pkg/biz/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func (s *testSuite) SetupTest() {
assert.NoError(err)

// Workflow
s.workflow, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID})
s.workflow, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID, Project: "test-project"})
assert.NoError(err)

// Integration configuration
Expand Down
2 changes: 1 addition & 1 deletion app/controlplane/pkg/biz/organization_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (s *OrgIntegrationTestSuite) SetupTest() {
assert.NoError(err)

// Workflow + contract
_, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID})
_, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID, Project: "test-project"})
assert.NoError(err)

// check integration, OCI repository and workflow and contracts are present in the db
Expand Down
4 changes: 2 additions & 2 deletions app/controlplane/pkg/biz/referrer_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,9 @@ func (s *referrerIntegrationTestSuite) SetupTest() {
s.org2UUID, err = uuid.Parse(s.org2.ID)
require.NoError(s.T(), err)

s.workflow1, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "wf", Team: "team", OrgID: s.org1.ID})
s.workflow1, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "wf", Team: "team", OrgID: s.org1.ID, Project: "test"})
require.NoError(s.T(), err)
s.workflow2, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "wf-from-org-2", Team: "team", OrgID: s.org2.ID})
s.workflow2, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "wf-from-org-2", Team: "team", OrgID: s.org2.ID, Project: "test"})
require.NoError(s.T(), err)

// user 1 has access to org 1 and 2
Expand Down
5 changes: 3 additions & 2 deletions app/controlplane/pkg/biz/robotaccount_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ func (s *robotAccountTestSuite) SetupTest() {
s.NoError(err)

wf, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{
Name: "myworkflow",
OrgID: s.org.ID,
Name: "myworkflow",
Project: "myproject",
OrgID: s.org.ID,
})
s.NoError(err)
s.ra, err = s.RobotAccount.Create(ctx, "myRobotAccount", s.org.ID, wf.ID.String())
Expand Down
8 changes: 5 additions & 3 deletions app/controlplane/pkg/biz/workflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ func NewWorkflowUsecase(wfr WorkflowRepo, schemaUC *WorkflowContractUseCase, log
func (uc *WorkflowUseCase) Create(ctx context.Context, opts *WorkflowCreateOpts) (*Workflow, error) {
if opts.Name == "" {
return nil, errors.New("workflow name is required")
} else if opts.Project == "" {
return nil, errors.New("project name is required")
}

// validate format of the name and the project
Expand Down Expand Up @@ -118,8 +120,7 @@ func (uc *WorkflowUseCase) Create(ctx context.Context, opts *WorkflowCreateOpts)
wf, err := uc.wfRepo.Create(ctx, opts)
if err != nil {
if IsErrAlreadyExists(err) {
// mask the error but return a custom error
return nil, NewErrAlreadyExistsStr("name already taken")
return nil, NewErrAlreadyExistsStr(fmt.Sprintf("workflow %q in project %q already exists", opts.Name, opts.Project))
}

return nil, fmt.Errorf("failed to create workflow: %w", err)
Expand Down Expand Up @@ -238,9 +239,10 @@ func (uc *WorkflowUseCase) FindByNameInOrg(ctx context.Context, orgID, projectNa

if workflowName == "" {
return nil, NewErrValidationStr("empty workflow name")
} else if projectName == "" {
return nil, NewErrValidationStr("empty project name")
}

// TODO: enforce projectName
wf, err := uc.wfRepo.GetOrgScopedByProjectAndName(ctx, orgUUID, projectName, workflowName)
if err != nil {
return nil, fmt.Errorf("failed to get workflow: %w", err)
Expand Down
37 changes: 24 additions & 13 deletions app/controlplane/pkg/biz/workflow_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,48 +89,59 @@ func (s *workflowIntegrationTestSuite) TestCreateDuplicatedName() {
ctx := context.Background()

const workflowName = "name"
existingWorkflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: workflowName})
existingWorkflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: workflowName, Project: "project"})
require.NoError(s.T(), err)

s.Run("can't create a workflow with the same name", func() {
_, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: workflowName})
s.ErrorContains(err, "name already taken")
s.Run("can't create a workflow with the same name in the same project", func() {
_, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: workflowName, Project: "project"})
s.ErrorContains(err, "already exists")
})

s.Run("but can do it in another project", func() {
_, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: workflowName, Project: "another-project"})
s.NoError(err)
})

s.Run("but if we delete it we can", func() {
err = s.Workflow.Delete(ctx, s.org.ID, existingWorkflow.ID.String())
require.NoError(s.T(), err)

_, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: workflowName})
_, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: workflowName, Project: "project"})
require.NoError(s.T(), err)
})
}

func (s *workflowIntegrationTestSuite) TestCreate() {
ctx := context.Background()
const project = "project"
testCases := []struct {
name string
opts *biz.WorkflowCreateOpts
wantErrMsg string
}{
{
name: "org missing",
opts: &biz.WorkflowCreateOpts{Name: "name"},
opts: &biz.WorkflowCreateOpts{Name: "name", Project: project},
wantErrMsg: "required",
},
{
name: "name missing",
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID},
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Project: project},
wantErrMsg: "required",
},
{
name: "project missing",
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name"},
wantErrMsg: "required",
},
{
name: "invalid name",
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "this/not/valid"},
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "this/not/valid", Project: project},
wantErrMsg: "RFC 1123",
},
{
name: "another invalid name",
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "this-not Valid"},
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "this-not Valid", Project: project},
wantErrMsg: "RFC 1123",
},
{
Expand All @@ -140,11 +151,11 @@ func (s *workflowIntegrationTestSuite) TestCreate() {
},
{
name: "non-existing contract will create it",
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name", ContractName: uuid.Generate().String()},
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name", ContractName: uuid.Generate().String(), Project: project},
},
{
name: "can create it with just the name and the org",
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name2"},
name: "can create it with just the name, the project and the org",
opts: &biz.WorkflowCreateOpts{OrgID: s.org.ID, Name: "name2", Project: project},
},
{
name: "with all items",
Expand Down Expand Up @@ -184,7 +195,7 @@ func (s *workflowIntegrationTestSuite) TestUpdate() {

org2, err := s.Organization.CreateWithRandomName(context.Background())
require.NoError(s.T(), err)
workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: name, OrgID: s.org.ID})
workflow, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: name, OrgID: s.org.ID, Project: project})
require.NoError(s.T(), err)

// Create two contracts in two different orgs
Expand Down
6 changes: 3 additions & 3 deletions app/controlplane/pkg/biz/workflowrun_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,12 @@ func setupWorkflowRunTestData(t *testing.T, suite *testhelpers.TestingUseCases,
assert.NoError(err)

// Workflow
s.workflowOrg1, err = suite.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID})
s.workflowOrg1, err = suite.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org.ID, Project: "test-project"})
assert.NoError(err)
s.workflowOrg2, err = suite.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org2.ID})
s.workflowOrg2, err = suite.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-workflow", OrgID: s.org2.ID, Project: "test-project"})
assert.NoError(err)
// Public workflow
s.workflowPublicOrg2, err = suite.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-public-workflow", OrgID: s.org2.ID, Public: true})
s.workflowPublicOrg2, err = suite.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "test-public-workflow", OrgID: s.org2.ID, Public: true, Project: "test-project"})
assert.NoError(err)

// Find contract revision
Expand Down
2 changes: 2 additions & 0 deletions app/controlplane/pkg/data/ent/migrate/migrations/20241019142805.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- Drop index "workflow_name_organization_id" from table: "workflows"
DROP INDEX "workflow_name_organization_id";
2 changes: 2 additions & 0 deletions app/controlplane/pkg/data/ent/migrate/migrations/20241019145455.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- Modify "workflows" table
ALTER TABLE "workflows" ALTER COLUMN "project" SET NOT NULL;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need a migration of previous workflows right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because they happened to be set and forced in another layer.

4 changes: 3 additions & 1 deletion app/controlplane/pkg/data/ent/migrate/migrations/atlas.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
h1:tuCJBEVVWbRmd0tPzFCI0VSQ2lKNfjfSCqtsW5SijOA=
h1:39lFYMXkGW9lu7cOjzwNO9fmePg5Y6aTG1el6AD9Mfs=
20230706165452_init-schema.sql h1:VvqbNFEQnCvUVyj2iDYVQQxDM0+sSXqocpt/5H64k8M=
20230710111950-cas-backend.sql h1:A8iBuSzZIEbdsv9ipBtscZQuaBp3V5/VMw7eZH6GX+g=
20230712094107-cas-backends-workflow-runs.sql h1:a5rzxpVGyd56nLRSsKrmCFc9sebg65RWzLghKHh5xvI=
Expand Down Expand Up @@ -45,3 +45,5 @@ h1:tuCJBEVVWbRmd0tPzFCI0VSQ2lKNfjfSCqtsW5SijOA=
20240920193509.sql h1:FREEMo5oD//cf+6iq72YgbVJgXN/fuy7DB/jvx6NgKM=
20241011075107.sql h1:8UrMNKA0QAKkhfdetnNCZfI7DBOTKLLxsqqH+c8VhdU=
20241016094434.sql h1:mSu1nkNggqnH5wZzjYhY4wo2m8NA+mEL5QOtPXc/Lqg=
20241019142805.sql h1:j4rd1ktedNnB5WDjL7lWmBhpFsNhT90/G4hzxkvu2D4=
20241019145455.sql h1:8Moysfwh/gJ/4Me5ujndurNFIbqFtKbNBTSAZq+S9EM=
10 changes: 1 addition & 9 deletions app/controlplane/pkg/data/ent/migrate/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ var (
WorkflowsColumns = []*schema.Column{
{Name: "id", Type: field.TypeUUID, Unique: true},
{Name: "name", Type: field.TypeString},
{Name: "project", Type: field.TypeString, Nullable: true},
{Name: "project", Type: field.TypeString},
{Name: "team", Type: field.TypeString, Nullable: true},
{Name: "runs_count", Type: field.TypeInt, Default: 0},
{Name: "created_at", Type: field.TypeTime, Default: "CURRENT_TIMESTAMP"},
Expand Down Expand Up @@ -370,14 +370,6 @@ var (
Where: "deleted_at IS NULL",
},
},
{
Name: "workflow_name_organization_id",
Unique: true,
Columns: []*schema.Column{WorkflowsColumns[1], WorkflowsColumns[9]},
Annotation: &entsql.IndexAnnotation{
Where: "deleted_at IS NULL",
},
},
{
Name: "workflow_organization_id_id",
Unique: true,
Expand Down
Loading
Loading