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

tiup/mirror: add ability to change owner of a component #1676

Merged
merged 2 commits into from
Dec 16, 2021

Conversation

AstroProfundis
Copy link
Contributor

What problem does this PR solve?

Change the owner of a component.

What is changed and how it works?

Use a process like rotating root.json:

  • On server side, the server admin run tiup mirror transfer-owner command to initialize a owner transfer process
  • The existence of new owner is checked, then open a temp HTTP server to serve the component.json
  • Client with the new owner's private key use tiup mirror sign to sign that temp URL
  • Server validate the signature received and update component.json along with index.json

The process is a pure administration operation, and could be used without the participate of old owner of the component, so that it also works if the old owner is absent for long time and can not be contacted.

The new owner must be granted before the transfer.

Check List

Tests

  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change
  • Has interface methods change

Release notes:

NONE

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Dec 15, 2021

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • lucklove

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.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 15, 2021
@AstroProfundis AstroProfundis added component/tiup Issues about the TiUP package management component itself size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 15, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2021

Codecov Report

Merging #1676 (dcc8478) into master (3523106) will increase coverage by 17.60%.
The diff coverage is 32.31%.

❗ Current head dcc8478 differs from pull request most recent head d88c403. Consider uploading reports for the commit d88c403 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1676       +/-   ##
===========================================
+ Coverage   14.81%   32.41%   +17.60%     
===========================================
  Files         152      247       +95     
  Lines       18751    28842    +10091     
===========================================
+ Hits         2777     9347     +6570     
- Misses      15457    18179     +2722     
- Partials      517     1316      +799     
Flag Coverage Δ
cluster 27.27% <0.00%> (?)
unittest ?

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

Impacted Files Coverage Δ
pkg/repository/model/publish.go 40.00% <0.00%> (-36.92%) ⬇️
cmd/mirror.go 46.08% <26.09%> (ø)
pkg/repository/model/model.go 37.24% <52.94%> (-6.18%) ⬇️
server/rotate/root.go
components/cluster/command/transfer.go 50.00% <0.00%> (ø)
pkg/cluster/manager/edit_config.go 28.72% <0.00%> (ø)
pkg/cluster/manager/cleanup.go 0.00% <0.00%> (ø)
pkg/cluster/task/scale_config.go 0.00% <0.00%> (ø)
... and 179 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 3523106...d88c403. Read the comment docs.

Copy link
Member

@lucklove lucklove left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 16, 2021
@AstroProfundis
Copy link
Contributor Author

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: d88c403

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 16, 2021
@ti-chi-bot ti-chi-bot merged commit 60ae371 into pingcap:master Dec 16, 2021
@AstroProfundis AstroProfundis deleted the edit-owner branch December 16, 2021 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tiup Issues about the TiUP package management component itself size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants