Skip to content

feat(crafting-schema): add schema v2 support to cli#2467

Merged
Piskoo merged 9 commits into
chainloop-dev:mainfrom
Piskoo:feat-use-contract-v2-for-contract-operations
Oct 28, 2025
Merged

feat(crafting-schema): add schema v2 support to cli#2467
Piskoo merged 9 commits into
chainloop-dev:mainfrom
Piskoo:feat-use-contract-v2-for-contract-operations

Conversation

@Piskoo

@Piskoo Piskoo commented Oct 14, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This PR adds support for contract schema v2 in CLI workflow contract create, update, and apply operations.

Note: The default contract body for create will be updated once v2 schema support is available everywhere.

Changes

  • CLI can now create and update contracts using v2 schema format:
  apiVersion: ...
  kind: ...
  metadata:
    name: ...
  spec:
    ...
  • Contract name can now be extracted from the v2 field metadata.name, removing the need for the --name flag.
  • Validation ensures only one name source is provided (either --name flag or metadata.name in the contract).

@Piskoo Piskoo marked this pull request as ready for review October 14, 2025 12:37
@Piskoo Piskoo mentioned this pull request Oct 17, 2025
Merged
migmartri
migmartri reviewed Oct 17, 2025
View reviewed changes
Comment thread app/cli/cmd/workflow_contract_create.go

cmd.Flags().StringVar(&name, "name", "", "contract name")
err := cmd.MarkFlagRequired("name")
cobra.CheckErr(err)

@migmartri migmartri Oct 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

missing validation

Comment thread app/controlplane/api/controlplane/v1/workflow_contract.proto Outdated
message: "must contain only lowercase letters, numbers, and hyphens."
expression: "this.name == '' || this.name.matches('^[a-z0-9]([-a-z0-9]*[a-z0-9])?$')"
};
option (buf.validate.message).cel = {

@migmartri migmartri Oct 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this validation doesnt mean much in legacy format

Comment thread app/controlplane/pkg/data/workflowcontract.go
return nil, fmt.Errorf("failed to unmarshal raw body: %w", err)
}
contract.Schema = schema
contract.Schema = parsedContract.Schema

@migmartri migmartri Oct 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This might be broken, because Schema is necessary for compatibility reasons.

@migmartri migmartri Oct 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Old CLIs must keep working with new contracts

Comment thread app/controlplane/pkg/data/workflowcontract.go Outdated
}
contract.Schema = schema
contract.Schema = parsedContract.Schema
contract.Schemav2 = parsedContract.Schemav2

@migmartri migmartri Oct 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This second option we don't want it.

Comment thread app/controlplane/internal/service/workflowcontract.go Outdated
}

// extractContractName extracts the contract name from v2 metadata or returns the fallback name
func extractContractName(rawContract []byte, name string) (string, error) {

@migmartri migmartri Oct 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

validate that exactly one item is present

Comment thread app/controlplane/pkg/biz/workflowcontract.go
if _, err := uc.findAndValidatePolicy(att, token); err != nil {
return NewErrValidation(err)
switch {
case c.isV1Schema():

@migmartri migmartri Oct 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

mark as deprecated

@Piskoo Piskoo marked this pull request as draft October 20, 2025 07:08
@Piskoo Piskoo mentioned this pull request Oct 20, 2025
Merged
@Piskoo Piskoo force-pushed the feat-use-contract-v2-for-contract-operations branch from 3918cc5 to 39deb09 Compare October 21, 2025 11:53
@Piskoo Piskoo marked this pull request as ready for review October 22, 2025 08:16
jiparis
jiparis reviewed Oct 22, 2025
View reviewed changes
Comment thread app/controlplane/pkg/biz/workflowcontract.go
jiparis
jiparis reviewed Oct 22, 2025
View reviewed changes
Comment thread app/controlplane/pkg/data/workflowcontract.go
} else if len(w.Body) == 0 {
schema := &schemav1.CraftingSchema{}
err := unmarshal.FromRaw(w.RawBody, w.RawBodyFormat, schema, false)
parsedContract, err := biz.UnmarshalAndValidateRawContract(w.RawBody, w.RawBodyFormat)

@jiparis jiparis Oct 22, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks awkward. Data layer shouldn't call biz layer logic.

@Piskoo Piskoo Oct 23, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/chainloop-dev/chainloop/blob/main/app/controlplane/pkg/data/workflowcontract.go#L354
That is already the case currently

@jiparis jiparis Oct 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see, let's address it in another PR then.

jiparis
jiparis approved these changes Oct 28, 2025
View reviewed changes

@jiparis jiparis left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I've been testing this locally with different server/client version combinations without any issue.

Piskoo added 9 commits October 28, 2025 12:06
@Piskoo
use contract v2 for contract operations
bd2990b 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
fix validation and update for v2 schema
232174b 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
lint
0a9563f 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
lint
8263dce 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
test fix
56207ec 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
fix validation and compatibility issues
408c9ca 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
lint
3976220 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
add name validation in service
5609fe3 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo
update error messages
a2824ae 
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo force-pushed the feat-use-contract-v2-for-contract-operations branch from 0dcd82f to a2824ae Compare October 28, 2025 11:06
@Piskoo Piskoo merged commit f355374 into chainloop-dev:main Oct 28, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@migmartri migmartri migmartri left review comments
@jiparis jiparis jiparis approved these changes
@javirln javirln Awaiting requested review from javirln

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Piskoo @migmartri @jiparis