mirrors/pinniped
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/pinniped.git synced 2026-01-08 07:11:53 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #1733 from vmware-tanzu/jtc/issue-1700/impersonation-proxy-token-request-api

Browse Source 
The Impersonation Proxy should use a short-lived token from the `TokenRequest` API
This commit is contained in:
Ryan Richard
2023-12-01 11:48:52 -08:00
committed by GitHub
parent dc1d563549 7616799adb
commit ef6369d71e
25 changed files with 1717 additions and 144 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
deploy/concierge/deployment.yaml
View File

@@ -45,8 +45,9 @@ metadata:
annotations:
#! we need to create this service account before we create the secret
kapp.k14s.io/change-group: "impersonation-proxy.concierge.pinniped.dev/serviceaccount"
secrets: #! make sure the token controller does not create any other secrets
- name: #@ defaultResourceNameWithSuffix("impersonation-proxy")
kubernetes.io/enforce-mountable-secrets: "true"
secrets: [] #! make sure the token controller does not create any secrets
automountServiceAccountToken: false
---
apiVersion: v1
kind: ConfigMap
@@ -77,6 +78,8 @@ data:
impersonationCACertificateSecret: (@= defaultResourceNameWithSuffix("impersonation-proxy-ca-certificate") @)
impersonationSignerSecret: (@= defaultResourceNameWithSuffix("impersonation-proxy-signer-ca-certificate") @)
agentServiceAccount: (@= defaultResourceNameWithSuffix("kube-cert-agent") @)
impersonationProxyServiceAccount: (@= defaultResourceNameWithSuffix("impersonation-proxy") @)
impersonationProxyLegacySecret: (@= defaultResourceNameWithSuffix("impersonation-proxy") @)
labels: (@= json.encode(labels()).rstrip() @)
kubeCertAgent:
namePrefix: (@= defaultResourceNameWithSuffix("kube-cert-agent-") @)
@@ -182,9 +185,6 @@ spec:
- name: podinfo
mountPath: /etc/podinfo
readOnly: true
- name: impersonation-proxy
mountPath: /var/run/secrets/impersonation-proxy.concierge.pinniped.dev/serviceaccount
readOnly: true
env:
#@ if data.values.https_proxy:
- name: HTTPS_PROXY
@@ -220,12 +220,6 @@ spec:
- name: config-volume
configMap:
name: #@ defaultResourceNameWithSuffix("config")
- name: impersonation-proxy
secret:
secretName: #@ defaultResourceNameWithSuffix("impersonation-proxy")
items: #! make sure our pod does not start until the token controller has a chance to populate the secret
- key: token
path: token
- name: podinfo
downwardAPI:
items:
@@ -353,16 +347,3 @@ spec:
#@ else:
annotations: #@ data.values.impersonation_proxy_spec.service.annotations
#@ end
---
apiVersion: v1
kind: Secret
metadata:
name: #@ defaultResourceNameWithSuffix("impersonation-proxy")
namespace: #@ namespace()
labels: #@ labels()
annotations:
#! wait until the SA exists to create this secret so that the token controller does not delete it
#! we have this secret at the end so that kubectl will create the service account first
kapp.k14s.io/change-rule: "upsert after upserting impersonation-proxy.concierge.pinniped.dev/serviceaccount"
kubernetes.io/service-account.name: #@ defaultResourceNameWithSuffix("impersonation-proxy")
type: kubernetes.io/service-account-token

9
deploy/concierge/rbac.yaml
View File

@@ -1,4 +1,4 @@
#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
#! Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
#! SPDX-License-Identifier: Apache-2.0
#@ load("@ytt:data", "data")
@@ -156,6 +156,13 @@ rules:
- apiGroups: [ coordination.k8s.io ]
resources: [ leases ]
verbs: [ create, get, update ]
#! We need to be able to get service accounts and create serviceaccounts/tokens so that we can create short-lived tokens for the impersonation proxy
- apiGroups: [""]
resources: [ serviceaccounts ]
verbs: [ get ]
- apiGroups: [""]
resources: [ serviceaccounts/token ]
verbs: [ create ]
---
kind: RoleBinding
apiVersion: rbac.authorization.k8s.io/v1

43
internal/backoff/infinitebackoff.go Normal file
View File

@@ -0,0 +1,43 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package backoff
import (
"math"
"time"
)
type InfiniteBackoff struct {
// The initial duration.
Duration time.Duration
// Factor is used to scale up the Duration until it reaches MaxDuration.
// Should be at least 1.0.
Factor float64
// A limit on step size. Once reached, this value will be used as the interval.
MaxDuration time.Duration
hasStepped bool
}
// Step returns the next duration in the backoff sequence.
// It modifies the receiver and is not thread-safe.
func (b *InfiniteBackoff) Step() time.Duration {
if !b.hasStepped {
b.hasStepped = true
return b.Duration
}
var next time.Duration
b.Factor = math.Max(1, b.Factor)
// Grow by the factor (which could be 1).
next = time.Duration(float64(b.Duration) * b.Factor)
// Stop growing the intervals once we exceed the max duration.
if b.MaxDuration > 0 && next > b.MaxDuration {
next = b.MaxDuration
}
b.Duration = next
return next
}

94
internal/backoff/infinitebackoff_test.go Normal file
View File

@@ -0,0 +1,94 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package backoff
import (
"math"
"testing"
"time"
"github.com/stretchr/testify/require"
)
func TestInfiniteBackoff(t *testing.T) {
tests := []struct {
name string
stepper Stepper
expectedSequence []time.Duration
}{
{
name: "zero initialization results in 0ns steps",
stepper: &InfiniteBackoff{},
expectedSequence: func() []time.Duration {
results := make([]time.Duration, 1000)
for i := 0; i < 1000; i++ {
results[i] = time.Duration(0)
}
return results
}(),
},
{
name: "double 5 ns forever",
stepper: &InfiniteBackoff{
Duration: 5 * time.Nanosecond,
Factor: 2,
},
expectedSequence: func() []time.Duration {
// limit to 60 to prevent int64 overflow
results := make([]time.Duration, 60)
results[0] = 5 * time.Nanosecond
for i := 1; i < 60; i++ {
results[i] = 2 * results[i-1]
}
return results
}(),
},
{
name: "grows slowly until limit",
stepper: &InfiniteBackoff{
Duration: 20 * time.Nanosecond,
Factor: 1.1,
MaxDuration: 40 * time.Nanosecond,
},
expectedSequence: func() []time.Duration {
results := make([]time.Duration, 1000)
results[0] = 20 * time.Nanosecond
for i := 1; i < 1000; i++ {
nanoseconds := 1.1 * float64(results[i-1])
results[i] = time.Duration(math.Min(nanoseconds, 40))
}
return results
}(),
},
{
name: "factor less than 1.0 is replaced with 1.0",
stepper: &InfiniteBackoff{
Duration: 20 * time.Nanosecond,
Factor: 0.9,
},
expectedSequence: func() []time.Duration {
results := make([]time.Duration, 1000)
for i := 0; i < 1000; i++ {
results[i] = 20 * time.Nanosecond
}
return results
}(),
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
require.NotEmpty(t, tt.expectedSequence)
for i, expected := range tt.expectedSequence {
actual := tt.stepper.Step()
require.Equalf(t, expected, actual, "incorrect result on step #%d, previous steps are %v", i, tt.expectedSequence[:i])
}
backoff, ok := tt.stepper.(*InfiniteBackoff)
require.True(t, ok)
require.NotNil(t, backoff)
require.GreaterOrEqual(t, backoff.Factor, 1.0)
})
}
}

56
internal/backoff/stepping.go Normal file
View File

@@ -0,0 +1,56 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package backoff
import (
"context"
"time"
"k8s.io/apimachinery/pkg/util/wait"
)
type Stepper interface {
Step() time.Duration
}
func wrapConditionWithNoPanics(ctx context.Context, condition wait.ConditionWithContextFunc) (done bool, err error) {
defer func() {
if r := recover(); r != nil {
if err2, ok := r.(error); ok {
err = err2
return
}
}
}()
return condition(ctx)
}
func WithContext(ctx context.Context, backoff Stepper, condition wait.ConditionWithContextFunc) error {
// Loop forever, unless we reach one of the return statements below.
for {
// Stop if the context is done.
select {
case <-ctx.Done():
return ctx.Err()
default:
}
// Stop trying unless the condition function returns false.
// Allow cancellation during the attempt if the condition function respects the ctx.
if ok, err := wrapConditionWithNoPanics(ctx, condition); err != nil || ok {
return err
}
// Calculate how long to wait before the next step.
waitBeforeRetry := backoff.Step()
// Wait before running again, allowing cancellation during the wait.
select {
case <-ctx.Done():
return ctx.Err()
case <-time.After(waitBeforeRetry):
}
}
}

124
internal/backoff/stepping_test.go Normal file
View File

@@ -0,0 +1,124 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package backoff
import (
"context"
"errors"
"testing"
"time"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/util/wait"
)
type MockStepper struct {
steps []time.Duration
currentStep int
}
func (m *MockStepper) Step() time.Duration {
result := m.steps[m.currentStep]
m.currentStep++
return result
}
func TestWithContext(t *testing.T) {
tests := []struct {
name string
steps []time.Duration
finalCondition wait.ConditionWithContextFunc
expectedErr error
}{
{
name: "cancelling results in cancellation error",
steps: []time.Duration{
time.Duration(0),
time.Duration(0),
time.Duration(0),
},
finalCondition: func(ctx context.Context) (done bool, err error) {
return false, nil
},
expectedErr: context.Canceled,
},
{
name: "when condition is done, exit early",
steps: []time.Duration{
time.Duration(0),
time.Duration(0),
time.Duration(0),
},
finalCondition: func(ctx context.Context) (done bool, err error) {
return true, nil
},
expectedErr: nil,
},
{
name: "when condition returns error, exit early",
steps: []time.Duration{
time.Duration(0),
time.Duration(0),
time.Duration(0),
time.Duration(0),
time.Duration(0),
},
finalCondition: func(ctx context.Context) (done bool, err error) {
return false, errors.New("error from condition")
},
expectedErr: errors.New("error from condition"),
},
{
name: "when condition panics, cover and exit",
steps: []time.Duration{
time.Duration(0),
},
finalCondition: func(ctx context.Context) (done bool, err error) {
panic(errors.New("panic error"))
},
expectedErr: errors.New("panic error"),
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
testContext, cancel := context.WithCancel(context.Background())
backoff := &MockStepper{
steps: tt.steps,
}
actualConditionCalls := 0
err := WithContext(testContext, backoff, func(ctx context.Context) (done bool, err error) {
actualConditionCalls++
if backoff.currentStep >= (len(backoff.steps) - 1) {
cancel()
return tt.finalCondition(ctx)
}
return false, nil
})
require.Equal(t, tt.expectedErr, err)
require.Equal(t, len(backoff.steps), actualConditionCalls)
})
}
t.Run("does not invoke any functions when run with a cancelled context", func(t *testing.T) {
testContext, cancel := context.WithCancel(context.Background())
cancel()
stepper := &MockStepper{}
conditionCalls := 0
condition := func(context.Context) (done bool, err error) {
conditionCalls++
return false, nil
}
err := WithContext(testContext, stepper, condition)
require.Equal(t, context.Canceled, err)
require.Equal(t, 0, conditionCalls)
require.Equal(t, 0, stepper.currentStep)
})
}

20
internal/concierge/apiserver/apiserver.go
View File

@@ -21,6 +21,7 @@ import (
"go.pinniped.dev/internal/pversion"
"go.pinniped.dev/internal/registry/credentialrequest"
"go.pinniped.dev/internal/registry/whoamirequest"
"go.pinniped.dev/internal/tokenclient"
)
type Config struct {
@@ -36,6 +37,7 @@ type ExtraConfig struct {
NegotiatedSerializer runtime.NegotiatedSerializer
LoginConciergeGroupVersion schema.GroupVersion
IdentityConciergeGroupVersion schema.GroupVersion
TokenClient *tokenclient.TokenClient
}
type PinnipedServer struct {
@@ -134,6 +136,24 @@ func (c completedConfig) New() (*PinnipedServer, error) {
},
)
s.GenericAPIServer.AddPostStartHookOrDie("fetch-impersonation-proxy-tokens",
func(postStartContext genericapiserver.PostStartHookContext) error {
plog.Debug("fetch-impersonation-proxy-tokens start hook starting")
defer plog.Debug("fetch-impersonation-proxy-tokens start hook completed")
controllersShutdownWaitGroup.Add(1)
go func() {
defer controllersShutdownWaitGroup.Done()
// Start the token client
c.ExtraConfig.TokenClient.Start(controllersCtx)
plog.Debug("fetch-impersonation-proxy-tokens start hook's background goroutine has finished")
}()
return nil
},
)
s.GenericAPIServer.AddPreShutdownHookOrDie("stop-controllers",
func() error {
plog.Debug("stop-controllers pre shutdown hook starting")

60
internal/concierge/impersonator/impersonator.go
View File

@@ -12,7 +12,6 @@ import (
"net/http"
"net/http/httputil"
"net/url"
"os"
"reflect"
"regexp"
"strings"
@@ -56,6 +55,7 @@ import (
"go.pinniped.dev/internal/httputil/securityheader"
"go.pinniped.dev/internal/kubeclient"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/tokenclient"
"go.pinniped.dev/internal/valuelesscontext"
)
@@ -68,26 +68,38 @@ type FactoryFunc func(
port int,
dynamicCertProvider dynamiccert.Private,
impersonationProxySignerCA dynamiccert.Public,
impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet,
) (func(stopCh <-chan struct{}) error, error)
func New(
port int,
dynamicCertProvider dynamiccert.Private,
impersonationProxySignerCA dynamiccert.Public,
impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet,
) (func(stopCh <-chan struct{}) error, error) {
return newInternal(port, dynamicCertProvider, impersonationProxySignerCA, kubeclient.Secure, nil, nil, nil)
return newInternal(port, dynamicCertProvider, impersonationProxySignerCA, kubeclient.Secure, impersonationProxyTokenCache, nil, nil, nil)
}
func newInternal( //nolint:funlen // yeah, it's kind of long.
//nolint:funlen // It is definitely too complicated. New calls newInternal, which makes another function.
func newInternal(
port int,
dynamicCertProvider dynamiccert.Private,
impersonationProxySignerCA dynamiccert.Public,
restConfigFunc ptls.RestConfigFunc, // for unit testing, should always be kubeclient.Secure in production
clientOpts []kubeclient.Option, // for unit testing, should always be nil in production
cache tokenclient.ExpiringSingletonTokenCacheGet,
baseConfig *rest.Config, // for unit testing, should always be nil in production
recOpts func(*genericoptions.RecommendedOptions), // for unit testing, should always be nil in production
recConfig func(*genericapiserver.RecommendedConfig), // for unit testing, should always be nil in production
) (func(stopCh <-chan struct{}) error, error) {
var listener net.Listener
var err error
if baseConfig == nil {
baseConfig, err = rest.InClusterConfig()
if err != nil {
return nil, err
}
}
constructServer := func() (func(stopCh <-chan struct{}) error, error) {
// Bare minimum server side scheme to allow for status messages to be encoded.
@@ -117,7 +129,7 @@ func newInternal( //nolint:funlen // yeah, it's kind of long.
// along with the Kube API server's CA.
// Note: any changes to the Authentication stack need to be kept in sync with any assumptions made
// by getTransportForUser, especially if we ever update the TCR API to start returning bearer tokens.
kubeClientUnsafeForProxying, err := kubeclient.New(clientOpts...)
kubeClientUnsafeForProxying, err := kubeclient.New(kubeclient.WithConfig(baseConfig))
if err != nil {
return nil, err
}
@@ -168,7 +180,8 @@ func newInternal( //nolint:funlen // yeah, it's kind of long.
)
// use the custom impersonation proxy service account credentials when reverse proxying to the API server
kubeClientForProxy, err := getReverseProxyClient(clientOpts)
kubeClientForProxy, err := getReverseProxyClient(baseConfig, cache)
if err != nil {
return nil, fmt.Errorf("failed to build reverse proxy client: %w", err)
}
@@ -321,11 +334,6 @@ func newInternal( //nolint:funlen // yeah, it's kind of long.
return nil, fmt.Errorf("invalid mutation of impersonation authorizer detected: %#v", preparedRun.Authorizer)
}
// Sanity check. Assert that we have a functioning token file to use and no bearer token.
if len(preparedRun.LoopbackClientConfig.BearerToken) != 0 || len(preparedRun.LoopbackClientConfig.BearerTokenFile) == 0 {
return nil, constable.Error("invalid impersonator loopback rest config has wrong bearer token semantics")
}
return preparedRun.Run, nil
}
@@ -341,28 +349,16 @@ func newInternal( //nolint:funlen // yeah, it's kind of long.
return result, nil
}
func getReverseProxyClient(clientOpts []kubeclient.Option) (*kubeclient.Client, error) {
// just use the overrides given during unit tests
if len(clientOpts) != 0 {
return kubeclient.New(clientOpts...)
func getReverseProxyClient(baseConfig *rest.Config, cache tokenclient.ExpiringSingletonTokenCacheGet) (*kubeclient.Client, error) {
impersonationProxyRestConfig := kubeclient.SecureAnonymousClientConfig(baseConfig)
authRoundTripper := func(base http.RoundTripper) http.RoundTripper {
return &authorizationRoundTripper{
cache: cache,
base: base,
}
}
// this is the magic path where the impersonation proxy SA token is mounted
const tokenFile = "/var/run/secrets/impersonation-proxy.concierge.pinniped.dev/serviceaccount/token" //nolint:gosec // this is not a credential
// make sure the token file we need exists before trying to use it
if _, err := os.Stat(tokenFile); err != nil {
return nil, err
}
// build an in cluster config that uses the impersonation proxy token file
impersonationProxyRestConfig, err := rest.InClusterConfig()
if err != nil {
return nil, err
}
impersonationProxyRestConfig = kubeclient.SecureAnonymousClientConfig(impersonationProxyRestConfig)
impersonationProxyRestConfig.BearerTokenFile = tokenFile
impersonationProxyRestConfig.Wrap(authRoundTripper)
return kubeclient.New(kubeclient.WithConfig(impersonationProxyRestConfig))
}

165
internal/concierge/impersonator/impersonator_test.go
View File

@@ -55,6 +55,7 @@ import (
"go.pinniped.dev/internal/httputil/roundtripper"
"go.pinniped.dev/internal/kubeclient"
"go.pinniped.dev/internal/testutil/tlsserver"
"go.pinniped.dev/internal/tokenclient"
)
func TestImpersonator(t *testing.T) {
@@ -84,24 +85,23 @@ func TestImpersonator(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIPriorityAndFairness, false)()
tests := []struct {
name string
clientCert *clientCert
clientImpersonateUser rest.ImpersonationConfig
clientMutateHeaders func(http.Header)
clientNextProtos []string
kubeAPIServerClientBearerTokenFile string
kubeAPIServerStatusCode int
kubeAPIServerHealthz http.Handler
anonymousAuthDisabled bool
wantKubeAPIServerRequestHeaders http.Header
wantError string
wantConstructionError string
wantAuthorizerAttributes []authorizer.AttributesRecord
name string
clientCert *clientCert
clientImpersonateUser rest.ImpersonationConfig
clientMutateHeaders func(http.Header)
clientNextProtos []string
kubeAPIServerStatusCode int
kubeAPIServerHealthz http.Handler
anonymousAuthDisabled bool
noServiceAcctTokenInCache bool // when true, no available service account token for the impersonator to use
wantKubeAPIServerRequestHeaders http.Header
wantError string
wantConstructionError string
wantAuthorizerAttributes []authorizer.AttributesRecord
}{
{
name: "happy path",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "happy path",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
wantKubeAPIServerRequestHeaders: http.Header{
"Impersonate-User": {"test-username"},
"Impersonate-Group": {"test-group1", "test-group2", "system:authenticated"},
@@ -119,9 +119,8 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "happy path with forbidden healthz",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "happy path with forbidden healthz",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
kubeAPIServerHealthz: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusForbidden)
_, _ = w.Write([]byte("no healthz for you"))
@@ -143,9 +142,8 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "happy path with unauthorized healthz",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "happy path with unauthorized healthz",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
kubeAPIServerHealthz: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusUnauthorized)
_, _ = w.Write([]byte("no healthz for you"))
@@ -168,9 +166,8 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "happy path with upgrade",
clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "happy path with upgrade",
clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
clientMutateHeaders: func(header http.Header) {
header.Add("Connection", "Upgrade")
header.Add("Upgrade", "spdy/3.1")
@@ -199,9 +196,8 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "happy path ignores forwarded header",
clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "happy path ignores forwarded header",
clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
clientMutateHeaders: func(header http.Header) {
header.Add("X-Forwarded-For", "example.com")
},
@@ -222,9 +218,8 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "happy path ignores forwarded header canonicalization",
clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "happy path ignores forwarded header canonicalization",
clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
clientMutateHeaders: func(header http.Header) {
header["x-FORWARDED-for"] = append(header["x-FORWARDED-for"], "example.com")
},
@@ -245,11 +240,10 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "user is authenticated but the kube API request returns an error",
kubeAPIServerStatusCode: http.StatusNotFound,
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: `the server could not find the requested resource (get namespaces)`,
name: "user is authenticated but the kube API request returns an error",
kubeAPIServerStatusCode: http.StatusNotFound,
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
wantError: `the server could not find the requested resource (get namespaces)`,
wantKubeAPIServerRequestHeaders: http.Header{
"Impersonate-User": {"test-username"},
"Impersonate-Group": {"test-group1", "test-group2", "system:authenticated"},
@@ -267,9 +261,8 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "when there is no client cert on request, it is an anonymous request",
clientCert: &clientCert{},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "when there is no client cert on request, it is an anonymous request",
clientCert: &clientCert{},
wantKubeAPIServerRequestHeaders: http.Header{
"Impersonate-User": {"system:anonymous"},
"Impersonate-Group": {"system:unauthenticated"},
@@ -294,7 +287,6 @@ func TestImpersonator(t *testing.T) {
req := &http.Request{Header: header}
req.SetBasicAuth("foo", "bar")
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantKubeAPIServerRequestHeaders: http.Header{
"Impersonate-User": {"system:anonymous"},
"Impersonate-Group": {"system:unauthenticated"},
@@ -313,17 +305,15 @@ func TestImpersonator(t *testing.T) {
},
},
{
name: "failed client cert authentication",
clientCert: newClientCert(t, unrelatedCA, "test-username", []string{"test-group1"}),
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: "Unauthorized",
wantAuthorizerAttributes: nil,
name: "failed client cert authentication",
clientCert: newClientCert(t, unrelatedCA, "test-username", []string{"test-group1"}),
wantError: "Unauthorized",
wantAuthorizerAttributes: nil,
},
{
name: "nested impersonation by regular users calls delegating authorizer",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
clientImpersonateUser: rest.ImpersonationConfig{UserName: "some-other-username"},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
name: "nested impersonation by regular users calls delegating authorizer",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
clientImpersonateUser: rest.ImpersonationConfig{UserName: "some-other-username"},
// this fails because the delegating authorizer in this test only allows system:masters and fails everything else
wantError: `users "some-other-username" is forbidden: User "test-username" ` +
`cannot impersonate resource "users" in API group "" at the cluster scope: ` +
@@ -359,7 +349,6 @@ func TestImpersonator(t *testing.T) {
"alpha.kubernetes.io/identity/user/domain/name": {"a-domain-name"},
},
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantKubeAPIServerRequestHeaders: http.Header{
"Impersonate-User": {"fire"},
"Impersonate-Group": {"elements", "system:authenticated"},
@@ -477,8 +466,7 @@ func TestImpersonator(t *testing.T) {
clientMutateHeaders: func(header http.Header) {
header["Impersonate-Uid"] = []string{"root"}
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantAuthorizerAttributes: []authorizer.AttributesRecord{
{
User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -501,8 +489,7 @@ func TestImpersonator(t *testing.T) {
clientMutateHeaders: func(header http.Header) {
header["imPerSoNaTE-uid"] = []string{"magic"}
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantAuthorizerAttributes: []authorizer.AttributesRecord{
{
User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -529,8 +516,7 @@ func TestImpersonator(t *testing.T) {
"something.impersonation-proxy.concierge.pinniped.dev": {"bad data"},
},
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantAuthorizerAttributes: []authorizer.AttributesRecord{
{
User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -569,8 +555,7 @@ func TestImpersonator(t *testing.T) {
"party~~time": {"danger"},
},
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
wantAuthorizerAttributes: []authorizer.AttributesRecord{
{
User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -600,7 +585,6 @@ func TestImpersonator(t *testing.T) {
"ROAR": {"tiger"}, // by the time our code sees this key, it is lowercased to "roar"
},
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantKubeAPIServerRequestHeaders: http.Header{
"Impersonate-User": {"panda"},
"Impersonate-Group": {"other-peeps", "system:authenticated"},
@@ -631,11 +615,6 @@ func TestImpersonator(t *testing.T) {
},
},
},
{
name: "no bearer token file in Kube API server client config",
wantConstructionError: "invalid impersonator loopback rest config has wrong bearer token semantics",
wantAuthorizerAttributes: nil,
},
{
name: "unexpected healthz response",
kubeAPIServerHealthz: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -651,7 +630,6 @@ func TestImpersonator(t *testing.T) {
clientMutateHeaders: func(header http.Header) {
header["imPerSonaTE-USer"] = []string{"PANDA"}
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: `users "PANDA" is forbidden: User "test-username" ` +
`cannot impersonate resource "users" in API group "" at the cluster scope: ` +
`decision made by impersonation-proxy.concierge.pinniped.dev`,
@@ -668,9 +646,8 @@ func TestImpersonator(t *testing.T) {
clientMutateHeaders: func(header http.Header) {
header["imPerSonaTE-uid"] = []string{"007"}
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID 007 authentication.k8s.io/v1 }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
wantAuthorizerAttributes: []authorizer.AttributesRecord{},
wantError: `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID 007 authentication.k8s.io/v1 }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
wantAuthorizerAttributes: []authorizer.AttributesRecord{},
},
{
name: "future UID header", // no longer future as it exists in Kube v1.22
@@ -678,9 +655,21 @@ func TestImpersonator(t *testing.T) {
clientMutateHeaders: func(header http.Header) {
header["Impersonate-Uid"] = []string{"008"}
},
kubeAPIServerClientBearerTokenFile: "required-to-be-set",
wantError: `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID 008 authentication.k8s.io/v1 }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
wantAuthorizerAttributes: []authorizer.AttributesRecord{},
wantError: `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID 008 authentication.k8s.io/v1 }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
wantAuthorizerAttributes: []authorizer.AttributesRecord{},
},
{
name: "when there is no service account token cached for the impersonator to use to call the KAS",
clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
noServiceAcctTokenInCache: true,
wantKubeAPIServerRequestHeaders: nil, // no request should have been made to the KAS on behalf of the user
wantError: `an error on the server ("") has prevented the request from succeeding (get namespaces)`,
wantAuthorizerAttributes: []authorizer.AttributesRecord{
{
User: &user.DefaultInfo{Name: "test-username", UID: "", Groups: []string{"test-group1", "test-group2", "system:authenticated"}, Extra: nil},
Verb: "list", Namespace: "", APIGroup: "", APIVersion: "v1", Resource: "namespaces", Subresource: "", Name: "", ResourceRequest: true, Path: "/api/v1/namespaces",
},
},
},
}
for _, tt := range tests {
@@ -697,7 +686,9 @@ func TestImpersonator(t *testing.T) {
require.NoError(t, err)
// After failing to start and after shutdown, the impersonator port should be available again.
defer requireCanBindToPort(t, port)
t.Cleanup(func() {
requireCanBindToPort(t, port)
})
if tt.kubeAPIServerStatusCode == 0 {
tt.kubeAPIServerStatusCode = http.StatusOK
@@ -830,11 +821,8 @@ func TestImpersonator(t *testing.T) {
// Create the client config that the impersonation server should use to talk to the Kube API server.
testKubeAPIServerKubeconfig := rest.Config{
Host: testKubeAPIServer.URL,
BearerToken: "some-service-account-token",
TLSClientConfig: rest.TLSClientConfig{CAData: tlsserver.TLSTestServerCA(testKubeAPIServer)},
BearerTokenFile: tt.kubeAPIServerClientBearerTokenFile,
}
clientOpts := []kubeclient.Option{kubeclient.WithConfig(&testKubeAPIServerKubeconfig)}
// Punch out just enough stuff to make New actually run without error.
recOpts := func(options *genericoptions.RecommendedOptions) {
@@ -872,8 +860,13 @@ func TestImpersonator(t *testing.T) {
return kubeclient.Secure(config)
}
serviceTokenCache := tokenclient.NewExpiringSingletonTokenCache()
if !tt.noServiceAcctTokenInCache {
serviceTokenCache.Set("some-service-account-token", 1*time.Hour)
}
// Create an impersonator. Use an invalid port number to make sure our listener override works.
runner, constructionErr := newInternal(-1000, certKeyContent, caContent, restConfigFunc, clientOpts, recOpts, recConfig)
runner, constructionErr := newInternal(-1000, certKeyContent, caContent, restConfigFunc, serviceTokenCache, &testKubeAPIServerKubeconfig, recOpts, recConfig)
if len(tt.wantConstructionError) > 0 {
require.EqualError(t, constructionErr, tt.wantConstructionError)
require.Nil(t, runner)
@@ -891,6 +884,13 @@ func TestImpersonator(t *testing.T) {
errCh <- stopErr
}()
// Stop the impersonator server at the end of the test, even if it fails.
t.Cleanup(func() {
close(stopCh)
exitErr := <-errCh
require.NoError(t, exitErr)
})
// Create a kubeconfig to talk to the impersonator as a client.
clientKubeconfig := &rest.Config{
Host: "https://127.0.0.1:" + strconv.Itoa(port),
@@ -922,7 +922,7 @@ func TestImpersonator(t *testing.T) {
client, err := kubeclient.New(kubeclient.WithConfig(clientKubeconfig))
require.NoError(t, err)
// The fake Kube API server knows how to to list namespaces, so make that request using the client
// The fake Kube API server knows how to list namespaces, so make that request using the client
// through the impersonator.
listResponse, err := client.Kubernetes.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
if len(tt.wantError) > 0 {
@@ -944,6 +944,12 @@ func TestImpersonator(t *testing.T) {
// of the original request mutated by the impersonator. Otherwise the headers should be nil.
require.Equal(t, tt.wantKubeAPIServerRequestHeaders, testKubeAPIServerSawHeaders)
// The rest of the test doesn't make sense for when there is no service account token available in the cache.
// In this case, the impersonator cannot make any calls to the Kube API server on behalf of any user.
if tt.noServiceAcctTokenInCache {
return
}
// these authorization checks are caused by the anonymous auth checks below
tt.wantAuthorizerAttributes = append(tt.wantAuthorizerAttributes,
authorizer.AttributesRecord{
@@ -1046,11 +1052,6 @@ func TestImpersonator(t *testing.T) {
_, errBadCert := tcrBadCert.PinnipedConcierge.LoginV1alpha1().TokenCredentialRequests().Create(ctx, &loginv1alpha1.TokenCredentialRequest{}, metav1.CreateOptions{})
require.True(t, errors.IsUnauthorized(errBadCert), errBadCert)
require.EqualError(t, errBadCert, "Unauthorized")
// Stop the impersonator server.
close(stopCh)
exitErr := <-errCh
require.NoError(t, exitErr)
})
}
}

43
internal/concierge/impersonator/roundtripper.go Normal file
View File

@@ -0,0 +1,43 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package impersonator
import (
"errors"
"fmt"
"net/http"
utilnet "k8s.io/apimachinery/pkg/util/net"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/tokenclient"
)
type authorizationRoundTripper struct {
cache tokenclient.ExpiringSingletonTokenCacheGet
base http.RoundTripper
}
var _ utilnet.RoundTripperWrapper = (*authorizationRoundTripper)(nil)
func (rt *authorizationRoundTripper) WrappedRoundTripper() http.RoundTripper {
return rt.base
}
func (rt *authorizationRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
req = utilnet.CloneRequest(req)
token := rt.cache.Get()
if token == "" {
plog.Error("could not RoundTrip impersonation proxy request to API server",
errors.New("no service account token available in in-memory cache"))
return nil, fmt.Errorf("no impersonator service account token available")
}
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
return rt.base.RoundTrip(req)
}

106
internal/concierge/impersonator/roundtripper_test.go Normal file
View File

@@ -0,0 +1,106 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package impersonator
import (
"context"
"errors"
"net/http"
"testing"
"github.com/stretchr/testify/require"
"golang.org/x/oauth2"
"go.pinniped.dev/internal/tokenclient"
)
func TestWrappedRoundTripper(t *testing.T) {
var base = new(oauth2.Transport)
roundTripper := authorizationRoundTripper{
base: base,
}
require.Equal(t, base, roundTripper.WrappedRoundTripper())
}
type fakeRoundTripper struct {
request *http.Request
response *http.Response
err error
}
func (t *fakeRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
t.request = request
return t.response, t.err
}
var _ http.RoundTripper = (*fakeRoundTripper)(nil)
type fakeCache struct {
token string
}
func (c *fakeCache) Get() string {
return c.token
}
var _ tokenclient.ExpiringSingletonTokenCacheGet = (*fakeCache)(nil)
func TestRoundTrip(t *testing.T) {
fakeResponse := new(http.Response)
for _, tt := range []struct {
name string
token string
baseResponse *http.Response
baseError string
wantResponse *http.Response
wantError string
}{
{
name: "happy path - mutate the request and return whatever the base returns",
token: "token",
baseResponse: fakeResponse,
baseError: "error from base",
wantResponse: fakeResponse,
wantError: "error from base",
},
{
name: "no token available",
token: "", // since the cache always returns a non-pointer string, this indicates empty
wantError: "no impersonator service account token available",
},
} {
tt := tt
t.Run(tt.name, func(t *testing.T) {
base := &fakeRoundTripper{
response: new(http.Response),
err: errors.New(tt.baseError),
}
cache := &fakeCache{
token: tt.token,
}
roundTripper := &authorizationRoundTripper{
cache: cache,
base: base,
}
request, err := http.NewRequestWithContext(context.Background(), "GET", "https://example.com", http.NoBody)
require.NoError(t, err)
//nolint:bodyclose // response.Body is nil so you can't call .Close() on it
response, err := roundTripper.RoundTrip(request)
require.Equal(t, tt.wantResponse, response)
require.ErrorContains(t, err, tt.wantError)
if tt.token != "" {
require.Equal(t, "Bearer "+tt.token, base.request.Header.Get("Authorization"))
} else {
require.Empty(t, base.request)
}
})
}
}

21
internal/concierge/server/server.go
View File

@@ -38,6 +38,7 @@ import (
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/pversion"
"go.pinniped.dev/internal/registry/credentialrequest"
"go.pinniped.dev/internal/tokenclient"
)
// App is an object that represents the pinniped-concierge application.
@@ -136,6 +137,8 @@ func (a *App) runServer(ctx context.Context) error {
// injected suffix).
scheme, loginGV, identityGV := conciergescheme.New(*cfg.APIGroupSuffix)
impersonationProxyTokenCache := tokenclient.NewExpiringSingletonTokenCache()
// Prepare to start the controllers, but defer actually starting them until the
// post start hook of the aggregated API server.
buildControllers, err := controllermanager.PrepareControllers(
@@ -154,6 +157,7 @@ func (a *App) runServer(ctx context.Context) error {
AuthenticatorCache: authenticators,
// This port should be safe to cast because the config reader already validated it.
ImpersonationProxyServerPort: int(*cfg.ImpersonationProxyServerPort),
ImpersonationProxyTokenCache: impersonationProxyTokenCache,
},
)
if err != nil {
@@ -181,6 +185,23 @@ func (a *App) runServer(ctx context.Context) error {
return fmt.Errorf("could not configure aggregated API server: %w", err)
}
// Configure a token client that retrieves relatively short-lived tokens from the API server.
// It uses a k8s client without leader election because all pods need tokens.
// This k8s client should not be reused for other purposes.
// The token client will retrieve tokens in the background for the lifetime of the concierge process,
// whether the impersonation proxy is enabled or not.
oneDayInSeconds := int64(24 * 60 * 60)
k8sClient, err := kubeclient.New()
if err != nil {
return fmt.Errorf("could not create default kubernetes client: %w", err)
}
aggregatedAPIServerConfig.ExtraConfig.TokenClient = tokenclient.New(
cfg.NamesConfig.ImpersonationProxyServiceAccount,
k8sClient.Kubernetes.CoreV1().ServiceAccounts(podInfo.Namespace),
impersonationProxyTokenCache.Set,
plog.New(),
tokenclient.WithExpirationSeconds(oneDayInSeconds))
// Complete the aggregated API server config and make a server instance.
server, err := aggregatedAPIServerConfig.Complete().New()
if err != nil {

6
internal/config/concierge/config.go
View File

@@ -161,6 +161,12 @@ func validateNames(names *NamesConfigSpec) error {
if names.AgentServiceAccount == "" {
missingNames = append(missingNames, "agentServiceAccount")
}
if names.ImpersonationProxyServiceAccount == "" {
missingNames = append(missingNames, "impersonationProxyServiceAccount")
}
if names.ImpersonationProxyLegacySecret == "" {
missingNames = append(missingNames, "impersonationProxyLegacySecret")
}
if len(missingNames) > 0 {
return constable.Error("missing required names: " + strings.Join(missingNames, ", "))
}

73
internal/config/concierge/config_test.go
View File

@@ -47,6 +47,8 @@ func TestFromPath(t *testing.T) {
impersonationSignerSecret: impersonationSignerSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
extraName: extraName-value
labels:
myLabelKey1: myLabelValue1
@@ -80,6 +82,8 @@ func TestFromPath(t *testing.T) {
ImpersonationCACertificateSecret: "impersonationCACertificateSecret-value",
ImpersonationSignerSecret: "impersonationSignerSecret-value",
AgentServiceAccount: "agentServiceAccount-value",
ImpersonationProxyServiceAccount: "impersonationProxyServiceAccount-value",
ImpersonationProxyLegacySecret: "impersonationProxyLegacySecret-value",
},
Labels: map[string]string{
"myLabelKey1": "myLabelValue1",
@@ -121,6 +125,8 @@ func TestFromPath(t *testing.T) {
impersonationSignerSecret: impersonationSignerSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
extraName: extraName-value
labels:
myLabelKey1: myLabelValue1
@@ -156,6 +162,8 @@ func TestFromPath(t *testing.T) {
ImpersonationCACertificateSecret: "impersonationCACertificateSecret-value",
ImpersonationSignerSecret: "impersonationSignerSecret-value",
AgentServiceAccount: "agentServiceAccount-value",
ImpersonationProxyServiceAccount: "impersonationProxyServiceAccount-value",
ImpersonationProxyLegacySecret: "impersonationProxyLegacySecret-value",
},
Labels: map[string]string{
"myLabelKey1": "myLabelValue1",
@@ -197,6 +205,8 @@ func TestFromPath(t *testing.T) {
impersonationSignerSecret: impersonationSignerSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
extraName: extraName-value
labels:
myLabelKey1: myLabelValue1
@@ -233,6 +243,8 @@ func TestFromPath(t *testing.T) {
ImpersonationCACertificateSecret: "impersonationCACertificateSecret-value",
ImpersonationSignerSecret: "impersonationSignerSecret-value",
AgentServiceAccount: "agentServiceAccount-value",
ImpersonationProxyServiceAccount: "impersonationProxyServiceAccount-value",
ImpersonationProxyLegacySecret: "impersonationProxyLegacySecret-value",
},
Labels: map[string]string{
"myLabelKey1": "myLabelValue1",
@@ -264,6 +276,7 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
log:
level: all
format: snorlax
@@ -284,6 +297,8 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantConfig: &Config{
DiscoveryInfo: DiscoveryInfoSpec{
@@ -308,6 +323,8 @@ func TestFromPath(t *testing.T) {
ImpersonationCACertificateSecret: "impersonationCACertificateSecret-value",
ImpersonationSignerSecret: "impersonationSignerSecret-value",
AgentServiceAccount: "agentServiceAccount-value",
ImpersonationProxyServiceAccount: "impersonationProxyServiceAccount-value",
ImpersonationProxyLegacySecret: "impersonationProxyLegacySecret-value",
},
Labels: map[string]string{},
KubeCertAgentConfig: KubeCertAgentSpec{
@@ -322,7 +339,7 @@ func TestFromPath(t *testing.T) {
wantError: "validate names: missing required names: servingCertificateSecret, credentialIssuer, " +
"apiService, impersonationLoadBalancerService, " +
"impersonationClusterIPService, impersonationTLSCertificateSecret, impersonationCACertificateSecret, " +
"impersonationSignerSecret, agentServiceAccount",
"impersonationSignerSecret, agentServiceAccount, impersonationProxyServiceAccount, impersonationProxyLegacySecret",
},
{
name: "Missing apiService name",
@@ -337,6 +354,8 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: apiService",
},
@@ -353,6 +372,8 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: credentialIssuer",
},
@@ -369,6 +390,8 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: servingCertificateSecret",
},
@@ -385,6 +408,8 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: impersonationLoadBalancerService",
},
@@ -401,6 +426,8 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: impersonationClusterIPService",
},
@@ -417,6 +444,8 @@ func TestFromPath(t *testing.T) {
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: impersonationTLSCertificateSecret",
},
@@ -433,6 +462,8 @@ func TestFromPath(t *testing.T) {
impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: impersonationCACertificateSecret",
},
@@ -449,9 +480,47 @@ func TestFromPath(t *testing.T) {
impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
impersonationCACertificateSecret: impersonationCACertificateSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: impersonationSignerSecret",
},
{
name: "Missing impersonationProxyServiceAccount name",
yaml: here.Doc(`
---
names:
servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate
credentialIssuer: pinniped-config
apiService: pinniped-api
impersonationLoadBalancerService: impersonationLoadBalancerService-value
impersonationClusterIPService: impersonationClusterIPService-value
impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: impersonationProxyServiceAccount",
},
{
name: "Missing impersonationProxyLegacySecret name",
yaml: here.Doc(`
---
names:
servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate
credentialIssuer: pinniped-config
apiService: pinniped-api
impersonationLoadBalancerService: impersonationLoadBalancerService-value
impersonationClusterIPService: impersonationClusterIPService-value
impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
impersonationCACertificateSecret: impersonationCACertificateSecret-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
`),
wantError: "validate names: missing required names: impersonationProxyLegacySecret",
},
{
name: "Missing several required names",
yaml: here.Doc(`
@@ -464,6 +533,8 @@ func TestFromPath(t *testing.T) {
impersonationClusterIPService: impersonationClusterIPService-value
impersonationSignerSecret: impersonationSignerSecret-value
agentServiceAccount: agentServiceAccount-value
impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
`),
wantError: "validate names: missing required names: " +
"impersonationTLSCertificateSecret, impersonationCACertificateSecret",

2
internal/config/concierge/types.go
View File

@@ -45,6 +45,8 @@ type NamesConfigSpec struct {
ImpersonationCACertificateSecret string `json:"impersonationCACertificateSecret"`
ImpersonationSignerSecret string `json:"impersonationSignerSecret"`
AgentServiceAccount string `json:"agentServiceAccount"`
ImpersonationProxyServiceAccount string `json:"impersonationProxyServiceAccount"`
ImpersonationProxyLegacySecret string `json:"impersonationProxyLegacySecret"`
}
// ServingCertificateConfigSpec contains the configuration knobs for the API's

6
internal/controller/impersonatorconfig/impersonator_config.go
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

8
internal/controller/impersonatorconfig/impersonator_config_test.go
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
var fakeExpiringSingletonTokenCacheGet = tokenclient.NewExpiringSingletonTokenCache()
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

107
internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller.go Normal file
View File

@@ -0,0 +1,107 @@
// 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"
)
// 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,
secretInformer corev1informers.SecretInformer,
withInformer pinnipedcontroller.WithInformerOptionFunc,
logger plog.Logger,
) controllerlib.Controller {
name := "legacy-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)
}
foundSecret := false
for _, secret := range secrets {
if secret.Name == c.legacySecretName && secret.Type == corev1.SecretTypeServiceAccountToken {
foundSecret = true
}
}
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
}

223
internal/controller/serviceaccounttokencleanup/legacy_service_account_token_cleanup_controller_test.go Normal file
View File

@@ -0,0 +1,223 @@
// 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
_ = NewLegacyServiceAccountTokenCleanupController(
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) {
t.Parallel()
kubeAPIClient, kubeInformers := setupKubernetes(t, tt.namespace)
if tt.addReactors != nil {
tt.addReactors(kubeAPIClient)
}
var log bytes.Buffer
controller := NewLegacyServiceAccountTokenCleanupController(
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
}

17
internal/controllermanager/prepare_controllers.go
View File

@@ -25,6 +25,7 @@ import (
"go.pinniped.dev/internal/controller/authenticator/webhookcachefiller"
"go.pinniped.dev/internal/controller/impersonatorconfig"
"go.pinniped.dev/internal/controller/kubecertagent"
"go.pinniped.dev/internal/controller/serviceaccounttokencleanup"
"go.pinniped.dev/internal/controllerinit"
"go.pinniped.dev/internal/controllerlib"
"go.pinniped.dev/internal/deploymentref"
@@ -34,6 +35,7 @@ import (
"go.pinniped.dev/internal/kubeclient"
"go.pinniped.dev/internal/leaderelection"
"go.pinniped.dev/internal/plog"
"go.pinniped.dev/internal/tokenclient"
)
const (
@@ -81,6 +83,9 @@ type Config struct {
// (Note that the impersonation proxy also accepts client certs signed by the Kube API server's cert.)
ImpersonationSigningCertProvider dynamiccert.Provider
// ImpersonationProxyTokenCache holds short-lived tokens for the impersonation proxy service account.
ImpersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet
// ServingCertDuration is the validity period, in seconds, of the API serving certificate.
ServingCertDuration time.Duration
@@ -276,6 +281,7 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol
c.NamesConfig.ImpersonationSignerSecret,
c.ImpersonationSigningCertProvider,
plog.Logr(), //nolint:staticcheck // old controller with lots of log statements
c.ImpersonationProxyTokenCache,
),
singletonWorker,
).
@@ -306,6 +312,17 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol
plog.New(),
),
singletonWorker,
).
WithController(
serviceaccounttokencleanup.NewLegacyServiceAccountTokenCleanupController(
c.ServerInstallationInfo.Namespace,
c.NamesConfig.ImpersonationProxyLegacySecret,
client.Kubernetes,
informers.installationNamespaceK8s.Core().V1().Secrets(),
controllerlib.WithInformer,
plog.New(),
),
singletonWorker,
)
return controllerinit.Prepare(controllerManager.Start, leaderElector,

50
internal/tokenclient/expiringtokencache.go Normal file
View File

@@ -0,0 +1,50 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package tokenclient
import (
"time"
"k8s.io/apimachinery/pkg/util/cache"
)
const tokenCacheKey = "token"
type ExpiringSingletonTokenCacheGet interface {
Get() string
}
type ExpiringSingletonTokenCache interface {
ExpiringSingletonTokenCacheGet
Set(token string, ttl time.Duration)
}
type expiringCacheImpl struct {
cache *cache.Expiring
}
var _ ExpiringSingletonTokenCacheGet = &expiringCacheImpl{}
var _ ExpiringSingletonTokenCache = &expiringCacheImpl{}
func NewExpiringSingletonTokenCache() ExpiringSingletonTokenCache {
return &expiringCacheImpl{cache: cache.NewExpiring()}
}
func (e *expiringCacheImpl) Get() string {
maybeToken, ok := e.cache.Get(tokenCacheKey)
if !ok {
return ""
}
token, ok := maybeToken.(string)
if !ok {
return ""
}
return token
}
func (e *expiringCacheImpl) Set(token string, ttl time.Duration) {
e.cache.Set(tokenCacheKey, token, ttl)
}

35
internal/tokenclient/expiringtokencache_test.go Normal file
View File

@@ -0,0 +1,35 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package tokenclient
import (
"testing"
"time"
"github.com/stretchr/testify/require"
k8sCache "k8s.io/apimachinery/pkg/util/cache"
)
func TestExpiringSingletonTokenCache(t *testing.T) {
cache := NewExpiringSingletonTokenCache()
require.NotNil(t, cache)
require.Empty(t, cache.Get())
cache.Set("i am a 12 hour token", 12*time.Hour)
require.Equal(t, "i am a 12 hour token", cache.Get())
cache.Set("i am a 0-TTL token", time.Duration(0))
time.Sleep(1 * time.Millisecond)
require.Empty(t, cache.Get())
cache.Set("i am a very short token", 1*time.Millisecond)
time.Sleep(2 * time.Millisecond)
require.Empty(t, cache.Get())
}
func TestExpiringSingletonTokenCache_WithNonString(t *testing.T) {
cache := &expiringCacheImpl{cache: k8sCache.NewExpiring()}
cache.cache.Set(tokenCacheKey, true, 1*time.Hour)
require.Empty(t, cache.Get())
}

135
internal/tokenclient/tokenclient.go Normal file
View File

@@ -0,0 +1,135 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package tokenclient
import (
"context"
"time"
"github.com/pkg/errors"
authenticationv1 "k8s.io/api/authentication/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
"k8s.io/utils/clock"
"go.pinniped.dev/internal/backoff"
"go.pinniped.dev/internal/plog"
)
type WhatToDoWithTokenFunc func(token string, ttl time.Duration)
type TokenClient struct {
serviceAccountName string
serviceAccountClient corev1client.ServiceAccountInterface
whatToDoWithToken WhatToDoWithTokenFunc
expirationSeconds int64
clock clock.Clock
logger plog.Logger
}
type Opt func(client *TokenClient)
func WithExpirationSeconds(expirationSeconds int64) Opt {
return func(client *TokenClient) {
client.expirationSeconds = expirationSeconds
}
}
func New(
serviceAccountName string,
serviceAccountClient corev1client.ServiceAccountInterface,
whatToDoWithToken WhatToDoWithTokenFunc,
logger plog.Logger,
opts ...Opt,
) *TokenClient {
client := &TokenClient{
serviceAccountName: serviceAccountName,
serviceAccountClient: serviceAccountClient,
whatToDoWithToken: whatToDoWithToken,
expirationSeconds: 600,
clock: clock.RealClock{},
logger: logger,
}
for _, opt := range opts {
opt(client)
}
return client
}
func (tc TokenClient) Start(ctx context.Context) {
sleeper := make(chan time.Time, 1)
// Make sure that the <-sleeper below gets run once immediately.
sleeper <- time.Now()
for {
select {
case <-ctx.Done():
tc.logger.Info("TokenClient was cancelled and is stopping")
return
case <-sleeper:
var tokenTTL time.Duration
err := backoff.WithContext(ctx, &backoff.InfiniteBackoff{
Duration: 10 * time.Millisecond,
MaxDuration: 10 * time.Second,
Factor: 2.0,
}, func(ctx context.Context) (bool, error) {
var (
err error
token string
)
token, tokenTTL, err = tc.fetchToken(ctx)
if err != nil {
// We got an error. Log it, swallow it, and ask for retry by returning false.
tc.logger.Error("TokenClient could not fetch short-lived service account token (will retry)", err,
"serviceAccountName", tc.serviceAccountName)
return false, nil
}
// We got a new token, so invoke the callback.
tc.whatToDoWithToken(token, tokenTTL)
// Stop backing off.
return true, nil
})
if err != nil {
// We were cancelled during our WithContext. We know it was not due to some other
// error because our last argument to WithContext above never returns any errors.
return
}
// Schedule ourselves to wake up in the future.
time.AfterFunc(tokenTTL*4/5, func() {
sleeper <- time.Now()
})
}
}
}
func (tc TokenClient) fetchToken(ctx context.Context) (token string, ttl time.Duration, _ error) {
tc.logger.Debug("TokenClient calling CreateToken to fetch a short-lived service account token")
tokenResponse, err := tc.serviceAccountClient.CreateToken(ctx,
tc.serviceAccountName,
&authenticationv1.TokenRequest{
Spec: authenticationv1.TokenRequestSpec{
ExpirationSeconds: &tc.expirationSeconds,
},
},
metav1.CreateOptions{},
)
if err != nil {
return "", 0, errors.Wrap(err, "error creating token")
}
if tokenResponse == nil {
return "", 0, errors.New("got nil CreateToken response")
}
return tokenResponse.Status.Token,
tokenResponse.Status.ExpirationTimestamp.Sub(tc.clock.Now()),
nil
}

404
internal/tokenclient/tokenclient_test.go Normal file
View File

@@ -0,0 +1,404 @@
// Copyright 2023 the Pinniped contributors. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
package tokenclient
import (
"bytes"
"context"
"errors"
"sync"
"testing"
"time"
"github.com/stretchr/testify/require"
authenticationv1 "k8s.io/api/authentication/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/fake"
corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
coretesting "k8s.io/client-go/testing"
"k8s.io/utils/clock"
clocktesting "k8s.io/utils/clock/testing"
"go.pinniped.dev/internal/plog"
)
const (
verb = "create"
resource = "serviceaccounts/token"
)
func TestNew(t *testing.T) {
mockWhatToDoWithTokenFunc := *new(WhatToDoWithTokenFunc)
mockClient := fake.NewSimpleClientset().CoreV1().ServiceAccounts("")
mockTime := time.Now()
mockClock := clocktesting.NewFakeClock(mockTime)
var log bytes.Buffer
testLogger := plog.TestLogger(t, &log)
type args struct {
serviceAccountName string
serviceAccountClient corev1client.ServiceAccountInterface
whatToDoWithToken WhatToDoWithTokenFunc
logger plog.Logger
opts []Opt
}
tests := []struct {
name string
args args
expected *TokenClient
}{
{
name: "defaults",
args: args{
serviceAccountName: "serviceAccountName",
serviceAccountClient: mockClient,
whatToDoWithToken: mockWhatToDoWithTokenFunc,
logger: testLogger,
},
expected: &TokenClient{
serviceAccountName: "serviceAccountName",
serviceAccountClient: mockClient,
whatToDoWithToken: mockWhatToDoWithTokenFunc,
expirationSeconds: 600,
clock: clock.RealClock{},
logger: testLogger,
},
},
{
name: "with all opts",
args: args{
serviceAccountName: "custom-serviceAccountName",
serviceAccountClient: mockClient,
whatToDoWithToken: mockWhatToDoWithTokenFunc,
logger: testLogger,
opts: []Opt{
WithExpirationSeconds(777),
withClock(mockClock),
},
},
expected: &TokenClient{
serviceAccountName: "custom-serviceAccountName",
serviceAccountClient: mockClient,
whatToDoWithToken: mockWhatToDoWithTokenFunc,
expirationSeconds: 777,
clock: mockClock,
logger: testLogger,
},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
actual := New(
tt.args.serviceAccountName,
tt.args.serviceAccountClient,
tt.args.whatToDoWithToken,
tt.args.logger,
tt.args.opts...,
)
require.Equal(t, tt.expected, actual)
})
}
}
// withClock should only be used for testing.
func withClock(clock clock.Clock) Opt {
return func(client *TokenClient) {
client.clock = clock
}
}
func TestFetchToken(t *testing.T) {
mockTime := metav1.Now()
type expected struct {
token string
ttl time.Duration
errMessage string
}
tests := []struct {
name string
expirationSeconds int64
serviceAccountName string
tokenResponseValue *authenticationv1.TokenRequest
tokenResponseError error
expected expected
}{
{
name: "happy path",
expirationSeconds: 555,
serviceAccountName: "happy-path-service-account-name",
tokenResponseValue: &authenticationv1.TokenRequest{
Status: authenticationv1.TokenRequestStatus{
Token: "token value",
ExpirationTimestamp: metav1.NewTime(mockTime.Add(25 * time.Minute)),
},
},
expected: expected{
token: "token value",
ttl: 25 * time.Minute,
},
},
{
name: "returns errors from howToFetchTokenFromAPIServer",
expirationSeconds: 444,
serviceAccountName: "service-account-name",
tokenResponseError: errors.New("has an error"),
expected: expected{
errMessage: "error creating token: has an error",
},
},
{
name: "errors when howToFetchTokenFromAPIServer returns nil",
expirationSeconds: 333,
serviceAccountName: "service-account-name",
expected: expected{
errMessage: "got nil CreateToken response",
},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
mockClock := clocktesting.NewFakeClock(mockTime.Time)
var log bytes.Buffer
require.NotEmpty(t, tt.serviceAccountName)
mockClient := fake.NewSimpleClientset()
tokenClient := New(
tt.serviceAccountName,
mockClient.CoreV1().ServiceAccounts("any-namespace-works"),
nil,
plog.TestLogger(t, &log),
WithExpirationSeconds(tt.expirationSeconds),
)
tokenClient.clock = mockClock
mockClient.PrependReactor(verb, resource, func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
require.Equal(t, tt.serviceAccountName, action.(coretesting.CreateActionImpl).Name)
tokenRequest := action.(coretesting.CreateAction).GetObject().(*authenticationv1.TokenRequest)
require.NotNil(t, tokenRequest)
require.Equal(t, tt.expirationSeconds, *tokenRequest.Spec.ExpirationSeconds)
require.Empty(t, tokenRequest.Spec.Audiences)
require.Empty(t, tokenRequest.Spec.BoundObjectRef)
return true, tt.tokenResponseValue, tt.tokenResponseError
})
token, ttl, err := tokenClient.fetchToken(context.Background())
if tt.expected.errMessage != "" {
require.ErrorContains(t, err, tt.expected.errMessage)
} else {
require.Equal(t, tt.expected.token, token)
require.Equal(t, tt.expected.ttl, ttl)
}
})
}
}
func TestStart(t *testing.T) {
type apiResponse struct {
token string
ttl time.Duration // how much in the future from the time of token response to set the expiration date
err error
}
type receivedToken struct {
token string
ttl time.Duration // expected ttl, within a fudge factor
}
type wanted struct {
receivedTokens []receivedToken
timeFudgeFactor time.Duration
approxTimesBetweenAPIInvocations []time.Duration
}
tests := []struct {
name string
apiResponses []apiResponse
want *wanted
}{
{
name: "several successful token requests",
apiResponses: []apiResponse{
{token: "t1", ttl: 200 * time.Millisecond},
{token: "t2", ttl: 400 * time.Millisecond},
{token: "t3", ttl: 300 * time.Millisecond},
{token: "t4", ttl: time.Hour},
},
want: &wanted{
timeFudgeFactor: 30 * time.Millisecond, // lots of fudge for busy CI workers
receivedTokens: []receivedToken{
{token: "t1", ttl: 200 * time.Millisecond},
{token: "t2", ttl: 400 * time.Millisecond},
{token: "t3", ttl: 300 * time.Millisecond},
{token: "t4", ttl: time.Hour},
},
approxTimesBetweenAPIInvocations: []time.Duration{
160 * time.Millisecond, // time between getting t1 and t2 (80% of t1's ttl)
320 * time.Millisecond, // time between getting t2 and t3 (80% of t2's ttl)
240 * time.Millisecond, // time between getting t4 and t4 (80% of t3's ttl)
},
},
},
{
name: "some errors in the middle",
apiResponses: []apiResponse{
{token: "t1", ttl: 100 * time.Millisecond},
{token: "t2", ttl: 200 * time.Millisecond},
{err: errors.New("err1")},
{err: errors.New("err2")},
{err: errors.New("err3")},
{err: errors.New("err4")},
{err: errors.New("err5")},
{err: errors.New("err6")},
{err: errors.New("err7")},
{token: "t3", ttl: 100 * time.Millisecond},
{token: "t4", ttl: time.Hour},
},
want: &wanted{
timeFudgeFactor: 30 * time.Millisecond, // lots of fudge for busy CI workers
receivedTokens: []receivedToken{
{token: "t1", ttl: 100 * time.Millisecond},
{token: "t2", ttl: 200 * time.Millisecond},
{token: "t3", ttl: 100 * time.Millisecond},
{token: "t4", ttl: time.Hour},
},
approxTimesBetweenAPIInvocations: []time.Duration{
80 * time.Millisecond, // time between getting t1 and t2 (80% of t1's ttl)
160 * time.Millisecond, // time between getting t2 and err1 (80% of t2's ttl)
10 * time.Millisecond, // time between getting err1 and err2 (1st step of exponential backoff)
20 * time.Millisecond, // time between getting err2 and err3 (2nd step of exponential backoff)
40 * time.Millisecond, // time between getting err3 and err4 (3rd step of exponential backoff)
80 * time.Millisecond, // time between getting err4 and err5 (4th step of exponential backoff)
160 * time.Millisecond, // time between getting err5 and err6 (5th step of exponential backoff)
320 * time.Millisecond, // time between getting err6 and err7 (6th step of exponential backoff)
640 * time.Millisecond, // time between getting err7 and t3 (7th step of exponential backoff)
80 * time.Millisecond, // time between getting t3 and t4 (80% of t3's ttl)
},
},
},
{
name: "getting errors before successfully fetching the first token",
apiResponses: []apiResponse{
{err: errors.New("err1")},
{err: errors.New("err2")},
{err: errors.New("err3")},
{err: errors.New("err4")},
{token: "t1", ttl: 100 * time.Millisecond},
{token: "t2", ttl: 200 * time.Millisecond},
{token: "t3", ttl: time.Hour},
},
want: &wanted{
timeFudgeFactor: 30 * time.Millisecond, // lots of fudge for busy CI workers
receivedTokens: []receivedToken{
{token: "t1", ttl: 100 * time.Millisecond},
{token: "t2", ttl: 200 * time.Millisecond},
{token: "t3", ttl: time.Hour},
},
approxTimesBetweenAPIInvocations: []time.Duration{
10 * time.Millisecond, // time between getting err1 and err2 (1st step of exponential backoff)
20 * time.Millisecond, // time between getting err2 and err3 (2nd step of exponential backoff)
40 * time.Millisecond, // time between getting err3 and err4 (3rd step of exponential backoff)
80 * time.Millisecond, // time between getting err4 and t1 (4th step of exponential backoff)
80 * time.Millisecond, // time between getting t1 and t2 (80% of t1's ttl)
160 * time.Millisecond, // time between getting t2 and t3 (80% of t2's ttl)
},
},
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
mockClient := fake.NewSimpleClientset()
var logs bytes.Buffer
var mutex sync.Mutex
// These variables are accessed by the reactor and by the callback function in the goroutine which is
// running Start() below. But they are also accessed by this test's main goroutine to make assertions later.
// Protect them with a mutex to make the data race detector happy.
var receivedTokens []receivedToken
var reactorCallTimestamps []time.Time
reactorCallCount := 0
subject := New(
"service-account-name",
mockClient.CoreV1().ServiceAccounts("any-namespace-works"),
func(token string, ttl time.Duration) {
mutex.Lock()
defer mutex.Unlock()
t.Logf("received token %q with ttl %q", token, ttl)
receivedTokens = append(receivedTokens, receivedToken{token: token, ttl: ttl})
},
plog.TestLogger(t, &logs),
)
mockClient.PrependReactor(verb, resource, func(action coretesting.Action) (handled bool, ret runtime.Object, err error) {
mutex.Lock()
defer mutex.Unlock()
require.Less(t, reactorCallCount, len(tt.apiResponses),
"more TokenRequests were made than fake reactor responses were prepared in the test setup")
response := &authenticationv1.TokenRequest{Status: authenticationv1.TokenRequestStatus{
Token: tt.apiResponses[reactorCallCount].token,
ExpirationTimestamp: metav1.NewTime(time.Now().Add(tt.apiResponses[reactorCallCount].ttl)),
}}
responseErr := tt.apiResponses[reactorCallCount].err
reactorCallCount++
reactorCallTimestamps = append(reactorCallTimestamps, time.Now())
t.Logf("fake CreateToken API returning response %q at time %s", response.Status, time.Now())
return true, response, responseErr
})
ctx, cancel := context.WithCancel(context.Background())
time.AfterFunc(4*time.Second, cancel) // cancel the context after a few seconds
go subject.Start(ctx) // Start() should only return after the context is cancelled
<-ctx.Done()
mutex.Lock()
defer mutex.Unlock()
// Should have used up all the reactor responses from the test table.
require.Equal(t, reactorCallCount, len(tt.apiResponses))
// Should have got the expected callbacks for new tokens.
require.Equal(t, len(tt.want.receivedTokens), len(receivedTokens))
for i := range tt.want.receivedTokens {
require.Equal(t, tt.want.receivedTokens[i].token, receivedTokens[i].token)
require.InDelta(t,
float64(tt.want.receivedTokens[i].ttl), float64(receivedTokens[i].ttl),
float64(tt.want.timeFudgeFactor),
)
}
// Should have observed the appropriate amount of elapsed time in between each call to the CreateToken API.
require.Equal(t, reactorCallCount-1, len(tt.want.approxTimesBetweenAPIInvocations), "wrong number of expected time deltas in test setup")
for i := range reactorCallTimestamps {
if i == 0 {
continue
}
actualDelta := reactorCallTimestamps[i].Sub(reactorCallTimestamps[i-1])
require.InDeltaf(t,
tt.want.approxTimesBetweenAPIInvocations[i-1], actualDelta,
float64(tt.want.timeFudgeFactor),
"for API invocation %d", i,
)
}
})
}
}

25
test/integration/pod_shutdown_test.go
View File

@@ -32,11 +32,24 @@ func TestPodShutdown_Disruptive(t *testing.T) {
env := testlib.IntegrationEnv(t, testlib.SkipPodRestartAssertions()).
WithKubeDistribution(testlib.KindDistro)
testShutdownAllPodsOfApp(t, env, env.ConciergeNamespace, env.ConciergeAppName, "-kube-cert-agent-")
testShutdownAllPodsOfApp(t, env, env.SupervisorNamespace, env.SupervisorAppName, "")
shutdownAllPodsOfApp(t, env, env.ConciergeNamespace, env.ConciergeAppName, true)
shutdownAllPodsOfApp(t, env, env.SupervisorNamespace, env.SupervisorAppName, false)
}
func testShutdownAllPodsOfApp(t *testing.T, env *testlib.TestEnv, namespace string, appName string, ignorePodsWithNameSubstring string) {
func shutdownAllPodsOfApp(
t *testing.T,
env *testlib.TestEnv,
namespace string,
appName string,
isConcierge bool,
) {
t.Helper()
ignorePodsWithNameSubstring := ""
if isConcierge {
ignorePodsWithNameSubstring = "-kube-cert-agent-"
}
// Precondition: the app should have some pods running initially.
initialPods := getRunningPodsByNamePrefix(t, namespace, appName+"-", ignorePodsWithNameSubstring)
require.Greater(t, len(initialPods), 0)
@@ -114,6 +127,12 @@ func testShutdownAllPodsOfApp(t *testing.T, env *testlib.TestEnv, namespace stri
"did not find expected message in pod log for pod %q", pl.pod.Name)
require.Containsf(t, pl.logsBuf.String(), `[graceful-termination] apiserver is exiting`,
"did not find expected message in pod log for pod %q", pl.pod.Name)
if isConcierge {
require.Containsf(t, pl.logsBuf.String(), `fetch-impersonation-proxy-tokens start hook's background goroutine has finished`,
"did not find expected message in pod log for pod %q", pl.pod.Name)
}
t.Logf("found expected graceful-termination messages in the logs of pod %q", pl.pod.Name)
}