-
Notifications
You must be signed in to change notification settings - Fork 5.5k
WiP: Local reply mapper #8126
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
Closed
Closed
WiP: Local reply mapper #8126
Changes from 6 commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
899ec21
Init work with configuration proposition
6da905d
Added missing dependency
8eb6f82
Merge branch 'master' into local_reply_mapper
9bf0e6b
Without passed config
fc3c545
WIP config
c9a686e
Response flags added
4213f2e
Applied changes from code review
f8df3ae
repeated match
dac4695
Merge branch 'master' into local_reply_mapper
052aeeb
WIP local reply formatter with access log reuse
1499746
Merge branch 'master' into local_reply_mapper
8f022b6
Added mapper config
419a59c
Changed to access log filter
468308a
Merge branch 'master' into local_reply_mapper
1cce878
Cleanup and initial work docs
1efc2e3
Cleanup and initial work docs
7c75f6c
added comments
2670d0f
Added possibility to define custom format for each mapper
955bf79
Merge branch 'master' into local_reply_mapper
cd31254
Extracted new format StringOrJson
a77c82f
Removed docs and fixed format. I will provide docs file with implemen…
20abff0
Merge branch 'master' into local_reply_mapper
9c8d757
Added more docs
cc55bb4
Merge branch 'master' into local_reply_mapper
7784130
Code review fix
31f3cbf
Merge branch 'master' into local_reply_mapper
5eadeca
Fix build
199dd43
Merge branch 'master' into local_reply_mapper
0aee07a
Generated v3 files
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ api_proto_library_internal( | |
| name = "accesslog", | ||
| srcs = ["accesslog.proto"], | ||
| visibility = [ | ||
| "//envoy/config/filter/network/http_connection_manager/v2:__pkg__", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this needed?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added this because ResponseFlags wasn't visible in proto. |
||
| "//envoy/service/accesslog/v2:__pkg__", | ||
| ], | ||
| deps = [ | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| #pragma once | ||
|
|
||
| #include <list> | ||
| #include <string> | ||
| #include <vector> | ||
|
|
||
| #include "envoy/config/filter/network/http_connection_manager/v2/http_connection_manager.pb.validate.h" | ||
| #include "envoy/config/filter/accesslog/v2/accesslog.pb.validate.h" | ||
|
|
||
| #include "common/common/matchers.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Http { | ||
| static uint64_t toBitMask(const envoy::data::accesslog::v2::ResponseFlags& response_flags){ | ||
| uint64_t bit_mask = 0; | ||
| if (response_flags.failed_local_healthcheck()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::FailedLocalHealthCheck; | ||
| } | ||
|
|
||
| if (response_flags.no_healthy_upstream()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::NoHealthyUpstream; | ||
| } | ||
|
|
||
| if (response_flags.upstream_request_timeout()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::UpstreamRequestTimeout; | ||
| } | ||
|
|
||
| if (response_flags.local_reset()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::LocalReset; | ||
| } | ||
|
|
||
| if (response_flags.upstream_remote_reset()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::UpstreamRemoteReset; | ||
| } | ||
|
|
||
| if (response_flags.upstream_connection_failure()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::UpstreamConnectionFailure; | ||
| } | ||
|
|
||
| if (response_flags.upstream_connection_termination()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::UpstreamConnectionTermination; | ||
| } | ||
|
|
||
| if (response_flags.upstream_overflow()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::UpstreamOverflow; | ||
| } | ||
|
|
||
| if (response_flags.no_route_found()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::NoRouteFound; | ||
| } | ||
|
|
||
| if (response_flags.delay_injected()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::DelayInjected; | ||
| } | ||
|
|
||
| if (response_flags.fault_injected()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::FaultInjected; | ||
| } | ||
|
|
||
| if (response_flags.rate_limited()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::RateLimited; | ||
| } | ||
|
|
||
| // if (response_flags.unauthorized_details()) { | ||
| // bit_mask |= StreamInfo::ResponseFlag::UnauthorizedExternalService; | ||
| // } | ||
|
|
||
| if (response_flags.rate_limit_service_error()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::RateLimitServiceError; | ||
| } | ||
|
|
||
| if (response_flags.downstream_connection_termination()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::DownstreamConnectionTermination; | ||
| } | ||
|
|
||
| if (response_flags.upstream_retry_limit_exceeded()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::UpstreamRetryLimitExceeded; | ||
| } | ||
|
|
||
| if (response_flags.stream_idle_timeout()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::StreamIdleTimeout; | ||
| } | ||
|
|
||
| if (response_flags.invalid_envoy_request_headers()) { | ||
| bit_mask |= StreamInfo::ResponseFlag::InvalidEnvoyRequestHeaders; | ||
| } | ||
|
|
||
| return bit_mask; | ||
| } | ||
|
|
||
| struct LocalReplyMatcher { | ||
| LocalReplyMatcher(std::vector<uint32_t>& status_codes, | ||
| const envoy::type::matcher::StringMatcher& body_pattern, | ||
| const envoy::data::accesslog::v2::ResponseFlags& response_flags) | ||
| : status_codes_(move(status_codes)), body_pattern_(body_pattern), response_flags_(toBitMask(response_flags)){}; | ||
|
|
||
| bool match(const absl::string_view value) const { | ||
| return body_pattern_.match(value); | ||
| } | ||
|
|
||
| std::vector<uint32_t> status_codes_; | ||
| Matchers::StringMatcherImpl body_pattern_; | ||
| uint64_t response_flags_; | ||
| }; | ||
|
|
||
| /** | ||
| * Structure which holds rewriter configuration from proto file for LocalReplyConfig. | ||
| */ | ||
| struct LocalReplyRewriter { | ||
| uint32_t status_; | ||
| }; | ||
|
|
||
| class LocalReplyConfig { | ||
| public: | ||
| LocalReplyConfig( | ||
| const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& | ||
| config) { | ||
| for (const auto& match_rewrite_config : config.send_local_reply_config()) { | ||
| std::vector<uint32_t> status_codes; | ||
| for (const auto code : match_rewrite_config.match().status_codes()) { | ||
| status_codes.emplace_back(code); | ||
| } | ||
|
|
||
| std::pair<LocalReplyMatcher, LocalReplyRewriter> pair = std::make_pair( | ||
| LocalReplyMatcher{status_codes, match_rewrite_config.match().body_pattern(), match_rewrite_config.match().response_flags()}, | ||
| LocalReplyRewriter{match_rewrite_config.rewrite().status()}); | ||
| match_rewrite_pair_list_.emplace_back(std::move(pair)); | ||
| } | ||
| }; | ||
| private: | ||
| std::list<std::pair<LocalReplyMatcher, LocalReplyRewriter>> match_rewrite_pair_list_; | ||
| }; | ||
|
|
||
| using LocalReplyConfigConstPtr = std::unique_ptr<const LocalReplyConfig>; | ||
|
|
||
| } // namespace Http | ||
| } // namespace Envoy |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I really, really, really would like to unify our HTTP match capabilities, but maybe we defer this to v4 as I think any other solution will be too disruptive. @htuch any thoughts here?
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.
I think that we should tackle this in v4, but meanwhile, for new mathers, can we align them behind either access log or TAP matchers as the basis (even if only subsetted)? I'd prefer not to add yet another matcher format (and surrounding code cruft for parsing and executing match expressions).
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.
My high level question here is do we really need a match/rewrite semantics here? Since local reply is the output from Envoy (either from core HCM, or extensions), can we change the code of local reply to emit some structured data, and map it to the actual response based on config? A
Rewritehere seems based on how current local reply works.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.
I am not sure if we want always to map some response. I thought that it would be nice to just map specifice responses to another one like it has been written in #7537.
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.
IMO it's a bit easier to allow the operator to have generic rules for local replies versus having to account for every possible location where a reply is generated, but I could definitely be convinced otherwise.
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.
I understand the context of #7537, but I'd like this to be generic enough to make it possible to convert all local reply to JSON. Particularly can
Rewritebe some sort of formatter instead?Rewriteproposed here seems based on how current local reply works (i.e. take what current sendLocalReply send, and rewrite the response header/body), instead can we just make sendLocalReply emit what formatter says for those?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.
Yeah agreed this makes sense to rethink rewrite is some combination of both rewriting fields as well as a final format phase?