Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Fix token cache issue #488

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fix token cache issue #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
617736f
Fix token cache issue
pmahindrakar-oss May 22, 2024
be4c4fd
Using same mutex for condition variable
pmahindrakar-oss May 22, 2024
5a07c88
Using latest flyteidl changes which has fix to use same mutex for con…
pmahindrakar-oss May 22, 2024
38f3799
fix
pmahindrakar-oss May 22, 2024
c72403d
Lock the locker in the wait to adher to cond.Wait() semantics
pmahindrakar-oss May 23, 2024
8ae9780
using noop locker as waitlist add is atomic operation
pmahindrakar-oss May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ require (
go.opentelemetry.io/otel/trace v1.21.0 // indirect
golang.org/x/crypto v0.21.0 // indirect
golang.org/x/exp v0.0.0-20240325151524-a685a6edb6d8 // indirect
golang.org/x/net v0.22.0 // indirect
golang.org/x/net v0.23.0 // indirect
golang.org/x/sync v0.6.0 // indirect
golang.org/x/sys v0.18.0 // indirect
golang.org/x/term v0.18.0 // indirect
Expand All @@ -178,8 +178,8 @@ require (
)

replace (
github.com/flyteorg/flyte/flyteidl => github.com/flyteorg/flyte/flyteidl v1.12.0-b0.0.20240517165944-d066b2050575
github.com/flyteorg/flyte/flyteplugins => github.com/flyteorg/flyte/flyteplugins v1.12.0-b0.0.20240503025718-07c0f238650c
github.com/flyteorg/flyte/flytepropeller => github.com/flyteorg/flyte/flytepropeller v1.12.0-b0.0.20240503025718-07c0f238650c
github.com/flyteorg/flyte/flytestdlib => github.com/flyteorg/flyte/flytestdlib v1.12.0-b0.0.20240503025718-07c0f238650c
github.com/flyteorg/flyte/flyteidl => github.com/flyteorg/flyte/flyteidl v1.12.1-0.20240523211648-f1200666f004
github.com/flyteorg/flyte/flyteplugins => github.com/flyteorg/flyte/flyteplugins v1.12.1-0.20240523211648-f1200666f004
github.com/flyteorg/flyte/flytepropeller => github.com/flyteorg/flyte/flytepropeller v1.12.1-0.20240523211648-f1200666f004
github.com/flyteorg/flyte/flytestdlib => github.com/flyteorg/flyte/flytestdlib v1.12.1-0.20240523211648-f1200666f004
)
20 changes: 10 additions & 10 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,14 +352,14 @@ github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg=
github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U=
github.com/flyteorg/flyte/flyteidl v1.12.0-b0.0.20240517165944-d066b2050575 h1:GAkFzSPzCOQOQR/WM5+4Zchw2LmOHJPiIBNwTOxnoJo=
github.com/flyteorg/flyte/flyteidl v1.12.0-b0.0.20240517165944-d066b2050575/go.mod h1:mKC19e/327nx6ouAWtrcZt0vFWl0/IFxXwyb+Ff9/wY=
github.com/flyteorg/flyte/flyteplugins v1.12.0-b0.0.20240503025718-07c0f238650c h1:ehuea7v/q0hZxyIMuO8NYUcfgAEUAOvv4Tyk5i0y644=
github.com/flyteorg/flyte/flyteplugins v1.12.0-b0.0.20240503025718-07c0f238650c/go.mod h1:bnW+Jb8u60I7FlufsCu/nE7XZZOlY4m7ngWPA7YFnQc=
github.com/flyteorg/flyte/flytepropeller v1.12.0-b0.0.20240503025718-07c0f238650c h1:JRzMLhFKiRtDj6f/U/RxaCsl8iBcS57SfA69USKVrE4=
github.com/flyteorg/flyte/flytepropeller v1.12.0-b0.0.20240503025718-07c0f238650c/go.mod h1:6NID2yzvHWId9U7pCCqCeUFvc3jSdrnR3iyFQja1bP0=
github.com/flyteorg/flyte/flytestdlib v1.12.0-b0.0.20240503025718-07c0f238650c h1:24xUhEtvDIrAlk+nt35YUvpuou/UQBoC2OZkj8VEuZw=
github.com/flyteorg/flyte/flytestdlib v1.12.0-b0.0.20240503025718-07c0f238650c/go.mod h1:bWhD/mbPO/RWATAVvtySSprmKZU7GB9SLej4pQBO5rc=
github.com/flyteorg/flyte/flyteidl v1.12.1-0.20240523211648-f1200666f004 h1:n3WS5FqdV3Esxu2/5+ncafqfe0Po9Py5QsZ+T3RP3j4=
github.com/flyteorg/flyte/flyteidl v1.12.1-0.20240523211648-f1200666f004/go.mod h1:ki0nYf4mHM5VyaXY3lpndDSRqvUvmqlnyLENVu06dSQ=
github.com/flyteorg/flyte/flyteplugins v1.12.1-0.20240523211648-f1200666f004 h1:d06Og4tFDDkOETeSwac6209HUnhPPiFpQQcsYGzV+n8=
github.com/flyteorg/flyte/flyteplugins v1.12.1-0.20240523211648-f1200666f004/go.mod h1:Zr3nUQN5FREE0Qzg0MteW2H46YFKYJMiu6F8oPmU46g=
github.com/flyteorg/flyte/flytepropeller v1.12.1-0.20240523211648-f1200666f004 h1:DS6KgCKYTP97zOXfeI38wFN1dS2jv9It3gSQUwI9t1A=
github.com/flyteorg/flyte/flytepropeller v1.12.1-0.20240523211648-f1200666f004/go.mod h1:Pqr+Usd2CPO/nynhy1O2zEzyLZerVflH80N/k9H89Ag=
github.com/flyteorg/flyte/flytestdlib v1.12.1-0.20240523211648-f1200666f004 h1:xYylOmZyz58p0FiuzerJ6C+DOP6AYjmKJUBtY9KAQNM=
github.com/flyteorg/flyte/flytestdlib v1.12.1-0.20240523211648-f1200666f004/go.mod h1:l1mK3nAptXTkVBKtrYT3v0ezLJj6BtgfBs/5ywPrO8o=
github.com/flyteorg/stow v0.3.10 h1:uEe+tI+CGKn21H93uXp9z05hqynEki2BO9KkW/GweY8=
github.com/flyteorg/stow v0.3.10/go.mod h1:fArjMpsYJNWkp/hyDKKdbcv07gxbuLmKFcb7YT1aSOM=
github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k=
Expand Down Expand Up @@ -1047,8 +1047,8 @@ golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v
golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM=
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc=
golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
golang.org/x/net v0.23.0 h1:7EYJ93RZ9vYSZAIb2x3lnuvqO5zneoD6IvWjuhfxjTs=
golang.org/x/net v0.23.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
Expand Down
29 changes: 14 additions & 15 deletions pkg/pkce/token_cache_keyring.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/flyteorg/flyte/flyteidl/clients/go/admin/cache"
"github.com/flyteorg/flyte/flytestdlib/logger"

"github.com/zalando/go-keyring"
"golang.org/x/oauth2"
)
Expand All @@ -22,18 +23,16 @@ type TokenCacheKeyringProvider struct {
ServiceName string
ServiceUser string
mu *sync.Mutex
condLocker *cache.NoopLocker
cond *sync.Cond
}

func (t *TokenCacheKeyringProvider) PurgeIfEquals(existing *oauth2.Token) (bool, error) {
if existingBytes, err := json.Marshal(existing); err != nil {
return false, fmt.Errorf("unable to marshal token to save in cache due to %w", err)
} else if tokenJSON, err := keyring.Get(t.ServiceName, t.ServiceUser); err != nil {
if err.Error() == "secret not found in keyring" {
return false, fmt.Errorf("unable to read token from cache. Error: %w", cache.ErrNotFound)
}

return false, fmt.Errorf("unable to read token from cache. Error: %w", err)
logger.Warnf(context.Background(), "unable to read token from cache but not failing the purge as the token might not have been saved at all. Error: %v", err)
return true, nil
} else if tokenJSON != string(existingBytes) {
return false, nil
}
Expand All @@ -52,19 +51,17 @@ func (t *TokenCacheKeyringProvider) Unlock() {

// TryLock the cache.
func (t *TokenCacheKeyringProvider) TryLock() bool {
if t.mu.TryLock() {
logger.Infof(context.Background(), "Locked the cache")
return true
}
logger.Infof(context.Background(), "Failed to lock the cache")
return false
return t.mu.TryLock()
}

// CondWait waits for the condition to be true.
// CondWait adds the current go routine to the condition waitlist and waits for another go routine to notify using CondBroadcast
// The current usage is that one who was able to acquire the lock using TryLock is the one who gets a valid token and notifies all the waitlist requesters so that they can use the new valid token.
// It also locks the Locker in the condition variable as the semantics of Wait is that it unlocks the Locker after adding
// the consumer to the waitlist and before blocking on notification.
func (t *TokenCacheKeyringProvider) CondWait() {
logger.Infof(context.Background(), "Signaling the condition")
t.cond.L.Lock()
t.cond.Wait()
logger.Infof(context.Background(), "Coming out of waiting")
t.cond.L.Unlock()
}

// CondBroadcast broadcasts the condition.
Expand Down Expand Up @@ -111,9 +108,11 @@ func (t *TokenCacheKeyringProvider) GetToken() (*oauth2.Token, error) {
}

func NewTokenCacheKeyringProvider(serviceName, serviceUser string) *TokenCacheKeyringProvider {
condLocker := &cache.NoopLocker{}
return &TokenCacheKeyringProvider{
mu: &sync.Mutex{},
cond: sync.NewCond(&sync.Mutex{}),
condLocker: condLocker,
cond: sync.NewCond(condLocker),
ServiceName: serviceName,
ServiceUser: serviceUser,
}
Expand Down
Loading