Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 3 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ bind(
# 2. Update .bazelversion, envoy.bazelrc and .bazelrc if needed.
#
# Note: this is needed by release builder to resolve envoy dep sha to tag.
# Commit date: 2021-06-04
ENVOY_SHA = "436946bb9df0acda0e4e709592205f0d199dfb79"
# Commit date: 2021-06-18
ENVOY_SHA = "c7c53b675c5a1337964403d91a2575c18075173c"

ENVOY_SHA256 = "bc6218475a7333b4ca5af106761ee034d47a94350d5507114a692860362f101a"
ENVOY_SHA256 = "d5fe518e092a6466657102c75fb0b4717f7614ba812ab9b2b332c367cc6a4ca0"

ENVOY_ORG = "envoyproxy"

Expand Down
4 changes: 4 additions & 0 deletions envoy.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ build:sanitizer --test_tag_filters=-no_san
build:clang --action_env=BAZEL_COMPILER=clang
build:clang --linkopt=-fuse-ld=lld

# Flags for Clang + PCH
build:clang-pch --spawn_strategy=local
build:clang-pch --define=ENVOY_CLANG_PCH=1

# Basic ASAN/UBSAN that works for gcc
build:asan --action_env=ENVOY_ASAN=1
build:asan --config=sanitizer
Expand Down
19 changes: 17 additions & 2 deletions extensions/common/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,16 @@ envoy_cc_library(
name = "context",
srcs = [
"context.cc",
"util.cc",
],
hdrs = [
"context.h",
"istio_dimensions.h",
"util.h",
],
repository = "@envoy",
visibility = ["//visibility:public"],
deps = [
":node_info_fb_cc",
":util",
"@proxy_wasm_cpp_host//:null_lib",
],
)
Expand All @@ -63,11 +62,27 @@ envoy_cc_library(
visibility = ["//visibility:public"],
deps = [
":node_info_fb_cc",
":util",
"@com_google_protobuf//:protobuf",
"@proxy_wasm_cpp_host//:null_lib",
],
)

envoy_cc_library(
name = "util",
srcs = [
"util.cc",
],
hdrs = [
"util.h",
],
repository = "@envoy",
visibility = ["//visibility:public"],
deps = [
"@proxy_wasm_cpp_host//:null_lib",
],
)

envoy_cc_library(
name = "istio_dimensions",
hdrs = [
Expand Down
18 changes: 9 additions & 9 deletions extensions/common/context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,9 @@ flatbuffers::DetachedBuffer extractLocalNodeFlatBuffer() {
}
}
if (getValue({"node", "metadata", "APP_CONTAINERS"}, &value)) {
std::vector<std::string_view> containers = absl::StrSplit(value, ',');
std::vector<absl::string_view> containers = absl::StrSplit(value, ',');
for (const auto& container : containers) {
app_containers.push_back(fbb.CreateString(container));
app_containers.push_back(fbb.CreateString(toStdStringView(container)));
}
}

Expand Down Expand Up @@ -300,7 +300,7 @@ bool extractPeerMetadataFromUpstreamHostMetadata(
&endpoint_labels)) {
return false;
}
std::vector<std::string_view> parts = absl::StrSplit(endpoint_labels, ';');
std::vector<absl::string_view> parts = absl::StrSplit(endpoint_labels, ';');
// workload label should semicolon separated four parts string:
// workload_name;namespace;canonical_service;canonical_revision;cluster_id.
if (parts.size() < 4) {
Expand All @@ -310,22 +310,22 @@ bool extractPeerMetadataFromUpstreamHostMetadata(
flatbuffers::Offset<flatbuffers::String> workload_name, namespace_,
cluster_id;
std::vector<flatbuffers::Offset<KeyVal>> labels;
workload_name = fbb.CreateString(parts[0]);
namespace_ = fbb.CreateString(parts[1]);
workload_name = fbb.CreateString(toStdStringView(parts[0]));
namespace_ = fbb.CreateString(toStdStringView(parts[1]));
if (!parts[2].empty()) {
labels.push_back(CreateKeyVal(fbb,
fbb.CreateString(kCanonicalServiceLabelName),
fbb.CreateString(parts[2])));
fbb.CreateString(toStdStringView(parts[2]))));
}
if (!parts[3].empty()) {
labels.push_back(
CreateKeyVal(fbb, fbb.CreateString(kCanonicalServiceRevisionLabelName),
fbb.CreateString(parts[3])));
fbb.CreateString(toStdStringView(parts[3]))));
}
if (parts.size() >= 5) {
// In case newer proxy runs with old control plane, only extract cluster
// name if there are the fifth part.
cluster_id = fbb.CreateString(parts[4]);
cluster_id = fbb.CreateString(toStdStringView(parts[4]));
}
auto labels_offset = fbb.CreateVectorOfSortedTables(&labels);

