feat(contracts): updates by name (#674)

Signed-off-by: Miguel Martinez Trivino <[email protected]>

Author: migmartri

app/cli/cmd/workflow_contract_update.go

@@ -32,6 +32,7 @@ func newWorkflowContractUpdateCmd() *cobra.Command {
 			if contractPath == "" && name == "" && description == "" {
 				return errors.New("no updates provided")
 			}
+
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -51,11 +52,13 @@ func newWorkflowContractUpdateCmd() *cobra.Command {
 	}
 
 	cmd.Flags().StringVar(&contractID, "id", "", "contract ID")
-	err := cmd.MarkFlagRequired("id")
+	err := cmd.Flags().MarkDeprecated("id", "use --name instead")
 	cobra.CheckErr(err)
 
 	cmd.Flags().StringVarP(&contractPath, "contract", "f", "", "path or URL to the contract schema")
 	cmd.Flags().StringVar(&name, "name", "", "name of the contract")
+
+	cobra.CheckErr(err)
 	cmd.Flags().StringVar(&description, "description", "", "description of the contract")
 
 	return cmd

app/controlplane/api/controlplane/v1/workflow_contract.pb.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,8 +17,8 @@ syntax = "proto3";
 
 package controlplane.v1;
 
-import "controlplane/v1/response_messages.proto";
 import "buf/validate/validate.proto";
+import "controlplane/v1/response_messages.proto";
 import "workflowcontract/v1/crafting_schema.proto";
 
 option go_package = "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1;v1";
@@ -52,8 +52,36 @@ message WorkflowContractServiceCreateResponse {
 }
 
 message WorkflowContractServiceUpdateRequest {
-  string id = 1 [(buf.validate.field).string.uuid = true];
-  string name = 2;
+  // TODO: remove once the id is fully removed
+  // Checks that either name or id is provided
+  // NOTE: we did not use oneof in this case because we don't want to break compatibility with other clients
+  // and we'll drop the id field in the future
+  option (buf.validate.message).cel = {
+    id: "either-name-or-id-required",
+    message: "either name or id are required",
+    expression: "has(this.id) || has(this.name)"
+  };
+
+  // Maintained for backward compatibility until we mover all identifiers to name
+  // Moving forward, the contract name will be used as the identifier
+  string id = 1 [
+    deprecated = true,
+    (buf.validate.field) = {
+      string: {uuid: true},
+      ignore_empty: true,
+    }
+  ];
+
+  // Name is the identifier of the contract
+  string name = 2 [(buf.validate.field) = {
+    ignore_empty: true,
+    // NOTE: validations can not be shared yet https://github.com/bufbuild/protovalidate/issues/51
+    cel: {
+      message: "name must be a valid DNS-1123 subdomain",
+      expression: "this.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$')",
+      id: "name.dns-1123",
+    },
+  }];
 
   oneof contract {
     workflowcontract.v1.CraftingSchema v1 = 3;

app/controlplane/api/controlplane/v1/workflow_contract.proto

@@ -44,8 +44,11 @@ func (s *workflowIntegrationTestSuite) TestContractLatestAvailable() {
 	})
 
 	s.Run("it will increment if the contract is updated", func() {
-		_, err := s.WorkflowContract.Update(ctx, s.org.ID, workflow.ContractID.String(),
-			&biz.WorkflowContractUpdateOpts{Name: "new-name", Schema: &v1.CraftingSchema{
+		contract, err := s.WorkflowContract.FindByIDInOrg(ctx, s.org.ID, workflow.ContractID.String())
+		require.NoError(s.T(), err)
+
+		_, err = s.WorkflowContract.Update(ctx, s.org.ID, contract.Name,
+			&biz.WorkflowContractUpdateOpts{Schema: &v1.CraftingSchema{
 				Runner: &v1.CraftingSchema_Runner{Type: v1.CraftingSchema_Runner_CIRCLECI_BUILD},
 			}})
 		s.NoError(err)

app/controlplane/api/controlplane/v1/workflow_contract_grpc.pb.go

@@ -56,7 +56,7 @@ type WorkflowContractRepo interface {
 	FindByIDInOrg(ctx context.Context, orgID, ID uuid.UUID) (*WorkflowContract, error)
 	Describe(ctx context.Context, orgID, contractID uuid.UUID, revision int) (*WorkflowContractWithVersion, error)
 	FindVersionByID(ctx context.Context, versionID uuid.UUID) (*WorkflowContractVersion, error)
-	Update(ctx context.Context, opts *ContractUpdateOpts) (*WorkflowContractWithVersion, error)
+	Update(ctx context.Context, orgID uuid.UUID, name string, opts *ContractUpdateOpts) (*WorkflowContractWithVersion, error)
 	SoftDelete(ctx context.Context, contractID uuid.UUID) error
 }
 
@@ -68,10 +68,8 @@ type ContractCreateOpts struct {
 }
 
 type ContractUpdateOpts struct {
-	Name              string
-	OrgID, ContractID uuid.UUID
-	Description       *string
-	ContractBody      []byte
+	Description  *string
+	ContractBody []byte
 }
 
 type WorkflowContractUseCase struct {
@@ -233,12 +231,11 @@ func (uc *WorkflowContractUseCase) FindVersionByID(ctx context.Context, versionI
 }
 
 type WorkflowContractUpdateOpts struct {
-	Name        string
 	Schema      *schemav1.CraftingSchema
 	Description *string
 }
 
-func (uc *WorkflowContractUseCase) Update(ctx context.Context, orgID, contractID string, opts *WorkflowContractUpdateOpts) (*WorkflowContractWithVersion, error) {
+func (uc *WorkflowContractUseCase) Update(ctx context.Context, orgID, name string, opts *WorkflowContractUpdateOpts) (*WorkflowContractWithVersion, error) {
 	if opts == nil {
 		return nil, NewErrValidationStr("no updates provided")
 	}
@@ -248,30 +245,14 @@ func (uc *WorkflowContractUseCase) Update(ctx context.Context, orgID, contractID
 		return nil, err
 	}
 
-	contractUUID, err := uuid.Parse(contractID)
-	if err != nil {
-		return nil, err
-	}
-
-	if opts.Name != "" {
-		if err := ValidateIsDNS1123(opts.Name); err != nil {
-			return nil, NewErrValidation(err)
-		}
-	}
-
 	rawSchema, err := proto.Marshal(opts.Schema)
 	if err != nil {
 		return nil, err
 	}
 
-	args := &ContractUpdateOpts{OrgID: orgUUID, ContractID: contractUUID, ContractBody: rawSchema, Name: opts.Name, Description: opts.Description}
-
-	c, err := uc.repo.Update(ctx, args)
+	args := &ContractUpdateOpts{ContractBody: rawSchema, Description: opts.Description}
+	c, err := uc.repo.Update(ctx, orgUUID, name, args)
 	if err != nil {
-		if errors.Is(err, ErrAlreadyExists) {
-			return nil, NewErrValidationStr("name already taken")
-		}
-
 		return nil, fmt.Errorf("failed to update contract: %w", err)
 	} else if c == nil {
 		return nil, NewErrNotFound("contract")

app/controlplane/api/gen/frontend/controlplane/v1/workflow_contract.ts

@@ -31,82 +31,58 @@ func (s *workflowContractIntegrationTestSuite) TestUpdate() {
 	ctx := context.Background()
 
 	testCases := []struct {
-		name            string
-		OrgID, ID       string
-		input           *biz.WorkflowContractUpdateOpts
-		inputSchema     *schemav1.CraftingSchema
-		wantErrMsg      string
-		wantRevision    int
-		wantName        string
-		wantDescription string
+		name                string
+		orgID, contractName string
+		input               *biz.WorkflowContractUpdateOpts
+		inputSchema         *schemav1.CraftingSchema
+		wantErrMsg          string
+		wantRevision        int
+		wantDescription     string
 	}{
 		{
 			name:       "non-updates",
 			wantErrMsg: "no updates",
 		},
 		{
-			name:       "non-existing contract",
-			wantName:   "non-existing",
-			OrgID:      s.org.ID,
-			input:      &biz.WorkflowContractUpdateOpts{},
-			ID:         uuid.NewString(),
-			wantErrMsg: "not found",
-		},
-		{
-			name:       "existing contract invalid name",
-			input:      &biz.WorkflowContractUpdateOpts{Name: "invalid name"},
-			OrgID:      s.org.ID,
-			ID:         s.contractOrg1.ID.String(),
-			wantErrMsg: "RFC 1123",
-		},
-		{
-			name:         "existing contract valid name, does not bump revision",
-			input:        &biz.WorkflowContractUpdateOpts{Name: "valid-name"},
-			wantName:     "valid-name",
-			OrgID:        s.org.ID,
-			ID:           s.contractOrg1.ID.String(),
-			wantRevision: 1,
+			name:         "non-existing contract",
+			orgID:        s.org.ID,
+			input:        &biz.WorkflowContractUpdateOpts{},
+			contractName: uuid.NewString(),
+			wantErrMsg:   "not found",
 		},
 		{
 			name:         "updating schema bumps revision",
-			OrgID:        s.org.ID,
-			ID:           s.contractOrg1.ID.String(),
-			wantName:     "valid-name",
+			orgID:        s.org.ID,
+			contractName: s.contractOrg1.Name,
 			input:        &biz.WorkflowContractUpdateOpts{Schema: &schemav1.CraftingSchema{SchemaVersion: "v123"}},
 			wantRevision: 2,
 		},
 		{
 			name:         "updating with same schema DOES NOT bump revision",
-			OrgID:        s.org.ID,
-			ID:           s.contractOrg1.ID.String(),
+			orgID:        s.org.ID,
+			contractName: s.contractOrg1.Name,
 			input:        &biz.WorkflowContractUpdateOpts{Schema: &schemav1.CraftingSchema{SchemaVersion: "v123"}},
-			wantName:     "valid-name",
 			wantRevision: 2,
 		},
 		{
-			name:            "you can update the description",
-			OrgID:           s.org.ID,
-			ID:              s.contractOrg1.ID.String(),
+			name:            "updating description bumps revision",
+			orgID:           s.org.ID,
+			contractName:    s.contractOrg1.Name,
 			input:           &biz.WorkflowContractUpdateOpts{Description: toPtrS("new description")},
-			wantName:        "valid-name",
 			wantDescription: "new description",
 			wantRevision:    2,
 		},
 	}
 
 	for _, tc := range testCases {
 		s.Run(tc.name, func() {
-			contract, err := s.WorkflowContract.Update(ctx, tc.OrgID, tc.ID, tc.input)
+			contract, err := s.WorkflowContract.Update(ctx, tc.orgID, tc.contractName, tc.input)
 			if tc.wantErrMsg != "" {
 				s.ErrorContains(err, tc.wantErrMsg)
 				return
 			}
 			require.NoError(s.T(), err)
 
-			if tc.wantName != "" {
-				s.Equal(tc.wantName, contract.Contract.Name)
-			}
-
 			if tc.wantDescription != "" {
 				s.Equal(tc.wantDescription, contract.Contract.Description)
 			}

app/controlplane/internal/biz/workflow_integration_test.go

@@ -269,8 +269,8 @@ func (s *workflowRunIntegrationTestSuite) TestContractInformation() {
 	})
 
 	s.Run("if the contract gets a new revision but it's not used, it shows spread", func() {
-		updatedContractRevision, err := s.WorkflowContract.Update(ctx, s.org.ID, s.contractVersion.Contract.ID.String(),
-			&biz.WorkflowContractUpdateOpts{Name: "new-name", Schema: &schemav1.CraftingSchema{
+		updatedContractRevision, err := s.WorkflowContract.Update(ctx, s.org.ID, s.contractVersion.Contract.Name,
+			&biz.WorkflowContractUpdateOpts{Schema: &schemav1.CraftingSchema{
 				Runner: &schemav1.CraftingSchema_Runner{Type: schemav1.CraftingSchema_Runner_CIRCLECI_BUILD},
 			}})
 		s.NoError(err)

app/controlplane/internal/biz/workflowcontract.go

@@ -35,7 +35,7 @@ type WorkflowContract struct {
 func (WorkflowContract) Fields() []ent.Field {
 	return []ent.Field{
 		field.UUID("id", uuid.UUID{}).Default(uuid.New).Unique(),
-		field.String("name"),
+		field.String("name").Immutable(),
 		field.Time("created_at").
 			Default(time.Now).
 			Immutable().