Skip to content

ext_authz: removed unnecessary assert from onSuccess gRPC client#6505

Merged
mattklein123 merged 4 commits intoenvoyproxy:masterfrom
gsagula:remove-assert
Apr 11, 2019
Merged

ext_authz: removed unnecessary assert from onSuccess gRPC client#6505
mattklein123 merged 4 commits intoenvoyproxy:masterfrom
gsagula:remove-assert

Conversation

@gsagula
Copy link
Member

@gsagula gsagula commented Apr 7, 2019

Description:
Since Grpc::Status::GrpcStatus::Unknown is a valid status, there is no reason for crashing Envoy when this code is received from a gRPC authorization response.

Risk Level: Low
Testing: No
Docs Changes: No
Release Notes: Yes
Fixes #6210

Signed-off-by: Gabriel gsagula@gmail.com

Gabriel added 2 commits April 6, 2019 17:35
Signed-off-by: Gabriel <gsagula@gmail.com>
Signed-off-by: Gabriel <gsagula@gmail.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM with small comments.

/wait

@mattklein123 mattklein123 self-assigned this Apr 8, 2019
Signed-off-by: Gabriel <gsagula@gmail.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thank you!

@mattklein123 mattklein123 merged commit dcd7f4c into envoyproxy:master Apr 11, 2019
alyssawilk pushed a commit that referenced this pull request May 7, 2019
…6821)

Description: PR #6211 updated the documentation of CheckResponse.status to reflect Envoy's actual behavior at the time. Later, PR #6505 changed that behavior to be in-line with the pre-6211 docs. So, revert that part of PR #6211.

Risk Level: Low
Testing: None
Docs Changes: Inline in API protos
Release Notes: none

Signed-off-by: Luke Shumaker <lukeshu@datawire.io>
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.

2 participants