From 5170d4da1d3ace3be3f16231c1a41c092871ae4f Mon Sep 17 00:00:00 2001
From: Joseph Stevens <thejosephstevens@gmail.com>
Date: Tue, 15 Oct 2024 18:07:21 -0700
Subject: [PATCH] finishing cleanup

---
 oci/auth/gcp/auth_test.go    | 8 ++++++++
 oci/auth/login/cache.go      | 8 --------
 oci/auth/login/login.go      | 7 -------
 oci/auth/login/login_test.go | 9 +++++++--
 4 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/oci/auth/gcp/auth_test.go b/oci/auth/gcp/auth_test.go
index a49b587f..31aba6fd 100644
--- a/oci/auth/gcp/auth_test.go
+++ b/oci/auth/gcp/auth_test.go
@@ -113,6 +113,7 @@ func TestLogin(t *testing.T) {
 		image     string
 		token     *oauth2.Token
 		tokenErr  error
+		testOIDC  bool
 		wantErr   bool
 	}{
 		{
@@ -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",
@@ -136,6 +138,7 @@ func TestLogin(t *testing.T) {
 			autoLogin: true,
 			image:     testValidGCRImage,
 			tokenErr:  fmt.Errorf("token error"),
+			testOIDC:  true,
 			wantErr:   true,
 		},
 		{
@@ -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))
+			}
 		})
 	}
 }
diff --git a/oci/auth/login/cache.go b/oci/auth/login/cache.go
index bf66dea1..13180ff6 100644
--- a/oci/auth/login/cache.go
+++ b/oci/auth/login/cache.go
@@ -17,7 +17,6 @@ limitations under the License.
 package login
 
 import (
-	"fmt"
 	"time"
 
 	"github.com/google/go-containerregistry/pkg/authn"
@@ -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
 }
diff --git a/oci/auth/login/login.go b/oci/auth/login/login.go
index fc03ad76..9ffaef94 100644
--- a/oci/auth/login/login.go
+++ b/oci/auth/login/login.go
@@ -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 {
@@ -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")
diff --git a/oci/auth/login/login_test.go b/oci/auth/login/login_test.go
index c0d479bd..e764d011 100644
--- a/oci/auth/login/login_test.go
+++ b/oci/auth/login/login_test.go
@@ -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,
 		},
 		{
@@ -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,
 		},
 	}