Validate and populate metadata fields in token request

pkg/registry/core/serviceaccount/storage/token.go

@@ -28,9 +28,11 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/apiserver/pkg/registry/rest"
+	"k8s.io/apiserver/pkg/warning"
 	authenticationapi "k8s.io/kubernetes/pkg/apis/authentication"
 	authenticationvalidation "k8s.io/kubernetes/pkg/apis/authentication/validation"
 	api "k8s.io/kubernetes/pkg/apis/core"
@@ -62,30 +64,67 @@ var gvk = schema.GroupVersionKind{
 }
 
 func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
-	if createValidation != nil {
+	req := obj.(*authenticationapi.TokenRequest)
-		if err := createValidation(ctx, obj.DeepCopyObject()); err != nil {
+
-			return nil, err
+	// Get the namespace from the context (populated from the URL).
-		}
+	namespace, ok := genericapirequest.NamespaceFrom(ctx)
+	if !ok {
+		return nil, errors.NewBadRequest("namespace is required")
 	}
 
-	out := obj.(*authenticationapi.TokenRequest)
+	// require name/namespace in the body to match URL if specified
-
+	if len(req.Name) > 0 && req.Name != name {
-	if errs := authenticationvalidation.ValidateTokenRequest(out); len(errs) != 0 {
+		errs := field.ErrorList{field.Invalid(field.NewPath("metadata").Child("name"), req.Name, "must match the service account name if specified")}
-		return nil, errors.NewInvalid(gvk.GroupKind(), "", errs)
+		return nil, errors.NewInvalid(gvk.GroupKind(), name, errs)
+	}
+	if len(req.Namespace) > 0 && req.Namespace != namespace {
+		errs := field.ErrorList{field.Invalid(field.NewPath("metadata").Child("namespace"), req.Namespace, "must match the service account namespace if specified")}
+		return nil, errors.NewInvalid(gvk.GroupKind(), name, errs)
 	}
 
+	// Lookup service account
 	svcacctObj, err := r.svcaccts.Get(ctx, name, &metav1.GetOptions{})
 	if err != nil {
 		return nil, err
 	}
 	svcacct := svcacctObj.(*api.ServiceAccount)
 
+	// Default unset spec audiences to API server audiences based on server config
+	if len(req.Spec.Audiences) == 0 {
+		req.Spec.Audiences = r.auds
+	}
+	// Populate metadata fields if not set
+	if len(req.Name) == 0 {
+		req.Name = svcacct.Name
+	}
+	if len(req.Namespace) == 0 {
+		req.Namespace = svcacct.Namespace
+	}
+
+	// Save current time before building the token, to make sure the expiration
+	// returned in TokenRequestStatus would be <= the exp field in token.
+	nowTime := time.Now()
+	req.CreationTimestamp = metav1.NewTime(nowTime)
+
+	// Clear status
+	req.Status = authenticationapi.TokenRequestStatus{}
+
+	// call static validation, then validating admission
+	if errs := authenticationvalidation.ValidateTokenRequest(req); len(errs) != 0 {
+		return nil, errors.NewInvalid(gvk.GroupKind(), "", errs)
+	}
+	if createValidation != nil {
+		if err := createValidation(ctx, obj.DeepCopyObject()); err != nil {
+			return nil, err
+		}
+	}
+
 	var (
 		pod    *api.Pod
 		secret *api.Secret
 	)
 
-	if ref := out.Spec.BoundObjectRef; ref != nil {
+	if ref := req.Spec.BoundObjectRef; ref != nil {
 		var uid types.UID
 
 		gvk := schema.FromAPIVersionAndKind(ref.APIVersion, ref.Kind)
@@ -116,13 +155,11 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object,
 			return nil, errors.NewConflict(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, ref.Name, fmt.Errorf("the UID in the bound object reference (%s) does not match the UID in record. The object might have been deleted and then recreated", ref.UID))
 		}
 	}
-	if len(out.Spec.Audiences) == 0 {
-		out.Spec.Audiences = r.auds
-	}
 
-	if r.maxExpirationSeconds > 0 && out.Spec.ExpirationSeconds > r.maxExpirationSeconds {
+	if r.maxExpirationSeconds > 0 && req.Spec.ExpirationSeconds > r.maxExpirationSeconds {
 		//only positive value is valid
-		out.Spec.ExpirationSeconds = r.maxExpirationSeconds
+		warning.AddWarning(ctx, "", fmt.Sprintf("requested expiration of %d seconds shortened to %d seconds", req.Spec.ExpirationSeconds, r.maxExpirationSeconds))
+		req.Spec.ExpirationSeconds = r.maxExpirationSeconds
 	}
 
 	// Tweak expiration for safe transition of projected service account token.
@@ -130,21 +167,20 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object,
 	// Fail after hard-coded extended expiration time.
 	// Only perform the extension when token is pod-bound.
 	var warnAfter int64
-	exp := out.Spec.ExpirationSeconds
+	exp := req.Spec.ExpirationSeconds
-	if r.extendExpiration && pod != nil && out.Spec.ExpirationSeconds == token.WarnOnlyBoundTokenExpirationSeconds && r.isKubeAudiences(out.Spec.Audiences) {
+	if r.extendExpiration && pod != nil && req.Spec.ExpirationSeconds == token.WarnOnlyBoundTokenExpirationSeconds && r.isKubeAudiences(req.Spec.Audiences) {
 		warnAfter = exp
 		exp = token.ExpirationExtensionSeconds
 	}
 
-	// Save current time before building the token, to make sure the expiration
+	sc, pc := token.Claims(*svcacct, pod, secret, exp, warnAfter, req.Spec.Audiences)
-	// returned in TokenRequestStatus would be earlier than exp field in token.
-	nowTime := time.Now()
-	sc, pc := token.Claims(*svcacct, pod, secret, exp, warnAfter, out.Spec.Audiences)
 	tokdata, err := r.issuer.GenerateToken(sc, pc)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate token: %v", err)
 	}
 
+	// populate status
+	out := req.DeepCopy()
 	out.Status = authenticationapi.TokenRequestStatus{
 		Token:               tokdata,
 		ExpirationTimestamp: metav1.Time{Time: nowTime.Add(time.Duration(out.Spec.ExpirationSeconds) * time.Second)},

test/integration/auth/svcaccttoken_test.go

@@ -29,6 +29,7 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -126,7 +127,11 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 	instanceConfig, _, closeFn := framework.RunAnAPIServer(controlPlaneConfig)
 	defer closeFn()
 
-	cs, err := clientset.NewForConfig(instanceConfig.GenericAPIServer.LoopbackClientConfig)
+	warningHandler := &recordingWarningHandler{}
+
+	configWithWarningHandler := rest.CopyConfig(instanceConfig.GenericAPIServer.LoopbackClientConfig)
+	configWithWarningHandler.WarningHandler = warningHandler
+	cs, err := clientset.NewForConfig(configWithWarningHandler)
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -182,16 +187,42 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 			},
 		}
 
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err creating token for nonexistant svcacct but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 		sa, delSvcAcct := createDeleteSvcAcct(t, cs, sa)
 		defer delSvcAcct()
 
+		treqWithBadName := treq.DeepCopy()
+		treqWithBadName.Name = "invalid-name"
+		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treqWithBadName, metav1.CreateOptions{}); err == nil || !strings.Contains(err.Error(), "must match the service account name") {
+			t.Fatalf("expected err creating token with mismatched name but got: %#v", resp)
+		}
+
+		treqWithBadNamespace := treq.DeepCopy()
+		treqWithBadNamespace.Namespace = "invalid-namespace"
+		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treqWithBadNamespace, metav1.CreateOptions{}); err == nil || !strings.Contains(err.Error(), "must match the service account namespace") {
+			t.Fatalf("expected err creating token with mismatched namespace but got: %#v", resp)
+		}
+
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
+
+		if treq.Name != sa.Name {
+			t.Errorf("expected name=%s, got %s", sa.Name, treq.Name)
+		}
+		if treq.Namespace != sa.Namespace {
+			t.Errorf("expected namespace=%s, got %s", sa.Namespace, treq.Namespace)
+		}
+		if treq.CreationTimestamp.IsZero() {
+			t.Errorf("expected non-zero creation timestamp")
+		}
 
 		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
@@ -220,34 +251,44 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 			},
 		}
 
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err creating token for nonexistant svcacct but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 		sa, del := createDeleteSvcAcct(t, cs, sa)
 		defer del()
 
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err creating token bound to nonexistant pod but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 		pod, delPod := createDeletePod(t, cs, pod)
 		defer delPod()
 
 		// right uid
 		treq.Spec.BoundObjectRef.UID = pod.UID
+		warningHandler.clear()
 		if _, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 		// wrong uid
 		treq.Spec.BoundObjectRef.UID = wrongUID
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err creating token bound to pod with wrong uid but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 		// no uid
 		treq.Spec.BoundObjectRef.UID = noUID
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
@@ -283,34 +324,44 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 			},
 		}
 
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err creating token for nonexistant svcacct but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 		sa, del := createDeleteSvcAcct(t, cs, sa)
 		defer del()
 
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err creating token bound to nonexistant secret but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 		secret, delSecret := createDeleteSecret(t, cs, secret)
 		defer delSecret()
 
 		// right uid
 		treq.Spec.BoundObjectRef.UID = secret.UID
+		warningHandler.clear()
 		if _, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 		// wrong uid
 		treq.Spec.BoundObjectRef.UID = wrongUID
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err creating token bound to secret with wrong uid but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 		// no uid
 		treq.Spec.BoundObjectRef.UID = noUID
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
@@ -341,9 +392,11 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		_, del = createDeletePod(t, cs, otherpod)
 		defer del()
 
