mirrors/pinniped
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/pinniped.git synced 2026-09-05 23:57:12 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Do not use long-lived service account tokens in secrets

Browse Source 
Co-authored-by: Ryan Richard <richardry@vmware.com>
This commit is contained in:
Joshua Casey
2023-11-30 09:57:57 -08:00
committed by Ryan Richard
co-authored by Ryan Richard
parent dc1d563549
commit 2603bbfcd6
27 changed files with 1462 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/controller/impersonatorconfig/impersonator_config.go
+6
View File
@@ -46,6 +46,7 @@ import (
"go.pinniped.dev/internal/dynamiccert"
"go.pinniped.dev/internal/endpointaddr"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/tokenclient"
)
const (
@@ -86,6 +87,8 @@ type impersonatorConfigController struct {
tlsServingCertDynamicCertProvider dynamiccert.Private
infoLog logr.Logger
debugLog logr.Logger
impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet
}
func NewImpersonatorConfigController(
@@ -108,6 +111,7 @@ func NewImpersonatorConfigController(
impersonationSignerSecretName string,
impersonationSigningCertProvider dynamiccert.Provider,
log logr.Logger,
impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet,
) controllerlib.Controller {
secretNames := sets.NewString(tlsSecretName, caSecretName, impersonationSignerSecretName)
log = log.WithName("impersonator-config-controller")
@@ -135,6 +139,7 @@ func NewImpersonatorConfigController(
tlsServingCertDynamicCertProvider: dynamiccert.NewServingCert("impersonation-proxy-serving-cert"),
infoLog: log.V(plog.KlogLevelInfo),
debugLog: log.V(plog.KlogLevelDebug),
impersonationProxyTokenCache: impersonationProxyTokenCache,
},
},
withInformer(credentialIssuerInformer,
@@ -487,6 +492,7 @@ func (c *impersonatorConfigController) ensureImpersonatorIsStarted(syncCtx contr
c.impersonationProxyPort,
c.tlsServingCertDynamicCertProvider,
c.impersonationSigningCertProvider,
c.impersonationProxyTokenCache,
)
if err != nil {
return err
internal/controller/impersonatorconfig/impersonator_config_test.go
+7 -1
View File
@@ -45,6 +45,7 @@ import (
"go.pinniped.dev/internal/kubeclient"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/testutil"
"go.pinniped.dev/internal/tokenclient"
)
func TestImpersonatorConfigControllerOptions(t *testing.T) {
@@ -93,6 +94,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) {
caSignerName,
nil,
plog.Logr(), //nolint:staticcheck // old test with no log assertions
nil,
)
credIssuerInformerFilter = observableWithInformerOption.GetFilterForInformer(credIssuerInformer)
servicesInformerFilter = observableWithInformerOption.GetFilterForInformer(servicesInformer)
@@ -272,6 +274,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
const httpsPort = ":443"
const fakeServerResponseBody = "hello, world!"
const externallyProvidedTLSSecretName = "external-tls-secret" //nolint:gosec // this is not a credential
fakeExpiringSingletonTokenCacheGet := *new(tokenclient.ExpiringSingletonTokenCacheGet)
var labels = map[string]string{"app": "app-name", "other-key": "other-value"}
var r *require.Assertions
@@ -308,11 +311,13 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
port int,
dynamicCertProvider dynamiccert.Private,
impersonationProxySignerCAProvider dynamiccert.Public,
expiringSingletonTokenCacheGet tokenclient.ExpiringSingletonTokenCacheGet,
) (func(stopCh <-chan struct{}) error, error) {
impersonatorFuncWasCalled++
r.Equal(8444, port)
r.NotNil(dynamicCertProvider)
r.NotNil(impersonationProxySignerCAProvider)
r.Equal(fakeExpiringSingletonTokenCacheGet, expiringSingletonTokenCacheGet)
if impersonatorFuncError != nil {
return nil, impersonatorFuncError
@@ -580,7 +585,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
impersonatorFunc,
mTLSClientCertCASecretName,
mTLSClientCertProvider,
plog.Logr(), //nolint:staticcheck // old test with no log assertions
plog.Logr(), //nolint:staticcheck // old test with no log assertions,
fakeExpiringSingletonTokenCacheGet,
)
controllerlib.TestWrap(t, subject, func(syncer controllerlib.Syncer) controllerlib.Syncer {
tlsServingCertDynamicCertProvider = syncer.(*impersonatorConfigController).tlsServingCertDynamicCertProvider
internal/controller/serviceaccounttokencleanup/secrets.go
+92
View File
@@ -0,0 +1,92 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package serviceaccounttokencleanup
import (
"fmt"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
corev1informers "k8s.io/client-go/informers/core/v1"
"k8s.io/client-go/kubernetes"
pinnipedcontroller "go.pinniped.dev/internal/controller"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/plog"
)
func NewServiceAccountTokenCleanupController(
namespace string,
legacySecretName string,
k8sClient kubernetes.Interface,
secretInformer corev1informers.SecretInformer,
withInformer pinnipedcontroller.WithInformerOptionFunc,
logger plog.Logger,
) controllerlib.Controller {
name := "service-account-token-cleanup-controller"
return controllerlib.New(controllerlib.Config{
Name: name,
Syncer: &serviceAccountTokenCleanupController{
name: name,
namespace: namespace,
legacySecretName: legacySecretName,
k8sClient: k8sClient,
secretInformer: secretInformer,
logger: logger.WithName(name),
},
},
withInformer(
secretInformer,
pinnipedcontroller.SimpleFilterWithSingletonQueue(func(obj metav1.Object) bool {
secret, ok := obj.(*corev1.Secret)
return obj.GetNamespace() == namespace &&
obj.GetName() == legacySecretName &&
ok &&
secret.Type == corev1.SecretTypeServiceAccountToken
}),
controllerlib.InformerOption{},
))
}
type serviceAccountTokenCleanupController struct {
name string
namespace string
legacySecretName string
k8sClient kubernetes.Interface
secretInformer corev1informers.SecretInformer
logger plog.Logger
}
func (c serviceAccountTokenCleanupController) Sync(syncCtx controllerlib.Context) error {
secrets, err := c.secretInformer.Lister().Secrets(c.namespace).List(labels.Everything())
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 {
if secret.Name == c.legacySecretName && secret.Type == corev1.SecretTypeServiceAccountToken {
foundSecret = true
}
}
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,
))
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)
}
}
return nil
}
internal/controller/serviceaccounttokencleanup/secrets_test.go
+221
View File
@@ -0,0 +1,221 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package serviceaccounttokencleanup
import (
"bytes"
"context"
"errors"
"testing"
"github.com/stretchr/testify/require"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
kubeinformers "k8s.io/client-go/informers"
kubernetesfake "k8s.io/client-go/kubernetes/fake"
kubetesting "k8s.io/client-go/testing"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/testutil"
)
func TestNewServiceAccountTokenCleanupController(t *testing.T) {
namespace := "a-namespace"
legacySecretName := "a-secret"
observableWithInformerOption := testutil.NewObservableWithInformerOption()
secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets()
var log bytes.Buffer
_ = NewServiceAccountTokenCleanupController(
namespace,
legacySecretName,
nil, // not needed for this test
secretsInformer,
observableWithInformerOption.WithInformer,
plog.TestLogger(t, &log),
)
secretsInformerFilter := observableWithInformerOption.GetFilterForInformer(secretsInformer)
legacySecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: legacySecretName, Namespace: namespace}, Type: corev1.SecretTypeServiceAccountToken}
wrongName := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrongName", Namespace: namespace}, Type: corev1.SecretTypeServiceAccountToken}
wrongType := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrongType", Namespace: namespace}, Type: "other-type"}
wrongNamespace := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrongNamespace", Namespace: "wrong-namespace"}, Type: corev1.SecretTypeServiceAccountToken}
wrongObject := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "config-map", Namespace: namespace}}
require.False(t, secretsInformerFilter.Add(wrongName))
require.False(t, secretsInformerFilter.Update(wrongName, wrongNamespace))
require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongName))
require.False(t, secretsInformerFilter.Delete(wrongName))
require.False(t, secretsInformerFilter.Add(wrongObject))
require.False(t, secretsInformerFilter.Update(wrongObject, wrongNamespace))
require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongObject))
require.False(t, secretsInformerFilter.Delete(wrongObject))
require.False(t, secretsInformerFilter.Add(wrongNamespace))
require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongObject))
require.False(t, secretsInformerFilter.Update(wrongObject, wrongNamespace))
require.False(t, secretsInformerFilter.Delete(wrongNamespace))
require.False(t, secretsInformerFilter.Add(wrongType))
require.False(t, secretsInformerFilter.Update(wrongType, wrongNamespace))
require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongType))
require.False(t, secretsInformerFilter.Delete(wrongType))
require.True(t, secretsInformerFilter.Add(legacySecret))
require.True(t, secretsInformerFilter.Update(legacySecret, wrongNamespace))
require.True(t, secretsInformerFilter.Update(wrongNamespace, legacySecret))
require.True(t, secretsInformerFilter.Delete(legacySecret))
}
func TestSync(t *testing.T) {
for _, tt := range []struct {
name string
secretNameToDelete string
namespace string
addReactors func(*kubernetesfake.Clientset)
expectedErrMessage string
expectedActions []kubetesting.Action
}{
{
name: "happy path",
secretNameToDelete: "secret-to-delete",
namespace: "some-namespace",
expectedActions: []kubetesting.Action{
kubetesting.NewDeleteAction(
schema.GroupVersionResource{
Group: "",
Version: "v1",
Resource: "secrets",
},
"some-namespace",
"secret-to-delete",
),
},
},
{
name: "no secret to delete",
secretNameToDelete: "not-an-existing-secret",
expectedActions: []kubetesting.Action{},
},
{
name: "returns API errors",
secretNameToDelete: "secret-to-delete",
namespace: "other-namespace",
addReactors: func(clientset *kubernetesfake.Clientset) {
clientset.PrependReactor(
"delete",
"secrets",
func(a kubetesting.Action) (bool, runtime.Object, error) {
return true, nil, errors.New("error from API client")
},
)
},
expectedErrMessage: "unable to delete secret secret-to-delete in namespace other-namespace: error from API client",
expectedActions: []kubetesting.Action{
kubetesting.NewDeleteAction(
schema.GroupVersionResource{
Group: "",
Version: "v1",
Resource: "secrets",
},
"other-namespace",
"secret-to-delete",
),
},
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
kubeAPIClient, kubeInformers := setupKubernetes(t, tt.namespace)
if tt.addReactors != nil {
tt.addReactors(kubeAPIClient)
}
var log bytes.Buffer
controller := NewServiceAccountTokenCleanupController(
tt.namespace,
tt.secretNameToDelete,
kubeAPIClient,
kubeInformers.Core().V1().Secrets(),
controllerlib.WithInformer,
plog.TestLogger(t, &log),
)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
// Must start informers before calling TestRunSynchronously().
kubeInformers.Start(ctx.Done())
controllerlib.TestRunSynchronously(t, controller)
err := controllerlib.TestSync(t, controller, controllerlib.Context{
Context: ctx,
})
if tt.expectedErrMessage == "" {
require.NoError(t, err)
} else {
require.ErrorContains(t, err, tt.expectedErrMessage)
}
actualActions := kubeAPIClient.Actions()
require.Equal(t, tt.expectedActions, actualActions)
})
}
}
func setupKubernetes(t *testing.T, namespace string) (*kubernetesfake.Clientset, kubeinformers.SharedInformerFactory) {
t.Helper()
kubeAPIClient := kubernetesfake.NewSimpleClientset()
kubeInformerClient := kubernetesfake.NewSimpleClientset()
kubeInformers := kubeinformers.NewSharedInformerFactory(
kubeInformerClient,
0,
)
secretToDelete := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "secret-to-delete",
Namespace: namespace,
},
Type: corev1.SecretTypeServiceAccountToken,
}
secretWithWrongName := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "wrong-name",
Namespace: namespace,
},
Type: corev1.SecretTypeServiceAccountToken,
}
secretWithWrongType := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "secret-to-leave-alone",
Namespace: namespace,
},
}
secretWithWrongNamespace := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "secret-to-delete",
Namespace: "other",
},
Type: corev1.SecretTypeServiceAccountToken,
}
require.NoError(t, kubeAPIClient.Tracker().Add(secretToDelete))
require.NoError(t, kubeInformerClient.Tracker().Add(secretToDelete))
require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongName))
require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongName))
require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongType))
require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongType))
require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongNamespace))
require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongNamespace))
return kubeAPIClient, kubeInformers
}