Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Don't log secret of s3. #292

Merged
merged 8 commits into from
May 15, 2020
Merged

Don't log secret of s3. #292

merged 8 commits into from
May 15, 2020

Conversation

YuJuncen
Copy link
Collaborator

@YuJuncen YuJuncen commented May 15, 2020

What problem does this PR solve?

fix bug hunting #71

What is changed and how it works?

I change the line that logging the secret information:

log.Info("try backup", zap.Any("backup request", req))

Check List

Tests

  • Unit test
  • Integration test
  • No code

Code changes

  • Has exported function/method change(move LogArguments from utils to task).

Release Note

  • No release note

@codecov
Copy link

codecov bot commented May 15, 2020

Codecov Report

Merging #292 into master will increase coverage by 0.17%.
The diff coverage is 67.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #292      +/-   ##
==========================================
+ Coverage   71.32%   71.50%   +0.17%     
==========================================
  Files          48       48              
  Lines        4997     5025      +28     
==========================================
+ Hits         3564     3593      +29     
+ Misses        973      971       -2     
- Partials      460      461       +1     
Impacted Files Coverage Δ
main.go 77.14% <ø> (ø)
pkg/storage/local_unix.go 100.00% <ø> (ø)
pkg/storage/noop.go 0.00% <ø> (ø)
pkg/utils/version.go 100.00% <ø> (ø)
pkg/utils/worker.go 72.00% <ø> (ø)
pkg/restore/util.go 73.38% <47.77%> (+0.10%) ⬆️
pkg/storage/flags.go 71.42% <50.00%> (ø)
pkg/backup/check.go 53.33% <53.33%> (ø)
pkg/task/restore.go 54.07% <54.07%> (+0.15%) ⬆️
pkg/task/backup_raw.go 56.52% <56.52%> (+0.63%) ⬆️
... and 66 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 00fe08c...0ea8169. Read the comment docs.

Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

rest LGTM

pkg/backup/client.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@kennytm kennytm left a comment

Choose a reason for hiding this comment

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

rest lgtm

pkg/task/common.go Outdated Show resolved Hide resolved
@YuJuncen YuJuncen removed the WIP label May 15, 2020
Co-authored-by: kennytm <kennytm@gmail.com>
@kennytm kennytm added the status/LGT1 LGTM1 label May 15, 2020
Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

Rest LGTM

@@ -0,0 +1,37 @@
package task
Copy link
Member

Choose a reason for hiding this comment

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

Please add license header.

"regexp"
"strings"

"github.com/pingcap/log"
"go.uber.org/zap"

Copy link
Member

Choose a reason for hiding this comment

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

Please remove the empty line.

Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

Cool, LGTM

@YuJuncen YuJuncen merged commit c2f193c into pingcap:master May 15, 2020
@sre-bot
Copy link
Contributor

sre-bot commented May 15, 2020

cherry pick to release-3.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented May 15, 2020

cherry pick to release-4.0 in PR #293

YuJuncen added a commit that referenced this pull request May 18, 2020
* backup: don't log secret of s3.

* br_s3: always print log.

* *: remove query string on logging arguments.

* br_s3: reset BR_LOG_TO_TERM on fail.

* Apply suggestions from code review

Co-authored-by: kennytm <kennytm@gmail.com>

* Apply suggestions from code review

Co-authored-by: kennytm <kennytm@gmail.com>

* *: remove query string on logging arguments.

Co-authored-by: Hillium <maruruku@stu.csust.edu.cn>
Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>
Co-authored-by: kennytm <kennytm@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P2-[4.0 bug hunting]-[BR]-BACKUP command writes secrets to the TiDB log file
4 participants