From 7616799adb8d6b6ed4f8d4e8cdd07ac4129374e5 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 30 Nov 2023 16:40:21 -0800 Subject: [PATCH] Minor refactors in legacy SA token Secret cleanup controller --- ...rvice_account_token_cleanup_controller.go} | 33 ++++++++++++++----- ..._account_token_cleanup_controller_test.go} | 4 +-- .../controllermanager/prepare_controllers.go | 2 +- 3 files changed, 27 insertions(+), 12 deletions(-) rename internal/controller/serviceaccounttokencleanup/{service_account_token_cleanup_controller.go => legacy_service_account_token_cleanup_controller.go} (64%) rename internal/controller/serviceaccounttokencleanup/{service_account_token_cleanup_controller_test.go => legacy_service_account_token_cleanup_controller_test.go} (98%) diff --git a/internal/controller/serviceaccounttokencleanup/service_account_token_cleanup_controller.go b/internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller.go similarity index 64% rename from internal/controller/serviceaccounttokencleanup/service_account_token_cleanup_controller.go rename to internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller.go index f544655d4..06e277e54 100644 --- a/internal/controller/serviceaccounttokencleanup/service_account_token_cleanup_controller.go +++ b/internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller.go @@ -17,7 +17,14 @@ import ( "go.pinniped.dev/internal/plog" ) -func NewServiceAccountTokenCleanupController( +// NewLegacyServiceAccountTokenCleanupController creates a controller whose purpose is to delete a legacy Secret +// that was created by installation of older versions of the Pinniped Concierge which is no longer needed. +// This Secret was used to request and to hold a long-lived service account token which was used by the Concierge +// impersonation proxy. It has been replaced by a goroutine which requests short-lived service account tokens +// by making calls to the Kubernetes API server, without any need to read or write the tokens to a Secret. +// Since the old Secret contains a long-lived token, we try to delete it here. That Secret may not exist if the user +// never installed an old version of the Concierge, in which case this controller should do pretty much nothing. +func NewLegacyServiceAccountTokenCleanupController( namespace string, legacySecretName string, k8sClient kubernetes.Interface, @@ -25,7 +32,7 @@ func NewServiceAccountTokenCleanupController( withInformer pinnipedcontroller.WithInformerOptionFunc, logger plog.Logger, ) controllerlib.Controller { - name := "service-account-token-cleanup-controller" + name := "legacy-service-account-token-cleanup-controller" return controllerlib.New(controllerlib.Config{ Name: name, Syncer: &serviceAccountTokenCleanupController{ @@ -65,7 +72,6 @@ func (c serviceAccountTokenCleanupController) Sync(syncCtx controllerlib.Context if err != nil { return fmt.Errorf("unable to list all secrets in namespace %s: %w", c.namespace, err) } - c.logger.Info(fmt.Sprintf("You have now arrived in the %s controller, found %d secrets", c.name, len(secrets))) foundSecret := false for _, secret := range secrets { @@ -74,18 +80,27 @@ func (c serviceAccountTokenCleanupController) Sync(syncCtx controllerlib.Context } } - c.logger.Info(fmt.Sprintf( - "The %s controller has found a secret of name %s to delete with type %s", - c.name, - c.legacySecretName, - corev1.SecretTypeServiceAccountToken, - )) + c.logger.Debug( + fmt.Sprintf("%s controller checked for legacy secret", c.name), + "secretName", c.legacySecretName, + "secretNamespace", c.namespace, + "secretType", corev1.SecretTypeServiceAccountToken, + "foundSecret", foundSecret, + ) if foundSecret { err = c.k8sClient.CoreV1().Secrets(c.namespace).Delete(syncCtx.Context, c.legacySecretName, metav1.DeleteOptions{}) + if err != nil { return fmt.Errorf("unable to delete secret %s in namespace %s: %w", c.legacySecretName, c.namespace, err) } + + c.logger.Debug( + fmt.Sprintf("%s controller succcessfully deleted legacy secret", c.name), + "secretName", c.legacySecretName, + "secretNamespace", c.namespace, + "secretType", corev1.SecretTypeServiceAccountToken, + ) } return nil diff --git a/internal/controller/serviceaccounttokencleanup/service_account_token_cleanup_controller_test.go b/internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller_test.go similarity index 98% rename from internal/controller/serviceaccounttokencleanup/service_account_token_cleanup_controller_test.go rename to internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller_test.go index 3edc58902..364393dab 100644 --- a/internal/controller/serviceaccounttokencleanup/service_account_token_cleanup_controller_test.go +++ b/internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller_test.go @@ -30,7 +30,7 @@ func TestNewServiceAccountTokenCleanupController(t *testing.T) { secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets() var log bytes.Buffer - _ = NewServiceAccountTokenCleanupController( + _ = NewLegacyServiceAccountTokenCleanupController( namespace, legacySecretName, nil, // not needed for this test @@ -140,7 +140,7 @@ func TestSync(t *testing.T) { } var log bytes.Buffer - controller := NewServiceAccountTokenCleanupController( + controller := NewLegacyServiceAccountTokenCleanupController( tt.namespace, tt.secretNameToDelete, kubeAPIClient, diff --git a/internal/controllermanager/prepare_controllers.go b/internal/controllermanager/prepare_controllers.go index 76653ec48..29acfe143 100644 --- a/internal/controllermanager/prepare_controllers.go +++ b/internal/controllermanager/prepare_controllers.go @@ -314,7 +314,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol singletonWorker, ). WithController( - serviceaccounttokencleanup.NewServiceAccountTokenCleanupController( + serviceaccounttokencleanup.NewLegacyServiceAccountTokenCleanupController( c.ServerInstallationInfo.Namespace, c.NamesConfig.ImpersonationProxyLegacySecret, client.Kubernetes,