-
Notifications
You must be signed in to change notification settings - Fork 72
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
Add predicate capturing results of test runs #152
Add predicate capturing results of test runs #152
Conversation
09b7dad
to
34456cd
Compare
f4e7335
to
51bda0e
Compare
FYI one of my colleagues who has done work in this area is going to take a look and provide some comments. Stay tuned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Aditya!
2879a16
to
3ee93c9
Compare
Thanks for the reviews! I've squashed my commits now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks useful. I had a few questions (inline) about this predicate specifically, and a couple of higher-level questions: one about the relationship between in-toto attestations and the broader in-toto spec, and one about inclusion of attestation framework parsing rules in each predicate.
The higher level questions can be addressed separately and should not block this PR.
spec/predicates/test-results.md
Outdated
|
||
The supply chain owner creates a policy that records the expected test | ||
configurations. During verification, the policy checks that the test attestation | ||
used the right configurations. A custom inspection may optionally parse the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first predicate to reference an inspection, should we be avoiding in-toto specification terms in the attestations repository (which aims to be policy engine agnostic)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's okay to mention them in use cases. I can clarify that it's with in-toto layouts / policy engine. We could similarly add a non in-toto verifier and how it'd do something...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated with an "if using with in-toto..." conditional, lmk if this is fine.
489b101
to
13c9473
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @adityasaky !! I really like the simplicity of this predicate. I have several questions/requests for updates to improve clarity.
Signed-off-by: Aditya Sirish <[email protected]>
13c9473
to
ec23143
Compare
Does it make more sense for the schema to be as follows? Since {
"_type": "https://in-toto.io/Statement/v1",
"subject": [{...}],
"predicateType": "https://in-toto.io/attestation/test-result/v0.1",
"predicate": {
"result": "pass|fail",
"url": "<URL>",
"configuration": ["<ResourceDescriptor>", ...],
"passedTests": ["<TEST_NAME>", ...],
"warnedTests": ["<TEST_NAME>", ...],
"failedTests": ["<TEST_NAME>", ...]
}
} |
nit: I'll reorder the fields to move required ones up. Configuration should be first since it's now required. |
Signed-off-by: Aditya Sirish <[email protected]>
7df09ab
to
9f792d0
Compare
Signed-off-by: Aditya Sirish <[email protected]>
9f792d0
to
1fa3f18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the edits, @adityasaky . LGTM.
No description provided.