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

v2 tcp proxy filter config with deprecated_v1 field conflict #2441

Closed
junr03 opened this issue Jan 24, 2018 · 5 comments
Closed

v2 tcp proxy filter config with deprecated_v1 field conflict #2441

junr03 opened this issue Jan 24, 2018 · 5 comments
Assignees
Milestone

Comments

@junr03
Copy link
Member

junr03 commented Jan 24, 2018

Description:
Currently, when the listener is creating network level filters, it uses the deprecated_v1 field as a boolean indicator to load a v1 config from an opaque value field.

if (filter_config->getBoolean("deprecated_v1", false)) {
callback = factory.createFilterFactory(*filter_config->getObject("value", true), context);
} else {
auto message = Config::Utility::translateToFactoryConfig(proto_config, factory);
callback = factory.createFilterFactoryFromProto(*message, context);
}
. However, this approach conflicts with the actual deprecated_v1 field in the tcp proxy config https://github.com/envoyproxy/data-plane-api/blob/master/api/filter/network/tcp_proxy.proto#L113. This means that it is not possible to use a v2 tcp filter config with a deprecated_v1 field in it.

So for example, this config will fail:

- address:
    socket_address: {address: 0.0.0.0, port_value: 9414, protocol: TCP}
  filter_chains:
  - filters:
    - name: envoy.tcp_proxy 
       config:
        deprecated_v1: {cluster: foo}
        stat_prefix: foo
@junr03 junr03 self-assigned this Jan 24, 2018
@junr03
Copy link
Member Author

junr03 commented Jan 24, 2018

cc @danielhochman @mattklein123 related to what happened at lyft tonight.
cc @htuch

@mattklein123
Copy link
Member

@junr03 I don't really see us fixing this. To workaround this I think you can just use v1 JSON config for tcp_proxy inside the v2 deprecated_v1 holder, right? I think we should just do a doc warning to "fix" this issue.

@mattklein123 mattklein123 added this to the 1.6.0 milestone Jan 25, 2018
@junr03
Copy link
Member Author

junr03 commented Jan 25, 2018

ok, I will add the warning.

@mattklein123
Copy link
Member

Fixed

@CamiloGarciaLaRotta
Copy link

Just a heads up:

The latest commit to envoy/config/filter/README, which is pointed to in the Attention box in the TCP proxy documentation, removes the example on how to use the deprecated_v1 field.

Shikugawa pushed a commit to Shikugawa/envoy that referenced this issue Mar 28, 2020
jpsim added a commit that referenced this issue Nov 28, 2022
Continued from #2441.

Description: Parallels Android implementation based on SharedPreferences.
Risk Level: Low
Testing: Application

Co-authored-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim added a commit that referenced this issue Nov 29, 2022
Continued from #2441.

Description: Parallels Android implementation based on SharedPreferences.
Risk Level: Low
Testing: Application

Co-authored-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants