diff --git a/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go b/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go index 04fd0f02d..094282612 100644 --- a/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go +++ b/pkg/serviceaccounts/controllers/create_dockercfg_secrets.go @@ -324,7 +324,7 @@ func (e *DockercfgController) syncServiceAccount(key string) error { return nil } if !needsDockercfgSecret(obj.(*v1.ServiceAccount)) { - return e.syncDockercfgOwnerRefs(obj.(*v1.ServiceAccount)) + return nil } serviceAccount := obj.(*v1.ServiceAccount).DeepCopyObject().(*v1.ServiceAccount) @@ -503,81 +503,12 @@ func (e *DockercfgController) createDockerPullSecret(serviceAccount *v1.ServiceA return nil, false, err } dockercfgSecret.Data[v1.DockerConfigKey] = dockercfgContent - blockDeletion := false - ownerRef := metav1.NewControllerRef(tokenSecret, v1.SchemeGroupVersion.WithKind("Secret")) - ownerRef.BlockOwnerDeletion = &blockDeletion - dockercfgSecret.SetOwnerReferences([]metav1.OwnerReference{*ownerRef}) // Save the secret createdSecret, err := e.client.CoreV1().Secrets(tokenSecret.Namespace).Create(dockercfgSecret) return createdSecret, err == nil, err } -func (e *DockercfgController) syncDockercfgOwnerRefs(serviceAccount *v1.ServiceAccount) error { - for _, secretRef := range serviceAccount.Secrets { - secret, exists, err := e.secretCache.GetByKey(fmt.Sprintf("%s/%s", secretRef.Namespace, secretRef.Name)) - if err != nil { - return err - } - if !exists { - continue - } - err = e.syncDockercfgOwner(secret.(*v1.Secret)) - if err != nil { - return err - } - } - for _, secretRef := range serviceAccount.ImagePullSecrets { - secret, exists, err := e.secretCache.GetByKey(fmt.Sprintf("%s/%s", serviceAccount.Namespace, secretRef.Name)) - if err != nil { - return err - } - if !exists { - continue - } - err = e.syncDockercfgOwner(secret.(*v1.Secret)) - if err != nil { - return err - } - } - return nil -} - -func (e *DockercfgController) syncDockercfgOwner(pullSecret *v1.Secret) error { - if pullSecret.Type != v1.SecretTypeDockercfg { - return nil - } - tokenName := pullSecret.Annotations[ServiceAccountTokenSecretNameKey] - // If there is no token name, this pull secret was likely linked by a user. - // No further work is needed. - if len(tokenName) == 0 { - return nil - } - // Make sure the token exists - tokenSecret, exists, err := e.secretCache.GetByKey(fmt.Sprintf("%s/%s", pullSecret.Namespace, tokenName)) - if err != nil { - return err - } - if !exists { - // If the pull secret exists, and the token does not, delete the pull secret. - klog.V(4).Infof("Deleting pull secret %s/%s because its associated token %s/%s is missing", pullSecret.Namespace, pullSecret.Name, pullSecret.Namespace, tokenName) - return e.client.CoreV1().Secrets(pullSecret.Namespace).Delete(pullSecret.Name, &metav1.DeleteOptions{}) - } - tokenSecretObj := tokenSecret.(*v1.Secret) - // If the pull secret has an owner reference to its associated token, no further work is needed. - if metav1.IsControlledBy(pullSecret, tokenSecretObj) { - return nil - } - pullSecret = pullSecret.DeepCopy() - blockDeletion := false - ownerRef := metav1.NewControllerRef(tokenSecretObj, v1.SchemeGroupVersion.WithKind("Secret")) - ownerRef.BlockOwnerDeletion = &blockDeletion - pullSecret.SetOwnerReferences([]metav1.OwnerReference{*ownerRef}) - klog.V(4).Infof("Adding token %s/%s as the owner of pull secret %s/%s", pullSecret.Namespace, tokenName, pullSecret.Namespace, pullSecret.Name) - _, err = e.client.CoreV1().Secrets(pullSecret.Namespace).Update(pullSecret) - return err -} - func getGeneratedDockercfgSecretNames(serviceAccount *v1.ServiceAccount) (sets.String, sets.String) { mountableDockercfgSecrets := sets.String{} imageDockercfgPullSecrets := sets.String{} diff --git a/pkg/serviceaccounts/controllers/deleted_token_secrets.go b/pkg/serviceaccounts/controllers/deleted_token_secrets.go index d0b682057..f83ecdffe 100644 --- a/pkg/serviceaccounts/controllers/deleted_token_secrets.go +++ b/pkg/serviceaccounts/controllers/deleted_token_secrets.go @@ -4,15 +4,17 @@ import ( "fmt" "time" - v1 "k8s.io/api/core/v1" "k8s.io/klog" + "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" utilruntime "k8s.io/apimachinery/pkg/util/runtime" informers "k8s.io/client-go/informers/core/v1" kclientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" + api "k8s.io/kubernetes/pkg/apis/core" ) // DockercfgTokenDeletedControllerOptions contains options for the DockercfgTokenDeletedController @@ -78,7 +80,8 @@ func (e *DockercfgTokenDeletedController) secretDeleted(obj interface{}) { if !ok { return } - dockercfgSecrets, err := findDockercfgSecrets(e.client, tokenSecret) + + dockercfgSecrets, err := e.findDockercfgSecrets(tokenSecret) if err != nil { klog.Error(err) return @@ -86,16 +89,30 @@ func (e *DockercfgTokenDeletedController) secretDeleted(obj interface{}) { if len(dockercfgSecrets) == 0 { return } + // remove the reference token secrets for _, dockercfgSecret := range dockercfgSecrets { - if metav1.IsControlledBy(dockercfgSecret, tokenSecret) { - // If the docker pull secret is owned by its associated token, let garbage collection take care of it. - klog.V(5).Infof("Ignoring deletion of pull secret %s/%s because it should be removed via garbage collection", dockercfgSecret.Namespace, dockercfgSecret.Name) - continue - } - klog.V(4).Infof("Deleting pull secret %s/%s because its associated token %s/%s has been deleted", dockercfgSecret.Namespace, dockercfgSecret.Name, tokenSecret.Namespace, tokenSecret.Name) if err := e.client.CoreV1().Secrets(dockercfgSecret.Namespace).Delete(dockercfgSecret.Name, nil); (err != nil) && !apierrors.IsNotFound(err) { utilruntime.HandleError(err) } } } + +// findDockercfgSecret checks all the secrets in the namespace to see if the token secret has any existing dockercfg secrets that reference it +func (e *DockercfgTokenDeletedController) findDockercfgSecrets(tokenSecret *v1.Secret) ([]*v1.Secret, error) { + dockercfgSecrets := []*v1.Secret{} + + options := metav1.ListOptions{FieldSelector: fields.OneTermEqualSelector(api.SecretTypeField, string(v1.SecretTypeDockercfg)).String()} + potentialSecrets, err := e.client.CoreV1().Secrets(tokenSecret.Namespace).List(options) + if err != nil { + return nil, err + } + + for i, currSecret := range potentialSecrets.Items { + if currSecret.Annotations[ServiceAccountTokenSecretNameKey] == tokenSecret.Name { + dockercfgSecrets = append(dockercfgSecrets, &potentialSecrets.Items[i]) + } + } + + return dockercfgSecrets, nil +} diff --git a/pkg/serviceaccounts/controllers/deleted_token_secrets_test.go b/pkg/serviceaccounts/controllers/deleted_token_secrets_test.go index e11f81c45..ee8bb70da 100644 --- a/pkg/serviceaccounts/controllers/deleted_token_secrets_test.go +++ b/pkg/serviceaccounts/controllers/deleted_token_secrets_test.go @@ -5,7 +5,7 @@ import ( "reflect" "testing" - v1 "k8s.io/api/core/v1" + "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/runtime" @@ -39,12 +39,7 @@ func regularSecretReferences() []v1.ObjectReference { // tokenSecretReferences is used by a service account that references a ServiceAccountToken secret func tokenSecretReferences() []v1.ObjectReference { - return []v1.ObjectReference{ - { - Name: "token-secret-1", - UID: "23456", - }, - } + return []v1.ObjectReference{{Name: "token-secret-1"}} } // addTokenSecretReference adds a reference to the ServiceAccountToken that will be created @@ -90,37 +85,6 @@ func createdDockercfgSecret() *v1.Secret { } } -// createdDockercfgSecretWithOwnerRef returns the ServiceAccountToken secret posted when creating a new token secret, including an owner reference. -// Named "default-token-fplln", since that is the first generated name after rand.Seed(1) -func createdDockercfgSecretWithOwnerRef() *v1.Secret { - isController := true - gv := v1.SchemeGroupVersion - return &v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: "default-dockercfg-fplln", - Namespace: "default", - Annotations: map[string]string{ - v1.ServiceAccountNameKey: "default", - v1.ServiceAccountUIDKey: "12345", - ServiceAccountTokenSecretNameKey: "token-secret-1", - }, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: gv.String(), - Kind: "Secret", - Name: "token-secret-1", - UID: "23456", - Controller: &isController, - }, - }, - }, - Type: v1.SecretTypeDockercfg, - Data: map[string][]byte{ - v1.DockerConfigKey: []byte(`{"docker-registry.default.svc.cluster.local":{"Username":"serviceaccount","Password":"ABC","Email":"serviceaccount@example.org"}}`), - }, - } -} - // opaqueSecret returns a persisted non-ServiceAccountToken secret named "regular-secret-1" func opaqueSecret() *v1.Secret { return &v1.Secret{ @@ -199,7 +163,7 @@ func TestTokenDeletion(t *testing.T) { }, }, "deleted token secret with serviceaccount without reference": { - ClientObjects: []runtime.Object{serviceAccount(addTokenSecretReference(tokenSecretReferences()), imagePullSecretReferences()), createdDockercfgSecret(), serviceAccountTokenSecret()}, + ClientObjects: []runtime.Object{serviceAccount(addTokenSecretReference(tokenSecretReferences()), imagePullSecretReferences()), createdDockercfgSecret()}, DeletedSecret: serviceAccountTokenSecret(), ExpectedActions: []clientgotesting.Action{ @@ -210,16 +174,6 @@ func TestTokenDeletion(t *testing.T) { clientgotesting.NewDeleteAction(schema.GroupVersionResource{Resource: "secrets", Version: "v1"}, "default", "default-dockercfg-fplln"), }, }, - "deleted token secret with pull secret owner reference": { - ClientObjects: []runtime.Object{serviceAccount(addTokenSecretReference(tokenSecretReferences()), imagePullSecretReferences()), createdDockercfgSecretWithOwnerRef(), serviceAccountTokenSecret()}, - DeletedSecret: serviceAccountTokenSecret(), - ExpectedActions: []clientgotesting.Action{ - clientgotesting.NewListAction( - schema.GroupVersionResource{Resource: "secrets", Version: "v1"}, - schema.GroupVersionKind{Kind: "Secret", Version: "v1"}, - "default", metav1.ListOptions{FieldSelector: dockercfgSecretFieldSelector.String()}), - }, - }, } for k, tc := range testcases { diff --git a/pkg/serviceaccounts/controllers/pull_secrets.go b/pkg/serviceaccounts/controllers/pull_secrets.go deleted file mode 100644 index 1deb261aa..000000000 --- a/pkg/serviceaccounts/controllers/pull_secrets.go +++ /dev/null @@ -1,29 +0,0 @@ -package controllers - -import ( - corev1 "k8s.io/api/core/v1" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" - "k8s.io/client-go/kubernetes" - api "k8s.io/kubernetes/pkg/apis/core" -) - -// findDockercfgSecret checks all the secrets in the namespace to see if the token secret has any existing dockercfg secrets that reference it -func findDockercfgSecrets(client kubernetes.Interface, tokenSecret *corev1.Secret) ([]*corev1.Secret, error) { - dockercfgSecrets := []*corev1.Secret{} - - options := metav1.ListOptions{FieldSelector: fields.OneTermEqualSelector(api.SecretTypeField, string(v1.SecretTypeDockercfg)).String()} - potentialSecrets, err := client.CoreV1().Secrets(tokenSecret.Namespace).List(options) - if err != nil { - return nil, err - } - - for i, currSecret := range potentialSecrets.Items { - if currSecret.Annotations[ServiceAccountTokenSecretNameKey] == tokenSecret.Name { - dockercfgSecrets = append(dockercfgSecrets, &potentialSecrets.Items[i]) - } - } - - return dockercfgSecrets, nil -}