envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
25.08k stars 4.82k forks source link

wasm: Add support for Dynamic Metadata #15148

Open Gsantomaggio opened 3 years ago

Gsantomaggio commented 3 years ago

Description: Currently in WASM is not possible to set Dynamic Metadata. This metadata emitted by a filter can be consumed by other filters and useful features can be built by stacking such filters For example, a logging filter can consume dynamic metadata from an RBAC filter

One of the use cases is the integration with RBAC for example how MYSQL does We (vmware platform team) have a WASM network filter and we'd need to integrate it with RBAC.

Do others find this valuable? Is there something that we should know before we start contributing this? ( cc @venilnoronha, I spoke with him about that)

ref: Rust SDK Issue: https://github.com/proxy-wasm/proxy-wasm-rust-sdk/issues/81

More details:

I did a test by changing the envoy proxy wasm context.cc ONLY for test ( thanks to @yuval-k for pointed there)

WasmResult Context::setProperty(absl::string_view path, absl::string_view value) {
  auto* stream_info = getRequestStreamInfo();
  if (!stream_info) {
    return WasmResult::NotFound;
  }
  // ENVOY_LOG(debug, "***** setProperty wasm path:{}  value:{}", path, value);
  ProtobufWkt::Struct metadata_val;
  auto& fields_a = *metadata_val.mutable_fields();
  std::string v1;
  absl::StrAppend(&v1, value);
  fields_a[v1].set_bool_value(true);
  std::string key3;
  absl::StrAppend(&key3, path);
  stream_info->setDynamicMetadata(key3, metadata_val);

Rust code:

self.set_property(vec!["my.filters.network.network"],
                              Some("myitem".as_ref()));

And actually the filter works:

[2021-02-20 16:45:17.749][7215663][debug][rbac] [source/extensions/filters/network/rbac/rbac_filter.cc:45] checking connection: requestedServerName: , sourceIP: 127.0.0.1:54608, directRemoteIP: 127.0.0.1:54608,remoteIP: 127.0.0.1:54608, localAddress: 127.0.0.1:5673, ssl: none, 
dynamicMetadata: filter_metadata {
  key: "my.filters.network.network"
  value {
    fields {
      key: "myitem"
      value {
        bool_value: true
      }
    }
  }
}

The RBAC seems to work:

[2021-02-19 20:02:20.480][6826232][debug][rbac] [source/extensions/filters/network/rbac/rbac_filter.cc:125]
enforced denied, matched policy myitem-policy

envoy conf:

filter_chains:
    - filters:
        - name: envoy.filters.network.wasm
          typed_config:
            "@type": type.googleapis.com/envoy.extensions.filters.network.wasm.v3.Wasm
            config:
              configuration: {"@type":"type.googleapis.com/google.protobuf.StringValue",
                              "value":""}
              name: "my.filters.network.network"
              root_id: "my.filters.network.network"
              vm_config:
                vm_id: "my.filters.network.network"
                runtime: envoy.wasm.runtime.v8
                code: {"local":{"filename":"wasm32-unknown-unknown/release/my_network_filter.wasm"}}
                allow_precompiled: true
        - name: envoy.filters.network.rbac
          typed_config:
            "@type": type.googleapis.com/envoy.extensions.filters.network.rbac.v3.RBAC
            stat_prefix: rbac
            rules:
              action: DENY
              policies:
                "myqueue-policy":
                  permissions:
                    - metadata:
                        filter: my.filters.network.network
                        path:
                          - key: myitem
                        value:
                           bool_match: true

Note: My RBAC filter does not work always, I am looking why.

yangminzhu commented 3 years ago

I think this will be very useful, it allows to decouple the logic and the WASM module could be simpler and focus on the metadata generation. And then use the dedicated RBAC filter for enforcement that is very flexible in access control API.

Gsantomaggio commented 3 years ago

Thank you for the feedback @yangminzhu. I am trying to understand how to add the feature, sorry but I am far to be an expert here. I am looking for some suggestions, as far as I have understood I have to change setProperty by adding stream_info->setDynamicMetadata(

What I am missing is the condition to trigger stream_info->setDynamicMetadata(

something like:

 switch (part_token->second) {
  case PropertyToken::DYNAMICMETADATA: {
      // /do dynamic metadata
    break;
  }
  default:
    // old behaviour 
    break;
  }

here the full code

Or maybe add another method, specific this, something like:

proxy_set_dynamicdata

I will be glad to close the issue, any help/suggestion is appreciated. cc @PiotrSikora

Thank you

Gsantomaggio commented 3 years ago

this comment by @kyessenov can be relevant

karanadep commented 3 years ago

Between Filter State and Dynamic metadata, Any comments on state locking, consistency, concurrency ? Edit - My understanding is all Envoy filters would eventually move away from Dynamic metadata to Filter state ? I am developing custom filter, there is step to set / get metadata - self.set_property and self.get_property using Filter state. set_property - is it eventual consistency or updated instantly ?

NomadXD commented 3 years ago

@Gsantomaggio Does getDynamicdata works in WASM ? i'm trying to set up dynamic metadata from ext_authz filter and then consume those metadata from my WASM http filter. I was trying to use the method which is defined for that but it's not working for me. What I'm doing is,


google::protobuf::Struct ext_metadata;
if (!getMessageValue<google::protobuf::Struct>(
           {"metadata", "filter_metadata", "envoy.filters.http.ext_authz"}, &ext_metadata)) {
     LOG_ERROR(std::string("filter_metadata Error ") + std::to_string(id()));
   }

I tried it with google::protobuf::Value also. It's the same. That method getMessageValue returns false and the log gets printed as below.


envoyproxy-websocket_1  | [2021-03-29 10:56:54.300][29][trace][wasm] [source/extensions/common/wasm/wasm_vm.cc:40] [vm->host] env.proxy_get_property(5458080, 54, 5345664, 5345616)
envoyproxy-websocket_1  | [2021-03-29 10:56:54.300][29][trace][wasm] [source/extensions/common/wasm/wasm_vm.cc:40] [host->vm] malloc(177)
envoyproxy-websocket_1  | [2021-03-29 10:56:54.300][29][trace][wasm] [source/extensions/common/wasm/wasm_vm.cc:40] [host<-vm] malloc return: 5346384
envoyproxy-websocket_1  | [2021-03-29 10:56:54.300][29][trace][wasm] [source/extensions/common/wasm/wasm_vm.cc:40] [vm<-host] env.proxy_get_property return: 0
envoyproxy-websocket_1  | [2021-03-29 10:56:54.300][29][trace][wasm] [source/extensions/common/wasm/wasm_vm.cc:40] [vm->host] env.proxy_log(2, 5449512, 92)

Any idea what the issue might be ? Dynamic metadata is there because I traced it and did the following check to see.

auto buf1 = getProperty<std::string>({"metadata", "filter_metadata", "envoy.filters.http.ext_authz"});
  if (buf1.has_value()) {
    LOG_INFO("Metadata exist");
  }

I was trying this for several hours and still no luck. Really appreciate if anyone could help

github-actions[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] commented 3 years ago

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

kyessenov commented 2 years ago

@PiotrSikora We have Lua filters that need to write metadata. We probably need this in Wasm ABI.

PiotrSikora commented 2 years ago

@kyessenov I agree. See: https://github.com/envoyproxy/envoy/pull/15196 (though, that PR was closed).

Gsantomaggio commented 2 years ago

Hi @PiotrSikora @kyessenov I stopped working on that feature. I thought that the Envoy team was no longer interested

cetanu commented 1 year ago

Can we resurrect this issue (if appropriate)?

I recently tried to set dynamic metadata from wasm and was not able to do so

vincentjames501 commented 1 year ago

+1. Would love to see this!

dmavrommatis commented 9 months ago

I will revive this issue as it is still relevant.

Right now in our case we add metadata through response headers and then delete them on the WASM filter but this is far from ideal.