- 
                Notifications
    
You must be signed in to change notification settings  - Fork 5.1k
 
ext_authz: perform request header checks progressively #41766
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
ext_authz: perform request header checks progressively #41766
Conversation
Signed-off-by: antoniovleonti <leonti@google.com>
| 
           /assign @tyxia  | 
    
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.
a high level question:
From description : This pr just makes it so that the ext authz filter checks to make sure header limits (size, count) have not been violated as the mutations are added rather than once at the end. Once this PR is merged, do we still need those validation/rejection at the end that you recently added?
If they are not needed, we should clean them up instead of having duplicated logics
| 
           /wait  | 
    
          
 I've removed the original check already 😄 near the original check is a new check that only happens if the path header got updated.  | 
    
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.
LGTM, thanks
Commit Message: ext_authz: perform request header checks progressively
Additional Description:
No behavior change. This pr just makes it so that the ext authz filter checks to make sure header limits (size, count) have not been violated as the mutations are added rather than once at the end. This prevents this filter from hogging the worker thread's time adding a bunch of headers when it will ultimately send a local reply.
Risk Level: low
Testing: unit tests updated for each case
Docs Changes: none necessary
Release Notes: none necessary (no externally visible behavior change)