-
Notifications
You must be signed in to change notification settings - Fork 153
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 last use of api key on webui #4124
Conversation
@kevin55156 Anyway, Thank you for your nice work. 🚀 |
092df88
to
233eeda
Compare
Thank you, I think so, too. |
…/pipecd into add-last-use-of-api-key-on-webui
75568e6
to
78392e4
Compare
} | ||
|
||
func NewVerifier(ctx context.Context, getter apiKeyGetter, logger *zap.Logger) *Verifier { | ||
const apiKeyLastUsedCacheHashKey = "HASHKEY:PIPED:API_KEYS" //nolint:gosec |
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.
[nits] I feel it's better to make it sharding per project.
Because it's supposed to be called per a project.
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.
Server must decide hash key name before NewVerifier is called, but which project is not decided at that time.
Piped client is associated with one project, but defaultPipedStatHashKey does not include project id because of the same reason.
806de36
to
b8b523b
Compare
6d59e7f
to
ba18ca9
Compare
@@ -69,7 +69,8 @@ var ( | |||
) | |||
|
|||
const ( | |||
defaultPipedStatHashKey = "HASHKEY:PIPED:STATS" | |||
defaultPipedStatHashKey = "HASHKEY:PIPED:STATS" | |||
apiKeyLastUsedCacheHashKey = "HASHKEY:PIPED:API_KEYS" //nolint:gosec |
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.
Just want to confirm, do we still need this nolint
annotation?
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.
Yes, CI golang-cli fails without this nolint
ce4aa88
to
c242dce
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.
Nice work 🙌
What this PR does / why we need it:
Enabling us to know when api keys are created and used at last time.
Which issue(s) this PR fixes:
Fixes #4118
Does this PR introduce a user-facing change?: