Test Gateway OAuth token renewal #14845
Conversation
Flow that deauthorizes oauth token Gateway should fetch new token and retry request
OAuth token can expire
kyma-bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
kyma-bot
added
the
size/L
It was missing in some tests
VOID404
requested review from
akgalwas,
koala7659,
janmedrek,
franpog859 and
mvshao
as code owners
kyma-bot
removed
the
do-not-merge/work-in-progress
| ttl := defaultTokenTTL | ||
|
|
||
| if ttlStr := r.URL.Query().Get(tokenLifetime); ttlStr != "" { | ||
| ttlOrErr, err := time.ParseDuration(ttlStr) |
There was a problem hiding this comment.
Maybe parsedTTL? I assume it's not a TTL or error :D
There was a problem hiding this comment.
It's a TTL or not, I don't know what the idiomatic name should be, parsedTTL sounds good
There was a problem hiding this comment.
exp might be more approperiate, considering expires in is the oauth standard, not time to live
| apiVersion: applicationconnector.kyma-project.io/v1alpha1 | ||
| kind: Application | ||
| metadata: | ||
| name: manual |
There was a problem hiding this comment.
I'd rather name it complex-cases or something like this. manual suggests that you run it by hand or somehow locally, not automatically in the pipeline
| - displayName: oauth-short | ||
| name: oauth-short | ||
| providerDisplayName: Kyma | ||
| description: Valid oauth configuration with 5s token |
There was a problem hiding this comment.
Maybe Should renew the OAuth token after the expiration time?
| labels: | ||
| app: manual-tests | ||
| services: | ||
| - displayName: oauth-short |
There was a problem hiding this comment.
Maybe oauth-expired-token-renewal?
There was a problem hiding this comment.
I was thinking this might be generic for all tests requiring a token with short expire time, but sure, that makes sense too
|
|
||
| func (gs *GatewaySuite) TestManual() { | ||
| gs.Run("OAuth token renewal", func() { | ||
| url := gatewayURL("manual", "oauth-short") + "/ok" |
There was a problem hiding this comment.
I think we can move the + "/ok" part to the Application CR
There was a problem hiding this comment.
I am guessing different endpoints might be needed for different manual tests, but we can change it when we get there and leave it precise (with /ok endpoint) for now
|
@VOID404: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Description
Related issue(s)
#14654