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

Update to v1.30 #333

Merged
merged 31 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
11ef0b0
Update envoy dependency to v1.30.1
ashishb-solo Apr 30, 2024
a3b8fd2
Change repository back to upstream envoy
ashishb-solo Apr 30, 2024
45338db
Remove wavm
ashishb-solo Apr 30, 2024
d246825
Add some compile flags to get CEL to compile
ashishb-solo May 1, 2024
e62caf8
Get matchers working
ashishb-solo May 1, 2024
330bbf7
Fix a few flags that were moved
ashishb-solo May 1, 2024
f3bc648
Get HTTP transformation to compile
ashishb-solo May 2, 2024
771fe14
Fix some compile errors
ashishb-solo May 2, 2024
47a056c
addWatch callback must now return absl::Status
ashishb-solo May 2, 2024
093698a
Fix type signature of createProtocolOptionsConfig
ashishb-solo May 2, 2024
aceeeb6
Add changelog
ashishb-solo May 2, 2024
31c242d
Update extensions_build_config.bzl
ashishb-solo May 2, 2024
b9c1149
Move changelog
ashishb-solo May 2, 2024
4f0e904
Get inja_transformer_test.cc to compile
ashishb-solo May 2, 2024
0058721
Fix a broken test
ashishb-solo May 8, 2024
1dbb2c4
Add router_check tool
ashishb-solo May 9, 2024
d7ba911
Merge branch 'master' into update-to-v1.30
ashishb-solo May 9, 2024
1de6578
Update bazel version
ashishb-solo May 9, 2024
07b4004
do_ci.sh debug flags
ashishb-solo May 9, 2024
00a16a6
Update envoy-build-ubuntu image
ashishb-solo May 10, 2024
e0a5027
Remove debug lines from do_ci.sh
ashishb-solo May 13, 2024
7dbd7cd
Remove envoy.string_matcher.lua
ashishb-solo May 13, 2024
80ab7da
Remove envoy.tracers.opentelemetry.samplers.dynatrace from bazel/exte…
ashishb-solo May 13, 2024
fd1628f
Update changelog/v1.30.1-patch1/update-to-upstream-envoy-v1.30.yaml
ashishb-solo May 13, 2024
f91e837
Update source/extensions/filters/http/aws_lambda/config.cc
ashishb-solo May 13, 2024
5b43f5a
Revert "Update source/extensions/filters/http/aws_lambda/config.cc"
ashishb-solo May 13, 2024
02afc7e
Fix one more review comment
ashishb-solo May 13, 2024
a706ea8
Revert `fmt::vformat` change in inja_transformer_test.cc
ashishb-solo May 13, 2024
831067b
Add comments explaining why `//test/tools` exist
ashishb-solo May 13, 2024
08d1401
CHange auto variables to `std::string`
ashishb-solo May 14, 2024
dea2369
Change `fmt::vformat` back to `fmt::format`
ashishb-solo May 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ common --experimental_allow_tags_propagation

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

# macOS
build:macos --cxxopt=-std=c++17 --host_cxxopt=-std=c++17
build:macos --cxxopt=-std=c++20 --host_cxxopt=-std=c++20
build:macos --action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
build:macos --host_action_env=PATH=/opt/homebrew/bin:/opt/local/bin:/usr/local/bin:/usr/bin:/bin
build:macos --define tcmalloc=disabled
Expand Down
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
6.3.2
6.5.0
16 changes: 15 additions & 1 deletion bazel/extensions/extensions_build_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ EXTENSIONS = {

"envoy.access_loggers.file": "//source/extensions/access_loggers/file:config",
"envoy.access_loggers.extension_filters.cel": "//source/extensions/access_loggers/filters/cel:config",
"envoy.access_loggers.fluentd" : "//source/extensions/access_loggers/fluentd:config",
"envoy.access_loggers.http_grpc": "//source/extensions/access_loggers/grpc:http_config",
"envoy.access_loggers.tcp_grpc": "//source/extensions/access_loggers/grpc:tcp_config",
"envoy.access_loggers.open_telemetry": "//source/extensions/access_loggers/open_telemetry:config",
Expand Down Expand Up @@ -114,6 +115,11 @@ EXTENSIONS = {

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

#
# StringMatchers
#
# "envoy.string_matcher.lua": "//source/extensions/string_matcher/lua:config",

#
# HTTP filters
#
Expand All @@ -132,6 +138,7 @@ EXTENSIONS = {
"envoy.filters.http.cors": "//source/extensions/filters/http/cors:config",
"envoy.filters.http.composite": "//source/extensions/filters/http/composite:config",
# "envoy.filters.http.connect_grpc_bridge": "//source/extensions/filters/http/connect_grpc_bridge:config",
"envoy.filters.http.credential_injector": "//source/extensions/filters/http/credential_injector:config",
"envoy.filters.http.csrf": "//source/extensions/filters/http/csrf:config",
# "envoy.filters.http.custom_response": "//source/extensions/filters/http/custom_response:factory",
"envoy.filters.http.decompressor": "//source/extensions/filters/http/decompressor:config",
Expand Down Expand Up @@ -276,6 +283,7 @@ EXTENSIONS = {
#

"envoy.tracers.opentelemetry.samplers.always_on": "//source/extensions/tracers/opentelemetry/samplers/always_on:config",
# "envoy.tracers.opentelemetry.samplers.dynatrace": "//source/extensions/tracers/opentelemetry/samplers/dynatrace:config",

#
# Transport sockets
Expand All @@ -288,6 +296,7 @@ EXTENSIONS = {
"envoy.transport_sockets.tap": "//source/extensions/transport_sockets/tap:config",
"envoy.transport_sockets.starttls": "//source/extensions/transport_sockets/starttls:config",
"envoy.transport_sockets.tcp_stats": "//source/extensions/transport_sockets/tcp_stats:config",
"envoy.transport_sockets.tls": "//source/extensions/transport_sockets/tls:config",
"envoy.transport_sockets.internal_upstream": "//source/extensions/transport_sockets/internal_upstream:config",

#
Expand Down Expand Up @@ -339,7 +348,6 @@ EXTENSIONS = {
"envoy.wasm.runtime.null": "//source/extensions/wasm_runtime/null:config",
"envoy.wasm.runtime.v8": "//source/extensions/wasm_runtime/v8:config",
"envoy.wasm.runtime.wamr": "//source/extensions/wasm_runtime/wamr:config",
"envoy.wasm.runtime.wavm": "//source/extensions/wasm_runtime/wavm:config",
"envoy.wasm.runtime.wasmtime": "//source/extensions/wasm_runtime/wasmtime:config",

#
Expand Down Expand Up @@ -388,6 +396,12 @@ EXTENSIONS = {
# "envoy.http.custom_response.redirect_policy": "//source/extensions/http/custom_response/redirect_policy:redirect_policy_lib",
# "envoy.http.custom_response.local_response_policy": "//source/extensions/http/custom_response/local_response_policy:local_response_policy_lib",

#
# Injected credentials
#

"envoy.http.injected_credentials.generic": "//source/extensions/http/injected_credentials/generic:config",

#
# QUIC extensions
#
Expand Down
6 changes: 3 additions & 3 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ REPOSITORY_LOCATIONS = dict(
# can't have more than one comment between envoy line and commit line in
# order to accommodate `check_extensions_build_config.sh`
envoy = dict(
# envoy 1.29.3 with backported ext_proc updates
commit = "e2dab93e60e93b56fa63632fb9cfa64930fc5240", # v1.29.3-fork1
remote = "https://github.com/solo-io/envoy-fork",
# envoy v1.30.1
commit = "816188b86a0a52095b116b107f576324082c7c02",
remote = "https://github.com/envoyproxy/envoy",
),
inja = dict(
# Includes unmerged modifications for
Expand Down
10 changes: 10 additions & 0 deletions changelog/v1.30.1-patch1/update-to-upstream-envoy-v1.30.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
changelog:
- type: DEPENDENCY_BUMP
dependencyOwner: envoyproxy
dependencyRepo: envoy
dependencyTag: v1.30.1
resolvesIssue: false
description: >-
Update Envoy to v1.30.1 and switch back off our fork
issueLink: https://github.com/solo-io/envoy-gloo-ee/issues/768

2 changes: 1 addition & 1 deletion ci/cloudbuild.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
steps:
- name: 'envoyproxy/envoy-build-ubuntu:41c5a05d708972d703661b702a63ef5060125c33'
- name: 'envoyproxy/envoy-build-ubuntu:f94a38f62220a2b017878b790b6ea98a0f6c5f9c'
id: 'do_ci'
args: ['ci/do_ci.sh', 'release', '//test/extensions/...', '//test/common/...', '//test/integration/...']
volumes:
Expand Down
2 changes: 2 additions & 0 deletions source/common/matcher/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@ envoy_cc_library(
deps = [
"@envoy//source/common/router:config_lib",
"@envoy//source/common/http:header_utility_lib",
"@envoy//envoy/common:regex_interface",
"@envoy_api//envoy/api/v2/route:pkg_cc_proto",
"@envoy_api//envoy/config/route/v3:pkg_cc_proto",
"//source/common/regex:regex_lib",
],
)

110 changes: 63 additions & 47 deletions source/common/matcher/solo_matcher.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#include "source/common/matcher/solo_matcher.h"

#include "envoy/common/regex.h"
#include "source/common/common/logger.h"
#include "source/common/common/regex.h"
#include "source/common/regex/regex.h"
#include "source/common/router/config_impl.h"

#include "absl/strings/match.h"
Expand All @@ -19,15 +21,16 @@ namespace {
class BaseMatcherImpl : public Matcher,
public Logger::Loggable<Logger::Id::filter> {
public:
BaseMatcherImpl(const RouteMatch &match)
BaseMatcherImpl(const RouteMatch &match,
Server::Configuration::CommonFactoryContext &context)
: case_sensitive_(
PROTOBUF_GET_WRAPPED_OR_DEFAULT(match, case_sensitive, true)),
config_headers_(
Http::HeaderUtility::buildHeaderDataVector(match.headers())) {
config_headers_(Http::HeaderUtility::buildHeaderDataVector(
match.headers(), context)) {
for (const auto &query_parameter : match.query_parameters()) {
config_query_parameters_.push_back(
std::make_unique<Router::ConfigUtility::QueryParameterMatcher>(
query_parameter));
query_parameter, context));
}
}

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

matches &= Http::HeaderUtility::matchHeaders(headers, config_headers_);
if (!config_query_parameters_.empty()) {
auto query_parameters =
Http::Utility::QueryParamsMulti::parseQueryString(
headers.Path()->value().getStringView());
auto query_parameters = Http::Utility::QueryParamsMulti::parseQueryString(
headers.Path()->value().getStringView());
matches &= ConfigUtility::matchQueryParams(query_parameters,
config_query_parameters_);
}
Expand All @@ -61,8 +63,9 @@ class BaseMatcherImpl : public Matcher,
*/
class PrefixMatcherImpl : public BaseMatcherImpl {
public:
PrefixMatcherImpl(const ::RouteMatch &match)
: BaseMatcherImpl(match), prefix_(match.prefix()) {}
PrefixMatcherImpl(const ::RouteMatch &match,
Server::Configuration::CommonFactoryContext &context)
: BaseMatcherImpl(match, context), prefix_(match.prefix()) {}

bool matches(const Http::RequestHeaderMap &headers) const override {
if (BaseMatcherImpl::matchRoute(headers) &&
Expand All @@ -87,8 +90,9 @@ class PrefixMatcherImpl : public BaseMatcherImpl {
*/
class PathMatcherImpl : public BaseMatcherImpl {
public:
PathMatcherImpl(const ::RouteMatch &match)
: BaseMatcherImpl(match), path_(match.path()) {}
PathMatcherImpl(const ::RouteMatch &match,
Server::Configuration::CommonFactoryContext &context)
: BaseMatcherImpl(match, context), path_(match.path()) {}

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

class CompiledStdMatcher : public Regex::CompiledMatcher {
public:
CompiledStdMatcher(std::regex &&regex) : regex_(std::move(regex)) {}

// CompiledMatcher
bool match(absl::string_view value) const override {
try {
return std::regex_match(value.begin(), value.end(), regex_);
} catch (const std::regex_error &e) {
return false;
}
}

// CompiledMatcher
std::string replaceAll(absl::string_view value,
absl::string_view substitution) const override {
try {
return std::regex_replace(std::string(value), regex_,
std::string(substitution));
} catch (const std::regex_error &e) {
return std::string(value);
}
}

private:
const std::regex regex_;
};

class StdRegexEngine : public Regex::Engine {
public:
Regex::CompiledMatcherPtr matcher(const std::string &regex) const override {
return std::make_unique<CompiledStdMatcher>(
Solo::Regex::Utility::parseStdRegex(regex));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need to do this if engine is now included by default in 1.30?

It sounds like we are cruising for a bruising in upcoming releases if we rely on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i don't believe std::regex is present in upstream envoy. i see a few references to that class, but no functions that are publicly exported that we can use for using std:regex. i could be wrong here though, let me know of a way that we can use it

}
};

/**
* Perform a match against any path with a regex rule.
* TODO(mattklein123): This code needs dedup with RegexRouteEntryImpl.
*/
class RegexMatcherImpl : public BaseMatcherImpl {
public:
RegexMatcherImpl(const RouteMatch &match) : BaseMatcherImpl(match) {
RegexMatcherImpl(const RouteMatch &match,
Server::Configuration::CommonFactoryContext &context)
: BaseMatcherImpl(match, context) {
ASSERT(match.path_specifier_case() == RouteMatch::kSafeRegex);
regex_ = Regex::Utility::parseRegex(match.safe_regex());
auto engine = Envoy::MatcherCopy::StdRegexEngine();
regex_ = Regex::Utility::parseRegex(match.safe_regex(), engine);
regex_str_ = match.safe_regex().regex();
}

Expand All @@ -140,56 +183,29 @@ class RegexMatcherImpl : public BaseMatcherImpl {
}

private:

static Regex::CompiledMatcherPtr parseStdRegexAsCompiledMatcher(const std::string& regex,
std::regex::flag_type flags = std::regex::optimize);
Regex::CompiledMatcherPtr regex_;
// raw regex string, for logging.
std::string regex_str_;
};

class CompiledStdMatcher : public Regex::CompiledMatcher {
public:
CompiledStdMatcher(std::regex&& regex) : regex_(std::move(regex)) {}

// CompiledMatcher
bool match(absl::string_view value) const override {
try {
return std::regex_match(value.begin(), value.end(), regex_);
} catch (const std::regex_error& e) {
return false;
}
}

// CompiledMatcher
std::string replaceAll(absl::string_view value, absl::string_view substitution) const override {
try {
return std::regex_replace(std::string(value), regex_, std::string(substitution));
} catch (const std::regex_error& e) {
return std::string(value);
}
}

private:
const std::regex regex_;
};

} // namespace

MatcherConstPtr Matcher::create(const RouteMatch &match) {
MatcherConstPtr
Matcher::create(const RouteMatch &match,
Server::Configuration::CommonFactoryContext &context) {
switch (match.path_specifier_case()) {
case RouteMatch::PathSpecifierCase::kPrefix:
return std::make_shared<PrefixMatcherImpl>(match);
return std::make_shared<PrefixMatcherImpl>(match, context);
case RouteMatch::PathSpecifierCase::kPath:
return std::make_shared<PathMatcherImpl>(match);
return std::make_shared<PathMatcherImpl>(match, context);
case RouteMatch::PathSpecifierCase::kSafeRegex:
return std::make_shared<RegexMatcherImpl>(match);
return std::make_shared<RegexMatcherImpl>(match, context);
// path specifier is required.
case RouteMatch::PathSpecifierCase::PATH_SPECIFIER_NOT_SET:
default:
PANIC_DUE_TO_CORRUPT_ENUM;
}
}

} // namespace Matcher
} // namespace MatcherCopy
} // namespace Envoy
4 changes: 3 additions & 1 deletion source/common/matcher/solo_matcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "envoy/config/route/v3/route.pb.h"
#include "envoy/http/header_map.h"
#include "envoy/server/factory_context.h"

namespace Envoy {
namespace MatcherCopy {
Expand Down Expand Up @@ -34,7 +35,8 @@ class Matcher {
*/

static MatcherConstPtr
create(const ::envoy::config::route::v3::RouteMatch &match);
create(const ::envoy::config::route::v3::RouteMatch &match,
Server::Configuration::CommonFactoryContext& context);
};

} // namespace Matcher
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ AWSLambdaFilterConfigFactory::createFilterFactoryFromProtoTyped(
};
}

Upstream::ProtocolOptionsConfigConstSharedPtr
absl::StatusOr<Upstream::ProtocolOptionsConfigConstSharedPtr>
AWSLambdaFilterConfigFactory::createProtocolOptionsConfig(
const Protobuf::Message &config,
Server::Configuration::ProtocolOptionsFactoryContext &) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class AWSLambdaFilterConfigFactory
AWSLambdaFilterConfigFactory()
: FactoryBase(SoloHttpFilterNames::get().AwsLambda) {}

Upstream::ProtocolOptionsConfigConstSharedPtr createProtocolOptionsConfig(
absl::StatusOr<Upstream::ProtocolOptionsConfigConstSharedPtr> createProtocolOptionsConfig(
const Protobuf::Message &config,
Server::Configuration::ProtocolOptionsFactoryContext &) override;
ProtobufTypes::MessagePtr createEmptyProtocolOptionsProto() override;
Expand Down
8 changes: 7 additions & 1 deletion source/extensions/filters/http/aws_lambda/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,18 @@ void AWSLambdaConfigImpl::AWSLambdaStsRefresher::init(Event::Dispatcher &dispatc
} else {
ENVOY_LOG(debug, "{}: STS enabled without time based refresh",__func__);
}
file_watcher_->addWatch(
absl::Status result = file_watcher_->addWatch(
parent_->token_file_, Filesystem::Watcher::Events::Modified,
[shared_this](uint32_t) {
// Force timer callback to happen immediately to pick up the change.
shared_this->timer_->enableTimer(std::chrono::milliseconds::zero());
return absl::OkStatus();
});
ashishb-solo marked this conversation as resolved.
Show resolved Hide resolved
// the result of the above function MUST NOT be ignored (see [[nodiscard]]).
// since we don't expect an error to occur, we'll just log a message for now
if (!result.ok()) {
ENVOY_LOG(error, "Unexpected error occurred on file watcher timer: {}", result.message());
}
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,12 @@ void NatsStreamingFilter::onResponse() { onCompletion(Http::Code::OK, ""); }

void NatsStreamingFilter::onFailure() {
onCompletion(Http::Code::InternalServerError, "nats streaming filter abort",
StreamInfo::ResponseFlag::NoHealthyUpstream);
StreamInfo::CoreResponseFlag::NoHealthyUpstream);
}

void NatsStreamingFilter::onTimeout() {
onCompletion(Http::Code::RequestTimeout, "nats streaming filter timeout",
StreamInfo::ResponseFlag::UpstreamRequestTimeout);
StreamInfo::CoreResponseFlag::UpstreamRequestTimeout);
}

void NatsStreamingFilter::retrieveRouteSpecificFilterConfig() {
Expand Down
Loading
Loading