Skip to content

Commit 83d90c9

Browse files
ashishb-solonfuden
andauthored
Update to v1.30 (#333)
* Update envoy dependency to v1.30.1 * Change repository back to upstream envoy * Remove wavm This was removed from upstream - see envoyproxy/envoy#32872 * Add some compile flags to get CEL to compile These flags are copied from upstream's .bazelrc. The most important one here is the `-fsized-deallocation` which was required to get CEL to compile. The rest may not be necessary, but since they changed in upstream from 1.29 to 1.30, I think it might be a good idea to include them. * Get matchers working There were some details in regex that were changed upstream that we had to incorporate * Fix a few flags that were moved * Get HTTP transformation to compile A few upstream Envoy functions that were being called in this library needed to be passed contexts, which required plumbing that object through the call hierarchy. Along the way, it seemed to appear that there were a few type signatures that were possibly not completely correct, and those have been adjusted to get things compiling successfully. * Fix some compile errors It seems that passing string arguments constructed with the `+` operator is not allowed in ENVOY_STREAM_LOG, and possibly other macros as well. This was my first encounter with this error, but there might be other instances lurking elsewhere in the repository. * addWatch callback must now return absl::Status * Fix type signature of createProtocolOptionsConfig The signature of this abstract class method was changed upstream - this change reflects the fix * Add changelog * Update extensions_build_config.bzl * Move changelog * Get inja_transformer_test.cc to compile Interesting lesson here - when calling `fmt::format`, we must use a constexpr, since the formatting function provides compile-time type checking. Neat! * Fix a broken test * Add router_check tool Upstream ci calls this, so we put in a dummy target here * Update bazel version * do_ci.sh debug flags * Update envoy-build-ubuntu image * Remove debug lines from do_ci.sh * Remove envoy.string_matcher.lua Co-authored-by: Nathan Fudenberg <[email protected]> * Remove envoy.tracers.opentelemetry.samplers.dynatrace from bazel/extensions/extensions_build_config.bzl Co-authored-by: Nathan Fudenberg <[email protected]> * Update changelog/v1.30.1-patch1/update-to-upstream-envoy-v1.30.yaml Co-authored-by: Nathan Fudenberg <[email protected]> * Update source/extensions/filters/http/aws_lambda/config.cc Co-authored-by: Nathan Fudenberg <[email protected]> * Revert "Update source/extensions/filters/http/aws_lambda/config.cc" This reverts commit f91e837. * Fix one more review comment * Revert `fmt::vformat` change in inja_transformer_test.cc * Add comments explaining why `//test/tools` exist * CHange auto variables to `std::string` * Change `fmt::vformat` back to `fmt::format` Previous iterations were still using the `vformat` call so they may not have been using the `constexpr` versino of formatting correctly --------- Co-authored-by: Nathan Fudenberg <[email protected]>
1 parent c503b14 commit 83d90c9

23 files changed

+184
-105
lines changed

.bazelrc

+4-2
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,11 @@ common --experimental_allow_tags_propagation
5555

5656
# Enable position independent code (this is the default on macOS and Windows)
5757
# (Workaround for https://github.com/bazelbuild/rules_foreign_cc/issues/421)
58+
build:linux --copt=-fdebug-types-section
5859
build:linux --copt=-fPIC
5960
build:linux --copt=-Wno-deprecated-declarations
60-
build:linux --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
61+
build:linux --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
62+
build:linux --cxxopt=-fsized-deallocation --host_cxxopt=-fsized-deallocation # needed to compile CEL on Envoy 1.30.x and above
6163
build:linux --conlyopt=-fexceptions
6264
build:linux --fission=dbg,opt
6365
build:linux --features=per_object_debug_info
@@ -117,7 +119,7 @@ build:clang-asan --linkopt --rtlib=compiler-rt
117119
build:clang-asan --linkopt --unwindlib=libgcc
118120

119121
# macOS
120-
build:macos --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
122+
build:macos --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
121123
build:macos --action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
122124
build:macos --host_action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
123125
build:macos --define tcmalloc=disabled

.bazelversion

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
6.3.2
1+
6.5.0

bazel/extensions/extensions_build_config.bzl

+15-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ EXTENSIONS = {
77

88
"envoy.access_loggers.file": "//source/extensions/access_loggers/file:config",
99
"envoy.access_loggers.extension_filters.cel": "//source/extensions/access_loggers/filters/cel:config",
10+
"envoy.access_loggers.fluentd" : "//source/extensions/access_loggers/fluentd:config",
1011
"envoy.access_loggers.http_grpc": "//source/extensions/access_loggers/grpc:http_config",
1112
"envoy.access_loggers.tcp_grpc": "//source/extensions/access_loggers/grpc:tcp_config",
1213
"envoy.access_loggers.open_telemetry": "//source/extensions/access_loggers/open_telemetry:config",
@@ -114,6 +115,11 @@ EXTENSIONS = {
114115

115116
# "envoy.matching.actions.format_string": "//source/extensions/matching/actions/format_string:config",
116117

118+
#
119+
# StringMatchers
120+
#
121+
# "envoy.string_matcher.lua": "//source/extensions/string_matcher/lua:config",
122+
117123
#
118124
# HTTP filters
119125
#
@@ -132,6 +138,7 @@ EXTENSIONS = {
132138
"envoy.filters.http.cors": "//source/extensions/filters/http/cors:config",
133139
"envoy.filters.http.composite": "//source/extensions/filters/http/composite:config",
134140
# "envoy.filters.http.connect_grpc_bridge": "//source/extensions/filters/http/connect_grpc_bridge:config",
141+
"envoy.filters.http.credential_injector": "//source/extensions/filters/http/credential_injector:config",
135142
"envoy.filters.http.csrf": "//source/extensions/filters/http/csrf:config",
136143
# "envoy.filters.http.custom_response": "//source/extensions/filters/http/custom_response:factory",
137144
"envoy.filters.http.decompressor": "//source/extensions/filters/http/decompressor:config",
@@ -276,6 +283,7 @@ EXTENSIONS = {
276283
#
277284

278285
"envoy.tracers.opentelemetry.samplers.always_on": "//source/extensions/tracers/opentelemetry/samplers/always_on:config",
286+
# "envoy.tracers.opentelemetry.samplers.dynatrace": "//source/extensions/tracers/opentelemetry/samplers/dynatrace:config",
279287

280288
#
281289
# Transport sockets
@@ -288,6 +296,7 @@ EXTENSIONS = {
288296
"envoy.transport_sockets.tap": "//source/extensions/transport_sockets/tap:config",
289297
"envoy.transport_sockets.starttls": "//source/extensions/transport_sockets/starttls:config",
290298
"envoy.transport_sockets.tcp_stats": "//source/extensions/transport_sockets/tcp_stats:config",
299+
"envoy.transport_sockets.tls": "//source/extensions/transport_sockets/tls:config",
291300
"envoy.transport_sockets.internal_upstream": "//source/extensions/transport_sockets/internal_upstream:config",
292301

293302
#
@@ -339,7 +348,6 @@ EXTENSIONS = {
339348
"envoy.wasm.runtime.null": "//source/extensions/wasm_runtime/null:config",
340349
"envoy.wasm.runtime.v8": "//source/extensions/wasm_runtime/v8:config",
341350
"envoy.wasm.runtime.wamr": "//source/extensions/wasm_runtime/wamr:config",
342-
"envoy.wasm.runtime.wavm": "//source/extensions/wasm_runtime/wavm:config",
343351
"envoy.wasm.runtime.wasmtime": "//source/extensions/wasm_runtime/wasmtime:config",
344352

345353
#
@@ -388,6 +396,12 @@ EXTENSIONS = {
388396
# "envoy.http.custom_response.redirect_policy": "//source/extensions/http/custom_response/redirect_policy:redirect_policy_lib",
389397
# "envoy.http.custom_response.local_response_policy": "//source/extensions/http/custom_response/local_response_policy:local_response_policy_lib",
390398

399+
#
400+
# Injected credentials
401+
#
402+
403+
"envoy.http.injected_credentials.generic": "//source/extensions/http/injected_credentials/generic:config",
404+
391405
#
392406
# QUIC extensions
393407
#

bazel/repository_locations.bzl

+3-3
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ REPOSITORY_LOCATIONS = dict(
22
# can't have more than one comment between envoy line and commit line in
33
# order to accommodate `check_extensions_build_config.sh`
44
envoy = dict(
5-
# envoy 1.29.3 with backported ext_proc updates
6-
commit = "e2dab93e60e93b56fa63632fb9cfa64930fc5240", # v1.29.3-fork1
7-
remote = "https://github.com/solo-io/envoy-fork",
5+
# envoy v1.30.1
6+
commit = "816188b86a0a52095b116b107f576324082c7c02",
7+
remote = "https://github.com/envoyproxy/envoy",
88
),
99
inja = dict(
1010
# Includes unmerged modifications for
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
changelog:
2+
- type: DEPENDENCY_BUMP
3+
dependencyOwner: envoyproxy
4+
dependencyRepo: envoy
5+
dependencyTag: v1.30.1
6+
resolvesIssue: false
7+
description: >-
8+
Update Envoy to v1.30.1 and switch back off our fork
9+
issueLink: https://github.com/solo-io/envoy-gloo-ee/issues/768
10+

ci/cloudbuild.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
steps:
2-
- name: 'envoyproxy/envoy-build-ubuntu:41c5a05d708972d703661b702a63ef5060125c33'
2+
- name: 'envoyproxy/envoy-build-ubuntu:f94a38f62220a2b017878b790b6ea98a0f6c5f9c'
33
id: 'do_ci'
44
args: ['ci/do_ci.sh', 'release', '//test/extensions/...', '//test/common/...', '//test/integration/...']
55
volumes:

source/common/matcher/BUILD

+2
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@ envoy_cc_library(
1919
deps = [
2020
"@envoy//source/common/router:config_lib",
2121
"@envoy//source/common/http:header_utility_lib",
22+
"@envoy//envoy/common:regex_interface",
2223
"@envoy_api//envoy/api/v2/route:pkg_cc_proto",
2324
"@envoy_api//envoy/config/route/v3:pkg_cc_proto",
25+
"//source/common/regex:regex_lib",
2426
],
2527
)
2628

source/common/matcher/solo_matcher.cc

+63-47
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
#include "source/common/matcher/solo_matcher.h"
22

3+
#include "envoy/common/regex.h"
34
#include "source/common/common/logger.h"
45
#include "source/common/common/regex.h"
6+
#include "source/common/regex/regex.h"
57
#include "source/common/router/config_impl.h"
68

79
#include "absl/strings/match.h"
@@ -19,15 +21,16 @@ namespace {
1921
class BaseMatcherImpl : public Matcher,
2022
public Logger::Loggable<Logger::Id::filter> {
2123
public:
22-
BaseMatcherImpl(const RouteMatch &match)
24+
BaseMatcherImpl(const RouteMatch &match,
25+
Server::Configuration::CommonFactoryContext &context)
2326
: case_sensitive_(
2427
PROTOBUF_GET_WRAPPED_OR_DEFAULT(match, case_sensitive, true)),
25-
config_headers_(
26-
Http::HeaderUtility::buildHeaderDataVector(match.headers())) {
28+
config_headers_(Http::HeaderUtility::buildHeaderDataVector(
29+
match.headers(), context)) {
2730
for (const auto &query_parameter : match.query_parameters()) {
2831
config_query_parameters_.push_back(
2932
std::make_unique<Router::ConfigUtility::QueryParameterMatcher>(
30-
query_parameter));
33+
query_parameter, context));
3134
}
3235
}
3336

@@ -38,9 +41,8 @@ class BaseMatcherImpl : public Matcher,
3841

3942
matches &= Http::HeaderUtility::matchHeaders(headers, config_headers_);
4043
if (!config_query_parameters_.empty()) {
41-
auto query_parameters =
42-
Http::Utility::QueryParamsMulti::parseQueryString(
43-
headers.Path()->value().getStringView());
44+
auto query_parameters = Http::Utility::QueryParamsMulti::parseQueryString(
45+
headers.Path()->value().getStringView());
4446
matches &= ConfigUtility::matchQueryParams(query_parameters,
4547
config_query_parameters_);
4648
}
@@ -61,8 +63,9 @@ class BaseMatcherImpl : public Matcher,
6163
*/
6264
class PrefixMatcherImpl : public BaseMatcherImpl {
6365
public:
64-
PrefixMatcherImpl(const ::RouteMatch &match)
65-
: BaseMatcherImpl(match), prefix_(match.prefix()) {}
66+
PrefixMatcherImpl(const ::RouteMatch &match,
67+
Server::Configuration::CommonFactoryContext &context)
68+
: BaseMatcherImpl(match, context), prefix_(match.prefix()) {}
6669

6770
bool matches(const Http::RequestHeaderMap &headers) const override {
6871
if (BaseMatcherImpl::matchRoute(headers) &&
@@ -87,8 +90,9 @@ class PrefixMatcherImpl : public BaseMatcherImpl {
8790
*/
8891
class PathMatcherImpl : public BaseMatcherImpl {
8992
public:
90-
PathMatcherImpl(const ::RouteMatch &match)
91-
: BaseMatcherImpl(match), path_(match.path()) {}
93+
PathMatcherImpl(const ::RouteMatch &match,
94+
Server::Configuration::CommonFactoryContext &context)
95+
: BaseMatcherImpl(match, context), path_(match.path()) {}
9296

9397
bool matches(const Http::RequestHeaderMap &headers) const override {
9498
if (BaseMatcherImpl::matchRoute(headers)) {
@@ -112,15 +116,54 @@ class PathMatcherImpl : public BaseMatcherImpl {
112116
const std::string path_;
113117
};
114118

119+
class CompiledStdMatcher : public Regex::CompiledMatcher {
120+
public:
121+
CompiledStdMatcher(std::regex &&regex) : regex_(std::move(regex)) {}
122+
123+
// CompiledMatcher
124+
bool match(absl::string_view value) const override {
125+
try {
126+
return std::regex_match(value.begin(), value.end(), regex_);
127+
} catch (const std::regex_error &e) {
128+
return false;
129+
}
130+
}
131+
132+
// CompiledMatcher
133+
std::string replaceAll(absl::string_view value,
134+
absl::string_view substitution) const override {
135+
try {
136+
return std::regex_replace(std::string(value), regex_,
137+
std::string(substitution));
138+
} catch (const std::regex_error &e) {
139+
return std::string(value);
140+
}
141+
}
142+
143+
private:
144+
const std::regex regex_;
145+
};
146+
147+
class StdRegexEngine : public Regex::Engine {
148+
public:
149+
Regex::CompiledMatcherPtr matcher(const std::string &regex) const override {
150+
return std::make_unique<CompiledStdMatcher>(
151+
Solo::Regex::Utility::parseStdRegex(regex));
152+
}
153+
};
154+
115155
/**
116156
* Perform a match against any path with a regex rule.
117157
* TODO(mattklein123): This code needs dedup with RegexRouteEntryImpl.
118158
*/
119159
class RegexMatcherImpl : public BaseMatcherImpl {
120160
public:
121-
RegexMatcherImpl(const RouteMatch &match) : BaseMatcherImpl(match) {
161+
RegexMatcherImpl(const RouteMatch &match,
162+
Server::Configuration::CommonFactoryContext &context)
163+
: BaseMatcherImpl(match, context) {
122164
ASSERT(match.path_specifier_case() == RouteMatch::kSafeRegex);
123-
regex_ = Regex::Utility::parseRegex(match.safe_regex());
165+
auto engine = Envoy::MatcherCopy::StdRegexEngine();
166+
regex_ = Regex::Utility::parseRegex(match.safe_regex(), engine);
124167
regex_str_ = match.safe_regex().regex();
125168
}
126169

@@ -140,56 +183,29 @@ class RegexMatcherImpl : public BaseMatcherImpl {
140183
}
141184

142185
private:
143-
144-
static Regex::CompiledMatcherPtr parseStdRegexAsCompiledMatcher(const std::string& regex,
145-
std::regex::flag_type flags = std::regex::optimize);
146186
Regex::CompiledMatcherPtr regex_;
147187
// raw regex string, for logging.
148188
std::string regex_str_;
149189
};
150190

151-
class CompiledStdMatcher : public Regex::CompiledMatcher {
152-
public:
153-
CompiledStdMatcher(std::regex&& regex) : regex_(std::move(regex)) {}
154-
155-
// CompiledMatcher
156-
bool match(absl::string_view value) const override {
157-
try {
158-
return std::regex_match(value.begin(), value.end(), regex_);
159-
} catch (const std::regex_error& e) {
160-
return false;
161-
}
162-
}
163-
164-
// CompiledMatcher
165-
std::string replaceAll(absl::string_view value, absl::string_view substitution) const override {
166-
try {
167-
return std::regex_replace(std::string(value), regex_, std::string(substitution));
168-
} catch (const std::regex_error& e) {
169-
return std::string(value);
170-
}
171-
}
172-
173-
private:
174-
const std::regex regex_;
175-
};
176-
177191
} // namespace
178192

179-
MatcherConstPtr Matcher::create(const RouteMatch &match) {
193+
MatcherConstPtr
194+
Matcher::create(const RouteMatch &match,
195+
Server::Configuration::CommonFactoryContext &context) {
180196
switch (match.path_specifier_case()) {
181197
case RouteMatch::PathSpecifierCase::kPrefix:
182-
return std::make_shared<PrefixMatcherImpl>(match);
198+
return std::make_shared<PrefixMatcherImpl>(match, context);
183199
case RouteMatch::PathSpecifierCase::kPath:
184-
return std::make_shared<PathMatcherImpl>(match);
200+
return std::make_shared<PathMatcherImpl>(match, context);
185201
case RouteMatch::PathSpecifierCase::kSafeRegex:
186-
return std::make_shared<RegexMatcherImpl>(match);
202+
return std::make_shared<RegexMatcherImpl>(match, context);
187203
// path specifier is required.
188204
case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET:
189205
default:
190206
PANIC_DUE_TO_CORRUPT_ENUM;
191207
}
192208
}
193209

194-
} // namespace Matcher
210+
} // namespace MatcherCopy
195211
} // namespace Envoy

source/common/matcher/solo_matcher.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#include "envoy/config/route/v3/route.pb.h"
44
#include "envoy/http/header_map.h"
5+
#include "envoy/server/factory_context.h"
56

67
namespace Envoy {
78
namespace MatcherCopy {
@@ -34,7 +35,8 @@ class Matcher {
3435
*/
3536

3637
static MatcherConstPtr
37-
create(const ::envoy::config::route::v3::RouteMatch &match);
38+
create(const ::envoy::config::route::v3::RouteMatch &match,
39+
Server::Configuration::CommonFactoryContext& context);
3840
};
3941

4042
} // namespace Matcher

source/extensions/filters/http/aws_lambda/aws_lambda_filter_config_factory.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ AWSLambdaFilterConfigFactory::createFilterFactoryFromProtoTyped(
4141
};
4242
}
4343

44-
Upstream::ProtocolOptionsConfigConstSharedPtr
44+
absl::StatusOr<Upstream::ProtocolOptionsConfigConstSharedPtr>
4545
AWSLambdaFilterConfigFactory::createProtocolOptionsConfig(
4646
const Protobuf::Message &config,
4747
Server::Configuration::ProtocolOptionsFactoryContext &) {

source/extensions/filters/http/aws_lambda/aws_lambda_filter_config_factory.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class AWSLambdaFilterConfigFactory
2323
AWSLambdaFilterConfigFactory()
2424
: FactoryBase(SoloHttpFilterNames::get().AwsLambda) {}
2525

26-
Upstream::ProtocolOptionsConfigConstSharedPtr createProtocolOptionsConfig(
26+
absl::StatusOr<Upstream::ProtocolOptionsConfigConstSharedPtr> createProtocolOptionsConfig(
2727
const Protobuf::Message &config,
2828
Server::Configuration::ProtocolOptionsFactoryContext &) override;
2929
ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() override;

source/extensions/filters/http/aws_lambda/config.cc

+7-1
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,18 @@ void AWSLambdaConfigImpl::AWSLambdaStsRefresher::init(Event::Dispatcher &dispatc
194194
} else {
195195
ENVOY_LOG(debug, "{}: STS enabled without time based refresh",__func__);
196196
}
197-
file_watcher_->addWatch(
197+
absl::Status result = file_watcher_->addWatch(
198198
parent_->token_file_, Filesystem::Watcher::Events::Modified,
199199
[shared_this](uint32_t) {
200200
// Force timer callback to happen immediately to pick up the change.
201201
shared_this->timer_->enableTimer(std::chrono::milliseconds::zero());
202+
return absl::OkStatus();
202203
});
204+
// the result of the above function MUST NOT be ignored (see [[nodiscard]]).
205+
// since we don't expect an error to occur, we'll just log a message for now
206+
if (!result.ok()) {
207+
ENVOY_LOG(error, "Unexpected error occurred on file watcher timer: {}", result.message());
208+
}
203209
}
204210

205211
/*

source/extensions/filters/http/nats/streaming/nats_streaming_filter.cc

+2-2
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,12 @@ void NatsStreamingFilter::onResponse() { onCompletion(Http::Code::OK, ""); }
116116

117117
void NatsStreamingFilter::onFailure() {
118118
onCompletion(Http::Code::InternalServerError, "nats streaming filter abort",
119-
StreamInfo::ResponseFlag::NoHealthyUpstream);
119+
StreamInfo::CoreResponseFlag::NoHealthyUpstream);
120120
}
121121

122122
void NatsStreamingFilter::onTimeout() {
123123
onCompletion(Http::Code::RequestTimeout, "nats streaming filter timeout",
124-
StreamInfo::ResponseFlag::UpstreamRequestTimeout);
124+
StreamInfo::CoreResponseFlag::UpstreamRequestTimeout);
125125
}
126126

127127
void NatsStreamingFilter::retrieveRouteSpecificFilterConfig() {

0 commit comments

Comments
 (0)