Skip to content

Commit

Permalink
wasm: remove redundant xds attributes
Browse files Browse the repository at this point in the history 
Change-Id: I152e259f3f8e5f69463a64fedce4181ce606824b
Signed-off-by: Kuat Yessenov <[email protected]>
  • Loading branch information
@kyessenov
kyessenov committed Oct 15, 2024
1 parent 5129541 commit 090c8a4
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 95 deletions.
3 changes: 3 additions & 0 deletions changelogs/current.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ date: Pending

behavior_changes:
# *Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required*
- area: wasm
change: |
Remove previously deprecated xDS attributes from ``get_property``, use ``xds`` attributes instead.
minor_behavior_changes:
# *Changes that may cause incompatibilities for some users, but should not for most*
Expand Down
8 changes: 0 additions & 8 deletions docs/root/intro/arch_overview/advanced/attributes.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,6 @@ In addition to all above, the following extra attributes are available to Wasm e
plugin_name, string, Plugin name
plugin_root_id, string, Plugin root ID
plugin_vm_id, string, Plugin VM ID
node, :ref:`Node<envoy_v3_api_msg_config.core.v3.node>`, Local node description. DEPRECATED: please use `xds` attributes.
cluster_name, string, Upstream cluster name. DEPRECATED: please use `xds` attributes.
cluster_metadata, :ref:`Metadata<envoy_v3_api_msg_config.core.v3.metadata>`, Upstream cluster metadata. DEPRECATED: please use `xds` attributes.
listener_direction, int, Enumeration value of the :ref:`listener traffic direction<envoy_v3_api_field_config.listener.v3.Listener.traffic_direction>`. DEPRECATED: please use `xds` attributes.
listener_metadata, :ref:`Metadata<envoy_v3_api_msg_config.core.v3.metadata>`, Listener metadata. DEPRECATED: please use `xds` attributes.
route_name, string, Route name. DEPRECATED: please use `xds` attributes.
route_metadata, :ref:`Metadata<envoy_v3_api_msg_config.core.v3.metadata>`, Route metadata. DEPRECATED: please use `xds` attributes.
upstream_host_metadata, :ref:`Metadata<envoy_v3_api_msg_config.core.v3.metadata>`, Upstream host metadata. DEPRECATED: please use `xds` attributes.

Path expressions
----------------
Expand Down
63 changes: 1 addition & 62 deletions source/extensions/common/wasm/context.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,6 @@ Http::RequestHeaderMapPtr buildRequestHeaderMapFromPairs(const Pairs& pairs) {

template <typename P> static uint32_t headerSize(const P& p) { return p ? p->size() : 0; }

Upstream::HostDescriptionConstSharedPtr getHost(const StreamInfo::StreamInfo* info) {
if (info && info->upstreamInfo() && info->upstreamInfo().value().get().upstreamHost()) {
return info->upstreamInfo().value().get().upstreamHost();
}
return nullptr;
}

} // namespace

// Test support.
Expand Down Expand Up @@ -435,10 +428,7 @@ WasmResult serializeValue(Filters::Common::Expr::CelValue value, std::string* re
return WasmResult::SerializationFailure;
}

#define PROPERTY_TOKENS(_f) \
_f(NODE) _f(LISTENER_DIRECTION) _f(LISTENER_METADATA) _f(CLUSTER_NAME) _f(CLUSTER_METADATA) \
_f(ROUTE_NAME) _f(ROUTE_METADATA) _f(PLUGIN_NAME) _f(UPSTREAM_HOST_METADATA) \
_f(PLUGIN_ROOT_ID) _f(PLUGIN_VM_ID) _f(CONNECTION_ID)
#define PROPERTY_TOKENS(_f) _f(PLUGIN_NAME) _f(PLUGIN_ROOT_ID) _f(PLUGIN_VM_ID) _f(CONNECTION_ID)

static inline std::string downCase(std::string s) {
std::transform(s.begin(), s.end(), s.begin(), [](unsigned char c) { return std::tolower(c); });
Expand Down Expand Up @@ -513,57 +503,6 @@ Context::findValue(absl::string_view name, Protobuf::Arena* arena, bool last) co
}
break;
}
case PropertyToken::NODE:
if (root_local_info_) {
return CelProtoWrapper::CreateMessage(&root_local_info_->node(), arena);
} else if (plugin_) {
return CelProtoWrapper::CreateMessage(&plugin()->localInfo().node(), arena);
}
break;
case PropertyToken::LISTENER_DIRECTION:
if (plugin_) {
return CelValue::CreateInt64(plugin()->direction());
}
break;
case PropertyToken::LISTENER_METADATA:
if (plugin_) {
return CelProtoWrapper::CreateMessage(plugin()->listenerMetadata(), arena);
}
break;
case PropertyToken::CLUSTER_NAME:
if (getHost(info)) {
return CelValue::CreateString(&getHost(info)->cluster().name());
} else if (info && info->route() && info->route()->routeEntry()) {
return CelValue::CreateString(&info->route()->routeEntry()->clusterName());
} else if (info && info->upstreamClusterInfo().has_value() &&
info->upstreamClusterInfo().value()) {
return CelValue::CreateString(&info->upstreamClusterInfo().value()->name());
}
break;
case PropertyToken::CLUSTER_METADATA:
if (getHost(info)) {
return CelProtoWrapper::CreateMessage(&getHost(info)->cluster().metadata(), arena);
} else if (info && info->upstreamClusterInfo().has_value() &&
info->upstreamClusterInfo().value()) {
return CelProtoWrapper::CreateMessage(&info->upstreamClusterInfo().value()->metadata(),
arena);
}
break;
case PropertyToken::UPSTREAM_HOST_METADATA:
if (getHost(info)) {
return CelProtoWrapper::CreateMessage(getHost(info)->metadata().get(), arena);
}
break;
case PropertyToken::ROUTE_NAME:
if (info) {
return CelValue::CreateString(&info->getRouteName());
}
break;
case PropertyToken::ROUTE_METADATA:
if (info && info->route()) {
return CelProtoWrapper::CreateMessage(&info->route()->metadata(), arena);
}
break;
case PropertyToken::PLUGIN_NAME:
if (plugin_) {
return CelValue::CreateStringView(plugin()->name_);
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/filters/http/wasm/test_data/metadata_rust.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl HttpContext for TestStream {
}

fn on_http_request_body(&mut self, _: usize, _: bool) -> Action {
if let Some(value) = self.get_property(vec!["node", "metadata", "wasm_node_get_key"]) {
if let Some(value) = self.get_property(vec!["xds", "node", "metadata", "wasm_node_get_key"]) {
error!("onBody {}", String::from_utf8(value).unwrap());
} else {
debug!("missing node metadata");
Expand Down
25 changes: 9 additions & 16 deletions test/extensions/filters/http/wasm/test_data/test_cpp.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ bool TestRootContext::onConfigure(size_t size) {
"string_state", "metadata", "request", "response", "connection",
"connection_id", "upstream", "source", "destination", "cluster_name",
"cluster_metadata", "route_name", "route_metadata", "upstream_host_metadata",
"filter_state",
"filter_state", "listener_direction" ,"listener_metadata",
};
for (const auto& property : properties) {
if (getProperty({property}).has_value()) {
Expand All @@ -80,8 +80,6 @@ bool TestRootContext::onConfigure(size_t size) {
std::vector<std::pair<std::vector<std::string>, std::string>> properties = {
{{"plugin_name"}, "plugin_name"},
{{"plugin_vm_id"}, "vm_id"},
{{"listener_direction"}, std::string("\x1\0\0\0\0\0\0\0\0", 8)}, // INBOUND
{{"listener_metadata"}, ""},
{{"xds", "node", "metadata", "istio.io/metadata"}, "sample_data"},
};
for (const auto& property : properties) {
Expand Down Expand Up @@ -324,7 +322,7 @@ FilterDataStatus TestContext::onRequestBody(size_t body_buffer_length, bool end_
}
} else if (test == "metadata") {
std::string value;
if (!getValue({"node", "metadata", "wasm_node_get_key"}, &value)) {
if (!getValue({"xds", "node", "metadata", "wasm_node_get_key"}, &value)) {
logDebug("missing node metadata");
}
logError(std::string("onBody ") + value);
Expand Down Expand Up @@ -377,7 +375,7 @@ void TestContext::onLog() {
}
} else if (test == "cluster_metadata") {
std::string cluster_metadata;
if (getValue({"cluster_metadata", "filter_metadata", "namespace", "key"}, &cluster_metadata)) {
if (getValue({"xds", "cluster_metadata", "filter_metadata", "namespace", "key"}, &cluster_metadata)) {
logWarn("cluster metadata: " + cluster_metadata);
}
} else if (test == "property") {
Expand All @@ -386,7 +384,7 @@ void TestContext::onLog() {
if (path->view() == "/test_context") {
logWarn("request.path: " + getProperty({"request", "path"}).value()->toString());
logWarn("node.metadata: " +
getProperty({"node", "metadata", "istio.io/metadata"}).value()->toString());
getProperty({"xds", "node", "metadata", "istio.io/metadata"}).value()->toString());
logWarn("metadata: " + getProperty({"metadata", "filter_metadata", "envoy.filters.http.wasm",
"wasm_request_get_key"})
.value()
Expand All @@ -396,7 +394,7 @@ void TestContext::onLog() {
logWarn("response.code: " + std::to_string(responseCode));
}
std::string upstream_host_metadata;
if (getValue({"upstream_host_metadata", "filter_metadata", "namespace", "key"}, &upstream_host_metadata)) {
if (getValue({"xds", "upstream_host_metadata", "filter_metadata", "namespace", "key"}, &upstream_host_metadata)) {
logWarn("upstream host metadata: " + upstream_host_metadata);
}
logWarn("state: " + getProperty({"wasm_state"}).value()->toString());
Expand Down Expand Up @@ -588,16 +586,11 @@ void TestContext::onLog() {
std::vector<std::pair<std::vector<std::string>, std::string>> properties = {
{{"plugin_name"}, "plugin_name"},
{{"plugin_vm_id"}, "vm_id"},
{{"listener_direction"}, std::string("\x1\0\0\0\0\0\0\0\0", 8)}, // INBOUND
{{"listener_metadata"}, ""},
{{"route_name"}, "route12"},
{{"cluster_name"}, "fake_cluster"},
{{"connection_id"}, std::string("\x4\0\0\0\0\0\0\0\0", 8)},
{{"connection", "requested_server_name"}, "w3.org"},
{{"source", "address"}, "127.0.0.1:0"},
{{"destination", "address"}, "127.0.0.2:0"},
{{"upstream", "address"}, "10.0.0.1:443"},
{{"route_metadata"}, ""},
};
for (const auto& property : properties) {
std::string value;
Expand Down Expand Up @@ -629,22 +622,22 @@ void TestRootContext::onTick() {
}
} else if (test_ == "metadata") { // NOLINT(clang-analyzer-optin.portability.UnixAPI)
std::string value;
if (!getValue({"node", "metadata", "wasm_node_get_key"}, &value)) { // NOLINT(clang-analyzer-optin.portability.UnixAPI)
if (!getValue({"xds", "node", "metadata", "wasm_node_get_key"}, &value)) { // NOLINT(clang-analyzer-optin.portability.UnixAPI)
logDebug("missing node metadata");
}
logDebug(std::string("onTick ") + value);

std::string list_value;
if (!getValue({"node", "metadata", "wasm_node_list_key", "0"}, &list_value)) {
if (!getValue({"xds", "node", "metadata", "wasm_node_list_key", "0"}, &list_value)) {
logDebug("missing node metadata list value");
}
if (list_value != "wasm_node_get_value") {
logWarn("unexpected list value: " + list_value);
}
if (getValue({"node", "metadata", "wasm_node_list_key", "bad_key"}, &list_value)) {
if (getValue({"xds", "node", "metadata", "wasm_node_list_key", "bad_key"}, &list_value)) {
logDebug("unexpected list value for a bad_key");
}
if (getValue({"node", "metadata", "wasm_node_list_key", "1"}, &list_value)) {
if (getValue({"xds", "node", "metadata", "wasm_node_list_key", "1"}, &list_value)) {
logDebug("unexpected list value outside the range");
}
} else if (test_ == "property") {
Expand Down
8 changes: 0 additions & 8 deletions test/extensions/filters/http/wasm/wasm_filter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1835,8 +1835,6 @@ TEST_P(WasmHttpFilterTest, ClusterMetadata) {
}
setupTest("", "cluster_metadata");
setupFilter();
EXPECT_CALL(filter(),
log_(spdlog::level::warn, Eq(absl::string_view("cluster metadata: cluster"))));
auto cluster = std::make_shared<NiceMock<Upstream::MockClusterInfo>>();
auto cluster_metadata = std::make_shared<envoy::config::core::v3::Metadata>(
TestUtility::parseYaml<envoy::config::core::v3::Metadata>(
Expand All @@ -1852,14 +1850,8 @@ TEST_P(WasmHttpFilterTest, ClusterMetadata) {

EXPECT_CALL(encoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(request_stream_info_));
EXPECT_CALL(*cluster, metadata()).WillRepeatedly(ReturnRef(*cluster_metadata));
EXPECT_CALL(*host_description, cluster()).WillRepeatedly(ReturnRef(*cluster));
request_stream_info_.upstreamInfo()->setUpstreamHost(host_description);
EXPECT_CALL(request_stream_info_, requestComplete)
.WillRepeatedly(Return(std::chrono::milliseconds(30)));
filter().log({&request_headers}, request_stream_info_);

// If upstream host is empty, fallback to upstream cluster info for cluster metadata.
request_stream_info_.upstreamInfo()->setUpstreamHost(nullptr);
EXPECT_CALL(request_stream_info_, upstreamClusterInfo()).WillRepeatedly(Return(cluster));
EXPECT_CALL(filter(),
log_(spdlog::level::warn, Eq(absl::string_view("cluster metadata: cluster"))));
Expand Down

0 comments on commit 090c8a4

Please sign in to comment.