Skip to content
This repository has been archived by the owner on Mar 22, 2023. It is now read-only.

adds support for limits on the number of tokens per owner #534

Merged
merged 5 commits into from Jan 22, 2019

Conversation

shamsimam
Copy link
Contributor

Changes proposed in this PR

  • adds support for limits on the number of tokens per owner

Why are we making these changes?

Allows us to manage the stress we can end placing on our key-value store. In addition, it prevents rogue processes from creating many tokens.

@shamsimam shamsimam self-assigned this Dec 14, 2018
@DaoWen
Copy link
Contributor

DaoWen commented Dec 15, 2018

I think we should hold off on this until we have good answers to all of these questions:

  1. Can users easily obtain a human-readable list of their tokens? This should be human-readable, and for just one account.
  2. Do users actually know how to delete tokens?
  3. What happens if this limit is hit during cross-cluster token synchronization?

@shamsimam
Copy link
Contributor Author

@DaoWen

Can users easily obtain a human-readable list of their tokens? This should be human-readable, and for just one account.

This was not previously publicized but can be obtained from the /tokens?owner-<owner> endpoint. I have added this entry to our REST api documentation (rest-api.md).

Do users actually know how to delete tokens?

We have supported token delete and have it documented in our REST api documentation (rest-api.md).

What happens if this limit is hit during cross-cluster token synchronization?

I have updated the PR to avoid validating the limits during admin mode updates.

@DaoWen
Copy link
Contributor

DaoWen commented Jan 9, 2019

I don't think the "human friendly" aspects of points 1 and 2 in my earlier feedback have been addressed. I think it would be good to wait until David's project is done before merging this. (But we can still review in the meantime.)

@shamsimam
Copy link
Contributor Author

@DaoWen Okay, please go ahead with the review.

waiter/src/waiter/token.clj Outdated Show resolved Hide resolved
@shamsimam
Copy link
Contributor Author

Build is green.

@DaoWen DaoWen merged commit 50ad792 into master Jan 22, 2019
@dposada dposada deleted the token-limits branch January 22, 2019 17:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants