api: mark internal listener as implemented and change internal listener binding #22274
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,15 +32,20 @@ message Pipe { | |
| } | ||
|
|
||
| // The address represents an envoy internal listener. | ||
| // [#comment: TODO(asraa): When address available, remove workaround from test/server/server_fuzz_test.cc:30.] | ||
| message EnvoyInternalAddress { | ||
| oneof address_name_specifier { | ||
| option (validate.required) = true; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Please keep the comment describing the field.
Contributor
Author
There was a problem hiding this comment. This comment is not true, address has nothing to do with the listener name.
Contributor
There was a problem hiding this comment. IIUC the value here needs to point to another listener.
Member
There was a problem hiding this comment. seems like it changed what the value expect to from the beginning. Maybe worth to ask again whether this field still name correctly, this can be a chance to change the name before expose to the end user.
Contributor
Author
There was a problem hiding this comment. Yes, I agree we need to fix this. I think we should use a dedicated field in InternalListener instead of internal address in the generic
Contributor
Author
There was a problem hiding this comment. The simplest thing is to just really use the listener name. That keeps LDS to a minimum and I can't think of any reason why more flexibility would be needed here (multiple addresses don't work anyways, but can always be added to
Member
There was a problem hiding this comment. I'm not sure the usecase of using multiple internal addresses, it seems useless. I guess the multiple internal addresses usecase is internal address mix with IP address. From this point of view, just using the listener name should be fine. I little prefer just use a string for the internal address, needn't limit that to the listener name. The only reason I have is a soft reason, it seems more straightforward for me when configuring the internal address, just matching the upstream endpoint internal address with the listener internal address. And we needn't add a lot of checks in the code like the user can't specific addresses when this is internal listener...etc. But anyway those are soft reasons.
Member
There was a problem hiding this comment.
suddenly found this may be the multiple internal addresses usecase https://github.com/envoyproxy/envoy/pull/22274/files#r935253240 |
||
| // Specifies the :ref:`name <envoy_v3_api_field_config.listener.v3.Listener.name>` of the | ||
| // internal listener. | ||
| string server_listener_name = 1; | ||
| } | ||
|
|
||
| // Specifies an endpoint identifier to distinguish between multiple endpoints for the same internal listener in a | ||
| // single upstream pool. Only used in the upstream addresses for tracking changes to individual endpoints. This, for | ||
| // example, may be set to the final destination IP for the target internal listener. | ||
| string endpoint_id = 2; | ||
|
Member
There was a problem hiding this comment. Is this kind of multiple internal addresses? if yes, then another option is just to support multiple internal addresses here, then you needn't add
Member
There was a problem hiding this comment. after looking at another PR, multiple addresses will bind the number of listener addresses and the number of endpoints together. That means when you add more endpoints to the same internal listener, then you have to update the internal listener with the new internal addresses also, I guess that isn't what people want.
Contributor
Author
There was a problem hiding this comment. Correct, we want to maximize reuse of the internal listeners - e.g. tunneling shouldn't be coupled to the origin of the traffic. So that implies a possibility of multiple endpoints to the same internal listener in one pool. Upstream de-duplicates endpoints by computing
Contributor
There was a problem hiding this comment. Is there any typical value of endpoint_id? e.g. the actually L4 address?
Contributor
Author
There was a problem hiding this comment. I think it'd be confusing to say "L4 address should be here". We don't say a listener corresponds to VIP, or a cluster corresponds to a VIP. That's too prescriptive. I will add an example of the usage. |
||
| } | ||
|
|
||
| // [#next-free-field: 7] | ||
|
|
@@ -143,7 +148,8 @@ message Address { | |
|
|
||
| Pipe pipe = 2; | ||
|
|
||
kyessenov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Specifies a user-space address handled by :ref:`internal listeners | ||
| // <envoy_v3_api_field_config.listener.v3.Listener.internal_listener>`. | ||
| EnvoyInternalAddress envoy_internal_address = 3; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| # This configuration takes incoming HTTP requests on port 10000 and encapsulates it in a CONNECT | ||
| # request which is sent upstream port 10001. | ||
| bootstrap_extensions: | ||
| - name: envoy.bootstrap.internal_listener | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.bootstrap.internal_listener.v3.InternalListener | ||
| static_resources: | ||
| listeners: | ||
| - name: http | ||
| address: | ||
| socket_address: | ||
| protocol: TCP | ||
| address: 127.0.0.1 | ||
| port_value: 10000 | ||
| filter_chains: | ||
| - filters: | ||
| - name: envoy.filters.network.http_connection_manager | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager | ||
| stat_prefix: ingress_http | ||
| route_config: | ||
| name: local_route | ||
| virtual_hosts: | ||
| - name: local_service | ||
| domains: ["*"] | ||
| routes: | ||
| - match: | ||
| prefix: "/" | ||
| route: | ||
| cluster: encap_cluster | ||
| http_filters: | ||
| - name: envoy.filters.http.router | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router | ||
| - name: encap | ||
| internal_listener: {} | ||
| filter_chains: | ||
| - filters: | ||
| - name: tcp | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy | ||
| stat_prefix: tcp_stats | ||
| cluster: cluster_0 | ||
| tunneling_config: | ||
| hostname: host.com:443 | ||
| clusters: | ||
| - name: encap_cluster | ||
| load_assignment: | ||
| cluster_name: encap_cluster | ||
| endpoints: | ||
| - lb_endpoints: | ||
| - endpoint: | ||
| address: | ||
| envoy_internal_address: | ||
| server_listener_name: encap | ||
| - name: cluster_0 | ||
| # This ensures HTTP/2 CONNECT is used for establishing the tunnel. | ||
| typed_extension_protocol_options: | ||
| envoy.extensions.upstreams.http.v3.HttpProtocolOptions: | ||
| "@type": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions | ||
| explicit_http_config: | ||
| http2_protocol_options: {} | ||
| load_assignment: | ||
| cluster_name: cluster_0 | ||
| endpoints: | ||
| - lb_endpoints: | ||
| - endpoint: | ||
| address: | ||
| socket_address: | ||
| address: 127.0.0.1 | ||
| port_value: 10001 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| # This configuration listens on port 9999 and creates TCP connections to port 10000 using an | ||
| # intermediate internal listener. | ||
| bootstrap_extensions: | ||
| - name: envoy.bootstrap.internal_listener | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.bootstrap.internal_listener.v3.InternalListener | ||
| static_resources: | ||
| listeners: | ||
| - name: ingress | ||
| address: | ||
| socket_address: | ||
| protocol: TCP | ||
| address: 127.0.0.1 | ||
| port_value: 9999 | ||
| filter_chains: | ||
| - filters: | ||
| - name: tcp | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy | ||
| stat_prefix: ingress | ||
| cluster: encap_cluster | ||
| - name: encap | ||
| internal_listener: {} | ||
| filter_chains: | ||
| - filters: | ||
| - name: tcp | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy | ||
| stat_prefix: encap | ||
| cluster: cluster_0 | ||
| clusters: | ||
| - name: encap_cluster | ||
| load_assignment: | ||
| cluster_name: encap_cluster | ||
| endpoints: | ||
| - lb_endpoints: | ||
| - endpoint: | ||
| address: | ||
| envoy_internal_address: | ||
| server_listener_name: encap | ||
| - name: cluster_0 | ||
| load_assignment: | ||
| cluster_name: cluster_0 | ||
| endpoints: | ||
| - lb_endpoints: | ||
| - endpoint: | ||
| address: | ||
| socket_address: | ||
| address: 127.0.0.1 | ||
| port_value: 10000 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # This configuration terminates h2 CONNECT on port 10001 and then chains an HTTP filter that always responds with 200 using | ||
| # an internal listener. | ||
| bootstrap_extensions: | ||
| - name: envoy.bootstrap.internal_listener | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.bootstrap.internal_listener.v3.InternalListener | ||
| static_resources: | ||
| listeners: | ||
| - name: listener_0 | ||
| address: | ||
| socket_address: | ||
| protocol: TCP | ||
| address: 127.0.0.1 | ||
| port_value: 10001 | ||
| filter_chains: | ||
| - filters: | ||
| - name: envoy.filters.network.http_connection_manager | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager | ||
| stat_prefix: ingress_http | ||
| route_config: | ||
| name: local_route | ||
| virtual_hosts: | ||
| - name: local_service | ||
| domains: | ||
| - "*" | ||
| routes: | ||
| - match: | ||
| connect_matcher: | ||
| {} | ||
| route: | ||
| cluster: decap_cluster | ||
| upgrade_configs: | ||
| - upgrade_type: CONNECT | ||
| connect_config: | ||
| {} | ||
| http_filters: | ||
| - name: envoy.filters.http.router | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router | ||
| http2_protocol_options: | ||
| allow_connect: true | ||
| upgrade_configs: | ||
| - upgrade_type: CONNECT | ||
| - name: decap | ||
| internal_listener: {} | ||
| filter_chains: | ||
| - filters: | ||
| - name: envoy.filters.network.http_connection_manager | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager | ||
| stat_prefix: ingress_http | ||
| route_config: | ||
| name: local_route | ||
| virtual_hosts: | ||
| - name: local_service | ||
| domains: ["*"] | ||
| routes: | ||
| - match: | ||
| prefix: "/" | ||
| direct_response: | ||
| status: 200 | ||
| body: | ||
| inline_string: "Hello, world!\n" | ||
| http_filters: | ||
| - name: envoy.filters.http.router | ||
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router | ||
| clusters: | ||
| - name: decap_cluster | ||
| load_assignment: | ||
| cluster_name: decap_cluster | ||
| endpoints: | ||
| - lb_endpoints: | ||
| - endpoint: | ||
| address: | ||
| envoy_internal_address: | ||
| server_listener_name: decap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe worth to note that this is only used for upstream endpoint address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.