-
Notifications
You must be signed in to change notification settings - Fork 13
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 worker token TTL #621
Add worker token TTL #621
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
e54f001
to
9d87f9c
Compare
cf54e39
to
83daee4
Compare
There was a problem hiding this 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! LGTM, Just a minor comment.
54b9ba9
to
bbf4d0d
Compare
// Note(ben): Never change the order or remove a migration as this would break the internal microcluster counter! | ||
SchemaExtensions = []schema.Update{ | ||
schemaApplyMigration("kubernetes-auth-tokens", "000-create.sql"), | ||
schemaApplyMigration("cluster-configs", "000-create.sql"), | ||
|
||
schemaApplyMigration("worker-tokens", "000-create.sql"), | ||
schemaApplyMigration("worker-tokens", "001-add-expiry.sql"), | ||
|
||
schemaApplyMigration("feature-status", "000-feature-status.sql"), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the addition of schemaApplyMigration("worker-tokens", "001-add-expiry.sql")
did not obey the comment at the top of this function:
// Note(ben): Never change the order or remove a migration as this would break the internal microcluster counter!
ALTER TABLE worker_tokens | ||
ADD COLUMN expiry DATETIME DEFAULT '2100-01-01 23:59:59'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bschimke95 what if the table worker_tokens
has a column named expiry
already. Won't this fail?
Add an optional TTL for worker tokens similar to #620
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.