Add matches filter

[XAMPPRocky]

Adds a minimal version of the quilkin matches filter discussed in #401.

I believe closes #401, #412

Breaking Changes

• ConfigType::Static now accepts serde_yaml::Value by value instead of by reference.

[XAMPPRocky]

I've updated the tests, and add one new feature to the Matches filter, which is the addition of fallthrough, which behaves like fallthrough in programming languages, except you have the options for "drop" for dropping packets if none match, "pass" for allowing passthrough, and "filter" which allows you to run a specific filter when nothing matches.

I've not added a test for the write side of the filter because currently the only filter we have that populates easily populates metadata is CaptureBytes and that does not support capturing from the write side (maybe it should?), when we have something that does, I can add it in a followup PR.

[iffyio]

Let's call out in the PR description that is a breaking change, so its visible in the release docs

[XAMPPRocky]

I've added the label, which I believe is what @markmandel is using.

[iffyio]

Yeah we need the label too! What I meant was in this case the label isn't enough since the breakage isn't obvious at all from the PR - usually with a release folks will go through the breaking PRs (which is where the label comes in) and it should be clear what broke. Here that won't be possible at all without reading the code which would be harsh, so let's mention that this PR does in fact contain a breaking change since adding a Matches filter isn't breaking in itself.

[markmandel]

Yes please - add a note about this in the description, so when i write the release notes I won't forget 👍🏻

[iffyio]

I think this can be Option<serde_yaml::Value> rather than a full config type or? since this is only used for the yaml config? in which case it probably allows us to avoid the custom serde we have for fallthrough? (same for Branch)

[XAMPPRocky]

It is needed since someone can send a Fallthrough::Filter through xDS/protobuf, not just in the static configuration

[iffyio]

Ah so iiuc its not necessarily it being used through xds but rather that to use deserialize we need to be able to convert from proto to yaml and there isn't a reasonable way to do that for the transparent filter config field? We don't have to use deserialize since its only a helper function, we can call the convert function ourselves and instantiate the filter i.e

fn create_filter(&self, args: CreateFilterArgs) -> Result<FilterInstance, Error> {
  let (config_json, on_read, on_write) = match args {
    ConfigType::Static(config) => {
      // Instantiate sub filters from static config
    },
    ConfigType::Dynamic(config) => {
      // Instantiate sub filters from dynamic config
    }
  }

  let filter = Matches::new(on_read, on_write)?;
  Ok(FilterInstance::new(
    config_json,
    Box::new(filter) as Box<dyn Filter>,
  ))
}

[XAMPPRocky]

Sure, though I'm not sure what that provides over the current approach.

[markmandel]

Let's add the documentation like we have it currently for filters. It means for users that don't know Rust they have to work it out from the Rust code.

[XAMPPRocky]

How are you generating that? Is there a specific format I should be following?

[markmandel]

From digging into the code, value can't be a base64 byte array, correct? It has be one of the Value types such as String or Int? (bool?). This makes sense, since the dynamic metadata could come from anywhere, and as such, should be a Value.

(Just making sure my understanding is correct).

If so, we might want to make that extra clear in the documentation.

[XAMPPRocky]

Yes, it's not base64, it's a string.

[markmandel]

nods that makes total sense -- I think we should be clear in that in the docs with a comment somewhere, as I could see people assuming it was a byte array and attempting to fill it as such.

[XAMPPRocky]

Well I mean it should be documented but that's also valid to do that. That's why the PartialEq of Value compares the value between strings and bytes, so that you can set a string here and use it to match the bytes, since a lot of marker values in my experience are at least string friendly if not human-readable.

[markmandel]

100% agree.

[markmandel]

Yes please - add a note about this in the description, so when i write the release notes I won't forget 👍🏻

[XAMPPRocky]

@markmandel Now with #478, do we want to merge this in, and I can add the configuration schema in that?

[markmandel]

@markmandel Now with #478, do we want to merge this in, and I can add the configuration schema in that?

I like it! Also good to get this in for 0.3.0 👍🏻

[quilkin-bot]

Build Succeeded 🥳

Build Id: 76e917c6-c950-4a53-bc44-9683c1dc1b84

To build this version:

git fetch git@github.com:googleforgames/quilkin.git pull/447/head:pr_447 && git checkout pr_447
cargo build