*: fix server-side lease expire when auth is enabled #9018
Conversation
auth/store_test.go
Outdated
|
|
||
| ai, aerr := as.AuthInfoFromCtx(ctx) | ||
| if aerr != nil { | ||
| t.Fatal(err) |
There was a problem hiding this comment.
Probably we should use t.Error instead of t.Fatal? It is same to the below error cases.
integration/v3_auth_test.go
Outdated
| }, | ||
| ) | ||
| if terr != nil { | ||
| t.Fatal(terr) |
There was a problem hiding this comment.
Should be t.Error? Same to the below error cases.
|
@gyuho thanks for the fix. It seems to be interesting and terrible gRPC update. Does this mean we have no way to detect misusage of |
|
@mitake Yeah, it was pretty confusing gRPC update. Only way to prevent the misuse was having metadata tests on user-side. |
"WithRoot" is only used within local node, and "AuthInfoFromCtx" expects token from incoming context. Embed token with "NewIncomingContext" so that token can be found in "AuthInfoFromCtx". Signed-off-by: Gyuho Lee <[email protected]>
Signed-off-by: Gyuho Lee <[email protected]>
Signed-off-by: Gyuho Lee <[email protected]>
|
@gyuho oh I see... we need to be careful in the future. |
|
@mitake Yeah, btw, just addressed your feedback. Let me merge when CIs turn green. Thanks! |
|
@gyuho lgtm after the CIs turn green, thanks! |
gyuho
changed the title
gyuho
changed the title
Codecov Report
@@ Coverage Diff @@
## master #9018 +/- ##
=========================================
Coverage ? 76.04%
=========================================
Files ? 359
Lines ? 29837
Branches ? 0
=========================================
Hits ? 22691
Misses ? 5569
Partials ? 1577
Continue to review full report at Codecov.
|
| return metadata.NewOutgoingContext(ctx, tokenMD) | ||
|
|
||
| // use "mdIncomingKey{}" since it's called from local etcdserver | ||
| return metadata.NewIncomingContext(ctx, tokenMD) |
There was a problem hiding this comment.
Funny, I was just reading about these new grpc metadata context functions for the first time today.
There was a problem hiding this comment.
Yeah, it was from gRPC v1.3.0 with grpc/grpc-go#1157.
|
UPDATE: just found out we didn't backport #8031, so no need to backport this. |
When auth is enabled, lease revoke loop was broken because we were embedding the token in the wrong way.
Address #9006.