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

feat(kuma-cp) generate filterChainMatchers based on TrafficRoutes #1294

Merged
merged 9 commits into from
Dec 14, 2020

Conversation

lobkovilya
Copy link
Contributor

Signed-off-by: Ilya Lobkov ilya.lobkov@konghq.com

Summary

In order to support TrafficRoutes in Ingress level, we used to generate FilterChainMatch for all possibles combinations of tags. It's not efficient and might cause a really big Envoy config even if there are not so many tags.

The current implementation relies on the TrafficRoute policy. We collect all destinations from all TrafficRoutes and generate FilterChainMatch only for them. SelectorSubsets are also generated now based on TrafficRoutes. So we don't need permutation package anymore.

Documentation

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
@lobkovilya lobkovilya requested a review from a team as a code owner December 11, 2020 12:21
lobkovilya and others added 4 commits December 11, 2020 19:26
Signed-off-by: Nikolay Nikolaev <nikolay.nikolaev@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz left a comment

Choose a reason for hiding this comment

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

looks solid. Can we comment somewhere that multiple tags in outbound does not work?

pkg/xds/generator/ingress_generator.go Outdated Show resolved Hide resolved
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
destination := destination.
WithoutTag(mesh_proto.ServiceTag).
WithTags("mesh", inbound.GetMesh())
sni := tls.SNIFromServiceAndTags(service, destination)
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO instead of introducing

SNIFromServiceAndTags

I'd keep the tls. API simple and do

meshDestination := destination.WithTags("mesh", inbound.GetMesh())
sni := tls.SNIFromTags(meshDestination)
...
Tags:        meshDestination.WithoutTag(mesh_proto.ServiceTag),

overall - as a rule of thumb I always double check the code when I'm introducing method with And if I can do it better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I did it at first, but then there is a problem - destination can have kuma.io/service: '*' and in this case, we should take service from the service variable

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>

# Conflicts:
#	go.sum
@lobkovilya lobkovilya merged commit ea214a4 into master Dec 14, 2020
@lobkovilya lobkovilya deleted the feat/ingress-config-opt branch December 14, 2020 10:32
mergify bot pushed a commit that referenced this pull request Dec 14, 2020
)

(cherry picked from commit ea214a4)

# Conflicts:
#	go.mod
#	go.sum
nickolaev pushed a commit that referenced this pull request Dec 14, 2020
…1294) (#1314)

* feat(kuma-cp) generate filterChainMatchers based on TrafficRoutes (#1294)

(cherry picked from commit ea214a4)

Signed-off-by: Ilya Lobkov <ilya.lobkov@konghq.com>

Co-authored-by: Ilya Lobkov <ilya.lobkov@konghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants