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

b/158262661: Move snake to json segment mapping to per-operation instead of global #218

Merged
merged 14 commits into from
Jul 14, 2020
7 changes: 2 additions & 5 deletions api/envoy/v6/http/path_matcher/config.proto
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,14 @@ message PathMatcherRule {
// Operation name (selector) used as a unique ID for the operation.
string operation = 2 [(validate.rules).string.min_bytes = 1];

// If set, the path parameters in the `pattern` must be extracted.
// Ref:
// If set, the path parameters in the `pattern` must be extracted. Ref:
// https://cloud.google.com/endpoints/docs/openapi/openapi-extensions#understanding_path_translation
PathParameterExtractionRule path_parameter_extraction = 3;
}

message PathParameterExtractionRule {
TAOXUY marked this conversation as resolved.
Show resolved Hide resolved
// Each rule has a mapping of snake_case segments to jsonCase segments.
// For example, for a URI template: /{foo_bar}
// * snake_name = "foo_bar"
// * json_name = "fooBar"
// If the map is empty, the path params still need to be extracted.
map<string, string> snake_to_json_segments = 2;
}

Expand Down
1 change: 1 addition & 0 deletions src/api_proxy/path_matcher/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ envoy_basic_cc_library(
":path_matcher_lib",
"//external:abseil_flat_hash_map",
"//external:abseil_strings",
"//external:protobuf",
],
)

Expand Down
4 changes: 2 additions & 2 deletions src/envoy/http/backend_auth/filter_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace backend_auth {
/**
* All stats for the backend auth filter. @see stats_macros.h
*/
#define ALL_PATH_MATCHER_FILTER_STATS(COUNTER) \
#define ALL_BACKEND_AUTH_FILTER_STATS(COUNTER) \
COUNTER(denied_by_no_operation) \
COUNTER(denied_by_no_token) \
COUNTER(allowed_by_no_configured_rules) \
nareddyt marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -36,7 +36,7 @@ namespace backend_auth {
* Wrapper struct for backend auth filter stats. @see stats_macros.h
*/
struct FilterStats {
ALL_PATH_MATCHER_FILTER_STATS(GENERATE_COUNTER_STRUCT)
ALL_BACKEND_AUTH_FILTER_STATS(GENERATE_COUNTER_STRUCT)
};

class FilterConfig {
Expand Down
2 changes: 1 addition & 1 deletion src/envoy/http/backend_auth/filter_config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class FilterConfigImpl
FilterStats generateStats(const std::string& prefix,
Envoy::Stats::Scope& scope) {
const std::string final_prefix = prefix + "backend_auth.";
return {ALL_PATH_MATCHER_FILTER_STATS(
return {ALL_BACKEND_AUTH_FILTER_STATS(
POOL_COUNTER_PREFIX(scope, final_prefix))};
nareddyt marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
2 changes: 1 addition & 1 deletion src/envoy/http/backend_auth/filter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class BackendAuthFilterTest : public ::testing::Test {
}

testing::NiceMock<Envoy::Stats::MockIsolatedStatsStore> scope_;
FilterStats stats_{ALL_PATH_MATCHER_FILTER_STATS(
FilterStats stats_{ALL_BACKEND_AUTH_FILTER_STATS(
POOL_COUNTER_PREFIX(scope_, "backend_auth."))};

std::shared_ptr<MockFilterConfigParser> mock_filter_config_parser_;
Expand Down
2 changes: 1 addition & 1 deletion src/envoy/http/path_matcher/filter_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ FilterConfig::FilterConfig(
// Only stores the operations that need path param extraction.
if (rule.has_path_parameter_extraction()) {
nareddyt marked this conversation as resolved.
Show resolved Hide resolved
path_param_extractions_[rule.operation()] =
rule.path_parameter_extraction();
&rule.path_parameter_extraction();
}
}
path_matcher_ = pmb.Build();
Expand Down
11 changes: 4 additions & 7 deletions src/envoy/http/path_matcher/filter_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ class FilterConfig : public Envoy::Logger::Loggable<Envoy::Logger::Id::filter> {
return nullptr;
}

// Relies on pointer safety in absl::flat_hash_map.
// The map will never change after filter config is generated, so it should
// be safe.
return &(operation_it->second);
return operation_it->second;
}

FilterStats& stats() { return stats_; }
Expand All @@ -100,9 +97,9 @@ class FilterConfig : public Envoy::Logger::Loggable<Envoy::Logger::Id::filter> {

// Map from operation id to a PathParameterExtractionRule.
// Only stores the operations that need path param extraction.
absl::flat_hash_map<
std::string,
::espv2::api::envoy::v6::http::path_matcher::PathParameterExtractionRule>
absl::flat_hash_map<std::string,
const ::espv2::api::envoy::v6::http::path_matcher::
PathParameterExtractionRule*>
nareddyt marked this conversation as resolved.
Show resolved Hide resolved
path_param_extractions_;

FilterStats stats_;
Expand Down