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.

Sign up for GitHub

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

Jump to bottom

Separate TiDB client certificate can be used for each component #2403

Merged
merged 3 commits into from
May 9, 2020
Merged

Separate TiDB client certificate can be used for each component #2403

merged 3 commits into from
May 9, 2020

Conversation

weekface
Copy link
Contributor

@weekface weekface commented May 8, 2020 •
edited
Loading

What problem does this PR solve?

fixes: #2384

What is changed and how does it work?

Check List

Tests

  • Unit test
  • E2E test
  • Stability test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

  • Has Go code change
  • Has CI related scripts change
  • Has Terraform scripts change

Side effects

  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Does this PR introduce a user-facing change?:

Separate TiDB client certificate can be used for each component

ACTION_REQUIRED: Users should migrate the old TLS configs of Backup and Restore to the new configs.

TLS configs in old Backup:

apiVersion: pingcap.com/v1alpha1
kind: Backup
spec:
  from:
    tlsClient:
      secretName: <secretName>

TLS configs in new Backup:

apiVersion: pingcap.com/v1alpha1
kind: Backup
spec:
  from:
    tlsClientSecretName: <secretName>

TLS configs in old Restore:

apiVersion: pingcap.com/v1alpha1
kind: Restore
spec:
  to:
    tlsClient:
      secretName: <secretName>

TLS configs in new Restore:

apiVersion: pingcap.com/v1alpha1
kind: Restore
spec:
  to:
    tlsClientSecretName: <secretName>

@weekface weekface changed the title Can use separate TiDB client certificate for each component Separate TiDB client certificate can be used for each component May 8, 2020
This was referenced May 8, 2020
Closed
Closed
DanielZhangQD
DanielZhangQD reviewed May 8, 2020
View reviewed changes
pkg/backup/backup/backup_manager.go
@@ -318,8 +318,8 @@ func (bm *backupManager) makeBackupJob(backup *v1alpha1.Backup) (*batchv1.Job, s
if tc.Spec.TiDB.TLSClient != nil && tc.Spec.TiDB.TLSClient.Enabled && !tc.SkipTLSWhenConnectTiDB() {
args = append(args, "--client-tls=true")
clientSecretName := util.TiDBClientTLSSecretName(backup.Spec.BR.Cluster)
if backup.Spec.From.TLSClient != nil && backup.Spec.From.TLSClient.TLSSecret != "" {
clientSecretName = backup.Spec.From.TLSClient.TLSSecret
if backup.Spec.From.TLSClientSecretName != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also check the length of *backup.Spec.From.TLSClientSecretName?

Copy link
Contributor

Choose a reason for hiding this comment

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

The same comment apply for the code in the other files.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to check it here.

DanielZhangQD
DanielZhangQD approved these changes May 9, 2020
View reviewed changes
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

shuijing198799
shuijing198799 approved these changes May 9, 2020
View reviewed changes
Copy link
Contributor

@shuijing198799 shuijing198799 left a comment

Choose a reason for hiding this comment

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

LGTM

@weekface
Copy link
Contributor Author

weekface commented May 9, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented May 9, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented May 9, 2020

@weekface merge failed.

@weekface
Copy link
Contributor Author

weekface commented May 9, 2020

/run-e2e-tests

@weekface
Copy link
Contributor Author

weekface commented May 9, 2020

/merge

@sre-bot
Copy link
Contributor

sre-bot commented May 9, 2020

/run-all-tests

@sre-bot sre-bot merged commit 35047c4 into pingcap:master May 9, 2020
@DanielZhangQD
Copy link
Contributor

/run-cherry-picker

sre-bot pushed a commit to sre-bot/tidb-operator that referenced this pull request May 13, 2020
@weekface @sre-bot
Separate TiDB client certificate can be used for each component (ping…
5b633bb 
…cap#2403)
@sre-bot sre-bot mentioned this pull request May 13, 2020
Merged
@sre-bot
Copy link
Contributor

sre-bot commented May 13, 2020

cherry pick to release-1.1 in PR #2450

sre-bot added a commit that referenced this pull request May 13, 2020
@sre-bot
Separate TiDB client certificate can be used for each component (#2403)…
9986ea2 
… (#2450)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shuijing198799 shuijing198799 shuijing198799 approved these changes

@DanielZhangQD DanielZhangQD DanielZhangQD approved these changes

@Yisaer Yisaer Awaiting requested review from Yisaer

@cofyc cofyc Awaiting requested review from cofyc

Assignees
No one assigned
Labels
CanMerge needs-cherry-pick-1.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Separate TLS certificates for TiDB clients.
4 participants
@weekface @sre-bot @DanielZhangQD @shuijing198799