filter: added reverse tunnel terminal network filter #41013
Conversation
agrawroh
force-pushed
the
reverse-tunnel-filter
branch
19 times, most recently
from
82b3136 to
b588dde
Compare
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
agrawroh
changed the title
agrawroh
force-pushed
the
reverse-tunnel-filter
branch
5 times, most recently
from
718d66b to
54c7927
Compare
|
CC @envoyproxy/coverage-shephards: FYI only for changes made to |
agrawroh
force-pushed
the
reverse-tunnel-filter
branch
from
d004988 to
fe267f8
Compare
agrawroh
assigned
yanavlasov and
botengyao and unassigned
adisuissa and
RyanTheOptimist
|
LGTM from me. Will wait for @adisuissa API review and then submit. /wait-any |
source/extensions/filters/network/reverse_tunnel/reverse_tunnel_filter.cc
Outdated
Show resolved
Hide resolved
source/extensions/bootstrap/reverse_tunnel/downstream_socket_interface/rc_connection_wrapper.cc
Outdated
Show resolved
Hide resolved
...ensions/bootstrap/reverse_tunnel/downstream_socket_interface/reverse_connection_io_handle.cc
Outdated
Show resolved
Hide resolved
api/envoy/extensions/filters/network/reverse_tunnel/v3/reverse_tunnel.proto
Outdated
Show resolved
Hide resolved
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
api/envoy/extensions/filters/network/reverse_tunnel/v3/reverse_tunnel.proto
Outdated
Show resolved
Hide resolved
| // Previous filters in the network chain can populate the connection's filter state | ||
| // with validation data (e.g., extracted from client certificates or authentication tokens) | ||
| // that this filter will use to validate incoming reverse tunnel requests. | ||
| message ValidationConfig { |
There was a problem hiding this comment.
This seems to have some assumptions, which I'm not sure if they are true. Specifically:
- Will there only be a single node/cluster/tenant_id_filter_state_key?
- AFAICT each of these defines a filter-state-key that must (although maybe it is only optional) be present in the filter-state, and that the value will match a node_id/cluster_id/tenant_id. Will it make sense that non-exat matching will be desired (e.g., prefix)?
- It is unclear if these are all required together, or if one is valid then it makes the reverse tunnel connection valid.
More generally will it make sense to add a generic matcher here?
There was a problem hiding this comment.
Thanks for the feedback @adisuissa. I think this part need some more consideration. I'll remove the validation piece for now as it's not blocking and will submit it as a follow-up.
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
|
@yanavlasov @botengyao Could one of you please re-stamp this? |
Description
This PR adds a new terminal network filter which accepts or rejects the incoming reverse connection requests sent from the reverse tunnels downstream connection interface and optionally validating the Node ID and Cluster ID values that come as part of the handshake protocol using the Envoy filter state metadata. Filter state could be populated by other network filters like "Set-Filter State", etc.
This new filter could be configured with the type URL
type.googleapis.com/envoy.extensions.filters.network.reverse_tunnel.v3.ReverseTunnel.Commit Message: filter: added reverse tunnel terminal network filter
Additional Description: Adds a new terminal network filter for accepting the incoming reverse tunnel handshake requests from the downstream instances.
Risk Level: Low
Testing: Added Unit & Integration Tests
Docs Changes: Added
Release Notes: Added