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

chore(kuma-cp) generify proxy template matching #588

Merged
merged 4 commits into from
Feb 21, 2020

Conversation

jakubdyszkiewicz
Copy link
Contributor

@jakubdyszkiewicz jakubdyszkiewicz commented Feb 18, 2020

Summary

Introduce DataplanePolicy and generify matching for ProxyTemplate. This enables us to reuse the same logic for TrafficTrace as well as soon to be refactored TrafficLog

@jakubdyszkiewicz jakubdyszkiewicz requested review from yskopets and a team February 18, 2020 10:49
@jakubdyszkiewicz jakubdyszkiewicz force-pushed the feature/proxy-template-generify-matcher branch from eb307ad to 2733c5b Compare February 19, 2020 08:03
Copy link
Contributor

@yskopets yskopets left a comment

Choose a reason for hiding this comment

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

If not part of this PR, let's open a separate issue to refactor matching algorithm to be consistent with SelectConnectionPolicies(), namely

  • ranking should distinguish between exact matches and wildcard ('*') matches

pkg/core/policy/dataplane_matcher.go Outdated Show resolved Hide resolved
@jakubdyszkiewicz
Copy link
Contributor Author

Refactored to support * and gateways. Please take a look.

// when
actual := policy.SelectDataplanePolicy(given.proxy.Dataplane, given.policies)
// then
if given.expected == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't Expect(actual).To(Equal(given.expected)) be enough for both cases ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently it is not

      Refusing to compare <nil> to <nil>.
      Be explicit and use BeNil() instead.  This is to avoid mistakes where both sides of an assertion are erroneously uninitialized.

Spec: mesh_proto.Dataplane{
Networking: &mesh_proto.Dataplane_Networking{
Inbound: []*mesh_proto.Dataplane_Networking_Inbound{
{},
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we want to test against invalid inbound listeners ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was in the previous test, but I agree - it doesn't make sense, deleted.

@jakubdyszkiewicz jakubdyszkiewicz merged commit 76199fa into master Feb 21, 2020
@jakubdyszkiewicz jakubdyszkiewicz deleted the feature/proxy-template-generify-matcher branch February 28, 2020 08:55
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

2 participants