Skip to content

Conversation

@migmartri
Copy link
Member

@migmartri migmartri commented Aug 16, 2024

This PR allows users to manage contracts in different formats during the whole lifecycle #1237

In practice, for the user, this means that the system will preserve the contracts in the format (preserving even comments and ordering). It also improves the validation errors, moves them server-side, and improves error messaging.

Multi-format support

For example, creating a contract pointing to a yaml file now will maintain that format

chainloop wf contract create --name test-in-yaml -f  ~/work/chainloop/chainloop/docs/examples/contracts/skynet/contract.yaml

image

the API also returns the detected format, this is a hint to help future clients of this API render the contacts

chainloop wf contract describe --name test-in-yaml -o json | jq .revision.rawBody.format
"FORMAT_YAML"

of course we can update the format at any time, let's move it to json
image

chainloop wf contract describe --name test-in-yaml -o json | jq .revision.rawBody.format
"FORMAT_JSON"

or cue

image

chainloop wf contract describe --name test-in-yaml -o json | jq .revision.rawBody.format
"FORMAT_CUE"

Validations

Validations have been moved to the server side and especially improved in the case of a yaml file by introducing protoyaml-go. Fixes #995

See below a contract that has errors and how it indicates the line and column where the error happens.

ERR failed to validate contract: validation error: :9:11 unknown enum value "NON_EXISTING", expected one of [MATERIAL_TYPE_UNSPECIFIED STRING CONTAINER_IMAGE ARTIFACT SBOM_CYCLONEDX_JSON SBOM_SPDX_JSON JUNIT_XML ...]
   9 |   - type: NON_EXISTING
   9 | ..........^

:1:16 schema_version: value must equal `v1` (string.const)
   1 | schemaVersion: v2
   1 | ...............^

:9:11 materials[0].type: value must not be in list [0] (enum.not_in)
   9 |   - type: NON_EXISTING
   9 | ..........^

exit status 1

On the technical side, this patch.

  • Adds two new database columns to raw_contract and contract_format that store the raw content and format of the contract, respectively. The body column that until today stored the binary representation of the proto will not be populated anymore,
  • Updates API to return the raw value and format alongside the current wired version in proto. The reason for returning both is to keep compatibility with clients that expect the binary proto, i.e attestations.
  • This implementation works with existing contracts. For those, we generate the json representation of the stored contract, which in practice means that nothing changes for the user.
  • Update the CLI to use this raw data instead during read operations with contracts

Closes #1237

@migmartri migmartri requested review from javirln and jiparis August 16, 2024 15:01
@migmartri migmartri marked this pull request as ready for review August 16, 2024 17:56
@migmartri migmartri changed the title feat(contracts): return raw version of contract feat(contracts): keep raw version of contract Aug 17, 2024
@migmartri migmartri requested a review from tknura August 17, 2024 17:52
@migmartri migmartri changed the title feat(contracts): keep raw version of contract feat(contracts): support multi-format Aug 18, 2024
Copy link
Member

@javirln javirln left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jiparis jiparis left a comment

Choose a reason for hiding this comment

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

Looks good. Only two minor comments.

return nil, err
}

contract, err = biz.SchemaToRawContract(schema)
Copy link
Member

Choose a reason for hiding this comment

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

I guess in this case we always return JSON because raw format it's still not known.

// Scenario 1: contracts that have been stored (and not updated) before the introduction of the raw_body field will have an empty raw_body
// so we will generate a json representation of the contract to populate the raw_body field in that case
// that way clients can always expect a raw_body field to be present
if len(contract.Raw) == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

I know it's the same, but I would put here len(w.RawBody), just to keep consistency with the else if below.

Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
Signed-off-by: Miguel <miguel@chainloop.dev>
@migmartri migmartri force-pushed the store-raw-contrafct branch from b7d735c to 2401475 Compare August 19, 2024 13:59
@migmartri migmartri merged commit 8962739 into chainloop-dev:main Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

contract format persistence and server-side validations make contract server-side validations as good as the client side ones in the CLI

3 participants