Skip to content

feat(referrer): support same digest for two different kinds #431

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

Jump to bottom
Merged
merged 4 commits into from
Nov 13, 2023
Merged

feat(referrer): support same digest for two different kinds #431

merged 4 commits into from
Nov 13, 2023

Conversation

migmartri
Copy link
Member

@migmartri migmartri commented Nov 10, 2023 •
edited
Loading

This PR

  • Improves the ingestion code to allow storing two referrers with the same digest but different kinds. it includes some refactoring too.
  • Updates the retrieval code to return a well-known error that:
    • makes the API return a validation error instead of a server error
    • shows actionable information to the user of what's available.

For example, now, on retrieval we get.

$ chainloop discover -d sha256:ebe2e6ffab93f4b2e2743d3557bd094427ea059adc78cfe2cd330a72f61731e8
ERR validation error: please provide the referrer kind: failed to get referrer: digest sha256:ebe2e6ffab93f4b2e2743d3557bd094427ea059adc78cfe2cd330a72f61731e8
 present in two kinds ["SBOM_CYCLONEDX_JSON" "ARTIFACT"]

In a future patch, the user will be able to filter out by kind.

@migmartri
improve storage
873750c 
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri
improve storage
34aa384 
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri
improve api
8022d6f 
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri
improve api
b93c5b8 
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>
@migmartri migmartri changed the title enhancement(referrer): handle same artifact digest feat(referrer): handle same artifact digest Nov 10, 2023
@migmartri migmartri changed the title feat(referrer): handle same artifact digest feat(referrer): support same digest for two different kinds Nov 10, 2023
danlishka
danlishka approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@danlishka danlishka left a comment

Choose a reason for hiding this comment

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

LGTM

app/controlplane/internal/biz/referrer.go
Downloadable bool
CreatedAt *time.Time
*Referrer
ID uuid.UUID
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need uuid?

Copy link
Member Author

Choose a reason for hiding this comment

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

In favor of another format? or having a primary ID at all?

@migmartri migmartri merged commit d5d0728 into chainloop-dev:main Nov 13, 2023
@migmartri migmartri deleted the same-referrer branch November 13, 2023 14:37
@migmartri migmartri mentioned this pull request Nov 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danlishka danlishka danlishka approved these changes

@buccarel buccarel Awaiting requested review from buccarel

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@migmartri @danlishka