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) Support many tags in Ingress #842

Merged
merged 2 commits into from
Jun 23, 2020

Conversation

jakubdyszkiewicz
Copy link
Contributor

Summary

This PR introduces support for many tag in Ingress - meaning you can apply TrafficRoute like this

type: TrafficRoute
mesh: default
name: tr-1
sources:
  - match:
      service: gateway
destinations:
  - match:
      service: backend
conf:
  - weight: 80
    destination:
      service: backend
      cluster: "1"
  - weight: 20
    destination:
      service: backend
      cluster: "2"

or this (when version: 2 is in different cluster)

type: TrafficRoute
mesh: default
name: tr-1
sources:
  - match:
      service: gateway
destinations:
  - match:
      service: backend
conf:
  - weight: 100
    destination:
      service: backend
      version: "2"

Documentation

Will be a part of Multi Cluster docs/demo. From user perspective is just a regular TrafficRoute

@jakubdyszkiewicz jakubdyszkiewicz requested a review from a team June 19, 2020 13:04
Copy link
Contributor

@nickolaev nickolaev left a comment

Choose a reason for hiding this comment

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

Looks OK to me.

}
if tlsContext != nil {
pbst, err := proto.MarshalAnyDeterministic(tlsContext)
// there might be a situation when there are multiple sam tags passed here for example two outbound listeners with the same tags, therefore we need to distinct them.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "we need to distinguish between them"

Copy link
Contributor

@lobkovilya lobkovilya left a comment

Choose a reason for hiding this comment

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

I have really stingy review :) Overall looks great, very seamless experience for the user that utilizes Ingress!

}

func TagsFromSNI(sni string) map[string]string {
r := regexp.MustCompile(`(.*)\{(.*)\}`)
Copy link
Contributor

Choose a reason for hiding this comment

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

I overlooked that during my PR, it makes sense to extract regex to global scope

}

func DistinctTags(tags []Tags) []Tags {
if len(tags) == 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this check since we iterate over tags in loop

func SNIFromTags(tags envoy.Tags) string {
nonServiceTags := tags.WithoutTag(mesh_proto.ServiceTag)
var pairs []string
for _, key := range nonServiceTags.Keys() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we reuse tags.String() method here?

@jakubdyszkiewicz jakubdyszkiewicz merged commit a9f0a0f into master Jun 23, 2020
@jakubdyszkiewicz jakubdyszkiewicz deleted the feat/ingress-many-tags branch June 23, 2020 13:37
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