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

signer: Capture signer rejections and report them #483

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

cdecker
Copy link
Collaborator

@cdecker cdecker commented Jul 18, 2024

We want to capture policy signer rejections too, as they can help us identify problems for false negatives, and can be used to fine-tune policies, or act as a regression test suite should the rejection be a true positive.

Sadly we were not actually sending a rejection, due to a misplaces ? that'd bubble up the error past the reporting spot. This PR addresses that by handling the response in a more fine-grained manner.

@cdecker cdecker requested review from nepet and Randy808 July 18, 2024 09:31
There was a `?` that would prematurely bubble the error up, past the
point we'd send a rejection notice to the scheduler. So handle those
errors with more care, and we can see when policies fail on the server
side, to adjust, tweak and right-size the policies, and act as a
regression test suite.
@cdecker cdecker force-pushed the 202407-signer-rejection-report branch from 3c46697 to b5de1cd Compare July 18, 2024 09:32
Copy link

@philipr-za philipr-za left a comment

Choose a reason for hiding this comment

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

Not massively well versed in this codebase yet but LGTM

@cdecker cdecker merged commit 2af60e9 into main Jul 18, 2024
10 of 12 checks passed
@cdecker cdecker deleted the 202407-signer-rejection-report branch July 18, 2024 12:50
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.

3 participants