Expand Down Expand Up @@ -518,7 +518,7 @@ bool populateGRPCInfo(RequestInfo* request_info) {
}
// The expected byte serialization of grpc_stats filter is "x,y" where "x"
// is the request message count and "y" is the response message count.
std::vector<std::string_view> parts = absl::StrSplit(value, ',');
std::vector<absl::string_view> parts = absl::StrSplit(value, ',');
if (parts.size() == 2) {
return absl::SimpleAtoi(parts[0], &request_info->request_message_count) &&
absl::SimpleAtoi(parts[1], &request_info->response_message_count);
Expand Down
3 changes: 0 additions & 3 deletions extensions/common/context.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ std::string_view AuthenticationPolicyString(ServiceAuthenticationPolicy policy);
std::string_view TCPConnectionStateString(TCPConnectionState state);
std::string_view ProtocolString(Protocol protocol);

// None response flag.
const std::string NONE = "-";

// RequestInfo represents the information collected from filter stream
// callbacks. This is used to fill metrics and logs.
struct RequestInfo {
Expand Down
5 changes: 3 additions & 2 deletions extensions/common/proto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include "absl/strings/str_join.h"
#include "absl/strings/str_split.h"
#include "extensions/common/util.h"

// WASM_PROLOG
#ifndef NULL_PLUGIN
Expand Down Expand Up @@ -70,10 +71,10 @@ flatbuffers::DetachedBuffer extractNodeFlatBufferFromStruct(
fbb.CreateString(platform_it.second.string_value())));
}
} else if (it.first == "APP_CONTAINERS") {
std::vector<std::string_view> containers =
std::vector<absl::string_view> containers =
absl::StrSplit(it.second.string_value(), ',');
for (const auto& container : containers) {
app_containers.push_back(fbb.CreateString(container));
app_containers.push_back(fbb.CreateString(toStdStringView(container)));
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions extensions/common/proto_util_speed_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "benchmark/benchmark.h"
#include "extensions/common/node_info_generated.h"
#include "extensions/common/proto_util.h"
#include "extensions/common/util.h"
#include "google/protobuf/util/json_util.h"
#include "source/common/stream_info/filter_state_impl.h"
#include "source/extensions/filters/common/expr/cel_state.h"
Expand Down Expand Up @@ -64,15 +65,16 @@ static void setData(Envoy::StreamInfo::FilterStateImpl& filter_state,
auto state_ptr =
std::make_unique<Envoy::Extensions::Filters::Common::Expr::CelState>(
prototype);
state_ptr->setValue(value);
filter_state.setData(key, std::move(state_ptr),
state_ptr->setValue(toAbslStringView(value));
filter_state.setData(toAbslStringView(key), std::move(state_ptr),
Envoy::StreamInfo::FilterState::StateType::Mutable);
}

static const std::string& getData(
Envoy::StreamInfo::FilterStateImpl& filter_state, std::string_view key) {
return filter_state
.getDataReadOnly<Envoy::Extensions::Filters::Common::Expr::CelState>(key)
.getDataReadOnly<Envoy::Extensions::Filters::Common::Expr::CelState>(
toAbslStringView(key))
.value();
}

Expand Down
5 changes: 2 additions & 3 deletions extensions/common/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@
* limitations under the License.
*/

#include <string>
#include "extensions/common/util.h"

#include "absl/strings/str_cat.h"
#include "extensions/common/context.h"

namespace Wasm {
namespace Common {
Expand Down Expand Up @@ -87,7 +86,7 @@ enum ResponseFlag {

void appendString(std::string& result, const absl::string_view& append) {
if (result.empty()) {
result = append;
result = std::string(append);
} else {
absl::StrAppend(&result, ",", append);
}
Expand Down
17 changes: 17 additions & 0 deletions extensions/common/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,28 @@

#include <string>

#include "absl/strings/string_view.h"

namespace Wasm {
namespace Common {

// None response flag.
const char NONE[] = "-";

// Parses an integer response flag into a readable short string.
const std::string parseResponseFlag(uint64_t response_flag);

// Used for converting sanctioned uses of std string_view (e.g. extensions) to
Comment thread
bianpengyuan marked this conversation as resolved.
// absl::string_view for internal use.
inline absl::string_view toAbslStringView(std::string_view view) {
return absl::string_view(view.data(), view.size());
}

// Used for converting internal absl::string_view to sanctioned uses of std
// string_view (e.g. extensions).
inline std::string_view toStdStringView(absl::string_view view) {
return std::string_view(view.data(), view.size());
}

} // namespace Common
} // namespace Wasm
1 change: 1 addition & 0 deletions extensions/common/wasm/json_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ namespace Common {
using JsonObject = ::nlohmann::json;

enum JsonParserResultDetail {
UNKNOWN,
OK,
OUT_OF_RANGE,
TYPE_ERROR,
Expand Down
14 changes: 11 additions & 3 deletions extensions/metadata_exchange/plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "absl/strings/str_split.h"
#include "extensions/common/context.h"
#include "extensions/common/proto_util.h"
#include "extensions/common/util.h"
#include "extensions/common/wasm/json_util.h"

#ifndef NULL_PLUGIN
Expand Down Expand Up @@ -103,8 +104,9 @@ bool PluginRootContext::configure(size_t configuration_size) {
// Parse configuration JSON string.
auto result = ::Wasm::Common::JsonParse(configuration_data->view());
if (!result.has_value()) {
LOG_WARN(absl::StrCat("cannot parse plugin configuration JSON string: ",
configuration_data->view()));
LOG_WARN(absl::StrCat(
"cannot parse plugin configuration JSON string: ",
::Wasm::Common::toAbslStringView(configuration_data->view())));
return false;
}

Expand All @@ -130,7 +132,13 @@ bool PluginRootContext::updatePeer(std::string_view key,
}
}

auto bytes = Base64::decodeWithoutPadding(peer_header);
#ifndef NULL_PLUGIN
auto peer_header_view = peer_header;
#else
auto peer_header_view = Wasm::Common::toAbslStringView(peer_header);
Comment thread
lambdai marked this conversation as resolved.
#endif

auto bytes = Base64::decodeWithoutPadding(peer_header_view);
google::protobuf::Struct metadata;
if (!metadata.ParseFromString(bytes)) {
return false;
Expand Down
15 changes: 10 additions & 5 deletions extensions/stackdriver/common/utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

#include "extensions/stackdriver/common/utils.h"

#include "extensions/common/util.h"
#include "extensions/stackdriver/common/constants.h"
#include "grpcpp/grpcpp.h"

Expand Down Expand Up @@ -106,9 +107,12 @@ std::string getGCEInstanceUID(const ::Wasm::Common::FlatNode &node) {
}

if (name.size() > 0 && project && location) {
return absl::StrCat("//compute.googleapis.com/projects/",
project->value()->string_view(), "/zones/",
location->value()->string_view(), "/instances/", name);
return absl::StrCat(
"//compute.googleapis.com/projects/",
::Wasm::Common::toAbslStringView(project->value()->string_view()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: doesn't absl StrCat handle std::string_view natively?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It doesn't - it supports absl::string_view, which by default is equivalent to std::string_view in C++17, but Envoy switched to non-standard null-terminated variants in envoyproxy/envoy#16029, so std::string_view is no longer supported.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@PiotrSikora Could you clarify wrt null?
is it null-terminated or nullptr constructed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good catch - nullptr-constructed.

"/zones/",
::Wasm::Common::toAbslStringView(location->value()->string_view()),
"/instances/", ::Wasm::Common::toAbslStringView(name));
}

return "";
Expand All @@ -131,8 +135,9 @@ std::string getOwner(const ::Wasm::Common::FlatNode &node) {
}
auto created_by = platform_metadata->LookupByKey(kGCECreatedByKey.data());
if (created_by) {
return absl::StrCat("//compute.googleapis.com/",
created_by->value()->string_view());
return absl::StrCat(
"//compute.googleapis.com/",
::Wasm::Common::toAbslStringView(created_by->value()->string_view()));
}

return getGCEInstanceUID(node);
Expand Down
5 changes: 4 additions & 1 deletion extensions/stackdriver/stackdriver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <unordered_map>

#include "extensions/common/proto_util.h"
#include "extensions/common/util.h"
#include "extensions/stackdriver/log/exporter.h"
#include "extensions/stackdriver/metric/registry.h"
#include "google/protobuf/util/time_util.h"
Expand Down Expand Up @@ -263,7 +264,9 @@ void fillAuthzDryRunInfo(
std::string shadow_deny_policy = "";
std::string shadow_allow_policy = "";
for (const auto& [key, val] : md.value()->pairs()) {
LOG_DEBUG(absl::StrCat("RBAC metadata found: key=", key, ", value=", val));
LOG_DEBUG(absl::StrCat(
"RBAC metadata found: key=", Wasm::Common::toAbslStringView(key),
", value=", Wasm::Common::toAbslStringView(val)));
if (key == kDryRunDenyShadowEngineResult) {
shadow_deny_result = (val == "allowed");
} else if (key == kDryRunAllowShadowEngineResult) {
Expand Down
10 changes: 6 additions & 4 deletions extensions/stats/plugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,9 @@ bool PluginRootContext::configure(size_t configuration_size) {

auto result = ::Wasm::Common::JsonParse(configuration_data->view());
if (!result.has_value()) {
LOG_WARN(absl::StrCat("cannot parse plugin configuration JSON string: ",
configuration_data->view()));
LOG_WARN(absl::StrCat(
"cannot parse plugin configuration JSON string: ",
::Wasm::Common::toAbslStringView(configuration_data->view())));
return false;
}

Expand Down Expand Up @@ -634,8 +635,9 @@ void PluginRootContext::report(::Wasm::Common::RequestInfo& request_info,
continue;
}
auto stat = statgen.resolve(istio_dimensions_);
LOG_DEBUG(absl::StrCat("metricKey cache miss ", statgen.name(), " ",
", stat=", stat.metric_id_,
LOG_DEBUG(absl::StrCat("metricKey cache miss ",
::Wasm::Common::toAbslStringView(statgen.name()),
" ", ", stat=", stat.metric_id_,
", recurrent=", stat.recurrent_));
if (end_stream || stat.recurrent_) {
stat.record(request_info);
Expand Down
7 changes: 3 additions & 4 deletions src/envoy/http/authn/authn_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void process(const Wasm::Common::JsonObject& json_obj,

// 1. Try to parse as string.
auto value_string =
Wasm::Common::JsonGetField<absl::string_view>(json_obj, key);
Wasm::Common::JsonGetField<std::string_view>(json_obj, key);
if (value_string.detail() == Wasm::Common::JsonParserResultDetail::OK) {
const auto list =
absl::StrSplit(value_string.value().data(), ' ', absl::SkipEmpty());
Expand All @@ -60,9 +60,8 @@ void process(const Wasm::Common::JsonObject& json_obj,
continue;
}
// 2. If not a string, try to parse as list of string.
auto value_list =
Wasm::Common::JsonGetField<std::vector<absl::string_view>>(json_obj,
key);
auto value_list = Wasm::Common::JsonGetField<std::vector<std::string_view>>(
json_obj, key);
if (value_list.detail() == Wasm::Common::JsonParserResultDetail::OK) {
for (const auto& s : value_list.value()) {
(*claims.mutable_fields())[key]
Expand Down
Loading