Commit 9f95cf7b authored by Andy Goldstein's avatar Andy Goldstein

serviceaccount admission: return correct tokens

Fix a bug in serviceaccount admission introduced when we switched everything to use shared informers. That change accidentally reused the list of secrets instead of creating a new one, resulting in all secrets in the namespace being returned as possible service account tokens, instead of limiting it only to the actual service account tokens, as it did before the shared informer conversion. This also adds a unit test to ensure there is no future regression here.
parent 176eb0e5
...@@ -43,12 +43,14 @@ go_test( ...@@ -43,12 +43,14 @@ go_test(
"//pkg/api:go_default_library", "//pkg/api:go_default_library",
"//pkg/client/clientset_generated/internalclientset/fake:go_default_library", "//pkg/client/clientset_generated/internalclientset/fake:go_default_library",
"//pkg/client/informers/informers_generated/internalversion:go_default_library", "//pkg/client/informers/informers_generated/internalversion:go_default_library",
"//pkg/client/listers/core/internalversion:go_default_library",
"//pkg/controller:go_default_library", "//pkg/controller:go_default_library",
"//pkg/kubelet/types:go_default_library", "//pkg/kubelet/types:go_default_library",
"//vendor:k8s.io/apimachinery/pkg/api/errors", "//vendor:k8s.io/apimachinery/pkg/api/errors",
"//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
"//vendor:k8s.io/apimachinery/pkg/types", "//vendor:k8s.io/apimachinery/pkg/types",
"//vendor:k8s.io/apiserver/pkg/admission", "//vendor:k8s.io/apiserver/pkg/admission",
"//vendor:k8s.io/client-go/tools/cache",
], ],
) )
......
...@@ -287,18 +287,20 @@ func (s *serviceAccount) getReferencedServiceAccountToken(serviceAccount *api.Se ...@@ -287,18 +287,20 @@ func (s *serviceAccount) getReferencedServiceAccountToken(serviceAccount *api.Se
// getServiceAccountTokens returns all ServiceAccountToken secrets for the given ServiceAccount // getServiceAccountTokens returns all ServiceAccountToken secrets for the given ServiceAccount
func (s *serviceAccount) getServiceAccountTokens(serviceAccount *api.ServiceAccount) ([]*api.Secret, error) { func (s *serviceAccount) getServiceAccountTokens(serviceAccount *api.ServiceAccount) ([]*api.Secret, error) {
tokens, err := s.secretLister.Secrets(serviceAccount.Namespace).List(labels.Everything()) secrets, err := s.secretLister.Secrets(serviceAccount.Namespace).List(labels.Everything())
if err != nil { if err != nil {
return nil, err return nil, err
} }
for _, token := range tokens { tokens := []*api.Secret{}
if token.Type != api.SecretTypeServiceAccountToken {
for _, secret := range secrets {
if secret.Type != api.SecretTypeServiceAccountToken {
continue continue
} }
if serviceaccount.InternalIsServiceAccountToken(token, serviceAccount) { if serviceaccount.InternalIsServiceAccountToken(secret, serviceAccount) {
tokens = append(tokens, token) tokens = append(tokens, secret)
} }
} }
return tokens, nil return tokens, nil
......
...@@ -25,9 +25,11 @@ import ( ...@@ -25,9 +25,11 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission"
"k8s.io/client-go/tools/cache"
"k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
corelisters "k8s.io/kubernetes/pkg/client/listers/core/internalversion"
"k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller"
kubelet "k8s.io/kubernetes/pkg/kubelet/types" kubelet "k8s.io/kubernetes/pkg/kubelet/types"
) )
...@@ -849,3 +851,59 @@ func TestMultipleReferencedSecrets(t *testing.T) { ...@@ -849,3 +851,59 @@ func TestMultipleReferencedSecrets(t *testing.T) {
t.Errorf("expected first referenced secret to be mounted, got %q", name) t.Errorf("expected first referenced secret to be mounted, got %q", name)
} }
} }
func newSecret(secretType api.SecretType, namespace, name, serviceAccountName, serviceAccountUID string) *api.Secret {
return &api.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Name: name,
Annotations: map[string]string{
api.ServiceAccountNameKey: serviceAccountName,
api.ServiceAccountUIDKey: serviceAccountUID,
},
},
Type: secretType,
}
}
func TestGetServiceAccountTokens(t *testing.T) {
admit := NewServiceAccount()
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
admit.secretLister = corelisters.NewSecretLister(indexer)
ns := "namespace"
serviceAccountUID := "12345"
sa := &api.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: DefaultServiceAccountName,
Namespace: ns,
UID: types.UID(serviceAccountUID),
},
}
nonSATokenSecret := newSecret(api.SecretTypeDockercfg, ns, "nonSATokenSecret", DefaultServiceAccountName, serviceAccountUID)
indexer.Add(nonSATokenSecret)
differentSAToken := newSecret(api.SecretTypeServiceAccountToken, ns, "differentSAToken", "someOtherSA", "someOtherUID")
indexer.Add(differentSAToken)
matchingSAToken := newSecret(api.SecretTypeServiceAccountToken, ns, "matchingSAToken", DefaultServiceAccountName, serviceAccountUID)
indexer.Add(matchingSAToken)
tokens, err := admit.getServiceAccountTokens(sa)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(tokens) != 1 {
names := make([]string, 0, len(tokens))
for _, token := range tokens {
names = append(names, token.Name)
}
t.Fatalf("expected only 1 token, got %v", names)
}
if e, a := matchingSAToken.Name, tokens[0].Name; e != a {
t.Errorf("expected token %s, got %s", e, a)
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment