Skip to content

Commit

Permalink
bring in changes for flytecl keyring from PR flytectl/pull/488
Browse files Browse the repository at this point in the history
Signed-off-by: pmahindrakar-oss <[email protected]>
  • Loading branch information
pmahindrakar-oss committed May 31, 2024
1 parent 0939347 commit 144cf30
Showing 1 changed file with 28 additions and 8 deletions.
36 changes: 28 additions & 8 deletions flytectl/pkg/pkce/token_cache_keyring.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package pkce

import (
"context"
"encoding/json"
"fmt"
"sync"

"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 @@ -21,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

Check warning on line 37 in flytectl/pkg/pkce/token_cache_keyring.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/pkce/token_cache_keyring.go#L30-L37

Added lines #L30 - L37 were not covered by tests
}
Expand All @@ -54,12 +54,30 @@ func (t *TokenCacheKeyringProvider) TryLock() bool {
return t.mu.TryLock()

Check warning on line 54 in flytectl/pkg/pkce/token_cache_keyring.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/pkce/token_cache_keyring.go#L53-L54

Added lines #L53 - L54 were not covered by tests
}

// 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.
// We use the condLocker which is noOp locker to get added to waitlist for notifications.
// The underlying notifcationList doesn't need to be guarded as it implementation is atomic and is thread safe
// Refer https://go.dev/src/runtime/sema.go
// Following is the function and its comments
// notifyListAdd adds the caller to a notify list such that it can receive
// notifications. The caller must eventually call notifyListWait to wait for
// such a notification, passing the returned ticket number.
//
// func notifyListAdd(l *notifyList) uint32 {
// // This may be called concurrently, for example, when called from
// // sync.Cond.Wait while holding a RWMutex in read mode.
// return l.wait.Add(1) - 1
// }
func (t *TokenCacheKeyringProvider) CondWait() {
t.condLocker.Lock()
t.cond.Wait()
t.condLocker.Unlock()

Check warning on line 77 in flytectl/pkg/pkce/token_cache_keyring.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/pkce/token_cache_keyring.go#L74-L77

Added lines #L74 - L77 were not covered by tests
}

// CondBroadcast signals the condition.
// CondBroadcast broadcasts the condition.
func (t *TokenCacheKeyringProvider) CondBroadcast() {
t.cond.Broadcast()

Check warning on line 82 in flytectl/pkg/pkce/token_cache_keyring.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/pkce/token_cache_keyring.go#L81-L82

Added lines #L81 - L82 were not covered by tests
}
Expand Down Expand Up @@ -103,9 +121,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,

Check warning on line 130 in flytectl/pkg/pkce/token_cache_keyring.go

View check run for this annotation

Codecov / codecov/patch

flytectl/pkg/pkce/token_cache_keyring.go#L123-L130

Added lines #L123 - L130 were not covered by tests
}
Expand Down

0 comments on commit 144cf30

Please sign in to comment.