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

Fix the unmatched request handling by set the grpc status in payload. #223

Merged
merged 4 commits into from
Jul 27, 2022

Conversation

incfly
Copy link

@incfly incfly commented Jul 26, 2022

Fix #140

Previously we only return the grpc status, but not set the actual response payload, response.status.
Verified works manually e2e with allow_unmatched_request: true/false having different effect.

@incfly incfly requested review from Shikugawa and removed request for BrenoDeMedeiros and liminw July 26, 2022 18:13
@incfly
Copy link
Author

incfly commented Jul 26, 2022

@dio @Shikugawa for review.

Signed-off-by: Jianfei Hu <[email protected]>
@@ -12,6 +12,7 @@
#include "common/config/version_converter.h"
#include "config/config.pb.h"
#include "envoy/common/exception.h"
// #include "envoy/grpc/status.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove?

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: incfly, Shikugawa

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@istio-testing istio-testing merged commit 1618d0c into istio-ecosystem:master Jul 27, 2022
@Shikugawa
Copy link
Collaborator

@incfly
Copy link
Author

incfly commented Jul 27, 2022

@Shikugawa suspect it's some build flakiness, rerunning, https://github.com/istio-ecosystem/authservice/runs/7545016277?check_suite_focus=true

incfly added a commit that referenced this pull request Jul 28, 2022
…#223)

* mst fix e2e with hardcode.

Signed-off-by: Jianfei Hu <[email protected]>

* wip organize the convert code.

Signed-off-by: Jianfei Hu <[email protected]>

* update example to use localhost

Signed-off-by: Jianfei Hu <[email protected]>

* make format.

Signed-off-by: Jianfei Hu <[email protected]>
incfly added a commit to incfly/authservice that referenced this pull request Jul 28, 2022
…istio-ecosystem#223)

* mst fix e2e with hardcode.

Signed-off-by: Jianfei Hu <[email protected]>

* wip organize the convert code.

Signed-off-by: Jianfei Hu <[email protected]>

* update example to use localhost

Signed-off-by: Jianfei Hu <[email protected]>

* make format.

Signed-off-by: Jianfei Hu <[email protected]>
incfly added a commit that referenced this pull request Jul 28, 2022
…#223) (#225)

* mst fix e2e with hardcode.

Signed-off-by: Jianfei Hu <[email protected]>

* wip organize the convert code.

Signed-off-by: Jianfei Hu <[email protected]>

* update example to use localhost

Signed-off-by: Jianfei Hu <[email protected]>

* make format.

Signed-off-by: Jianfei Hu <[email protected]>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Authservice should implicitly deny requests that don't match config prefixes
4 participants