xDS metadata keys should be filter instance names, not filter implementation names

[markdroth]

In #12274, we changed the keys in typed_per_filter_config to match the HTTP filter instance name (the HttpFilter.name field) instead of the legacy filter implementation name (e.g., "envoy.filters.http.gcp_authn"). However, the CDS metadata keys are still using the legacy names (example).

I think we should make the same change for CDS metadata keys that we did for typed_per_filter_config, so that it becomes possible to specify different metadata for multiple instances of the same filter.

[wbpcode]

I think not only the CDS metadata, we have no restriction to the using of any metadata. It's completely depend on the filter implementation. And it's much harder to change this point. 🤔

[markdroth]

You're right, this is not just an issue for CDS, but for metadata in any resource. Updated the issue title accordingly.

I think we should probably build a common library for finding metadata entries applicable to a given filter in various parts of the config (e.g., routes, clusters, endpoints, etc), and then we should modify all existing filters to use that common library.

[markdroth]

For context, the reason I filed this issue is that I am proposing a feature in gRPC that will be the first to use CDS metadata (see gRFC A83: xDS GCP Authentication Filter), and I want to make sure we agree on the right semantics before we implement it. I don't think we want Envoy and gRPC to have incompatible interpretations of this data, but at the same time, Envoy's existing behavior seems broken to me, and I don't want to implement this broken behavior in gRPC and then have to fix it later.

[adisuissa]

I agree that it makes sense. We probably want Envoy to be backward compatible, i.e., search b a filter name, and if not found, search for a filter of that type (keeping the current behavior).

[markdroth]

Yup, absolutely. We can use basically the same migration story that we did for #12274.

[github-actions]

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.