Skip to content

Commit

Permalink
finishing cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
thejosephstevens committed Oct 16, 2024
1 parent 2a3f985 commit 5170d4d
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 17 deletions.
8 changes: 8 additions & 0 deletions oci/auth/gcp/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ func TestLogin(t *testing.T) {
image string
token *oauth2.Token
tokenErr error
testOIDC bool
wantErr bool
}{
{
Expand All @@ -125,6 +126,7 @@ func TestLogin(t *testing.T) {
name: "with auto login",
autoLogin: true,
image: testValidGCRImage,
testOIDC: true,
token: &oauth2.Token{
AccessToken: "some-token",
TokenType: "Bearer",
Expand All @@ -136,6 +138,7 @@ func TestLogin(t *testing.T) {
autoLogin: true,
image: testValidGCRImage,
tokenErr: fmt.Errorf("token error"),
testOIDC: true,
wantErr: true,
},
{
Expand Down Expand Up @@ -167,6 +170,11 @@ func TestLogin(t *testing.T) {

_, err = gc.Login(context.TODO(), tt.autoLogin, tt.image, ref)
g.Expect(err != nil).To(Equal(tt.wantErr))

if tt.testOIDC {
_, err = gc.OIDCLogin(context.TODO())
g.Expect(err != nil).To(Equal(tt.wantErr))
}
})
}
}
8 changes: 0 additions & 8 deletions oci/auth/login/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package login

import (
"fmt"
"time"

"github.com/google/go-containerregistry/pkg/authn"
Expand All @@ -33,20 +32,13 @@ func cacheObject[T authn.Authenticator](store cache.Expirable[cache.StoreObject[

err := store.Set(obj)
if err != nil {
fmt.Println("error setting object", err)
return err
}
fmt.Println("object set")

return store.SetExpiration(obj, expiresAt)
}

func getObjectFromCache[T authn.Authenticator](cache cache.Expirable[cache.StoreObject[T]], key string) (T, bool, error) {
val, exists, err := cache.GetByKey(key)
keys, err := cache.ListKeys()
if err != nil {
return val.Object, false, err
}
fmt.Println("keys", keys)
return val.Object, exists, err
}
7 changes: 0 additions & 7 deletions oci/auth/login/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ func (m *Manager) Login(ctx context.Context, url string, ref name.Reference, opt
)
if opts.Cache != nil {
key, err = m.keyFromURL(url, provider)
fmt.Println("key", key)
if err != nil {
logr.FromContextOrDiscard(ctx).Error(err, "failed to get cache key")
} else {
Expand All @@ -164,17 +163,11 @@ func (m *Manager) Login(ctx context.Context, url string, ref name.Reference, opt

switch provider {
case oci.ProviderAWS:
fmt.Println("logging in to AWS ECR")
auth, expiresAt, err := m.ecr.LoginWithExpiry(ctx, opts.AwsAutoLogin, url)
fmt.Println("auth", auth)
fmt.Println("expiresAt", expiresAt)
fmt.Println("err", err)
if err != nil {
return nil, err
}
fmt.Println("opts.cache", opts.Cache)
if opts.Cache != nil {
fmt.Println("caching auth object")
err := cacheObject(opts.Cache, auth, key, expiresAt)
if err != nil {
logr.FromContextOrDiscard(ctx).Error(err, "failed to cache auth object")
Expand Down
9 changes: 7 additions & 2 deletions oci/auth/login/login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ func TestLogin(t *testing.T) {
// NOTE: This fails because the azure exchanger uses the image host
// to exchange token which can't be modified here without
// interfering with image name based categorization of the login
// provider. This is tested in detail in the azure package.
// provider that's actually being tested here. This is tested in
// detail in the azure package.
wantErr: true,
},
{
Expand Down Expand Up @@ -249,7 +250,11 @@ func TestLogin_WithCache(t *testing.T) {

*image = "foo.azurecr.io/bar:v1"
},
// NOTE: This test may still fail because of the way the Azure exchanger uses the image host.
// NOTE: This fails because the azure exchanger uses the image host
// to exchange token which can't be modified here without
// interfering image name based categorization of the login
// provider, that's actually being tested here. This is tested in
// detail in the azure package.
wantErr: true,
},
}
Expand Down

0 comments on commit 5170d4d

Please sign in to comment.