Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use expiring cache with the OIDC tokens #7335

Merged
merged 2 commits into from
Oct 6, 2023
Merged
Changes from all commits
Commits
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
22 changes: 21 additions & 1 deletion pkg/auth/token_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,33 +19,43 @@ package auth
import (
"context"
"fmt"
"time"

"go.uber.org/zap"
authv1 "k8s.io/api/authentication/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/cache"
"k8s.io/client-go/kubernetes"
kubeclient "knative.dev/pkg/client/injection/kube/client"
"knative.dev/pkg/logging"
)

const (
expirationBufferTime = time.Second * 30
)
Comment on lines +34 to +36
Copy link
Member

@pierDipi pierDipi Oct 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd make this higher to also account for clock skew which could be in the minutes order at times, something like 5/10 minutes seems safer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cali0707 @creydr any thoughts here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clock skew is a good point. I'll create an issue to fix it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created #7351

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh whoops, sorry I missed this! @pierDipi or @creydr - what is clock skew in this context? I've only encountered it before in digital circuits


type OIDCTokenProvider struct {
logger *zap.SugaredLogger
kubeClient kubernetes.Interface
tokenCache cache.Expiring
}

func NewOIDCTokenProvider(ctx context.Context) *OIDCTokenProvider {
tokenProvider := &OIDCTokenProvider{
logger: logging.FromContext(ctx).With("component", "oidc-token-provider"),
kubeClient: kubeclient.Get(ctx),
tokenCache: *cache.NewExpiring(),
}

return tokenProvider
}

// GetJWT returns a JWT from the given service account for the given audience.
func (c *OIDCTokenProvider) GetJWT(serviceAccount types.NamespacedName, audience string) (string, error) {
// TODO: check the cache
if val, ok := c.tokenCache.Get(cacheKey(serviceAccount, audience)); ok {
return val.(string), nil
}

// if not found in cache: request new token
tokenRequest := authv1.TokenRequest{
Expand All @@ -63,5 +73,15 @@ func (c *OIDCTokenProvider) GetJWT(serviceAccount types.NamespacedName, audience
return "", fmt.Errorf("could not request a token for %s: %w", serviceAccount, err)
}

// we need a duration until this token expires, use the expiry time - (now + 30s)
// this gives us a buffer so that it doesn't expire between when we retrieve it and when we use it
expiryTtl := tokenRequestResponse.Status.ExpirationTimestamp.Time.Sub(time.Now().Add(expirationBufferTime))

c.tokenCache.Set(cacheKey(serviceAccount, audience), tokenRequestResponse.Status.Token, expiryTtl)

return tokenRequestResponse.Status.Token, nil
}

func cacheKey(serviceAccount types.NamespacedName, audience string) string {
return fmt.Sprintf("%s/%s/%s", serviceAccount.Namespace, serviceAccount.Name, audience)
}