docs: Create proposal for annotation-based filtering
Description
What this PR does / why we need it:
This PR adds a proposal for filtering artifacts based on their annotations before forwarding them to verification by one of the configured verifiers.]
The PR refers to issue #1772
Type of change
Added document to proposal directory.
Hi @asafalgawi , thank you for the proposal. We have discussed this during the community meeting. We agree with the "latest" vulnerability report user scenario, however we need to understand the feasibility and scope of change. @binbin-li brought up good point today, the individual verifier might not have filtering abilities. Major refactoring might be required in executor code path. We still require more time to understand the notary scenarios a bit more, thankyou for your patience.
Hi @susanshi, thanks for the reviewing the proposal and for the update :)
Hi @asafalgawi , thank you for the proposal. We have discussed this during the community meeting. We agree with the "latest" vulnerability report user scenario, however we need to understand the feasibility and scope of change. @binbin-li brought up good point today, the individual verifier might not have filtering abilities. Major refactoring might be required in executor code path. We still require more time to understand the notary scenarios a bit more, thankyou for your patience.
Hi @asafalgawi, reviewing our current executor path below, we want to explore a bit more on the proposed implementation. How will the executor be refactored for filtering? Note we will not be able to change the verifier interfaces to maintain backward compatibility. If the current PR review meeting and community meeting time doesn't work, would you be open to meet and discuss in the once off meeting?
for _, referrerStore := range executor.ReferrerStores {
for _, reference := range referrersResult.Referrers {
for _, verifier := range executor.Verifiers {
}
}
}
```
@susanshi I've updated the proposal based on our conversation, also added two implementation proposal. a "simple" one which theoretically impacts Ratify's scalability, the second, a bit more complex, which affects Ratify executor design
@susanshi I've updated the proposal based on our conversation, also added two implementation proposal. a "simple" one which theoretically impacts Ratify's scalability, the second, a bit more complex, which affects Ratify executor design
thanks for the updates and comparison between the two implementation methods. My understanding option1 is more straight forward, and given a 1k referrer list, there are no perf and memory concerns. Option 2 memory benefits is unclear. Major refactoring in the executor is risky, unless the refactoring improves code readability /allows for easier future maintaince/ aligns with the direction of ratify v2 executor change. @binbin-li , do you see for executor refactoring benefits for ratify v2?
@asafalgawi , just a note, we are not a cncf sandbox project and there is a new requirement on DCO. You might need to squash the current commits and sign a new commit with the -s flag , git commit -s -S -m <commit_message>. see details at https://github.com/ratify-project/ratify/blob/dev/CONTRIBUTING.md#commit :)
@susanshi I've updated the proposal based on our conversation, also added two implementation proposal. a "simple" one which theoretically impacts Ratify's scalability, the second, a bit more complex, which affects Ratify executor design
thanks for the updates and comparison between the two implementation methods. My understanding option1 is more straight forward, and given a 1k referrer list, there are no perf and memory concerns. Option 2 memory benefits is unclear. Major refactoring in the executor is risky, unless the refactoring improves code readability /allows for easier future maintaince/ aligns with the direction of ratify v2 executor change. @binbin-li , do you see for executor refactoring benefits for ratify v2?
For this feature, I'd like to keep executor untouched. For v2 design, I plan to make executor just handle the core coordination among different components(policy, verifier, store). The last-n feature and possibly more filtering options sound more like a verifier feature, therefore executor is not a good place to handle it.
@asafalgawi I agree with Susan and I think Option 1 seems like a better approach for now. If larger than 1k referrers becomes a user ask, we can revisit this.
Great I'll start with a POC for option 1
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Akash Singhal @.> Sent: Wednesday, October 16, 2024 8:36:54 PM To: ratify-project/ratify @.> Cc: Asaf Algawi @.>; Mention @.> Subject: Re: [ratify-project/ratify] docs: Create proposal for verifying 'last-n' artifacts only. (PR #1797)
@asafalgawihttps://github.com/asafalgawi I agree with Susan and I think Option 1 seems like a better approach for now. If larger than 1k referrers becomes a user ask, we can revisit this.
— Reply to this email directly, view it on GitHubhttps://github.com/ratify-project/ratify/pull/1797#issuecomment-2417484847, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BCRNDOFRKWSDBXYYW4SNGNTZ32P3NAVCNFSM6AAAAABOC5QPAGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJXGQ4DIOBUG4. You are receiving this because you were mentioned.Message ID: @.***>