(bug) Fix service account token secret reference (#2862)

Problem: The filterSecretsBySAName function attempts to identify all
service account token secrets related to a serviceAccount. To do so,
the filterSecretsBySAName function uses a range-for loop to iterate
over entries in the secrets argument. If a valid service account token
secret is found, a pointer to the range-for loop's value variable is
added to a map of results. Unfortunately, if a valid entry is found in
the middle of the list of secrets, the value returned by the range-for
loop is updated, causes the entry in the map to change.

Solution: Add a pointer to the actual secret instead of the range-for
loop's value variable.

Signed-off-by: Alexander Greene <[email protected]>

Signed-off-by: Alexander Greene <[email protected]>

Author: awgreene

pkg/lib/scoped/token_retriever.go

@@ -82,6 +82,12 @@ func getAPISecret(logger logrus.FieldLogger, kubeclient operatorclient.ClientInt
 func filterSecretsBySAName(saName string, secrets *corev1.SecretList) map[string]*corev1.Secret {
 	secretMap := make(map[string]*corev1.Secret)
 	for _, ref := range secrets.Items {
+		// Avoid referencing the "ref" created by the range-for loop as
+		// the secret stored in the map will change if there are more
+		// entries in the list of secrets that the range-for loop is
+		// iterating over.
+		ref := ref
+
 		annotations := ref.GetAnnotations()
 		value := annotations[corev1.ServiceAccountNameKey]
 		if value == saName {

pkg/lib/scoped/token_retriever_test.go

@@ -0,0 +1,117 @@
+package scoped
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+const serviceAccountName = "foo"
+
+func TestFilterSecretsBySAName(t *testing.T) {
+	tests := []struct {
+		name              string
+		secrets           *corev1.SecretList
+		wantedSecretNames []string
+	}{
+		{
+			name: "NoSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret"),
+					*newSecret("zSecret"),
+				},
+			},
+			wantedSecretNames: []string{},
+		},
+		{
+			name: "FirstSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("someSecret"),
+					*newSecret("zSecret"),
+				},
+			},
+			wantedSecretNames: []string{"aSecret"},
+		},
+		{
+			name: "SecondSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("zSecret"),
+				},
+			},
+			wantedSecretNames: []string{"someSecret"},
+		},
+		{
+			name: "ThirdSecretFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret"),
+					*newSecret("zSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+				},
+			},
+			wantedSecretNames: []string{"zSecret"},
+		},
+		{
+			name: "TwoSecretsFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret"),
+					*newSecret("someSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("zSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+				},
+			},
+			wantedSecretNames: []string{"someSecret", "zSecret"},
+		},
+		{
+			name: "AllSecretsFound",
+			secrets: &corev1.SecretList{
+				Items: []corev1.Secret{
+					*newSecret("aSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("someSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+					*newSecret("zSecret", withAnnotations(map[string]string{corev1.ServiceAccountNameKey: serviceAccountName})),
+				},
+			},
+			wantedSecretNames: []string{"aSecret", "someSecret", "zSecret"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := filterSecretsBySAName(serviceAccountName, tt.secrets)
+			require.Equal(t, len(tt.wantedSecretNames), len(got))
+			for _, wantedSecretName := range tt.wantedSecretNames {
+				require.NotNil(t, got[wantedSecretName])
+				require.Equal(t, wantedSecretName, got[wantedSecretName].GetName())
+			}
+		})
+	}
+}
+
+type secretOption func(*corev1.Secret)
+
+func withAnnotations(annotations map[string]string) secretOption {
+	return func(s *corev1.Secret) {
+		s.SetAnnotations(annotations)
+	}
+}
+
+func newSecret(name string, opts ...secretOption) *corev1.Secret {
+	s := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+	}
+	for _, opt := range opts {
+		opt(s)
+	}
+	return s
+}