Skip to content

Commit

Permalink
fix: update auth cache miss error handling (ratify-project#1105)
Browse files Browse the repository at this point in the history
  • Loading branch information
akashsinghal authored Oct 11, 2023
1 parent baf48ef commit 7645096
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 1 deletion.
4 changes: 4 additions & 0 deletions pkg/cache/dapr/dapr.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ func (d *daprCache) Set(ctx context.Context, key string, value interface{}) bool
}

func (d *daprCache) SetWithTTL(ctx context.Context, key string, value interface{}, ttl time.Duration) bool {
if ttl < 0 {
logger.GetLogger(ctx, logOpt).Errorf("Error saving value to redis: ttl provided must be >= 0: %v", ttl)
return false
}
bytes, err := json.Marshal(value)
if err != nil {
logger.GetLogger(ctx, logOpt).Error("Error marshalling value for redis: ", err)
Expand Down
11 changes: 11 additions & 0 deletions pkg/cache/dapr/dapr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,17 @@ func TestDaprCacheProvider_SetWithTTL_Error(t *testing.T) {
}
}

func TestDaprCacheProvider_SetWithTTL_InvalidTTL(t *testing.T) {
d := &daprCache{
daprClient: &TestDaprClient{},
cacheName: testCacheName,
}
ok := d.SetWithTTL(context.Background(), testKey, testValue, -10)
if ok {
t.Errorf("expected ok to be false")
}
}

func TestDaprCacheProvider_Delete_Expected(t *testing.T) {
d := &daprCache{
daprClient: &TestDaprClient{
Expand Down
4 changes: 4 additions & 0 deletions pkg/cache/ristretto/ristretto.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ func (r *ristrettoCache) Set(ctx context.Context, key string, value interface{})
}

func (r *ristrettoCache) SetWithTTL(ctx context.Context, key string, value interface{}, ttl time.Duration) bool {
if ttl < 0 {
logger.GetLogger(ctx, logOpt).Errorf("Error saving value to ristretto: ttl provided must be >= 0: %v", ttl)
return false
}
bytes, err := json.Marshal(value)
if err != nil {
logger.GetLogger(ctx, logOpt).Error("Error marshalling value for ristretto: ", err)
Expand Down
19 changes: 19 additions & 0 deletions pkg/cache/ristretto/ristretto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,25 @@ func TestSetWithTTL_Expected(t *testing.T) {
}
}

// TestSetWithTTL_InvalidTTL tests the SetWithTTL function with an invalid TTL
func TestSetWithTTL_InvalidTTL(t *testing.T) {
ctx := context.Background()
var err error
// first attempt to get the cache provider if it's already been initialized
cacheProvider := cache.GetCacheProvider()
if cacheProvider == nil {
// if no cache provider has been initialized, initialize one
cacheProvider, err = cache.NewCacheProvider(ctx, RistrettoCacheType, cache.DefaultCacheName, cache.DefaultCacheSize)
if err != nil {
t.Errorf("Expected no error, but got %v", err)
}
}
ok := cacheProvider.SetWithTTL(ctx, "test_ttl", "test", -10)
if ok {
t.Errorf("Expected ok to be false")
}
}

// TestGet_Expected tests the Get function
func TestGet_Expected(t *testing.T) {
ctx := context.Background()
Expand Down
6 changes: 6 additions & 0 deletions pkg/common/oras/authprovider/authprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
re "github.com/deislabs/ratify/errors"
"github.com/docker/cli/cli/config"
"github.com/docker/cli/cli/config/configfile"
"github.com/docker/cli/cli/config/types"
)

// This config represents the credentials that should be used
Expand Down Expand Up @@ -60,6 +61,7 @@ type defaultAuthProviderConf struct {
}

const DefaultAuthProviderName string = "dockerConfig"
const DefaultDockerAuthTTL = 1 * time.Hour

// init calls Register for our default provider, which simply reads the .dockercfg file.
func init() {
Expand Down Expand Up @@ -123,10 +125,14 @@ func (d *defaultAuthProvider) Provide(_ context.Context, artifact string) (AuthC
}

dockerAuthConfig := cfg.AuthConfigs[artifactHostName]
if dockerAuthConfig == (types.AuthConfig{}) {
return AuthConfig{}, nil
}
authConfig := AuthConfig{
Username: dockerAuthConfig.Username,
Password: dockerAuthConfig.Password,
IdentityToken: dockerAuthConfig.IdentityToken,
ExpiresOn: time.Now().Add(DefaultDockerAuthTTL),
Provider: d,
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/common/oras/authprovider/authprovider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"os"
"path/filepath"
"testing"
"time"
)

const (
Expand Down Expand Up @@ -87,6 +88,10 @@ func TestProvide_ExternalDockerConfigPath_ExpectedResults(t *testing.T) {
if authConfig.Username != testUserName || authConfig.Password != testPassword {
t.Fatalf("incorrect username %v or password %v returned", authConfig.Username, authConfig.Password)
}

if time.Now().Add(DefaultDockerAuthTTL - time.Minute).After(authConfig.ExpiresOn) {
t.Fatalf("incorrect expiration time %v returned", authConfig.ExpiresOn)
}
}

func TestProvide_ExternalDockerConfigPathWithIdentityToken_ExpectedResults(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/referrerstore/oras/oras.go
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,9 @@ func createDefaultRepository(ctx context.Context, store *orasStore, targetRef co
authConfig, err = store.authProvider.Provide(ctx, targetRef.Original)
if err != nil {
logger.GetLogger(ctx, logOpt).Warnf("auth provider failed with err, %v", err)
logger.GetLogger(ctx, logOpt).Info("attempting to use anonymous credentials")
logger.GetLogger(ctx, logOpt).Debug("attempting to use anonymous credentials")
} else if authConfig == (authprovider.AuthConfig{}) {
logger.GetLogger(ctx, logOpt).Debug("no credentials found, attempting to use anonymous credentials")
} else {
if cacheProvider != nil {
success := cacheProvider.SetWithTTL(ctx, fmt.Sprintf(cache.CacheKeyOrasAuth, artifactRef.Registry), authConfig, time.Until(authConfig.ExpiresOn))
Expand Down

0 comments on commit 7645096

Please sign in to comment.