Omitting deprecated `"users.*"` settings for TiFlash by JaySon-Huang · Pull Request #1211 · pingcap/tiup

JaySon-Huang · 2021-03-15T09:02:49Z

Signed-off-by: JaySon-Huang tshent@qq.com

What problem does this PR solve?

A part of pingcap/tiflash#1497.
TiFlash PRs on 5.0.x and 4.0.x: pingcap/tiflash#1526, pingcap/tiflash#1535

Some access control settings are inherited from Clickhouse and useless for TiFlash, we make these settings to be the default value if empty since v4.0.12 and v5.0.0. Some settings (like password = "") make trouble for TiFlash users when their configuration files are scanned by a security scanner.

This PR make TiUP don't generate those deprecated settings for cluster version >= v4.0.12 and not euqals to v5.0.0-rc

What is changed and how it works?

Ignore the 'users.*' and 'quotas.*' settings in TiFlash if the cluster version >= v4.0.12 and not equals to v5.0.0-rc.
But keep those settings for version <= v4.0.11

Check List

Tests

Unit test

Code changes

Has persistent data change

Side effects

N/A

Related changes

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

Release notes:

Omitting deprecated `"users.*"` settings in the configuration file of TiFlash.

Signed-off-by: JaySon-Huang <tshent@qq.com>

codecov-io · 2021-03-15T09:08:44Z

Codecov Report

Merging #1211 (c1bbd33) into master (197a8a3) will increase coverage by 20.82%.
The diff coverage is 90.00%.

@@             Coverage Diff             @@
##           master    #1211       +/-   ##
===========================================
+ Coverage   26.24%   47.06%   +20.82%     
===========================================
  Files         263      254        -9     
  Lines       18678    17730      -948     
===========================================
+ Hits         4902     8345     +3443     
+ Misses      13023     8009     -5014     
- Partials      753     1376      +623

Flag	Coverage Δ
cluster	`45.00% <90.00%> (?)`
dm	`25.07% <0.00%> (?)`
integrate	`47.06% <90.00%> (+30.53%)`	⬆️
tiup	`?`
unittest	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/cluster/spec/tiflash.go	`56.37% <90.00%> (+4.63%)`	⬆️
pkg/repository/store/store.go	`0.00% <0.00%> (-100.00%)`	⬇️
components/dm/ansible/worker.go	`0.00% <0.00%> (-100.00%)`	⬇️
pkg/repository/utils/hash.go	`0.00% <0.00%> (-81.82%)`	⬇️
pkg/meta/err.go	`0.00% <0.00%> (-81.25%)`	⬇️
pkg/telemetry/meta.go	`0.00% <0.00%> (-80.00%)`	⬇️
pkg/telemetry/scrub.go	`6.66% <0.00%> (-80.00%)`	⬇️
pkg/cluster/api/error.go	`0.00% <0.00%> (-80.00%)`	⬇️
pkg/repository/store/local.go	`0.00% <0.00%> (-76.48%)`	⬇️
pkg/repository/model/publish.go	`0.00% <0.00%> (-71.43%)`	⬇️
... and 242 more

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 197a8a3...c1bbd33. Read the comment docs.

JaySon-Huang · 2021-03-15T09:19:00Z

~~Do not merge until we confirm that v4.0.12 will contain that commit in TiFlash.~~
Confirm that v4.0.12 will contains this commit pingcap/tiflash#1535

JaySon-Huang · 2021-03-18T10:06:39Z

@AstroProfundis PTAL

AstroProfundis · 2021-03-23T03:47:38Z

 		pathConfig = fmt.Sprintf(`path: "%s"`, cfg.DataDir)
 	}

