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

wasm: Add support for Dynamic Metadata #15148

Open
Gsantomaggio opened this issue Feb 23, 2021 · 13 comments
Open

wasm: Add support for Dynamic Metadata #15148

Gsantomaggio opened this issue Feb 23, 2021 · 13 comments
Labels
area/wasm enhancement Feature requests. Not bugs or questions. help wanted Needs help!

Comments

@Gsantomaggio
Copy link

Gsantomaggio commented Feb 23, 2021

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: proxy-wasm/proxy-wasm-rust-sdk#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.

@Gsantomaggio Gsantomaggio added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Feb 23, 2021
@yangminzhu
Copy link
Contributor

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
Copy link
Author

Gsantomaggio commented Feb 24, 2021

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
Copy link
Author

this comment by @kyessenov can be relevant

@karanadep
Copy link

karanadep commented Mar 1, 2021

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 ?

Gsantomaggio added a commit to Gsantomaggio/envoy that referenced this issue Mar 10, 2021
Signed-off-by: Gabriele Santomaggio <g.santomaggio@gmail.com>
@NomadXD
Copy link
Contributor

NomadXD commented Mar 29, 2021

@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
Copy link

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 github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Apr 28, 2021
@github-actions
Copy link

github-actions bot commented May 5, 2021

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
Copy link
Contributor

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

@PiotrSikora
Copy link
Contributor

@kyessenov I agree. See: #15196 (though, that PR was closed).

@Gsantomaggio
Copy link
Author

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

@cetanu
Copy link
Contributor

cetanu commented Apr 17, 2023

Can we resurrect this issue (if appropriate)?

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

@kyessenov kyessenov reopened this May 4, 2023
@kyessenov kyessenov added help wanted Needs help! no stalebot Disables stalebot from closing an issue and removed stale stalebot believes this issue/PR has not been touched recently labels May 4, 2023
@vincentjames501
Copy link

+1. Would love to see this!

@dmavrommatis
Copy link

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.

@alyssawilk alyssawilk removed the no stalebot Disables stalebot from closing an issue label Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/wasm enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Development

No branches or pull requests