Merge pull request #108309 from zshihang/token

no auto-generation of secret-based service account token
This commit is contained in:
Kubernetes Prow Robot
2022-03-02 06:19:15 -08:00
committed by GitHub
7 changed files with 241 additions and 325 deletions

View File

@@ -22,7 +22,7 @@ import (
"fmt"
"time"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
@@ -68,6 +68,9 @@ type TokensControllerOptions struct {
// MaxRetries controls the maximum number of times a particular key is retried before giving up
// If zero, a default max is used
MaxRetries int
// AutoGenerate decides the auto-generation of secret-based token for service accounts.
AutoGenerate bool
}
// NewTokensController returns a new *TokensController.
@@ -85,7 +88,8 @@ func NewTokensController(serviceAccounts informers.ServiceAccountInformer, secre
syncServiceAccountQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "serviceaccount_tokens_service"),
syncSecretQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "serviceaccount_tokens_secret"),
maxRetries: maxRetries,
maxRetries: maxRetries,
autoGenerate: options.AutoGenerate,
}
if cl != nil && cl.CoreV1().RESTClient().GetRateLimiter() != nil {
if err := ratelimiter.RegisterMetricAndTrackRateLimiterUsage("serviceaccount_tokens_controller", cl.CoreV1().RESTClient().GetRateLimiter()); err != nil {
@@ -160,7 +164,8 @@ type TokensController struct {
// key is a secretQueueKey{}
syncSecretQueue workqueue.RateLimitingInterface
maxRetries int
maxRetries int
autoGenerate bool
}
// Run runs controller blocks until stopCh is closed
@@ -255,7 +260,7 @@ func (e *TokensController) syncServiceAccount() {
if err != nil {
klog.Errorf("error deleting serviceaccount tokens for %s/%s: %v", saInfo.namespace, saInfo.name, err)
}
default:
case e.autoGenerate:
// ensure a token exists and is referenced by this service account
retry, err = e.ensureReferencedToken(sa)
if err != nil {

View File

@@ -24,19 +24,20 @@ import (
"github.com/davecgh/go-spew/spew"
"gopkg.in/square/go-jose.v2/jwt"
"k8s.io/klog/v2"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
utilrand "k8s.io/apimachinery/pkg/util/rand"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes/fake"
core "k8s.io/client-go/testing"
featuregatetesting "k8s.io/component-base/featuregate/testing"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/features"
)
type testGenerator struct {
@@ -207,8 +208,9 @@ func TestTokenCreation(t *testing.T) {
testcases := map[string]struct {
ClientObjects []runtime.Object
IsAsync bool
MaxRetries int
IsAsync bool
MaxRetries int
autoGenerateDisabled bool
Reactors []reaction
@@ -235,6 +237,14 @@ func TestTokenCreation(t *testing.T) {
core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(emptySecretReferences()))),
},
},
"new serviceaccount with no secrets [autogenerate disabled]": {
autoGenerateDisabled: true,
ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences())},
AddedServiceAccount: serviceAccount(emptySecretReferences()),
ExpectedActions: []core.Action{},
},
"new serviceaccount with no secrets encountering create error": {
ClientObjects: []runtime.Object{serviceAccount(emptySecretReferences())},
MaxRetries: 10,
@@ -306,6 +316,14 @@ func TestTokenCreation(t *testing.T) {
core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(missingSecretReferences()))),
},
},
"new serviceaccount with missing secrets [autogenerate disabled]": {
autoGenerateDisabled: true,
ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())},
AddedServiceAccount: serviceAccount(missingSecretReferences()),
ExpectedActions: []core.Action{},
},
"new serviceaccount with missing secrets and a local secret in the cache": {
ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())},
@@ -313,6 +331,15 @@ func TestTokenCreation(t *testing.T) {
AddedSecretLocal: serviceAccountTokenSecret(),
ExpectedActions: []core.Action{},
},
"new serviceaccount with missing secrets and a local secret in the cache [autogenerate disabled]": {
autoGenerateDisabled: true,
ClientObjects: []runtime.Object{serviceAccount(missingSecretReferences())},
AddedServiceAccount: serviceAccount(tokenSecretReferences()),
AddedSecretLocal: serviceAccountTokenSecret(),
ExpectedActions: []core.Action{},
},
"new serviceaccount with non-token secrets": {
ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), opaqueSecret()},
@@ -323,6 +350,14 @@ func TestTokenCreation(t *testing.T) {
core.NewUpdateAction(schema.GroupVersionResource{Version: "v1", Resource: "serviceaccounts"}, metav1.NamespaceDefault, serviceAccount(addTokenSecretReference(regularSecretReferences()))),
},
},
"new serviceaccount with non-token secrets [autogenerate disabled]": {
autoGenerateDisabled: true,
ClientObjects: []runtime.Object{serviceAccount(regularSecretReferences()), opaqueSecret()},
AddedServiceAccount: serviceAccount(regularSecretReferences()),
ExpectedActions: []core.Action{},
},
"new serviceaccount with token secrets": {
ClientObjects: []runtime.Object{serviceAccount(tokenSecretReferences()), serviceAccountTokenSecret()},
ExistingSecrets: []*v1.Secret{serviceAccountTokenSecret()},
@@ -330,6 +365,15 @@ func TestTokenCreation(t *testing.T) {
AddedServiceAccount: serviceAccount(tokenSecretReferences()),
ExpectedActions: []core.Action{},
},
"new serviceaccount with token secrets [autogenerate disabled]": {
autoGenerateDisabled: false,
ClientObjects: []runtime.Object{serviceAccount(tokenSecretReferences()), serviceAccountTokenSecret()},
ExistingSecrets: []*v1.Secret{serviceAccountTokenSecret()},
AddedServiceAccount: serviceAccount(tokenSecretReferences()),
ExpectedActions: []core.Action{},
},
"new serviceaccount with no secrets with resource conflict": {
ClientObjects: []runtime.Object{updatedServiceAccount(emptySecretReferences()), createdTokenSecret()},
IsAsync: true,
@@ -577,124 +621,126 @@ func TestTokenCreation(t *testing.T) {
}
for k, tc := range testcases {
klog.Infof(k)
t.Run(k, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.LegacyServiceAccountTokenNoAutoGeneration, tc.autoGenerateDisabled)()
// Re-seed to reset name generation
utilrand.Seed(1)
// Re-seed to reset name generation
utilrand.Seed(1)
generator := &testGenerator{Token: "ABC"}
generator := &testGenerator{Token: "ABC"}
client := fake.NewSimpleClientset(tc.ClientObjects...)
for _, reactor := range tc.Reactors {
client.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactor(t))
}
informers := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc())
secretInformer := informers.Core().V1().Secrets().Informer()
secrets := secretInformer.GetStore()
serviceAccounts := informers.Core().V1().ServiceAccounts().Informer().GetStore()
controller, err := NewTokensController(informers.Core().V1().ServiceAccounts(), informers.Core().V1().Secrets(), client, TokensControllerOptions{TokenGenerator: generator, RootCA: []byte("CA Data"), MaxRetries: tc.MaxRetries})
if err != nil {
t.Fatalf("error creating Tokens controller: %v", err)
}
if tc.ExistingServiceAccount != nil {
serviceAccounts.Add(tc.ExistingServiceAccount)
}
for _, s := range tc.ExistingSecrets {
secrets.Add(s)
}
if tc.AddedServiceAccount != nil {
serviceAccounts.Add(tc.AddedServiceAccount)
controller.queueServiceAccountSync(tc.AddedServiceAccount)
}
if tc.UpdatedServiceAccount != nil {
serviceAccounts.Add(tc.UpdatedServiceAccount)
controller.queueServiceAccountUpdateSync(nil, tc.UpdatedServiceAccount)
}
if tc.DeletedServiceAccount != nil {
serviceAccounts.Delete(tc.DeletedServiceAccount)
controller.queueServiceAccountSync(tc.DeletedServiceAccount)
}
if tc.AddedSecret != nil {
secrets.Add(tc.AddedSecret)
controller.queueSecretSync(tc.AddedSecret)
}
if tc.AddedSecretLocal != nil {
controller.updatedSecrets.Mutation(tc.AddedSecretLocal)
}
if tc.UpdatedSecret != nil {
secrets.Add(tc.UpdatedSecret)
controller.queueSecretUpdateSync(nil, tc.UpdatedSecret)
}
if tc.DeletedSecret != nil {
secrets.Delete(tc.DeletedSecret)
controller.queueSecretSync(tc.DeletedSecret)
}
// This is the longest we'll wait for async tests
timeout := time.Now().Add(30 * time.Second)
waitedForAdditionalActions := false
for {
if controller.syncServiceAccountQueue.Len() > 0 {
controller.syncServiceAccount()
client := fake.NewSimpleClientset(tc.ClientObjects...)
for _, reactor := range tc.Reactors {
client.Fake.PrependReactor(reactor.verb, reactor.resource, reactor.reactor(t))
}
if controller.syncSecretQueue.Len() > 0 {
controller.syncSecret()
informers := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc())
secretInformer := informers.Core().V1().Secrets().Informer()
secrets := secretInformer.GetStore()
serviceAccounts := informers.Core().V1().ServiceAccounts().Informer().GetStore()
controller, err := NewTokensController(informers.Core().V1().ServiceAccounts(), informers.Core().V1().Secrets(), client, TokensControllerOptions{TokenGenerator: generator, RootCA: []byte("CA Data"), MaxRetries: tc.MaxRetries, AutoGenerate: !tc.autoGenerateDisabled})
if err != nil {
t.Fatalf("error creating Tokens controller: %v", err)
}
// The queues still have things to work on
if controller.syncServiceAccountQueue.Len() > 0 || controller.syncSecretQueue.Len() > 0 {
continue
if tc.ExistingServiceAccount != nil {
serviceAccounts.Add(tc.ExistingServiceAccount)
}
for _, s := range tc.ExistingSecrets {
secrets.Add(s)
}
// If we expect this test to work asynchronously...
if tc.IsAsync {
// if we're still missing expected actions within our test timeout
if len(client.Actions()) < len(tc.ExpectedActions) && time.Now().Before(timeout) {
// wait for the expected actions (without hotlooping)
time.Sleep(time.Millisecond)
if tc.AddedServiceAccount != nil {
serviceAccounts.Add(tc.AddedServiceAccount)
controller.queueServiceAccountSync(tc.AddedServiceAccount)
}
if tc.UpdatedServiceAccount != nil {
serviceAccounts.Add(tc.UpdatedServiceAccount)
controller.queueServiceAccountUpdateSync(nil, tc.UpdatedServiceAccount)
}
if tc.DeletedServiceAccount != nil {
serviceAccounts.Delete(tc.DeletedServiceAccount)
controller.queueServiceAccountSync(tc.DeletedServiceAccount)
}
if tc.AddedSecret != nil {
secrets.Add(tc.AddedSecret)
controller.queueSecretSync(tc.AddedSecret)
}
if tc.AddedSecretLocal != nil {
controller.updatedSecrets.Mutation(tc.AddedSecretLocal)
}
if tc.UpdatedSecret != nil {
secrets.Add(tc.UpdatedSecret)
controller.queueSecretUpdateSync(nil, tc.UpdatedSecret)
}
if tc.DeletedSecret != nil {
secrets.Delete(tc.DeletedSecret)
controller.queueSecretSync(tc.DeletedSecret)
}
// This is the longest we'll wait for async tests
timeout := time.Now().Add(30 * time.Second)
waitedForAdditionalActions := false
for {
if controller.syncServiceAccountQueue.Len() > 0 {
controller.syncServiceAccount()
}
if controller.syncSecretQueue.Len() > 0 {
controller.syncSecret()
}
// The queues still have things to work on
if controller.syncServiceAccountQueue.Len() > 0 || controller.syncSecretQueue.Len() > 0 {
continue
}
// if we exactly match our expected actions, wait a bit to make sure no other additional actions show up
if len(client.Actions()) == len(tc.ExpectedActions) && !waitedForAdditionalActions {
time.Sleep(time.Second)
waitedForAdditionalActions = true
continue
// If we expect this test to work asynchronously...
if tc.IsAsync {
// if we're still missing expected actions within our test timeout
if len(client.Actions()) < len(tc.ExpectedActions) && time.Now().Before(timeout) {
// wait for the expected actions (without hotlooping)
time.Sleep(time.Millisecond)
continue
}
// if we exactly match our expected actions, wait a bit to make sure no other additional actions show up
if len(client.Actions()) == len(tc.ExpectedActions) && !waitedForAdditionalActions {
time.Sleep(time.Second)
waitedForAdditionalActions = true
continue
}
}
}
break
}
if controller.syncServiceAccountQueue.Len() > 0 {
t.Errorf("%s: unexpected items in service account queue: %d", k, controller.syncServiceAccountQueue.Len())
}
if controller.syncSecretQueue.Len() > 0 {
t.Errorf("%s: unexpected items in secret queue: %d", k, controller.syncSecretQueue.Len())
}
actions := client.Actions()
for i, action := range actions {
if len(tc.ExpectedActions) < i+1 {
t.Errorf("%s: %d unexpected actions: %+v", k, len(actions)-len(tc.ExpectedActions), actions[i:])
break
}
expectedAction := tc.ExpectedActions[i]
if !reflect.DeepEqual(expectedAction, action) {
t.Errorf("%s:\nExpected:\n%s\ngot:\n%s", k, spew.Sdump(expectedAction), spew.Sdump(action))
continue
if controller.syncServiceAccountQueue.Len() > 0 {
t.Errorf("%s: unexpected items in service account queue: %d", k, controller.syncServiceAccountQueue.Len())
}
if controller.syncSecretQueue.Len() > 0 {
t.Errorf("%s: unexpected items in secret queue: %d", k, controller.syncSecretQueue.Len())
}
}
if len(tc.ExpectedActions) > len(actions) {
t.Errorf("%s: %d additional expected actions", k, len(tc.ExpectedActions)-len(actions))
for _, a := range tc.ExpectedActions[len(actions):] {
t.Logf(" %+v", a)
actions := client.Actions()
for i, action := range actions {
if len(tc.ExpectedActions) < i+1 {
t.Errorf("%s: %d unexpected actions: %+v", k, len(actions)-len(tc.ExpectedActions), actions[i:])
break
}
expectedAction := tc.ExpectedActions[i]
if !reflect.DeepEqual(expectedAction, action) {
t.Errorf("%s:\nExpected:\n%s\ngot:\n%s", k, spew.Sdump(expectedAction), spew.Sdump(action))
continue
}
}
}
if len(tc.ExpectedActions) > len(actions) {
t.Errorf("%s: %d additional expected actions", k, len(tc.ExpectedActions)-len(actions))
for _, a := range tc.ExpectedActions[len(actions):] {
t.Logf(" %+v", a)
}
}
})
}
}