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: Support SNI based TLS Route #547

Closed
Tracked by #1309
tokers opened this issue Jun 17, 2021 · 16 comments · Fixed by #1051
Closed
Tracked by #1309

feat: Support SNI based TLS Route #547

tokers opened this issue Jun 17, 2021 · 16 comments · Fixed by #1051
Assignees
Labels
enhancement New feature or request triage/accepted Indicates an issue or PR is ready to be actively worked on.
Milestone

Comments

@tokers
Copy link
Contributor

tokers commented Jun 17, 2021

Now Apache APISIX supports to match the stream route with TLS SNI (apache/apisix#4433), we can implement it in ApisixRoute.

@tao12345666333 tao12345666333 changed the title Support SNI based TLS Route feat: Support SNI based TLS Route Mar 23, 2022
@tao12345666333
Copy link
Member

LGTM

@tao12345666333 tao12345666333 added enhancement New feature or request triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Mar 23, 2022
@svendberg
Copy link

Any ETA for this? At the moment we have to manually call the API and set SNI for stream routes to support TLS. It would be nice to use the ApisixRoute CRD without having to manually update the streamroute with SNI afterwards.

@tao12345666333
Copy link
Member

It will be added in the next version v1.5 at the earliest, and v1.6 at the latest.

@tao12345666333
Copy link
Member

@svendberg Is this feature of APISIX currently used in your production environment?

@svendberg
Copy link

svendberg commented May 10, 2022

We use it in production to expose MQTT endpoints ( TCP ) with TLS for a multi-tenant solution. Right now, we use an ApisixRoute to create the upstream and a "dummy" stream_route in Apisix. Then we use the Apisix API to create a new stream_route with an SNI defined, reusing the upstream created by ApisixRoute.

@mangoGoForward
Copy link
Contributor

WeOpen Star

I'd like to work on this issue.

@tao12345666333
Copy link
Member

thanks @mangoGoForward
Assigned

@mangoGoForward
Copy link
Contributor

Hi @tao12345666333 . On the implementation, I want to confirm a few things:

  1. We should add sni in types.
    type StreamRoute struct {
    // TODO metadata should use Metadata type
    ID string `json:"id,omitempty" yaml:"id,omitempty"`
    Desc string `json:"desc,omitempty" yaml:"desc,omitempty"`
    Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"`
    ServerPort int32 `json:"server_port,omitempty" yaml:"server_port,omitempty"`
    UpstreamId string `json:"upstream_id,omitempty" yaml:"upstream_id,omitempty"`
    Upstream *Upstream `json:"upstream,omitempty" yaml:"upstream,omitempty"`
    }
  2. When ApisixRoute translate to StreamRoute, we should add sni to StreamRoute instance?
    sr.ServerPort = part.Match.IngressPort

    then create or update it.
    But I am confused about that which field in ApisixRoute we can use.

@tokers
Copy link
Contributor Author

tokers commented May 18, 2022

@mangoGoForward What about just using Host or SNI?

@tao12345666333
Copy link
Member

I prefer Host

@mangoGoForward
Copy link
Contributor

I prefer Host

But seems ApisixRouteSpec.Stream doesn't have this property? If I'm missed the meaning, please correct me, thanks.

@tao12345666333
Copy link
Member

@mangoGoForward sorry for delay.

But seems ApisixRouteSpec.Stream doesn't have this property?

yes, since it's a new feature, we can add it

@tokers
Copy link
Contributor Author

tokers commented May 25, 2022

I prefer Host

But seems ApisixRouteSpec.Stream doesn't have this property? If I'm missed the meaning, please correct me, thanks.

Yeah, that's the point that we need to implement it :).

@mangoGoForward
Copy link
Contributor

I have been commit a PR #1051, if you have free time, please have a review~

@tao12345666333
Copy link
Member

Thanks

@tao12345666333 tao12345666333 linked a pull request Jul 27, 2022 that will close this issue
8 tasks
@tao12345666333 tao12345666333 added this to the v1.6.0 milestone Jul 27, 2022
@tao12345666333 tao12345666333 moved this from Todo to In Progress in Apache APISIX Ingress controller Jul 27, 2022
Repository owner moved this from In Progress to Done in Apache APISIX Ingress controller Nov 4, 2022
@tao12345666333
Copy link
Member

@svendberg #1051 has been merged. Thanks @mangoGoForward

This feature will be released in v1.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

Successfully merging a pull request may close this issue.

4 participants