From 67444247c14b9c73d2e194a827a5aaa448aa21a3 Mon Sep 17 00:00:00 2001 From: Michal Fojtik Date: Mon, 17 Oct 2016 11:22:26 +0200 Subject: [PATCH] serviceaccount: add secret informer to create_dockercfg_secret --- pkg/cmd/server/origin/run_components.go | 2 +- .../controllers/create_dockercfg_secrets.go | 224 +++++++++++++----- 2 files changed, 166 insertions(+), 60 deletions(-) diff --git a/pkg/cmd/server/origin/run_components.go b/pkg/cmd/server/origin/run_components.go index 857009c202f4..beab0c8efa0e 100644 --- a/pkg/cmd/server/origin/run_components.go +++ b/pkg/cmd/server/origin/run_components.go @@ -304,7 +304,7 @@ func (c *MasterConfig) RunBuildPodController() { func (c *MasterConfig) RunBuildImageChangeTriggerController() { bcClient, _ := c.BuildImageChangeTriggerControllerClients() bcInstantiator := buildclient.NewOSClientBuildConfigInstantiatorClient(bcClient) - bcIndex := &oscache.StoreToBuildConfigListerImpl{c.Informers.BuildConfigs().Indexer()} + bcIndex := &oscache.StoreToBuildConfigListerImpl{Indexer: c.Informers.BuildConfigs().Indexer()} bcIndexSynced := c.Informers.BuildConfigs().Informer().HasSynced factory := buildcontrollerfactory.ImageChangeControllerFactory{Client: bcClient, BuildConfigInstantiator: bcInstantiator, BuildConfigIndex: bcIndex, BuildConfigIndexSynced: bcIndexSynced} go func() { diff --git a/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go b/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go index 3c67dbef6db2..f9a0b19bb011 100644 --- a/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go +++ b/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go @@ -15,8 +15,10 @@ import ( "k8s.io/kubernetes/pkg/controller" "k8s.io/kubernetes/pkg/controller/framework" "k8s.io/kubernetes/pkg/credentialprovider" + "k8s.io/kubernetes/pkg/fields" "k8s.io/kubernetes/pkg/registry/secret" "k8s.io/kubernetes/pkg/runtime" + "k8s.io/kubernetes/pkg/types" utilruntime "k8s.io/kubernetes/pkg/util/runtime" "k8s.io/kubernetes/pkg/util/sets" "k8s.io/kubernetes/pkg/util/wait" @@ -33,6 +35,14 @@ const ( // ServiceAccountTokenValueAnnotation stores the actual value of the token so that a dockercfg secret can be // made without having a value dockerURL ServiceAccountTokenValueAnnotation = "openshift.io/token-secret.value" + + // CreateDockercfgSecretsController is the name of this controller that should be + // attached to all token secrets this controller create + CreateDockercfgSecretsController = "openshift.io/create-dockercfg-secrets" + + // PendingTokenAnnotation contains the name of the token secret that is waiting for the + // token data population + PendingTokenAnnotation = "openshift.io/create-dockercfg-secrets.pending-token" ) // DockercfgControllerOptions contains options for the DockercfgController @@ -81,8 +91,29 @@ func NewDockercfgController(cl client.Interface, options DockercfgControllerOpti }, }, ) - e.serviceAccountCache = NewEtcdMutationCache(serviceAccountCache) + + tokenSecretSelector := fields.OneTermEqualSelector(api.SecretTypeField, string(api.SecretTypeServiceAccountToken)) + e.secretCache, e.secretController = framework.NewInformer( + &cache.ListWatch{ + ListFunc: func(options api.ListOptions) (runtime.Object, error) { + options.FieldSelector = tokenSecretSelector + return e.client.Secrets(api.NamespaceAll).List(options) + }, + WatchFunc: func(options api.ListOptions) (watch.Interface, error) { + options.FieldSelector = tokenSecretSelector + return e.client.Secrets(api.NamespaceAll).Watch(options) + }, + }, + &api.Secret{}, + options.Resync, + framework.ResourceEventHandlerFuncs{ + AddFunc: func(cur interface{}) { e.handleTokenSecretUpdate(nil, cur) }, + UpdateFunc: func(old, cur interface{}) { e.handleTokenSecretUpdate(old, cur) }, + DeleteFunc: e.handleTokenSecretDelete, + }, + ) + e.syncHandler = e.syncServiceAccount return e @@ -98,6 +129,8 @@ type DockercfgController struct { serviceAccountCache MutationCache serviceAccountController *framework.Controller + secretCache cache.Store + secretController *framework.Controller queue workqueue.RateLimitingInterface @@ -105,6 +138,71 @@ type DockercfgController struct { syncHandler func(serviceKey string) error } +// handleTokenSecretUpdate checks if the service account token secret is populated with +// token data and triggers re-sync of service account when the data are observed. +func (e *DockercfgController) handleTokenSecretUpdate(oldObj, newObj interface{}) { + secret := newObj.(*api.Secret) + if secret.Annotations[api.CreatedByAnnotation] != CreateDockercfgSecretsController { + return + } + isPopulated := len(secret.Data[api.ServiceAccountTokenKey]) > 0 + + wasPopulated := false + if oldObj != nil { + oldSecret := oldObj.(*api.Secret) + wasPopulated = len(oldSecret.Data[api.ServiceAccountTokenKey]) > 0 + glog.V(5).Infof("Updating token secret %s/%s", secret.Namespace, secret.Name) + } else { + glog.V(5).Infof("Adding token secret %s/%s", secret.Namespace, secret.Name) + } + + if !wasPopulated && isPopulated { + e.enqueueServiceAccountForToken(secret) + } +} + +// handleTokenSecretDelete handles token secrets deletion and re-sync the service account +// which will cause a token to be re-created. +func (e *DockercfgController) handleTokenSecretDelete(obj interface{}) { + secret, isSecret := obj.(*api.Secret) + if !isSecret { + tombstone, objIsTombstone := obj.(cache.DeletedFinalStateUnknown) + if !objIsTombstone { + glog.V(2).Infof("Expected tombstone object when deleting token, got %v", obj) + return + } + secret, isSecret = tombstone.Obj.(*api.Secret) + if !isSecret { + glog.V(2).Infof("Expected tombstone object to contain secret, got: %v", obj) + return + } + } + if secret.Annotations[api.CreatedByAnnotation] != CreateDockercfgSecretsController { + return + } + if len(secret.Data[api.ServiceAccountTokenKey]) > 0 { + // Let deleted_token_secrets handle deletion of populated tokens + return + } + e.enqueueServiceAccountForToken(secret) +} + +func (e *DockercfgController) enqueueServiceAccountForToken(tokenSecret *api.Secret) { + serviceAccount := &api.ServiceAccount{ + ObjectMeta: api.ObjectMeta{ + Name: tokenSecret.Annotations[api.ServiceAccountNameKey], + Namespace: tokenSecret.Namespace, + UID: types.UID(tokenSecret.Annotations[api.ServiceAccountUIDKey]), + }, + } + key, err := controller.KeyFunc(serviceAccount) + if err != nil { + utilruntime.HandleError(fmt.Errorf("error syncing token secret %s/%s: %v", tokenSecret.Namespace, tokenSecret.Name, err)) + return + } + e.queue.Add(key) +} + func (e *DockercfgController) Run(workers int, stopCh <-chan struct{}) { defer utilruntime.HandleCrash() @@ -119,6 +217,11 @@ func (e *DockercfgController) Run(workers int, stopCh <-chan struct{}) { glog.Infof("Dockercfg secret controller initialized, starting.") go e.serviceAccountController.Run(stopCh) + go e.secretController.Run(stopCh) + for !e.serviceAccountController.HasSynced() || !e.secretController.HasSynced() { + time.Sleep(100 * time.Millisecond) + } + for i := 0; i < workers; i++ { go wait.Until(e.worker, time.Second, stopCh) } @@ -200,7 +303,6 @@ func (e *DockercfgController) SetDockerURLs(newDockerURLs ...string) { } func needsDockercfgSecret(serviceAccount *api.ServiceAccount) bool { - mountableDockercfgSecrets, imageDockercfgPullSecrets := getGeneratedDockercfgSecretNames(serviceAccount) // look for an ImagePullSecret in the form @@ -244,6 +346,8 @@ func (e *DockercfgController) syncServiceAccount(key string) error { case foundMountableSecret: serviceAccount.ImagePullSecrets = append(serviceAccount.ImagePullSecrets, api.LocalObjectReference{Name: mountableDockercfgSecrets.List()[0]}) } + // Clear the pending token annotation when updating + delete(serviceAccount.Annotations, PendingTokenAnnotation) updatedSA, err := e.client.ServiceAccounts(serviceAccount.Namespace).Update(serviceAccount) if err == nil { @@ -252,10 +356,14 @@ func (e *DockercfgController) syncServiceAccount(key string) error { return err } - dockercfgSecret, err := e.createDockerPullSecret(serviceAccount) + dockercfgSecret, created, err := e.createDockerPullSecret(serviceAccount) if err != nil { return err } + if !created { + glog.V(5).Infof("The dockercfg secret was not created for service account %s/%s, will retry", serviceAccount.Namespace, serviceAccount.Name) + return nil + } first := true err = client.RetryOnConflict(client.DefaultBackoff, func() error { @@ -281,6 +389,8 @@ func (e *DockercfgController) syncServiceAccount(key string) error { serviceAccount.Secrets = append(serviceAccount.Secrets, api.ObjectReference{Name: dockercfgSecret.Name}) serviceAccount.ImagePullSecrets = append(serviceAccount.ImagePullSecrets, api.LocalObjectReference{Name: dockercfgSecret.Name}) + // Clear the pending token annotation when updating + delete(serviceAccount.Annotations, PendingTokenAnnotation) updatedSA, err := e.client.ServiceAccounts(serviceAccount.Namespace).Update(serviceAccount) if err == nil { @@ -298,62 +408,74 @@ func (e *DockercfgController) syncServiceAccount(key string) error { return err } -const ( - tokenSecretWaitInterval = 20 * time.Millisecond - tokenSecretWaitTimes = 100 -) - // createTokenSecret creates a token secret for a given service account. Returns the name of the token -func (e *DockercfgController) createTokenSecret(serviceAccount *api.ServiceAccount) (*api.Secret, error) { +func (e *DockercfgController) createTokenSecret(serviceAccount *api.ServiceAccount) (*api.Secret, bool, error) { + pendingTokenName := serviceAccount.Annotations[PendingTokenAnnotation] + + // If this service account has no record of a pending token name, record one + if len(pendingTokenName) == 0 { + pendingTokenName = secret.Strategy.GenerateName(osautil.GetTokenSecretNamePrefix(serviceAccount)) + if serviceAccount.Annotations == nil { + serviceAccount.Annotations = map[string]string{} + } + serviceAccount.Annotations[PendingTokenAnnotation] = pendingTokenName + updatedServiceAccount, err := e.client.ServiceAccounts(serviceAccount.Namespace).Update(serviceAccount) + // Conflicts mean we'll get called to sync this service account again + if kapierrors.IsConflict(err) { + return nil, false, nil + } + if err != nil { + return nil, false, err + } + serviceAccount = updatedServiceAccount + } + + // Return the token from cache + existingTokenSecretObj, exists, err := e.secretCache.GetByKey(serviceAccount.Namespace + "/" + pendingTokenName) + if err != nil { + return nil, false, err + } + if exists { + existingTokenSecret := existingTokenSecretObj.(*api.Secret) + return existingTokenSecret, len(existingTokenSecret.Data[api.ServiceAccountTokenKey]) > 0, nil + } + + // Try to create the named pending token tokenSecret := &api.Secret{ ObjectMeta: api.ObjectMeta{ - Name: secret.Strategy.GenerateName(osautil.GetTokenSecretNamePrefix(serviceAccount)), + Name: pendingTokenName, Namespace: serviceAccount.Namespace, Annotations: map[string]string{ api.ServiceAccountNameKey: serviceAccount.Name, api.ServiceAccountUIDKey: string(serviceAccount.UID), + api.CreatedByAnnotation: CreateDockercfgSecretsController, }, }, Type: api.SecretTypeServiceAccountToken, Data: map[string][]byte{}, } - _, err := e.client.Secrets(tokenSecret.Namespace).Create(tokenSecret) - if err != nil { - return nil, err - } - - // now we have to wait for the service account token controller to make this valid - // TODO remove this once we have a create-token endpoint - for i := 0; i <= tokenSecretWaitTimes; i++ { - liveTokenSecret, err2 := e.client.Secrets(tokenSecret.Namespace).Get(tokenSecret.Name) - if err2 != nil { - return nil, err2 - } - - if len(liveTokenSecret.Data[api.ServiceAccountTokenKey]) > 0 { - return liveTokenSecret, nil - } - - time.Sleep(wait.Jitter(tokenSecretWaitInterval, 0.0)) - + glog.V(4).Infof("Creating token secret %q for service account %s/%s", tokenSecret.Name, serviceAccount.Namespace, serviceAccount.Name) + token, err := e.client.Secrets(tokenSecret.Namespace).Create(tokenSecret) + // Already exists but not in cache means we'll get an add watch event and resync + if kapierrors.IsAlreadyExists(err) { + return nil, false, nil } - - // the token wasn't ever created, attempt deletion - glog.Warningf("Deleting unfilled token secret %s/%s", tokenSecret.Namespace, tokenSecret.Name) - if deleteErr := e.client.Secrets(tokenSecret.Namespace).Delete(tokenSecret.Name); (deleteErr != nil) && !kapierrors.IsNotFound(deleteErr) { - utilruntime.HandleError(deleteErr) + if err != nil { + return nil, false, err } - return nil, fmt.Errorf("token never generated for %s", tokenSecret.Name) + return token, len(token.Data[api.ServiceAccountTokenKey]) > 0, nil } // createDockerPullSecret creates a dockercfg secret based on the token secret -func (e *DockercfgController) createDockerPullSecret(serviceAccount *api.ServiceAccount) (*api.Secret, error) { - glog.V(4).Infof("Creating secret for %s/%s", serviceAccount.Namespace, serviceAccount.Name) - - tokenSecret, err := e.createTokenSecret(serviceAccount) +func (e *DockercfgController) createDockerPullSecret(serviceAccount *api.ServiceAccount) (*api.Secret, bool, error) { + tokenSecret, isPopulated, err := e.createTokenSecret(serviceAccount) if err != nil { - return nil, err + return nil, false, err + } + if !isPopulated { + glog.V(5).Infof("Token secret for service account %s/%s is not populated yet", serviceAccount.Namespace, serviceAccount.Name) + return nil, false, nil } dockercfgSecret := &api.Secret{ @@ -370,6 +492,7 @@ func (e *DockercfgController) createDockerPullSecret(serviceAccount *api.Service Type: api.SecretTypeDockercfg, Data: map[string][]byte{}, } + glog.V(4).Infof("Creating dockercfg secret %q for service account %s/%s", dockercfgSecret.Name, serviceAccount.Namespace, serviceAccount.Name) // prevent updating the DockerURL until we've created the secret e.dockerURLLock.Lock() @@ -385,30 +508,13 @@ func (e *DockercfgController) createDockerPullSecret(serviceAccount *api.Service } dockercfgContent, err := json.Marshal(&dockercfg) if err != nil { - return nil, err + return nil, false, err } dockercfgSecret.Data[api.DockerConfigKey] = dockercfgContent // Save the secret createdSecret, err := e.client.Secrets(tokenSecret.Namespace).Create(dockercfgSecret) - if err != nil { - // Clean up the generated token secret if we're not going to use it - glog.V(2).Infof("deleting unused token secret %s/%s, error creating dockercfgSecret: %v", tokenSecret.Namespace, tokenSecret.Name, err) - if deleteErr := e.client.Secrets(tokenSecret.Namespace).Delete(tokenSecret.Name); (deleteErr != nil) && !kapierrors.IsNotFound(deleteErr) { - utilruntime.HandleError(deleteErr) - } - return nil, err - } - - return createdSecret, err -} - -func getSecretReferences(serviceAccount *api.ServiceAccount) sets.String { - references := sets.NewString() - for _, secret := range serviceAccount.Secrets { - references.Insert(secret.Name) - } - return references + return createdSecret, err == nil, err } func getGeneratedDockercfgSecretNames(serviceAccount *api.ServiceAccount) (sets.String, sets.String) {