Authorization support for Portworx #324
Conversation
|
I see vendor updates in the PR but no change in Gopkg.lock. |
harsh-px
added
the
release-note
| Expiration: time.Now().Add(time.Hour).Unix(), | ||
| } | ||
|
|
||
| token, err := auth.Token(claims, signature, options) |
There was a problem hiding this comment.
Why generate this token for every call? We should store it and regenerate when it expires.
There was a problem hiding this comment.
This is a good point, but that would mean tracking tokens with time. If a token is expired generated a new one, and so on. I think we can do that, but that complexity I would like to move to another PR for later, if you don't mind. Currently generating a token each time is very fast.
There was a problem hiding this comment.
Generating a token is expensive if done very often, they involve crypto calls. Please file a bug to track that.
Example of how kubernetes deals with OIDC tokens to alleviate this: https://github.com/kubernetes/client-go/blob/master/plugin/pkg/client/auth/oidc/oidc.go#L332
There was a problem hiding this comment.
OIDC is different because it is an RPC connection to get the public auth key. We only generate it. But I'm not against it. I can change it to do this, I just suggested we do this after.
lpabon
force-pushed
the
auth-2
branch
2 times, most recently
from
9a2105b to
eb07bb9
Compare
Codecov Report
@@ Coverage Diff @@
## master #324 +/- ##
==========================================
- Coverage 72.71% 72.67% -0.05%
==========================================
Files 24 24
Lines 2104 2104
==========================================
- Hits 1530 1529 -1
- Misses 444 445 +1
Partials 130 130
Continue to review full report at Codecov.
|
| Expiration: time.Now().Add(time.Hour).Unix(), | ||
| } | ||
|
|
||
| token, err := auth.Token(claims, signature, options) |
There was a problem hiding this comment.
Generating a token is expensive if done very often, they involve crypto calls. Please file a bug to track that.
Example of how kubernetes deals with OIDC tokens to alleviate this: https://github.com/kubernetes/client-go/blob/master/plugin/pkg/client/auth/oidc/oidc.go#L332
Based from work on #302 by Paul Theunis Signed-off-by: Luis Pabón <[email protected]>
What type of PR is this?
feature
This change is a continuation of @pault84 's PR #302
Moved here from #323 to bring the branch from a fork to
origin.What this PR does / why we need it:
Stork/Portworx driver must have the appropriate token to communicate with Px. Some functions require stork to generate its own token based on a shared secret JWT. Most calls have stork extract the token from a secret pointed by the annotations in the CRD.
Also this PR has the following updates (saved as separate commits):
Does this PR change a user-facing CRD or CLI?:
Yes, This requires only that a secret is saved in the annotations.
Is a release note needed?:
Not sure. Maybe regular documentations will be needed in the docs for Portworx
Does this change need to be cherry-picked to a release branch?:
I think so, but I am not sure which branch would be needed for Portworx release.