Skip to content
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.

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

feat: use provenance-repository input for slsa-verifier #300

Merged
merged 1 commit into from
Jan 23, 2024
Merged

feat: use provenance-repository input for slsa-verifier #300

merged 1 commit into from
Jan 23, 2024

Conversation

saisatishkarra
Copy link
Contributor

@saisatishkarra saisatishkarra commented Jan 23, 2024

refers: slsa-framework/slsa-verifier#724 and slsa-framework/slsa-github-generator#3099.

Deprecate exporting COSIGN_REPOSITORY for slsa-verifier during container provenance verification when the --provenance-repository is specified.

Copy link
Member

@ianlewis ianlewis left a comment

Choose a reason for hiding this comment

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

LGTM

@ianlewis
Copy link
Member

@laurentsimon We only test the container builder verification at HEAD but I think it's a GA feature in slsa-verifier? Should we be testing versions other than HEAD if so we'd need to be able to skip earlier versions when testing --provenance-repostitory.

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>
@saisatishkarra
Copy link
Contributor Author

saisatishkarra commented Jan 23, 2024

@laurentsimon We only test the container builder verification at HEAD but I think it's a GA feature in slsa-verifier? Should we be testing versions other than HEAD if so we'd need to be able to skip earlier versions when testing --provenance-repostitory.

@ianlewis will the above scenario not be handled by: https://github.com/slsa-framework/example-package/compare/main...saisatishkarra:refactor/provenance-repository?expand=1#diff-ec5943f44574c4151b994e3164f5cfc4fc5a7cbdce50cceea731f7ca126c6bb9R356-R358 ?

@ianlewis
Copy link
Member

@ianlewis will this scenario not be handled by: https://github.com/slsa-framework/example-package/compare/main...saisatishkarra:refactor/provenance-repository?expand=1#diff-ec5943f44574c4151b994e3164f5cfc4fc5a7cbdce50cceea731f7ca126c6bb9R356-R358 ?

We have code that tests using various versions of slsa-verifier in order to make sure we are backwards compatible. However, right now we only test the container generator at HEAD.

I think we probably need to test at previous versions because it is a GA feature but I'm not sure. And if so, I'm not sure what version we should test from. This we need to check with @laurentsimon and can be an issue for a later PR.

Because we only use slsa-verifier at HEAD this PR should work fine as is so no changes are necessary for now.

@laurentsimon
Copy link
Collaborator

@ianlewis will this scenario not be handled by: https://github.com/slsa-framework/example-package/compare/main...saisatishkarra:refactor/provenance-repository?expand=1#diff-ec5943f44574c4151b994e3164f5cfc4fc5a7cbdce50cceea731f7ca126c6bb9R356-R358 ?

We have code that tests using various versions of slsa-verifier in order to make sure we are backwards compatible. However, right now we only test the container generator at HEAD.

I think we probably need to test at previous versions because it is a GA feature but I'm not sure. And if so, I'm not sure what version we should test from. This we need to check with @laurentsimon and can be an issue for a later PR.

Because we only use slsa-verifier at HEAD this PR should work fine as is so no changes are necessary for now.

I think we should be able to update this line in a follow-up PR https://github.com/slsa-framework/example-package/blob/main/.github/workflows/scripts/e2e.container.default.verify.sh#L50

Created #300 for tracking

@laurentsimon laurentsimon merged commit bf5d5fc into slsa-framework:main Jan 23, 2024
2 of 4 checks passed
@laurentsimon
Copy link
Collaborator

Thank you @saisatishkarra !
Let's keep an eye on the results of the test. I'll start one manually.
re: release. I'll be OOO until 11 Feb, so I may be unresponsive until then :)

@saisatishkarra
Copy link
Contributor Author

saisatishkarra commented Jan 23, 2024

@laurentsimon Does changing the workflow file name also require the 3 fields concurrency / image name / provenance docker repo name to be updated in the workflow?

Error. What does that mean/expected?

@laurentsimon
Copy link
Collaborator

good point. You're right, we're using the workflow path to validate the content of the provenance. I've made the changes and re-launched a workflow run

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.

None yet

3 participants