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

[ADR] Server token rotation #8215

Merged
merged 3 commits into from
Sep 25, 2023
Merged

Conversation

dereknola
Copy link
Contributor

Proposed Changes

ADR for Server Token Rotation

Linked Issues

rancher/rke2#2141

User-Facing Change


Further Comments

Signed-off-by: Derek Nola <derek.nola@suse.com>
@dereknola dereknola requested a review from a team as a code owner August 16, 2023 21:32
@dereknola
Copy link
Contributor Author

@Oats87 @jakefhyde Heads up for potential future v2 prov work. This doesn't change any existing CLI, its only additive

Signed-off-by: Derek Nola <derek.nola@suse.com>
Copy link
Member

@Oats87 Oats87 left a comment

Choose a reason for hiding this comment

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

At a high level, this looks great.

I do have a few questions/comments on the implementation side:

  1. Can we ensure that there is a method to retrieve the outputs of the various subcommands for token in json format? This would greatly help with automation/reliable parsing of status.
  2. How will this work with the config file parser? Considering if you pass in token etc. into the config file, that gets "parsed" and rendered into the CLI. I would say it's likely a user can accidentally run into a situation where they are rotating the token and mismatching tokens. Perhaps the CLI should parse in the config file(s) and make sure that the old token isn't referenced anymore -- I expect that we will pave a new config file onto disk with the new token before/during our usage of k3s token rotate with CAPR.
  3. Does K3s encrypt bootstrap data? If so, does this mean that etcd snapshots taken with the older token are no longer able to be restored?

Copy link
Contributor

@brandond brandond left a comment

Choose a reason for hiding this comment

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

spelling/grammar nits. You might run the whole thing though a check, this is just what jumped out at me.

docs/adrs/server-token-rotation.md Outdated Show resolved Hide resolved
docs/adrs/server-token-rotation.md Outdated Show resolved Hide resolved
docs/adrs/server-token-rotation.md Outdated Show resolved Hide resolved
Signed-off-by: Derek Nola <derek.nola@suse.com>
@dereknola dereknola merged commit bd9dad8 into k3s-io:master Sep 25, 2023
2 checks passed
@dereknola dereknola deleted the server_token_adr branch October 2, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants