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

*: add test for online reload new TLS certificates #2162

Merged
merged 5 commits into from
Feb 27, 2020

Conversation

nolouch
Copy link
Contributor

@nolouch nolouch commented Feb 27, 2020

Signed-off-by: nolouch nolouch@gmail.com

What problem does this PR solve?

ref pingcap/tidb#14666, #2112
If the key/cert expired, we need to reload it online. the method same as etcd, it is already supported, this PR is to add a test.

What is changed and how it works?

add test verify it.

Check List

Tests

  • Unit test

Signed-off-by: nolouch <nolouch@gmail.com>
@nolouch nolouch added component/testing CI testing. component/client Client logic. labels Feb 27, 2020
Signed-off-by: nolouch <nolouch@gmail.com>
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@9e2a27d). Click here to learn what that means.
The diff coverage is 62.06%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2162   +/-   ##
=========================================
  Coverage          ?   76.14%           
=========================================
  Files             ?      195           
  Lines             ?    20604           
  Branches          ?        0           
=========================================
  Hits              ?    15689           
  Misses            ?     3726           
  Partials          ?     1189
Impacted Files Coverage Δ
server/api/config.go 56.75% <0%> (ø)
server/schedule/checker/rule_checker.go 76.07% <100%> (ø)
server/config/config.go 85.21% <33.33%> (ø)
server/server.go 79.7% <66.66%> (ø)
server/config_manager/config_manager.go 76.63% <75%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e2a27d...d1345bf. Read the comment docs.

@nolouch nolouch added the needs-cherry-pick-release-3.1 The PR needs to cherry pick to release-3.1 branch. label Feb 27, 2020
@@ -260,7 +260,7 @@ func (c *baseClient) getOrCreateGRPCConn(addr string) (*grpc.ClientConn, error)
tlsCfg, err := grpcutil.SecurityConfig{
CAPath: c.security.CAPath,
CertPath: c.security.CertPath,
KeyPath: c.security.CertPath,
KeyPath: c.security.KeyPath,
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a bug :(

tests/client/client_tsl_test.go Outdated Show resolved Hide resolved
@nolouch nolouch changed the title *: add test for online reload of TLS *: add test for online reload new TLS certificates Feb 27, 2020
Copy link
Member

@rleungx rleungx left a comment

Choose a reason for hiding this comment

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

LGTM

@disksing
Copy link
Contributor

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 27, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Feb 27, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Feb 27, 2020

cherry pick to release-3.1 in PR #2165

@nolouch nolouch deleted the tls-test branch February 27, 2020 09:55
nolouch pushed a commit that referenced this pull request Feb 27, 2020
* *: add test for online reload of TLS

Signed-off-by: nolouch <nolouch@gmail.com>
july2993 added a commit to pingcap/tidb-binlog that referenced this pull request Mar 15, 2020
we can make sure the version after this tikv/pd#2162 can support reload new TLS certificates.

we just pass the file path into the client, the reload is supported by the client internally.
https://github.com/pingcap/pd/blob/0f3272c9b0ef661bdaea7f4ed66b13955c503b72/client/client.go#L129
@nolouch nolouch added needs-cherry-pick-release-3.0 The PR needs to cherry pick to release-3.0 branch. and removed needs-cherry-pick-release-3.1 The PR needs to cherry pick to release-3.1 branch. labels Mar 17, 2020
@nolouch
Copy link
Contributor Author

nolouch commented Mar 17, 2020

/run-cherry-picker

@sre-bot
Copy link
Contributor

sre-bot commented Mar 17, 2020

cherry pick to release-3.0 failed

nolouch pushed a commit to nolouch/pd that referenced this pull request Mar 18, 2020
…2165)

* *: add test for online reload of TLS

Signed-off-by: nolouch <nolouch@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/client Client logic. component/testing CI testing. needs-cherry-pick-release-3.0 The PR needs to cherry pick to release-3.0 branch. status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants