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

revert: fix: repetitive and incorrect log lines on witness verify (… #335

Merged
merged 2 commits into from
Aug 23, 2024

Conversation

mikhailswift
Copy link
Member

@mikhailswift mikhailswift commented Aug 22, 2024

#317)"

This reverts commit 26ee2dc.

This commit breaks policy evaluations, causing good policies to fail.

Also added is a test to catch these types of errors in the future. This is just a base case test; we will need to expand our testing around policy verification to ensure this is more resilient to breakages.

What this PR does / why we need it

Description

Which issue(s) this PR fixes (optional)

(optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged)*

Fixes #

Acceptance Criteria Met

  • Docs changes if needed
  • Testing changes if needed
  • All workflow checks passing (automatically enforced)
  • All review conversations resolved (automatically enforced)
  • DCO Sign-off

Special notes for your reviewer:

jkjell
jkjell previously approved these changes Aug 22, 2024
@mikhailswift mikhailswift force-pushed the chore/revert-broken-policy-eval branch 3 times, most recently from 956ee36 to 4cfc894 Compare August 23, 2024 00:03
@matglas
Copy link
Contributor

matglas commented Aug 23, 2024

@mikhailswift am I correct that you basically copied the tests from witness into go-witness to do the verify_test? It all looks good to me. I can't approve but if I could I would 😄

…317)"

This reverts commit 26ee2dc.

This commit breaks policy evaluations, causing good policies to fail.

Signed-off-by: Mikhail Swift <[email protected]>
@mikhailswift mikhailswift force-pushed the chore/revert-broken-policy-eval branch from 4cfc894 to b46fc00 Compare August 23, 2024 13:08
@mikhailswift
Copy link
Member Author

@mikhailswift am I correct that you basically copied the tests from witness into go-witness to do the verify_test? It all looks good to me. I can't approve but if I could I would 😄

Correct, with some tweaks. We'll need to add more testing around policy evaluations to prevent this type of thing from happening.

@mikhailswift mikhailswift merged commit 960e8a1 into main Aug 23, 2024
16 checks passed
@mikhailswift mikhailswift deleted the chore/revert-broken-policy-eval branch August 23, 2024 13:12
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.

4 participants