Skip to content

feat: support TLS connection with etcd.#2548

Merged
membphis merged 5 commits into
apache:masterfrom
tokers:feature/etcd_tls_support
Nov 3, 2020
Merged

feat: support TLS connection with etcd.#2548
membphis merged 5 commits into
apache:masterfrom
tokers:feature/etcd_tls_support

Conversation

@tokers

@tokers tokers commented Oct 28, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Support the TLS connection when communicating with etcd cluster. We added a configuration item to custom the certificate verification. Whether to setup TLS connection or not depends on the endpoints' scheme, for instance, when endpoints are:

etcd:
  host:
    - "https://127.0.0.1:2379"
    - "https://127.0.0.1:3379"

APISIX will originate TLS connection automatically, and the Server Name Indication extention will be set by the endpoint host (127.0.0.1 in above case). Note by default APISIX will verify the certificate, close the verification in configuration explicitly if you want to bypass it.

etcd:
  tls:
    verfiy: false

Commits will be rebased after the changes are LGTM.

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible?

@membphis

membphis commented Oct 28, 2020

Copy link
Copy Markdown
Member

missing test case ^_^

@tokers

tokers commented Oct 28, 2020 via email

Copy link
Copy Markdown
Contributor Author

@spacewander spacewander mentioned this pull request Oct 29, 2020
Closed
@Miss-you

Miss-you commented Oct 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

I think we can also use the etcd proxy localily as an interim solution

[Apache APISIX -> etcd proxy(cert) ]--http mTLS-->[etcd]

juzhiyuan
juzhiyuan reviewed Oct 29, 2020
View reviewed changes
Comment thread apisix/core/config_etcd.lua Outdated
@moonming

moonming commented Oct 29, 2020

Copy link
Copy Markdown
Member

I think we can also use the etcd proxy localily as an interim solution

[Apache APISIX -> etcd proxy(cert) ]--http mTLS-->[etcd]

Do you means add sidecar for etcd proxy(cert)?

@tokers
tokers force-pushed the feature/etcd_tls_support branch 2 times, most recently from 8911931 to 2084efa Compare October 30, 2020 02:52
@tokers

tokers commented Oct 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I think we can also use the etcd proxy localily as an interim solution
[Apache APISIX -> etcd proxy(cert) ]--http mTLS-->[etcd]

Do you means add sidecar for etcd proxy(cert)?

I think not the sidecar but a "central bus", a bunch of etcd proxy instances to delegate the backend etcd cluster.
Actually it loses the real effect to use client certificate auth, since the proxy can be accessed by any users.

@tokers
tokers force-pushed the feature/etcd_tls_support branch from 2084efa to d3cd815 Compare November 2, 2020 03:09
@tokers

tokers commented Nov 2, 2020

Copy link
Copy Markdown
Contributor Author

Will add test cases after the corresponding pr in lua-resty-etcd is merged.
…
-- Zhang Chao
On 2020年10月28日 at 22:32:05, YuanSheng Wang @.***) wrote: missing test case ^_^ — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2548 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPR7LPLLIBF5UR7XRALZLDSNATOLANCNFSM4TB6Q56Q .

I have added some test cases.

@tokers
tokers force-pushed the feature/etcd_tls_support branch from d3cd815 to d647e90 Compare November 2, 2020 03:28
@juzhiyuan

juzhiyuan commented Nov 2, 2020

Copy link
Copy Markdown
Member

We'd better keep far away from git force push if possible because this action will flush all review histories.

@tokers

tokers commented Nov 2, 2020

Copy link
Copy Markdown
Contributor Author

We'd better keep far away from git force push if possible because this action will flush all review histories.

OK.

@tokers
tokers force-pushed the feature/etcd_tls_support branch from 90936ed to d7960b1 Compare November 2, 2020 09:14
membphis
membphis reviewed Nov 2, 2020
View reviewed changes

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we need more test cases.
After enabling TLS, confirm that we can successfully set/get route.

@tokers

tokers commented Nov 2, 2020

Copy link
Copy Markdown
Contributor Author

we need more test cases.
After enabling TLS, confirm that we can successfully set/get route.

Fair enough.

@tokers

tokers commented Nov 3, 2020

Copy link
Copy Markdown
Contributor Author

we need more test cases.
After enabling TLS, confirm that we can successfully set/get route.

@membphis Cases supplemented

@membphis
membphis merged commit 5191374 into apache:master Nov 3, 2020
@membphis

membphis commented Nov 3, 2020

Copy link
Copy Markdown
Member

@tokers merged, many thx for your contribution.

@tokers
tokers deleted the feature/etcd_tls_support branch November 3, 2020 09:36
@moonming moonming added this to the 2.1 milestone Nov 21, 2020
@moonming moonming mentioned this pull request Nov 29, 2020
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juzhiyuan juzhiyuan juzhiyuan left review comments
@spacewander spacewander spacewander approved these changes
@membphis membphis membphis approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.1

Development

Successfully merging this pull request may close these issues.

6 participants

@tokers @membphis @Miss-you @moonming @juzhiyuan @spacewander