diff --git a/pkg/registry/core/serviceaccount/storage/token.go b/pkg/registry/core/serviceaccount/storage/token.go index 19d6c55a2cc..8ba30ed8446 100644 --- a/pkg/registry/core/serviceaccount/storage/token.go +++ b/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 { - if err := createValidation(ctx, obj.DeepCopyObject()); err != nil { - return nil, err - } + req := obj.(*authenticationapi.TokenRequest) + + // 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) - - if errs := authenticationvalidation.ValidateTokenRequest(out); len(errs) != 0 { - return nil, errors.NewInvalid(gvk.GroupKind(), "", errs) + // require name/namespace in the body to match URL if specified + if len(req.Name) > 0 && req.Name != name { + 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(), 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 - if r.extendExpiration && pod != nil && out.Spec.ExpirationSeconds == token.WarnOnlyBoundTokenExpirationSeconds && r.isKubeAudiences(out.Spec.Audiences) { + exp := req.Spec.ExpirationSeconds + 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 - // 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) + sc, pc := token.Claims(*svcacct, pod, secret, exp, warnAfter, req.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)}, diff --git a/test/integration/auth/svcaccttoken_test.go b/test/integration/auth/svcaccttoken_test.go index baaccebaf0e..85bfc0fc46d 100644 --- a/test/integration/auth/svcaccttoken_test.go +++ b/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) + } +}