diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index 4a60c51f..e3978b47 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -248,7 +248,7 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre } // Make a live API call to avoid the cost of having an informer watch all node changes on the cluster, - // since there could be lots and we don't especially care about node changes. + // since there could be lots, and we don't especially care about node changes. // Once we have concluded that there is or is not a visible control plane, then cache that decision // to avoid listing nodes very often. if c.hasControlPlaneNodes == nil { @@ -296,8 +296,13 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre } var impersonationCABundle []byte - if c.shouldHaveImpersonator(impersonationSpec) { - impersonationCABundle, err = c.ensureCAAndTLSSecrets(ctx, nameInfo) + if c.shouldHaveImpersonator(impersonationSpec) { //nolint:nestif // This is complex but readable + if impersonationSpec.TLS != nil { + impersonationCABundle, err = c.evaluateExternallyProvidedTLSSecret(ctx, impersonationSpec.TLS) + } else { + impersonationCABundle, err = c.ensureCAAndTLSSecrets(ctx, nameInfo) + } + if err != nil { return nil, err } @@ -321,7 +326,9 @@ func (c *impersonatorConfigController) doSync(syncCtx controllerlib.Context, cre return credentialIssuerStrategyResult, nil } -func (c *impersonatorConfigController) ensureCAAndTLSSecrets(ctx context.Context, nameInfo *certNameInfo) ([]byte, error) { +func (c *impersonatorConfigController) ensureCAAndTLSSecrets( + ctx context.Context, + nameInfo *certNameInfo) ([]byte, error) { var ( impersonationCA *certauthority.CA err error @@ -340,6 +347,44 @@ func (c *impersonatorConfigController) ensureCAAndTLSSecrets(ctx context.Context return nil, nil } +func (c *impersonatorConfigController) evaluateExternallyProvidedTLSSecret( + ctx context.Context, + tlsSpec *v1alpha1.ImpersonationProxyTLSSpec) ([]byte, error) { + if tlsSpec.SecretName == "" { + return nil, fmt.Errorf("must provide impersonationSpec.TLS.secretName if impersonationSpec.TLS is provided") + } + + c.infoLog.Info("configuring the impersonation proxy to use an externally provided TLS secret", + "secretName", tlsSpec.SecretName) + + // Ensure that any TLS secret generated by this controller is removed + err := c.ensureTLSSecretIsRemoved(ctx) + if err != nil { + return nil, fmt.Errorf("unable to remove generated TLS secret with name %s: %w", c.tlsSecretName, err) + } + + // The CA Bundle may come from either the TLS secret or the CertificateAuthorityData. + // Check CertificateAuthorityData last so that it will take priority. + + var caBundle []byte + caBundle, err = c.readExternalTLSSecret(tlsSpec.SecretName) + if err != nil { + return nil, fmt.Errorf("could not load the externally provided TLS secret for the impersonation proxy: %w", err) + } + + if tlsSpec.CertificateAuthorityData != "" { + caBundle, err = base64.StdEncoding.DecodeString(tlsSpec.CertificateAuthorityData) + if err != nil { + return nil, fmt.Errorf("could not decode impersonationSpec.TLS.certificateAuthorityData: %w", err) + } + + c.infoLog.Info("the impersonation proxy will advertise its CA Bundle from impersonationSpec.TLS.CertificateAuthorityData", + "CertificateAuthorityData", caBundle) + } + + return caBundle, nil +} + func (c *impersonatorConfigController) loadImpersonationProxyConfiguration(credIssuer *v1alpha1.CredentialIssuer) (*v1alpha1.ImpersonationProxySpec, error) { // Make a copy of the spec since we got this object from informer cache. spec := credIssuer.Spec.DeepCopy().ImpersonationProxy @@ -661,6 +706,26 @@ func (c *impersonatorConfigController) createOrUpdateService(ctx context.Context return err } +func (c *impersonatorConfigController) readExternalTLSSecret(externalTLSSecretName string) (impersonationCABundle []byte, err error) { + secretFromInformer, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(externalTLSSecretName) + if err != nil { + c.infoLog.Info("could not find externally provided TLS secret for the impersonation proxy", + "secretName", externalTLSSecretName) + return nil, err + } + + c.infoLog.Info("found externally provided TLS secret for the impersonation proxy", + "secretName", externalTLSSecretName) + + err = c.loadTLSCertFromSecret(secretFromInformer) + if err != nil { + plog.Error("error loading cert from externally provided TLS secret for the impersonation proxy", err) + return nil, err + } + + return secretFromInformer.Data[caCrtKey], nil +} + func (c *impersonatorConfigController) ensureTLSSecret(ctx context.Context, nameInfo *certNameInfo, ca *certauthority.CA) error { secretFromInformer, err := c.secretsInformer.Lister().Secrets(c.namespace).Get(c.tlsSecretName) notFound := k8serrors.IsNotFound(err) diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index 89dfee9d..7f2ef810 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -271,6 +271,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { const localhostIP = "127.0.0.1" const httpsPort = ":443" const fakeServerResponseBody = "hello, world!" + const externallyProvidedTLSSecretName = "external-tls-secret" //nolint:gosec // this is not a credential var labels = map[string]string{"app": "app-name", "other-key": "other-value"} var r *require.Assertions @@ -300,6 +301,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var testHTTPServerInterruptCh chan struct{} var queue *testQueue var validClientCert *tls.Certificate + var externalCA *certauthority.CA + var externalTLSSecret *corev1.Secret var impersonatorFunc = func( port int, @@ -336,7 +339,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { // and that the second parameter will always be nil in that case. // rawCerts will be raw ASN.1 certificates provided by the peer. if len(rawCerts) != 1 { - return fmt.Errorf("expected to get one client cert on incoming request to test server") + return fmt.Errorf("expected to get one client cert on incoming request to test server, found %d", len(rawCerts)) } clientCert := rawCerts[0] currentClientCertCA := impersonationProxySignerCAProvider.CurrentCABundleContent() @@ -464,8 +467,12 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { var tr *http.Transport if caCrt == nil { tr = &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, //nolint:gosec - DialContext: overrideDialContext, + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, //nolint:gosec // this is used to test when the impersonation proxy does not advertise a CA bundle + // Client cert which is supposed to work against the server's dynamic CAContentProvider + Certificates: []tls.Certificate{*validClientCert}, + }, + DialContext: overrideDialContext, } } else { rootCAs := x509.NewCertPool() @@ -1122,14 +1129,17 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { frozenNow = time.Date(2021, time.March, 2, 7, 42, 0, 0, time.Local) signingCertProvider = dynamiccert.NewCA(name) - ca := newCA() - signingCACertPEM = ca.Bundle() + signingCA := newCA() + signingCACertPEM = signingCA.Bundle() var err error - signingCAKeyPEM, err = ca.PrivateKeyToPEM() + signingCAKeyPEM, err = signingCA.PrivateKeyToPEM() r.NoError(err) signingCASecret = newSigningKeySecret(caSignerName, signingCACertPEM, signingCAKeyPEM) - validClientCert, err = ca.IssueClientCert("username", nil, time.Hour) + validClientCert, err = signingCA.IssueClientCert("username", nil, time.Hour) r.NoError(err) + + externalCA = newCA() + externalTLSSecret = newActualTLSSecret(externalCA, externallyProvidedTLSSecretName, localhostIP) }) it.After(func() { @@ -1159,7 +1169,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) }) - when("the configuration is auto mode with an endpoint and service type none", func() { + when("the configuration is auto mode with an endpoint and service type none, using generated TLS secrets", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ @@ -1211,6 +1221,133 @@ func TestImpersonatorConfigControllerSync(t *testing.T) { }) }) + when("using external TLS secrets", func() { + when("the configuration is auto mode with an endpoint and service type none", func() { + it.Before(func() { + addSecretToTrackers(signingCASecret, kubeInformerClient) + addSecretToTrackers(externalTLSSecret, kubeInformerClient) + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, + TLS: &v1alpha1.ImpersonationProxyTLSSpec{ + CertificateAuthorityData: base64.StdEncoding.EncodeToString(externalCA.Bundle()), + SecretName: externallyProvidedTLSSecretName, + }, + }, + }, + }, pinnipedInformerClient, pinnipedAPIClient) + }) + + when("there are not visible control plane nodes", func() { + it.Before(func() { + addNodeWithRoleToTracker("worker", kubeAPIClient) + }) + + it("starts the impersonator according to the settings in the CredentialIssuer", func() { + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 1) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireTLSServerIsRunning(externalCA.Bundle(), testServerAddr(), nil) + requireCredentialIssuer(newSuccessStrategy(localhostIP, externalCA.Bundle())) + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) + }) + + when("there is an existing generated TLS secret", func() { + it.Before(func() { + addSecretToTrackers(newEmptySecret(tlsSecretName), kubeInformerClient) + }) + + it("removes the existing generated TLS secret", func() { + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 2) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireTLSSecretWasDeleted(kubeAPIClient.Actions()[1]) + requireTLSServerIsRunning(externalCA.Bundle(), testServerAddr(), nil) + requireCredentialIssuer(newSuccessStrategy(localhostIP, externalCA.Bundle())) + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) + }) + }) + }) + }) + + when("the CertificateAuthorityData is not configured", func() { + when("the externally provided TLS secret has a ca.crt field", func() { + it.Before(func() { + addSecretToTrackers(signingCASecret, kubeInformerClient) + externalTLSSecret.Data["ca.crt"] = externalCA.Bundle() + addSecretToTrackers(externalTLSSecret, kubeInformerClient) + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, + TLS: &v1alpha1.ImpersonationProxyTLSSpec{ + SecretName: externallyProvidedTLSSecretName, + }, + }, + }, + }, pinnipedInformerClient, pinnipedAPIClient) + addNodeWithRoleToTracker("worker", kubeAPIClient) + }) + + it("will advertise ca.crt from the externally provided secret", func() { + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 1) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireTLSServerIsRunning(externalTLSSecret.Data["ca.crt"], testServerAddr(), nil) + requireCredentialIssuer(newSuccessStrategy(localhostIP, externalTLSSecret.Data["ca.crt"])) + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) + }) + }) + + when("the externally provided TLS secret does not have a ca.crt field", func() { + it.Before(func() { + addSecretToTrackers(signingCASecret, kubeInformerClient) + addSecretToTrackers(externalTLSSecret, kubeInformerClient) + addCredentialIssuerToTrackers(v1alpha1.CredentialIssuer{ + ObjectMeta: metav1.ObjectMeta{Name: credentialIssuerResourceName}, + Spec: v1alpha1.CredentialIssuerSpec{ + ImpersonationProxy: &v1alpha1.ImpersonationProxySpec{ + Mode: v1alpha1.ImpersonationProxyModeAuto, + ExternalEndpoint: localhostIP, + Service: v1alpha1.ImpersonationProxyServiceSpec{ + Type: v1alpha1.ImpersonationProxyServiceTypeNone, + }, + TLS: &v1alpha1.ImpersonationProxyTLSSpec{ + SecretName: externallyProvidedTLSSecretName, + }, + }, + }, + }, pinnipedInformerClient, pinnipedAPIClient) + addNodeWithRoleToTracker("worker", kubeAPIClient) + }) + + it("will advertise an empty CA bundle", func() { + startInformersAndController() + r.NoError(runControllerSync()) + r.Len(kubeAPIClient.Actions(), 1) + requireNodesListed(kubeAPIClient.Actions()[0]) + requireTLSServerIsRunning(nil, testServerAddr(), nil) + requireCredentialIssuer(newSuccessStrategy(localhostIP, nil)) + requireSigningCertProviderHasLoadedCerts(signingCACertPEM, signingCAKeyPEM) + }) + }) + }) + }) + when("the configuration is auto mode", func() { it.Before(func() { addSecretToTrackers(signingCASecret, kubeInformerClient) diff --git a/test/integration/kube_api_discovery_test.go b/test/integration/kube_api_discovery_test.go index 524f9116..9d7f63e8 100644 --- a/test/integration/kube_api_discovery_test.go +++ b/test/integration/kube_api_discovery_test.go @@ -441,7 +441,7 @@ func TestGetAPIResourceList(t *testing.T) { //nolint:gocyclo // each t.Run is pr // over time, make a rudimentary assertion that this test exercised the whole tree of all fields of all // Pinniped API resources. Without this, the test could accidentally skip parts of the tree if the // format has changed. - require.Equal(t, 227, foundFieldNames, + require.Equal(t, 230, foundFieldNames, "Expected to find all known fields of all Pinniped API resources. "+ "You may will need to update this expectation if you added new fields to the API types.", )