mirror of
https://github.com/k3s-io/kubernetes.git
synced 2026-08-08 15:25:26 +00:00
kubelet: Add service account UID to token cache key for proper invalidation
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
This commit is contained in:
@@ -34,6 +34,7 @@ import (
|
||||
|
||||
authenticationv1 "k8s.io/api/authentication/v1"
|
||||
v1 "k8s.io/api/core/v1"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||
"k8s.io/apimachinery/pkg/runtime/serializer"
|
||||
@@ -279,8 +280,11 @@ func (s *serviceAccountProvider) getServiceAccountData(namespace, name string) (
|
||||
}
|
||||
|
||||
// getServiceAccountToken returns a service account token for the service account.
|
||||
func (s *serviceAccountProvider) getServiceAccountToken(podNamespace, podName, serviceAccountName string, podUID types.UID) (string, error) {
|
||||
func (s *serviceAccountProvider) getServiceAccountToken(podNamespace, podName, serviceAccountName string, serviceAccountUID, podUID types.UID) (string, error) {
|
||||
tr, err := s.getServiceAccountTokenFunc(podNamespace, serviceAccountName, &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: serviceAccountUID,
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{s.audience},
|
||||
// expirationSeconds is not set explicitly here. It has the same default value of "ExpirationSeconds" in the TokenRequestSpec.
|
||||
@@ -387,7 +391,7 @@ func (p *pluginProvider) provide(image, podNamespace, podName string, podUID typ
|
||||
return credentialprovider.DockerConfig{}
|
||||
}
|
||||
|
||||
if serviceAccountToken, err = p.serviceAccountProvider.getServiceAccountToken(podNamespace, podName, serviceAccountName, podUID); err != nil {
|
||||
if serviceAccountToken, err = p.serviceAccountProvider.getServiceAccountToken(podNamespace, podName, serviceAccountName, serviceAccountUID, podUID); err != nil {
|
||||
klog.Errorf("Error getting service account token %s/%s: %v", podNamespace, serviceAccountName, err)
|
||||
return credentialprovider.DockerConfig{}
|
||||
}
|
||||
|
||||
@@ -206,5 +206,14 @@ func keyFunc(name, namespace string, tr *authenticationv1.TokenRequest) string {
|
||||
ref = *tr.Spec.BoundObjectRef
|
||||
}
|
||||
|
||||
return fmt.Sprintf("%q/%q/%#v/%#v/%#v", name, namespace, tr.Spec.Audiences, exp, ref)
|
||||
var uid types.UID
|
||||
if len(tr.UID) > 0 {
|
||||
// If UID is set in the token request it is used as a precondition
|
||||
// to ensure that the token request is for the same service account.
|
||||
// This is useful to prevent stale tokens from being returned after a service account
|
||||
// is deleted and recreated with the same name.
|
||||
uid = tr.UID
|
||||
}
|
||||
|
||||
return fmt.Sprintf("%q/%q/%#v/%#v/%#v/%q", name, namespace, tr.Spec.Audiences, exp, ref, uid)
|
||||
}
|
||||
|
||||
@@ -84,6 +84,90 @@ func TestTokenCachingAndExpiration(t *testing.T) {
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "service account recreated - cache miss due to different UID",
|
||||
exp: time.Hour,
|
||||
f: func(t *testing.T, s *suite) {
|
||||
// First, get a token for service account with UID-1
|
||||
tr1 := &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "service-account-uid-1",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(3600),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
if _, err := s.mgr.GetServiceAccountToken("a", "b", tr1); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if s.tg.count != 2 { // First call from setup + this call
|
||||
t.Fatalf("expected first token request: call count was %d", s.tg.count)
|
||||
}
|
||||
|
||||
// Now request token for "recreated" service account with UID-2
|
||||
tr2 := &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "service-account-uid-2",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(3600),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
if _, err := s.mgr.GetServiceAccountToken("a", "b", tr2); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if s.tg.count != 3 { // Should be 3 calls total (no cache hit)
|
||||
t.Fatalf("expected cache miss due to different service account UID: call count was %d", s.tg.count)
|
||||
}
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "service account UID consistent - cache hit",
|
||||
exp: time.Hour,
|
||||
f: func(t *testing.T, s *suite) {
|
||||
// Request token twice with same service account UID
|
||||
tr := &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "consistent-service-account-uid",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(3600),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
if _, err := s.mgr.GetServiceAccountToken("a", "b", tr); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if _, err := s.mgr.GetServiceAccountToken("a", "b", tr); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if s.tg.count != 2 { // Setup call + first call, second should be cache hit
|
||||
t.Fatalf("expected cache hit with same service account UID: call count was %d", s.tg.count)
|
||||
}
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
@@ -565,6 +649,168 @@ func TestKeyFunc(t *testing.T) {
|
||||
},
|
||||
shouldHit: false,
|
||||
},
|
||||
{
|
||||
name: "not hit due to different service account UID",
|
||||
trus: []tokenRequestUnit{
|
||||
{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "old-service-account-uid-123",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
target: tokenRequestUnit{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "new-service-account-uid-456", // Different service account UID
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
shouldHit: false,
|
||||
},
|
||||
{
|
||||
name: "hit with same service account UID",
|
||||
trus: []tokenRequestUnit{
|
||||
{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "same-service-account-uid-123",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
target: tokenRequestUnit{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "same-service-account-uid-123", // Same service account UID
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
shouldHit: true,
|
||||
},
|
||||
{
|
||||
name: "hit with empty UID (backward compatibility)",
|
||||
trus: []tokenRequestUnit{
|
||||
{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
// No UID set
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
target: tokenRequestUnit{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
// No UID set
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
shouldHit: true,
|
||||
},
|
||||
{
|
||||
name: "not hit when one has UID and other doesn't",
|
||||
trus: []tokenRequestUnit{
|
||||
{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "service-account-uid-123",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
target: tokenRequestUnit{
|
||||
name: "foo-sa",
|
||||
namespace: "foo-ns",
|
||||
tr: &authenticationv1.TokenRequest{
|
||||
// No UID set - should not hit cached entry with UID
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"foo1", "foo2"},
|
||||
ExpirationSeconds: getInt64Point(2000),
|
||||
BoundObjectRef: &authenticationv1.BoundObjectReference{
|
||||
Kind: "pod",
|
||||
Name: "foo-pod",
|
||||
UID: "foo-uid",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
shouldHit: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, c := range cases {
|
||||
@@ -586,7 +832,95 @@ func TestKeyFunc(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestServiceAccountRecreationCacheInvalidation(t *testing.T) {
|
||||
mgr := NewManager(nil)
|
||||
mgr.clock = testingclock.NewFakeClock(time.Time{}.Add(30 * 24 * time.Hour))
|
||||
|
||||
callCount := 0
|
||||
mgr.getToken = func(name, namespace string, tr *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
|
||||
callCount++
|
||||
expSecs := int64(3600)
|
||||
return &authenticationv1.TokenRequest{
|
||||
ObjectMeta: tr.ObjectMeta, // Preserve the UID from request
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
ExpirationSeconds: &expSecs,
|
||||
},
|
||||
Status: authenticationv1.TokenRequestStatus{
|
||||
Token: fmt.Sprintf("token-%d", callCount),
|
||||
ExpirationTimestamp: metav1.Time{Time: mgr.clock.Now().Add(time.Hour)},
|
||||
},
|
||||
}, nil
|
||||
}
|
||||
|
||||
// 1. Get token for service account with original UID
|
||||
originalTR := &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "original-sa-uid-123",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"test-audience"},
|
||||
ExpirationSeconds: getInt64Point(3600),
|
||||
},
|
||||
}
|
||||
|
||||
token1, err := mgr.GetServiceAccountToken("test-ns", "test-sa", originalTR)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if callCount != 1 {
|
||||
t.Fatalf("expected 1 API call, got %d", callCount)
|
||||
}
|
||||
if token1.Status.Token != "token-1" {
|
||||
t.Fatalf("unexpected token: %s", token1.Status.Token)
|
||||
}
|
||||
|
||||
// 2. Request same token again - should be cache hit
|
||||
token2, err := mgr.GetServiceAccountToken("test-ns", "test-sa", originalTR)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if callCount != 1 {
|
||||
t.Fatalf("expected cache hit, but got %d API calls", callCount)
|
||||
}
|
||||
if token2.Status.Token != "token-1" {
|
||||
t.Fatalf("unexpected token from cache: %s", token2.Status.Token)
|
||||
}
|
||||
|
||||
// 3. Service account recreated with new UID - should be cache miss
|
||||
recreatedTR := &authenticationv1.TokenRequest{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
UID: "recreated-sa-uid-456",
|
||||
},
|
||||
Spec: authenticationv1.TokenRequestSpec{
|
||||
Audiences: []string{"test-audience"},
|
||||
ExpirationSeconds: getInt64Point(3600),
|
||||
},
|
||||
}
|
||||
|
||||
token3, err := mgr.GetServiceAccountToken("test-ns", "test-sa", recreatedTR)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if callCount != 2 {
|
||||
t.Fatalf("expected cache miss due to UID change, but got %d API calls", callCount)
|
||||
}
|
||||
if token3.Status.Token != "token-2" {
|
||||
t.Fatalf("unexpected token for recreated SA: %s", token3.Status.Token)
|
||||
}
|
||||
|
||||
// 4. Request for recreated SA again - should be cache hit
|
||||
token4, err := mgr.GetServiceAccountToken("test-ns", "test-sa", recreatedTR)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if callCount != 2 {
|
||||
t.Fatalf("expected cache hit for recreated SA, but got %d API calls", callCount)
|
||||
}
|
||||
if token4.Status.Token != "token-2" {
|
||||
t.Fatalf("unexpected token from cache for recreated SA: %s", token4.Status.Token)
|
||||
}
|
||||
}
|
||||
|
||||
func getTokenRequest() *authenticationv1.TokenRequest {
|
||||
|
||||
Reference in New Issue
Block a user