From 7a812ac5ed0f3b67dc121f8fbba662f03f16cba2 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Tue, 10 Aug 2021 12:05:04 -0400 Subject: [PATCH] impersonatorconfig: only unload dynamiccert when proxy is disabled In the upstream dynamiccertificates package, we rely on two pieces of code: 1. DynamicServingCertificateController.newTLSContent which calls - clientCA.CurrentCABundleContent - servingCert.CurrentCertKeyContent 2. unionCAContent.VerifyOptions which calls - unionCAContent.CurrentCABundleContent This results in calls to our tlsServingCertDynamicCertProvider and impersonationSigningCertProvider. If we Unset these providers, we subtly break these consumers. At best this results in test slowness and flakes while we wait for reconcile loops to converge. At worst, it results in actual errors during runtime. For example, we previously would Unset the impersonationSigningCertProvider on any sync loop error (even a transient one caused by a network blip or a conflict between writes from different replicas of the concierge). This would cause us to transiently fail to issue new certificates from the token credential require API. It would also cause us to transiently fail to authenticate previously issued client certs (which results in occasional Unauthorized errors in CI). Signed-off-by: Monis Khan --- .../controller/apicerts/certs_expirer_test.go | 39 +-- .../impersonatorconfig/impersonator_config.go | 94 +++---- .../impersonator_config_test.go | 229 ++++++++++++++---- .../kubecertagent/kubecertagent_test.go | 1 - internal/testutil/delete.go | 47 ++++ .../concierge_impersonation_proxy_test.go | 69 +++++- 6 files changed, 344 insertions(+), 135 deletions(-) create mode 100644 internal/testutil/delete.go diff --git a/internal/controller/apicerts/certs_expirer_test.go b/internal/controller/apicerts/certs_expirer_test.go index da508c55..01040423 100644 --- a/internal/controller/apicerts/certs_expirer_test.go +++ b/internal/controller/apicerts/certs_expirer_test.go @@ -20,9 +20,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" kubeinformers "k8s.io/client-go/informers" - "k8s.io/client-go/kubernetes" kubernetesfake "k8s.io/client-go/kubernetes/fake" - corev1client "k8s.io/client-go/kubernetes/typed/core/v1" kubetesting "k8s.io/client-go/testing" "go.pinniped.dev/internal/controllerlib" @@ -253,7 +251,8 @@ func TestExpirerControllerSync(t *testing.T) { 0, ) - trackDeleteClient := &clientWrapper{Interface: kubeAPIClient, opts: &[]metav1.DeleteOptions{}} + opts := &[]metav1.DeleteOptions{} + trackDeleteClient := testutil.NewDeleteOptionsRecorder(kubeAPIClient, opts) c := NewCertsExpirerController( namespace, @@ -297,44 +296,16 @@ func TestExpirerControllerSync(t *testing.T) { require.Equal(t, exActions, acActions) if test.wantDelete { - require.Len(t, *trackDeleteClient.opts, 1) + require.Len(t, *opts, 1) require.Equal(t, metav1.DeleteOptions{ Preconditions: &metav1.Preconditions{ UID: &testUID, ResourceVersion: &testRV, }, - }, (*trackDeleteClient.opts)[0]) + }, (*opts)[0]) } else { - require.Len(t, *trackDeleteClient.opts, 0) + require.Len(t, *opts, 0) } }) } } - -type clientWrapper struct { - kubernetes.Interface - opts *[]metav1.DeleteOptions -} - -func (c *clientWrapper) CoreV1() corev1client.CoreV1Interface { - return &coreWrapper{CoreV1Interface: c.Interface.CoreV1(), opts: c.opts} -} - -type coreWrapper struct { - corev1client.CoreV1Interface - opts *[]metav1.DeleteOptions -} - -func (c *coreWrapper) Secrets(namespace string) corev1client.SecretInterface { - return &secretsWrapper{SecretInterface: c.CoreV1Interface.Secrets(namespace), opts: c.opts} -} - -type secretsWrapper struct { - corev1client.SecretInterface - opts *[]metav1.DeleteOptions -} - -func (s *secretsWrapper) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { - *s.opts = append(*s.opts, opts) - return s.SecretInterface.Delete(ctx, name, opts) -} diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index f5845013..c4296877 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -183,8 +183,6 @@ func (c *impersonatorConfigController) Sync(syncCtx controllerlib.Context) error Message: err.Error(), LastUpdateTime: metav1.NewTime(c.clock.Now()), } - // The impersonator is not ready, so clear the signer CA from the dynamic provider. - c.clearSignerCA() } err = utilerrors.NewAggregate([]error{err, issuerconfig.Update( @@ -281,27 +279,32 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre nameInfo, err := c.findDesiredTLSCertificateName(impersonationSpec) if err != nil { - // Unexpected error while determining the name that should go into the certs, so clear any existing certs. - c.tlsServingCertDynamicCertProvider.UnsetCertKeyContent() return nil, err } var impersonationCA *certauthority.CA - if c.shouldHaveTLSSecret(impersonationSpec) { + if c.shouldHaveImpersonator(impersonationSpec) { if impersonationCA, err = c.ensureCASecretIsCreated(ctx); err != nil { return nil, err } if err = c.ensureTLSSecret(ctx, nameInfo, impersonationCA); err != nil { return nil, err } - } else if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { - return nil, err + } else { + if err = c.ensureTLSSecretIsRemoved(ctx); err != nil { + return nil, err + } + c.clearTLSSecret() } credentialIssuerStrategyResult := c.doSyncResult(nameInfo, impersonationSpec, impersonationCA) - if err = c.loadSignerCA(credentialIssuerStrategyResult.Status); err != nil { - return nil, err + if c.shouldHaveImpersonator(impersonationSpec) { + if err = c.loadSignerCA(); err != nil { + return nil, err + } + } else { + c.clearSignerCA() } return credentialIssuerStrategyResult, nil @@ -350,20 +353,16 @@ func (c *impersonatorConfigController) shouldHaveClusterIPService(config *v1alph return c.shouldHaveImpersonator(config) && config.Service.Type == v1alpha1.ImpersonationProxyServiceTypeClusterIP } -func (c *impersonatorConfigController) shouldHaveTLSSecret(config *v1alpha1.ImpersonationProxySpec) bool { - return c.shouldHaveImpersonator(config) -} - -func (c *impersonatorConfigController) serviceExists(serviceName string) (bool, error) { - _, err := c.servicesInformer.Lister().Services(c.namespace).Get(serviceName) +func (c *impersonatorConfigController) serviceExists(serviceName string) (bool, *v1.Service, error) { + service, err := c.servicesInformer.Lister().Services(c.namespace).Get(serviceName) notFound := k8serrors.IsNotFound(err) if notFound { - return false, nil + return false, nil, nil } if err != nil { - return false, err + return false, nil, err } - return true, nil + return true, service, nil } func (c *impersonatorConfigController) tlsSecretExists() (bool, *v1.Secret, error) { @@ -477,7 +476,7 @@ func (c *impersonatorConfigController) ensureLoadBalancerIsStarted(ctx context.C } func (c *impersonatorConfigController) ensureLoadBalancerIsStopped(ctx context.Context) error { - running, err := c.serviceExists(c.generatedLoadBalancerServiceName) + running, service, err := c.serviceExists(c.generatedLoadBalancerServiceName) if err != nil { return err } @@ -488,7 +487,12 @@ func (c *impersonatorConfigController) ensureLoadBalancerIsStopped(ctx context.C c.infoLog.Info("deleting load balancer for impersonation proxy", "service", klog.KRef(c.namespace, c.generatedLoadBalancerServiceName), ) - err = c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedLoadBalancerServiceName, metav1.DeleteOptions{}) + err = c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedLoadBalancerServiceName, metav1.DeleteOptions{ + Preconditions: &metav1.Preconditions{ + UID: &service.UID, + ResourceVersion: &service.ResourceVersion, + }, + }) return utilerrors.FilterOut(err, k8serrors.IsNotFound) } @@ -517,7 +521,7 @@ func (c *impersonatorConfigController) ensureClusterIPServiceIsStarted(ctx conte } func (c *impersonatorConfigController) ensureClusterIPServiceIsStopped(ctx context.Context) error { - running, err := c.serviceExists(c.generatedClusterIPServiceName) + running, service, err := c.serviceExists(c.generatedClusterIPServiceName) if err != nil { return err } @@ -528,7 +532,12 @@ func (c *impersonatorConfigController) ensureClusterIPServiceIsStopped(ctx conte c.infoLog.Info("deleting cluster ip for impersonation proxy", "service", klog.KRef(c.namespace, c.generatedClusterIPServiceName), ) - err = c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedClusterIPServiceName, metav1.DeleteOptions{}) + err = c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedClusterIPServiceName, metav1.DeleteOptions{ + Preconditions: &metav1.Preconditions{ + UID: &service.UID, + ResourceVersion: &service.ResourceVersion, + }, + }) return utilerrors.FilterOut(err, k8serrors.IsNotFound) } @@ -942,7 +951,6 @@ func (c *impersonatorConfigController) loadTLSCertFromSecret(tlsSecret *v1.Secre keyPEM := tlsSecret.Data[v1.TLSPrivateKeyKey] if err := c.tlsServingCertDynamicCertProvider.SetCertKeyContent(certPEM, keyPEM); err != nil { - c.tlsServingCertDynamicCertProvider.UnsetCertKeyContent() return fmt.Errorf("could not parse TLS cert PEM data from Secret: %w", err) } @@ -955,39 +963,33 @@ func (c *impersonatorConfigController) loadTLSCertFromSecret(tlsSecret *v1.Secre } func (c *impersonatorConfigController) ensureTLSSecretIsRemoved(ctx context.Context) error { - tlsSecretExists, _, err := c.tlsSecretExists() + tlsSecretExists, secret, err := c.tlsSecretExists() if err != nil { return err } if !tlsSecretExists { return nil } - c.infoLog.Info("deleting TLS certificates for impersonation proxy", + c.infoLog.Info("deleting TLS serving certificate for impersonation proxy", "secret", klog.KRef(c.namespace, c.tlsSecretName), ) - err = c.k8sClient.CoreV1().Secrets(c.namespace).Delete(ctx, c.tlsSecretName, metav1.DeleteOptions{}) - notFound := k8serrors.IsNotFound(err) - if notFound { - // its okay if we tried to delete and we got a not found error. This probably means - // another instance of the concierge got here first so there's nothing to delete. - return nil - } - if err != nil { - return err - } - - c.tlsServingCertDynamicCertProvider.UnsetCertKeyContent() - - return nil + err = c.k8sClient.CoreV1().Secrets(c.namespace).Delete(ctx, c.tlsSecretName, metav1.DeleteOptions{ + Preconditions: &metav1.Preconditions{ + UID: &secret.UID, + ResourceVersion: &secret.ResourceVersion, + }, + }) + // it is okay if we tried to delete and we got a not found error. This probably means + // another instance of the concierge got here first so there's nothing to delete. + return utilerrors.FilterOut(err, k8serrors.IsNotFound) } -func (c *impersonatorConfigController) loadSignerCA(status v1alpha1.StrategyStatus) error { - // Clear it when the impersonator is not completely ready. - if status != v1alpha1.SuccessStrategyStatus { - c.clearSignerCA() - return nil - } +func (c *impersonatorConfigController) clearTLSSecret() { + c.debugLog.Info("clearing TLS serving certificate for impersonation proxy") + c.tlsServingCertDynamicCertProvider.UnsetCertKeyContent() +} +func (c *impersonatorConfigController) loadSignerCA() error { signingCertSecret, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(c.impersonationSignerSecretName) if err != nil { return fmt.Errorf("could not load the impersonator's credential signing secret: %w", err) @@ -997,7 +999,7 @@ func (c *impersonatorConfigController) loadSignerCA(status v1alpha1.StrategyStat keyPEM := signingCertSecret.Data[apicerts.CACertificatePrivateKeySecretKey] if err := c.impersonationSigningCertProvider.SetCertKeyContent(certPEM, keyPEM); err != nil { - return fmt.Errorf("could not load the impersonator's credential signing secret: %w", err) + return fmt.Errorf("could not set the impersonator's credential signing secret: %w", err) } c.infoLog.Info("loading credential signing certificate for impersonation proxy", diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 33bd17ff..af29711b 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -29,11 +29,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/clock" "k8s.io/apimachinery/pkg/util/intstr" kubeinformers "k8s.io/client-go/informers" + "k8s.io/client-go/kubernetes" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" + "k8s.io/utils/pointer" "go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1" pinnipedfake "go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake" @@ -266,6 +269,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var subject controllerlib.Controller var kubeAPIClient *kubernetesfake.Clientset + var deleteOptions *[]metav1.DeleteOptions + var deleteOptionsRecorder kubernetes.Interface var pinnipedAPIClient *pinnipedfake.Clientset var pinnipedInformerClient *pinnipedfake.Clientset var pinnipedInformers pinnipedinformers.SharedInformerFactory @@ -275,6 +280,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var cancelContextCancelFunc context.CancelFunc var syncContext *controllerlib.Context var frozenNow time.Time + var tlsServingCertDynamicCertProvider dynamiccert.Private var signingCertProvider dynamiccert.Provider var signingCACertPEM, signingCAKeyPEM []byte var signingCASecret *corev1.Secret @@ -418,6 +424,20 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { } } + var requireTLSSecretProviderHasLoadedCerts = func() { + actualCert, actualKey := tlsServingCertDynamicCertProvider.CurrentCertKeyContent() + r.NotEmpty(actualCert) + r.NotEmpty(actualKey) + _, err := tls.X509KeyPair(actualCert, actualKey) + r.NoError(err) + } + + var requireTLSSecretProviderIsEmpty = func() { + actualCert, actualKey := tlsServingCertDynamicCertProvider.CurrentCertKeyContent() + r.Nil(actualCert) + r.Nil(actualKey) + } + var requireTLSServerIsRunning = func(caCrt []byte, addr string, dnsOverrides map[string]string) { r.Greater(impersonatorFuncWasCalled, 0) @@ -469,6 +489,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(resp.Body.Close()) r.NoError(err) r.Equal(fakeServerResponseBody, string(body)) + + requireTLSSecretProviderHasLoadedCerts() } var requireTLSServerIsRunningWithoutCerts = func() { @@ -490,6 +512,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }, 20*time.Second, 50*time.Millisecond) r.Error(err) r.Regexp(expectedErrorRegex, err.Error()) + + requireTLSSecretProviderIsEmpty() } var requireTLSServerIsNoLongerRunning = func() { @@ -508,10 +532,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }, 20*time.Second, 50*time.Millisecond) r.Error(err) r.Regexp(expectedErrorRegex, err.Error()) + + requireTLSSecretProviderIsEmpty() } var requireTLSServerWasNeverStarted = func() { r.Equal(0, impersonatorFuncWasCalled) + + requireTLSSecretProviderIsEmpty() } // Defer starting the informers until the last possible moment so that the @@ -521,7 +549,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { subject = NewImpersonatorConfigController( installedInNamespace, credentialIssuerResourceName, - kubeAPIClient, + deleteOptionsRecorder, pinnipedAPIClient, pinnipedInformers.Config().V1alpha1().CredentialIssuers(), kubeInformers.Core().V1().Services(), @@ -538,6 +566,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { signingCertProvider, testLog, ) + controllerlib.TestWrap(t, subject, func(syncer controllerlib.Syncer) controllerlib.Syncer { + tlsServingCertDynamicCertProvider = syncer.(*impersonatorConfigController).tlsServingCertDynamicCertProvider + return syncer + }) // Set this at the last second to support calling subject.Name(). syncContext = &controllerlib.Context{ @@ -564,8 +596,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var newSecretWithData = func(resourceName string, data map[string][]byte) *corev1.Secret { return &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: resourceName, - Namespace: installedInNamespace, + Name: resourceName, + Namespace: installedInNamespace, + UID: "uid-1234", // simulate KAS filling out UID and RV + ResourceVersion: "rv-5678", }, Data: data, } @@ -705,6 +739,14 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var addObjectFromCreateActionToInformerAndWait = func(action coretesting.Action, informer controllerlib.InformerGetter) { createdObject, ok := action.(coretesting.CreateAction).GetObject().(kubeclient.Object) r.True(ok, "should have been able to cast this action's object to kubeclient.Object: %v", action) + + if secret, ok := createdObject.(*corev1.Secret); ok && len(secret.ResourceVersion) == 0 { + secret = secret.DeepCopy() + secret.UID = "uid-1234" // simulate KAS filling out UID and RV + secret.ResourceVersion = "rv-5678" + createdObject = secret + } + addObjectToKubeInformerAndWait(createdObject, informer) } @@ -986,6 +1028,18 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Equal("delete", deleteAction.GetVerb()) r.Equal(tlsSecretName, deleteAction.GetName()) r.Equal("secrets", deleteAction.GetResource().Resource) + + // validate that we set delete preconditions correctly + r.NotEmpty(*deleteOptions) + for _, opt := range *deleteOptions { + uid := types.UID("uid-1234") + r.Equal(metav1.DeleteOptions{ + Preconditions: &metav1.Preconditions{ + UID: &uid, + ResourceVersion: pointer.String("rv-5678"), + }, + }, opt) + } } var requireCASecretWasCreated = func(action coretesting.Action) []byte { @@ -1064,6 +1118,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { kubeinformers.WithNamespace(installedInNamespace), ) kubeAPIClient = kubernetesfake.NewSimpleClientset() + deleteOptions = &[]metav1.DeleteOptions{} + deleteOptionsRecorder = testutil.NewDeleteOptionsRecorder(kubeAPIClient, deleteOptions) pinnipedAPIClient = pinnipedfake.NewSimpleClientset() frozenNow = time.Date(2021, time.March, 2, 7, 42, 0, 0, time.Local) signingCertProvider = dynamiccert.NewCA(name) @@ -1222,7 +1278,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) @@ -1241,7 +1297,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireNodesListed(kubeAPIClient.Actions()[0]) requireCASecretWasCreated(kubeAPIClient.Actions()[1]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) @@ -1260,7 +1316,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireNodesListed(kubeAPIClient.Actions()[0]) requireCASecretWasCreated(kubeAPIClient.Actions()[1]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) @@ -1451,10 +1507,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { errString := "could not find valid IP addresses or hostnames from load balancer some-namespace/some-service-resource-name" r.EqualError(runControllerSync(), errString) - r.Len(kubeAPIClient.Actions(), 1) // no new actions - requireTLSServerIsRunningWithoutCerts() + r.Len(kubeAPIClient.Actions(), 1) // no new actions + requireTLSServerIsRunning(caCrt, testServerAddr(), nil) // serving certificate is not unloaded in this case requireCredentialIssuer(newErrorStrategy(errString)) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) }) @@ -1510,7 +1566,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) it("returns an error when the impersonation TLS server fails to start", func() { @@ -1545,7 +1601,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireCASecretWasCreated(kubeAPIClient.Actions()[1]) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) it("returns an error when the impersonation TLS server fails to start", func() { @@ -1685,7 +1741,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) @@ -1780,7 +1836,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { // Check that the server is running without certs. requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) @@ -2129,7 +2185,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Len(kubeAPIClient.Actions(), 4) requireTLSSecretWasDeleted(kubeAPIClient.Actions()[3]) // tried to delete cert but failed requireCredentialIssuer(newErrorStrategy("error on tls secret delete")) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) }) @@ -2160,7 +2216,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // load when enabled // Simulate the informer cache's background update from its watch. addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[1], kubeInformers.Core().V1().Services()) @@ -2178,7 +2234,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Len(kubeAPIClient.Actions(), 4) requireServiceWasDeleted(kubeAPIClient.Actions()[3], loadBalancerServiceName) requireCredentialIssuer(newManuallyDisabledStrategy()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderIsEmpty() // only unload when disabled deleteServiceFromTracker(loadBalancerServiceName, kubeInformerClient) waitForObjectToBeDeletedFromInformer(loadBalancerServiceName, kubeInformers.Core().V1().Services()) @@ -2195,7 +2251,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Len(kubeAPIClient.Actions(), 5) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[4]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // load again when enabled }) }) @@ -2226,7 +2282,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireClusterIPWasCreated(kubeAPIClient.Actions()[1]) requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // load when enabled // Simulate the informer cache's background update from its watch. addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[1], kubeInformers.Core().V1().Services()) @@ -2244,7 +2300,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Len(kubeAPIClient.Actions(), 4) requireServiceWasDeleted(kubeAPIClient.Actions()[3], clusterIPServiceName) requireCredentialIssuer(newManuallyDisabledStrategy()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderIsEmpty() // only unload when disabled deleteServiceFromTracker(clusterIPServiceName, kubeInformerClient) waitForObjectToBeDeletedFromInformer(clusterIPServiceName, kubeInformers.Core().V1().Services()) @@ -2264,7 +2320,88 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Len(kubeAPIClient.Actions(), 5) requireClusterIPWasCreated(kubeAPIClient.Actions()[4]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // load again when enabled + }) + }) + + when("service type none with a hostname", func() { + const fakeHostname = "hello.com" + it.Before(func() { + addSecretToTrackers(signingCASecret, kubeInformerClient) + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, + }, + }, + }, pinnipedInformerClient, pinnipedAPIClient) + addNodeWithRoleToTracker("worker", kubeAPIClient) + }) + + it("starts the impersonator, then shuts it down, then starts it again", func() { + startInformersAndController() + + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 3) + requireNodesListed(kubeAPIClient.Actions()[0]) + ca := requireCASecretWasCreated(kubeAPIClient.Actions()[1]) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[2], ca) + requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + requireCredentialIssuer(newSuccessStrategy(fakeHostname, ca)) + + // load when enabled + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) + requireTLSSecretProviderHasLoadedCerts() + + // Simulate the informer cache's background update from its watch. + addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[1], kubeInformers.Core().V1().Secrets()) + addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[2], kubeInformers.Core().V1().Secrets()) + + // Update the CredentialIssuer. + updateCredentialIssuerInInformerAndWait(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeDisabled, + }, + }, pinnipedInformers.Config().V1alpha1().CredentialIssuers()) + + r.NoError(runControllerSync()) + requireTLSServerIsNoLongerRunning() + r.Len(kubeAPIClient.Actions(), 4) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[3]) + requireCredentialIssuer(newManuallyDisabledStrategy()) + + // only unload when disabled requireSigningCertProviderIsEmpty() + requireTLSSecretProviderIsEmpty() + + deleteSecretFromTracker(tlsSecretName, kubeInformerClient) + waitForObjectToBeDeletedFromInformer(tlsSecretName, kubeInformers.Core().V1().Secrets()) + + // Update the CredentialIssuer again. + updateCredentialIssuerInInformerAndWait(credentialIssuerResourceName, v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeEnabled, + ExternalEndpoint: fakeHostname, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, + }, + }, pinnipedInformers.Config().V1alpha1().CredentialIssuers()) + + r.NoError(runControllerSync()) + requireTLSServerIsRunning(ca, fakeHostname, map[string]string{fakeHostname + httpsPort: testServerAddr()}) + r.Len(kubeAPIClient.Actions(), 5) + requireTLSSecretWasCreated(kubeAPIClient.Actions()[4], ca) + requireCredentialIssuer(newSuccessStrategy(fakeHostname, ca)) + + // load again when enabled + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) + requireTLSSecretProviderHasLoadedCerts() }) }) }) @@ -2315,9 +2452,9 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Len(kubeAPIClient.Actions(), 5) requireLoadBalancerWasCreated(kubeAPIClient.Actions()[3]) requireTLSSecretWasDeleted(kubeAPIClient.Actions()[4]) // the Secret was deleted because it contained a cert with the wrong IP - requireTLSServerIsRunningWithoutCerts() + requireTLSServerIsRunning(ca, testServerAddr(), nil) // serving certificate is not unloaded in this case requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Simulate the informer cache's background update from its watch. addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[3], kubeInformers.Core().V1().Services()) @@ -2326,10 +2463,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { // The controller should be waiting for the load balancer's ingress to become available. r.NoError(runControllerSync()) - r.Len(kubeAPIClient.Actions(), 5) // no new actions while it is waiting for the load balancer's ingress - requireTLSServerIsRunningWithoutCerts() + r.Len(kubeAPIClient.Actions(), 5) // no new actions while it is waiting for the load balancer's ingress + requireTLSServerIsRunning(ca, testServerAddr(), nil) // serving certificate is not unloaded in this case requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Update the ingress of the LB in the informer's client and run Sync again. fakeIP := "127.0.0.123" @@ -2767,7 +2904,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Simulate the informer cache's background update from its watch. addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[1], kubeInformers.Core().V1().Services()) @@ -2777,7 +2914,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.Equal(1, impersonatorFuncWasCalled) // wasn't started a second time requireTLSServerIsRunningWithoutCerts() // still running requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) r.Len(kubeAPIClient.Actions(), 3) // no new API calls }) @@ -2790,7 +2927,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { ca := requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Simulate the informer cache's background update from its watch. addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[1], kubeInformers.Core().V1().Services()) @@ -2827,7 +2964,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { ca := requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Simulate the informer cache's background update from its watch. addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[1], kubeInformers.Core().V1().Services()) @@ -2918,6 +3055,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { when("the impersonator start function returned by the impersonatorFunc returns an error immediately", func() { it.Before(func() { + addSecretToTrackers(signingCASecret, kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) impersonatorFuncReturnedFuncError = errors.New("some immediate impersonator startup error") addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ @@ -2948,7 +3086,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Simulate the informer cache's background update from its watch. addObjectFromCreateActionToInformerAndWait(kubeAPIClient.Actions()[1], kubeInformers.Core().V1().Services()) @@ -2966,7 +3104,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { // sync should be able to detect the error and return it. r.EqualError(runControllerSync(), "some immediate impersonator startup error") requireCredentialIssuer(newErrorStrategy("some immediate impersonator startup error")) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Next time the controller starts the server, the server will start successfully. impersonatorFuncReturnedFuncError = nil @@ -2976,12 +3114,13 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(runControllerSync()) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) when("the impersonator server dies for no apparent reason after running for a while", func() { it.Before(func() { + addSecretToTrackers(signingCASecret, kubeInformerClient) addNodeWithRoleToTracker("worker", kubeAPIClient) addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, @@ -3004,7 +3143,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1]) requireCASecretWasCreated(kubeAPIClient.Actions()[2]) requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) requireTLSServerIsRunningWithoutCerts() // Simulate the informer cache's background update from its watch. @@ -3026,7 +3165,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { // sync should be able to detect the error and return it. r.EqualError(runControllerSync(), "unexpected shutdown of proxy server") requireCredentialIssuer(newErrorStrategy("unexpected shutdown of proxy server")) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) // Next time the controller starts the server, the server should behave as normal. testHTTPServerInterruptCh = nil @@ -3036,7 +3175,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { r.NoError(runControllerSync()) requireTLSServerIsRunningWithoutCerts() requireCredentialIssuer(newPendingStrategyWaitingForLB()) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) @@ -3471,16 +3610,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }, }, pinnipedInformerClient, pinnipedAPIClient) addNodeWithRoleToTracker("worker", kubeAPIClient) - tlsSecret := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: tlsSecretName, - Namespace: installedInNamespace, - }, - Data: map[string][]byte{ - // "aGVsbG8gd29ybGQK" is "hello world" base64 encoded which is not a valid cert - corev1.TLSCertKey: []byte("-----BEGIN CERTIFICATE-----\naGVsbG8gd29ybGQK\n-----END CERTIFICATE-----\n"), - }, - } + tlsSecret := newSecretWithData(tlsSecretName, map[string][]byte{ + // "aGVsbG8gd29ybGQK" is "hello world" base64 encoded which is not a valid cert + corev1.TLSCertKey: []byte("-----BEGIN CERTIFICATE-----\naGVsbG8gd29ybGQK\n-----END CERTIFICATE-----\n"), + }) addSecretToTrackers(tlsSecret, kubeAPIClient, kubeInformerClient) }) @@ -3705,7 +3838,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("returns the error", func() { startInformersAndController() - errString := `could not load the impersonator's credential signing secret: TestImpersonatorConfigControllerSync: attempt to set invalid key pair: tls: failed to find any PEM data in certificate input` + errString := `could not set the impersonator's credential signing secret: TestImpersonatorConfigControllerSync: attempt to set invalid key pair: tls: failed to find any PEM data in certificate input` r.EqualError(runControllerSync(), errString) requireCredentialIssuer(newErrorStrategy(errString)) requireSigningCertProviderIsEmpty() @@ -3720,7 +3853,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { it("returns the error", func() { startInformersAndController() - errString := `could not load the impersonator's credential signing secret: TestImpersonatorConfigControllerSync: attempt to set invalid key pair: tls: failed to find any PEM data in certificate input` + errString := `could not set the impersonator's credential signing secret: TestImpersonatorConfigControllerSync: attempt to set invalid key pair: tls: failed to find any PEM data in certificate input` r.EqualError(runControllerSync(), errString) requireCredentialIssuer(newErrorStrategy(errString)) requireSigningCertProviderIsEmpty() @@ -3756,10 +3889,10 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { addSecretToTrackers(updatedSigner, kubeInformerClient) waitForObjectToAppearInInformer(updatedSigner, kubeInformers.Core().V1().Secrets()) - errString := `could not load the impersonator's credential signing secret: TestImpersonatorConfigControllerSync: attempt to set invalid key pair: tls: failed to find any PEM data in certificate input` + errString := `could not set the impersonator's credential signing secret: TestImpersonatorConfigControllerSync: attempt to set invalid key pair: tls: failed to find any PEM data in certificate input` r.EqualError(runControllerSync(), errString) requireCredentialIssuer(newErrorStrategy(errString)) - requireSigningCertProviderIsEmpty() + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) }) }) }) diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index 119a5476..5d829002 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -961,7 +961,6 @@ func runControllerUntilQuiet(ctx context.Context, t *testing.T, controller contr errorStream := make(chan error) controllerlib.TestWrap(t, controller, func(syncer controllerlib.Syncer) controllerlib.Syncer { - controller.Name() return controllerlib.SyncFunc(func(ctx controllerlib.Context) error { err := syncer.Sync(ctx) errorStream <- err diff --git a/internal/testutil/delete.go b/internal/testutil/delete.go new file mode 100644 index 00000000..7a6a9fe5 --- /dev/null +++ b/internal/testutil/delete.go @@ -0,0 +1,47 @@ +// Copyright 2021 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "context" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" +) + +func NewDeleteOptionsRecorder(client kubernetes.Interface, opts *[]metav1.DeleteOptions) kubernetes.Interface { + return &clientWrapper{ + Interface: client, + opts: opts, + } +} + +type clientWrapper struct { + kubernetes.Interface + opts *[]metav1.DeleteOptions +} + +func (c *clientWrapper) CoreV1() corev1client.CoreV1Interface { + return &coreWrapper{CoreV1Interface: c.Interface.CoreV1(), opts: c.opts} +} + +type coreWrapper struct { + corev1client.CoreV1Interface + opts *[]metav1.DeleteOptions +} + +func (c *coreWrapper) Secrets(namespace string) corev1client.SecretInterface { + return &secretsWrapper{SecretInterface: c.CoreV1Interface.Secrets(namespace), opts: c.opts} +} + +type secretsWrapper struct { + corev1client.SecretInterface + opts *[]metav1.DeleteOptions +} + +func (s *secretsWrapper) Delete(ctx context.Context, name string, opts metav1.DeleteOptions) error { + *s.opts = append(*s.opts, opts) + return s.SecretInterface.Delete(ctx, name, opts) +} diff --git a/test/integration/concierge_impersonation_proxy_test.go b/test/integration/concierge_impersonation_proxy_test.go index 21e90f69..a4a514c6 100644 --- a/test/integration/concierge_impersonation_proxy_test.go +++ b/test/integration/concierge_impersonation_proxy_test.go @@ -197,7 +197,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // We do this to ensure that future tests that use the impersonation proxy (e.g., // TestE2EFullIntegration) will start with a known-good state. if clusterSupportsLoadBalancers { - performImpersonatorDiscovery(ctx, t, env, adminConciergeClient) + performImpersonatorDiscovery(ctx, t, env, adminClient, adminConciergeClient, refreshCredential) } }) @@ -277,7 +277,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // to discover the impersonator's URL and CA certificate. Until it has finished starting, it may not be included // in the strategies array or it may be included in an error state. It can be in an error state for // awhile when it is waiting for the load balancer to be assigned an ip/hostname. - impersonationProxyURL, impersonationProxyCACertPEM := performImpersonatorDiscovery(ctx, t, env, adminConciergeClient) + impersonationProxyURL, impersonationProxyCACertPEM := performImpersonatorDiscovery(ctx, t, env, adminClient, adminConciergeClient, refreshCredential) if !clusterSupportsLoadBalancers { // In this case, we specified the endpoint in the configmap, so check that it was reported correctly in the CredentialIssuer. require.Equal(t, "https://"+proxyServiceEndpoint, impersonationProxyURL) @@ -1422,7 +1422,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl require.Equal(t, &corev1.Pod{}, pod) }) - // - request to whoami (pinniped resource endpoing) + // - request to whoami (pinniped resource endpoint) // - through the impersonation proxy // - should succeed 200 // - should respond "you are system:anonymous" @@ -1733,10 +1733,10 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl // wait until the credential issuer is updated with the new url testlib.RequireEventuallyWithoutError(t, func() (bool, error) { - newImpersonationProxyURL, _ := performImpersonatorDiscovery(ctx, t, env, adminConciergeClient) + newImpersonationProxyURL, _ := performImpersonatorDiscoveryURL(ctx, t, env, adminConciergeClient) return newImpersonationProxyURL == "https://"+clusterIPServiceURL, nil }, 30*time.Second, 500*time.Millisecond) - newImpersonationProxyURL, newImpersonationProxyCACertPEM := performImpersonatorDiscovery(ctx, t, env, adminConciergeClient) + newImpersonationProxyURL, newImpersonationProxyCACertPEM := performImpersonatorDiscovery(ctx, t, env, adminClient, adminConciergeClient, refreshCredential) anonymousClient := newAnonymousImpersonationProxyClientWithProxy(t, newImpersonationProxyURL, newImpersonationProxyCACertPEM, nil).PinnipedConcierge refreshedCredentials := refreshCredentialHelper(t, anonymousClient) @@ -1941,7 +1941,64 @@ func expectedWhoAmIRequestResponse(username string, groups []string, extra map[s } } -func performImpersonatorDiscovery(ctx context.Context, t *testing.T, env *testlib.TestEnv, adminConciergeClient pinnipedconciergeclientset.Interface) (string, []byte) { +func performImpersonatorDiscovery(ctx context.Context, t *testing.T, env *testlib.TestEnv, + adminClient kubernetes.Interface, adminConciergeClient pinnipedconciergeclientset.Interface, + refreshCredential func(t *testing.T, impersonationProxyURL string, impersonationProxyCACertPEM []byte) *loginv1alpha1.ClusterCredential) (string, []byte) { + t.Helper() + + impersonationProxyURL, impersonationProxyCACertPEM := performImpersonatorDiscoveryURL(ctx, t, env, adminConciergeClient) + + if len(env.Proxy) == 0 { + t.Log("no test proxy is available, skipping readiness checks for concierge impersonation proxy pods") + return impersonationProxyURL, impersonationProxyCACertPEM + } + + impersonationProxyParsedURL, err := url.Parse(impersonationProxyURL) + require.NoError(t, err) + + expectedGroups := make([]string, 0, len(env.TestUser.ExpectedGroups)+1) // make sure we do not mutate env.TestUser.ExpectedGroups + expectedGroups = append(expectedGroups, env.TestUser.ExpectedGroups...) + expectedGroups = append(expectedGroups, "system:authenticated") + + // probe each pod directly for readiness since the concierge status is a lie - it just means a single pod is ready + testlib.RequireEventually(t, func(requireEventually *require.Assertions) { + pods, err := adminClient.CoreV1().Pods(env.ConciergeNamespace).List(ctx, + metav1.ListOptions{LabelSelector: "app=" + env.ConciergeAppName + ",!kube-cert-agent.pinniped.dev"}) // TODO replace with deployment.pinniped.dev=concierge + requireEventually.NoError(err) + requireEventually.Len(pods.Items, 2) // has to stay in sync with the defaults in our YAML + + for _, pod := range pods.Items { + t.Logf("checking if concierge impersonation proxy pod %q is ready", pod.Name) + + requireEventually.NotEmptyf(pod.Status.PodIP, "pod %q does not have an IP", pod.Name) + + credentials := refreshCredential(t, impersonationProxyURL, impersonationProxyCACertPEM).DeepCopy() + credentials.Token = "not a valid token" // demonstrates that client certs take precedence over tokens by setting both on the requests + + config := newImpersonationProxyConfigWithCredentials(t, credentials, impersonationProxyURL, impersonationProxyCACertPEM, nil) + config = rest.CopyConfig(config) + config.Proxy = kubeconfigProxyFunc(t, env.Proxy) // always use the proxy since we are talking directly to a pod IP + config.Host = "https://" + pod.Status.PodIP + ":8444" // hardcode the internal port - it should not change + config.TLSClientConfig.ServerName = impersonationProxyParsedURL.Hostname() // make SNI hostname TLS verification work even when using IP + + whoAmI, err := testlib.NewKubeclient(t, config).PinnipedConcierge.IdentityV1alpha1().WhoAmIRequests(). + Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) + requireEventually.NoError(err) + requireEventually.Equal( + expectedWhoAmIRequestResponse( + env.TestUser.ExpectedUsername, + expectedGroups, + nil, + ), + whoAmI, + ) + } + }, 10*time.Minute, 10*time.Second) + + return impersonationProxyURL, impersonationProxyCACertPEM +} + +func performImpersonatorDiscoveryURL(ctx context.Context, t *testing.T, env *testlib.TestEnv, adminConciergeClient pinnipedconciergeclientset.Interface) (string, []byte) { t.Helper() var impersonationProxyURL string