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

Add control-plane token expiry #620

Merged
merged 5 commits into from
Aug 26, 2024
Merged

Add control-plane token expiry #620

merged 5 commits into from
Aug 26, 2024

Conversation

bschimke95
Copy link
Contributor

Microcluster v2 supports token TTL. This PR exposes this configuration option to the API/CLI.

Note that is only applicable for the control-plane tokens right now. Workers will be done in separate work (since we don't use microcluster for that it is more involved as we need to implement the logic ourselves).

This PR requires the (backward-compatible) update in the k8s-snap-api.
canonical/k8s-snap-api#5
Once this PR is reviewed and approved, I will merge the API change, create a new tag (1.0.3) and update this k8s-snap PR to use the new API version.

@bschimke95 bschimke95 requested a review from a team as a code owner August 23, 2024 07:45
@bschimke95 bschimke95 mentioned this pull request Aug 23, 2024
@@ -9,10 +9,13 @@ import (
"github.com/spf13/cobra"
)

const minTokenTTL = 3 * time.Second
Copy link
Contributor

Choose a reason for hiding this comment

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

ok, so you can't make a token that is valid for 2 seconds but 4 seconds is fine? Am i reading that right... no complaints just making sure i understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, I added this because we also do this for the timeout flag. My assumption was that this is done to avoid error if the user assumes a unit (e.g. seconds) and does not set them in the flag. However, it seems like this would just fail:

➜ sudo k8s get-join-token test --expires-in 10
Error: invalid argument "10" for "--expires-in" flag: time: missing unit in duration "10"
Usage:
  k8s get-join-token <node-name> [flags]

Flags:
      --expires-in duration   the time until the token expires (default 24h0m0s)
  -h, --help                  help for get-join-token
      --timeout duration      the max time to wait for the command to execute (default 1m30s)
      --worker                generate a join token for a worker node

So, I don't see a reason to actually have this check - removed it.

Copy link
Contributor

@HomayoonAlimohammadi HomayoonAlimohammadi left a comment

Choose a reason for hiding this comment

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

Thanks a lot @bschimke95! Just a minor comment.

src/k8s/pkg/k8sd/api/cluster_tokens.go Show resolved Hide resolved
Copy link
Contributor

@HomayoonAlimohammadi HomayoonAlimohammadi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot!

@bschimke95 bschimke95 merged commit 84551b1 into main Aug 26, 2024
19 checks passed
@bschimke95 bschimke95 deleted the KU-1246/token-expiration branch August 26, 2024 10:12
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.

4 participants