Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

envoy/lds: create inbound filter chains per service port #2140

Merged
merged 1 commit into from
Dec 3, 2020

Conversation

shashankram
Copy link
Member

@shashankram shashankram commented Dec 3, 2020

Description:
This change makes the inbound filter chains for in-mesh
and ingress a per port construct, so that protocol
specific filters and rules can be enforced per port.
Since protocol specific filters (ex. TCP proxy vs HTTP
Connection Manager) cannot be filters on the same filter
chain, a port specific filter chain is created to be
able to handle per port:protocol filtering.

Additionally, this change adds new tests for the ingress
filter chain and updates the existing tests for mesh filter
chains. The file lds/response_test.go was ingress specific
and has been rewritten to lds/ingress_test.go.

This is required for TCP filtering and routing: #1521

Affected area:

  • New Functionality [ ]
  • Documentation [ ]
  • Install [ ]
  • Control Plane [X]
  • CLI Tool [ ]
  • Certificate Management [ ]
  • Networking [X]
  • Metrics [ ]
  • SMI Policy [ ]
  • Security [ ]
  • Tests [ ]
  • CI System [ ]
  • Performance [ ]
  • Other [ ]

Please answer the following questions with yes/no.

  • Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?
    No

This change makes the inbound filter chains for in-mesh
and ingress a per port construct, so that protocol
specific filters and rules can be enforced per port.
Since protocol specific filters (ex. TCP proxy vs HTTP
Connection Manager) cannot be filter on the same filter
chain, a port specific filter chain is created to be
able to handle per port:protocol filtering.

Additionally, this change adds new tests for the ingress
filter chain and updates the existing tests for mesh filter
chains.

This is required for TCP filtering and routing: openservicemesh#1521

Signed-off-by: Shashank Ram <shashank08@gmail.com>
@shashankram shashankram requested a review from a team as a code owner December 3, 2020 22:54
@codecov-io
Copy link

Codecov Report

Merging #2140 (3a79df0) into main (16be519) will decrease coverage by 0.05%.
The diff coverage is 59.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2140      +/-   ##
==========================================
- Coverage   57.86%   57.81%   -0.06%     
==========================================
  Files         144      144              
  Lines        5995     6021      +26     
==========================================
+ Hits         3469     3481      +12     
- Misses       2523     2537      +14     
  Partials        3        3              
Impacted Files Coverage Δ
pkg/envoy/lds/response.go 7.27% <0.00%> (ø)
pkg/envoy/lds/inmesh.go 70.70% <27.77%> (-6.04%) ⬇️
pkg/envoy/lds/ingress.go 84.61% <82.60%> (-5.63%) ⬇️
pkg/envoy/lds/listener.go 31.46% <100.00%> (+1.57%) ⬆️
pkg/envoy/route/config.go 95.23% <0.00%> (-0.80%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16be519...3a79df0. Read the comment docs.

Copy link
Contributor

@eduser25 eduser25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just minors

pkg/envoy/lds/ingress.go Show resolved Hide resolved
@@ -57,27 +65,43 @@ func newIngressFilterChain(cfg configurator.Configurator, svc service.MeshServic
}
}

func getIngressFilterChains(svc service.MeshService, cfg configurator.Configurator) []*xds_listener.FilterChain {
func (lb *listenerBuilder) getIngressFilterChains(svc service.MeshService) []*xds_listener.FilterChain {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, unrelated to the commit, should it be ForService?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, fair enough.

pkg/envoy/lds/ingress.go Show resolved Hide resolved
@shashankram shashankram merged commit 8a6ed0f into openservicemesh:main Dec 3, 2020
@shashankram shashankram deleted the fc-per-port branch December 3, 2020 23:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants