pr feedback updates

aws · Nov 18, 2024 · a37069c · a37069c
1 parent 0178602
commit a37069c
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 21 deletions.
diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go
@@ -304,12 +304,14 @@ func New(defaultAudience,
 
 	go func() {
 		for req := range saFetchRequests {
-			sa, err := fetchFromAPI(SAGetter, req)
-			if err != nil {
-				klog.Errorf("fetching SA: %s, but got error from API: %v", req.CacheKey(), err)
-				continue
-			}
-			c.addSA(sa)
+			go func() {
+				sa, err := fetchFromAPI(SAGetter, req)
+				if err != nil {
+					klog.Errorf("fetching SA: %s, but got error from API: %v", req.CacheKey(), err)
+					return
+				}
+				c.addSA(sa)
+			}()
 		}
 	}()
 
@@ -367,22 +369,14 @@ func fetchFromAPI(getter corev1.ServiceAccountsGetter, req *Request) (*v1.Servic
 	defer cancel()
 
 	klog.V(5).Infof("fetching SA: %s", req.CacheKey())
-	saList, err := getter.ServiceAccounts(req.Namespace).List(
-		ctx,
-		metav1.ListOptions{},
-	)
+
+	sa, err := getter.ServiceAccounts(req.Namespace).Get(ctx, req.Name, metav1.GetOptions{})
+
 	if err != nil {
 		return nil, err
 	}
 
-	// Find the ServiceAccount
-	for _, sa := range saList.Items {
-		if sa.Name == req.Name {
-			return &sa, nil
-
-		}
-	}
-	return nil, fmt.Errorf("no SA found in namespace: %s", req.CacheKey())
+	return sa, nil
 }
 
 func (c *serviceAccountCache) populateCacheFromCM(oldCM, newCM *v1.ConfigMap) error {

diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go
@@ -157,7 +157,7 @@ func TestNotification(t *testing.T) {
 func TestFetchFromAPIServer(t *testing.T) {
 	testSA := &v1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "default",
+			Name:      "my-sa",
 			Namespace: "default",
 			Annotations: map[string]string{
 				"eks.amazonaws.com/role-arn":         "arn:aws:iam::111122223333:role/s3-reader",
@@ -196,15 +196,15 @@ func TestFetchFromAPIServer(t *testing.T) {
 		t.Fatalf("informer never called client: %v", err)
 	}
 
-	resp := cache.Get(Request{Name: "default", Namespace: "default", RequestNotification: true})
+	resp := cache.Get(Request{Name: "my-sa", Namespace: "default", RequestNotification: true})
 	assert.False(t, resp.FoundInCache, "Expected cache entry to not be found")
 
 	// wait for the notification while we fetch the SA from the API server:
 	select {
 	case <-resp.Notifier:
 		// expected
 		// test that the requested SA is now in the cache
-		resp := cache.Get(Request{Name: "default", Namespace: "default", RequestNotification: false})
+		resp := cache.Get(Request{Name: "my-sa", Namespace: "default", RequestNotification: false})
 		assert.True(t, resp.FoundInCache, "Expected cache entry to be found in cache")
 	case <-time.After(1 * time.Second):
 		t.Fatal("timeout waiting for notification")