+	if (semver.Compare(clusterVersion, "v4.0.12") >= 0 && semver.Compare(clusterVersion, "v5.0.0-rc") != 0) || clusterVersion == "nightly" {


It's better to use utils.Version(clusterVersion).IsNightly() to check for nightly versions, the actual version in topo will be expanded from nightly to a specific version at that date.

I push a commit to use utils.Version(clusterVersion).IsNightly() for TiFlash.

But I'm wondering should I also change this code in playground.go?

tiup/components/playground/playground.go

Line 688 in bd3cf97

if options.version != "nightly" {

I think so, it's better to change playground.go too.

…_tiflash_settings

Signed-off-by: JaySon-Huang <tshent@qq.com>

ti-chi-bot · 2021-03-23T06:33:15Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

AstroProfundis

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by writing /lgtm in a comment.
Reviewer can cancel approval by writing /lgtm cancel in a comment.

AstroProfundis · 2021-03-23T06:33:22Z

/merge

ti-chi-bot · 2021-03-23T06:33:24Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: c1bbd33

codecov-commenter · 2025-10-21T21:40:52Z

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.06%. Comparing base (197a8a3) to head (c1bbd33).
⚠️ Report is 670 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/cluster/spec/tiflash.go	90.00%	1 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #1211       +/-   ##
===========================================
+ Coverage   26.24%   47.06%   +20.82%     
===========================================
  Files         263      254        -9     
  Lines       18678    17730      -948     
===========================================
+ Hits         4902     8345     +3443     
+ Misses      13023     8009     -5014     
- Partials      753     1376      +623

Flag	Coverage Δ
cluster	`45.00% <90.00%> (?)`
dm	`25.07% <0.00%> (?)`
integrate	`47.06% <90.00%> (+30.53%)`	⬆️
tiup	`?`
unittest	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Remove deprecated settings for TiFlash

5230295

Signed-off-by: JaySon-Huang <tshent@qq.com>

ti-chi-bot requested a review from AstroProfundis March 15, 2021 09:02

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 15, 2021

JaySon-Huang mentioned this pull request Mar 15, 2021

The configuration item of password is empty in tiflash.toml pingcap/tiflash#1497

Closed

JaySon-Huang changed the title ~~Remove deprecated settings for TiFlash~~ Omitting deprecated "users.*" settings for TiFlash Mar 15, 2021

JaySon-Huang changed the title ~~Omitting deprecated "users.*" settings for TiFlash~~ [DNM] Omitting deprecated "users.*" settings for TiFlash Mar 15, 2021

JaySon-Huang changed the title ~~[DNM] Omitting deprecated "users.*" settings for TiFlash~~ Omitting deprecated "users.*" settings for TiFlash Mar 18, 2021

AstroProfundis reviewed Mar 23, 2021

View reviewed changes

JaySon-Huang added 3 commits March 23, 2021 11:57

Merge remote-tracking branch 'upstream/master' into remove_deprecated…

4bb1fd6

…_tiflash_settings

Use utils.Version(clusterVersion).IsNightly()

3de4b57

Signed-off-by: JaySon-Huang <tshent@qq.com>

Use utils.Version(clusterVersion).IsNightly() in playground.go

c1bbd33

Signed-off-by: JaySon-Huang <tshent@qq.com>

AstroProfundis approved these changes Mar 23, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 23, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 23, 2021

ti-chi-bot merged commit 4d5347d into pingcap:master Mar 23, 2021

JaySon-Huang deleted the remove_deprecated_tiflash_settings branch March 23, 2021 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Omitting deprecated `"users.*"` settings for TiFlash#1211

Omitting deprecated `"users.*"` settings for TiFlash#1211
ti-chi-bot merged 4 commits intopingcap:masterfrom
JaySon-Huang:remove_deprecated_tiflash_settings

JaySon-Huang commented Mar 15, 2021 •

edited

Loading

Uh oh!

codecov-io commented Mar 15, 2021 •

edited

Loading

Uh oh!

JaySon-Huang commented Mar 15, 2021 •

edited

Loading

Uh oh!

JaySon-Huang commented Mar 18, 2021

Uh oh!

AstroProfundis Mar 23, 2021

Uh oh!

JaySon-Huang Mar 23, 2021

Uh oh!

lucklove Mar 23, 2021

Uh oh!

JaySon-Huang Mar 23, 2021

Uh oh!

ti-chi-bot commented Mar 23, 2021

Uh oh!

AstroProfundis commented Mar 23, 2021

Uh oh!

ti-chi-bot commented Mar 23, 2021

Uh oh!

codecov-commenter commented Oct 21, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

JaySon-Huang commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Uh oh!

codecov-io commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JaySon-Huang commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JaySon-Huang commented Mar 18, 2021

Uh oh!

AstroProfundis Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

lucklove Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Mar 23, 2021

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot commented Mar 23, 2021

Uh oh!

AstroProfundis commented Mar 23, 2021

Uh oh!

ti-chi-bot commented Mar 23, 2021

Uh oh!

codecov-commenter commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

JaySon-Huang commented Mar 15, 2021 •

edited

Loading

codecov-io commented Mar 15, 2021 •

edited

Loading

JaySon-Huang commented Mar 15, 2021 •

edited

Loading

codecov-commenter commented Oct 21, 2025 •

edited

Loading