+		warningHandler.clear()
 		if resp, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err == nil {
 			t.Fatalf("expected err but got: %#v", resp)
 		}
+		warningHandler.assertEqual(t, nil)
 	})
 
 	t.Run("expired token", func(t *testing.T) {
@@ -356,10 +409,12 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		sa, del := createDeleteSvcAcct(t, cs, sa)
 		defer del()
 
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		doTokenReview(t, cs, treq, false)
 
@@ -405,10 +460,12 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		defer delPod()
 		treq.Spec.BoundObjectRef.UID = pod.UID
 
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		doTokenReview(t, cs, treq, false)
 
@@ -459,10 +516,12 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		defer delPod()
 		treq.Spec.BoundObjectRef.UID = pod.UID
 
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		// Give some tolerance to avoid flakiness since we are using real time.
 		var leeway int64 = 10
@@ -499,10 +558,12 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		sa, del := createDeleteSvcAcct(t, cs, sa)
 		defer del()
 
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		doTokenReview(t, cs, treq, true)
 	})
@@ -515,10 +576,12 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		sa, del := createDeleteSvcAcct(t, cs, sa)
 		defer del()
 
+		warningHandler.clear()
 		treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{})
 		if err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
 
@@ -543,9 +606,11 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		defer originalDelPod()
 
 		treq.Spec.BoundObjectRef.UID = originalPod.UID
+		warningHandler.clear()
 		if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
@@ -584,9 +649,11 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		defer originalDelSecret()
 
 		treq.Spec.BoundObjectRef.UID = originalSecret.UID
+		warningHandler.clear()
 		if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
@@ -627,9 +694,11 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		defer originalDelSecret()
 
 		treq.Spec.BoundObjectRef.UID = originalSecret.UID
+		warningHandler.clear()
 		if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 
 		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
@@ -671,9 +740,11 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		defer originalDelSecret()
 
 		treq.Spec.BoundObjectRef.UID = originalSecret.UID
+		warningHandler.clear()
 		if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(context.TODO(), sa.Name, treq, metav1.CreateOptions{}); err != nil {
 			t.Fatalf("err: %v", err)
 		}
+		warningHandler.assertEqual(t, []string{fmt.Sprintf("requested expiration of %d seconds shortened to %d seconds", tooLongExpirationTime, maxExpirationSeconds)})
 
 		checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
 		checkPayload(t, treq.Status.Token, `["api"]`, "aud")
@@ -698,6 +769,7 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		defer del()
 
 		t.Log("get token")
+		warningHandler.clear()
 		tokenRequest, err := cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(
 			context.TODO(),
 			sa.Name,
@@ -709,6 +781,7 @@ func TestServiceAccountTokenCreate(t *testing.T) {
 		if err != nil {
 			t.Fatalf("unexpected error creating token: %v", err)
 		}
+		warningHandler.assertEqual(t, nil)
 		token := tokenRequest.Status.Token
 		if token == "" {
 			t.Fatal("no token")
@@ -972,3 +1045,29 @@ func (f *fakeIndexer) GetByKey(key string) (interface{}, bool, error) {
 	obj, err := f.get(namespace, name)
 	return obj, err == nil, err
 }
+
+type recordingWarningHandler struct {
+	warnings []string
+
+	sync.Mutex
+}
+
+func (r *recordingWarningHandler) HandleWarningHeader(code int, agent string, message string) {
+	r.Lock()
+	defer r.Unlock()
+	r.warnings = append(r.warnings, message)
+}
+
+func (r *recordingWarningHandler) clear() {
+	r.Lock()
+	defer r.Unlock()
+	r.warnings = nil
+}
+func (r *recordingWarningHandler) assertEqual(t *testing.T, expected []string) {
+	t.Helper()
+	r.Lock()
+	defer r.Unlock()
+	if !reflect.DeepEqual(r.warnings, expected) {
+		t.Errorf("expected\n\t%v\ngot\n\t%v", expected, r.warnings)
+	}
+}