diff --git a/test/integration/e2e_test.go b/test/integration/e2e_test.go index 5d513d9dd..dc6897df7 100644 --- a/test/integration/e2e_test.go +++ b/test/integration/e2e_test.go @@ -38,7 +38,6 @@ import ( supervisorconfigv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" supervisorclient "go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/config/v1alpha1" - "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/crud" "go.pinniped.dev/internal/here" "go.pinniped.dev/internal/testutil" @@ -66,6 +65,16 @@ func TestE2EFullIntegration_Browser(t *testing.T) { topSetupCtx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Minute) defer cancelFunc() + supervisorClient := testlib.NewSupervisorClientset(t) + kubeClient := testlib.NewKubernetesClientset(t) + temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret( + topSetupCtx, + t, + env.SupervisorNamespace, + env.DefaultTLSCertSecretName(), + supervisorClient, + kubeClient, + ) // Build pinniped CLI. pinnipedExe := testlib.PinnipedCLIPath(t) @@ -74,58 +83,28 @@ func TestE2EFullIntegration_Browser(t *testing.T) { // Generate a CA bundle with which to serve this provider. t.Logf("generating test CA") - federationDomainSelfSignedCA, err := certauthority.New("Downstream Test CA", 1*time.Hour) - require.NoError(t, err) + tlsServingCertForSupervisorSecretName := "federation-domain-serving-cert-" + testlib.RandHex(t, 8) + + federationDomainSelfSignedCA := createTLSServingCertSecretForSupervisor( + topSetupCtx, + t, + env, + supervisorIssuer, + tlsServingCertForSupervisorSecretName, + kubeClient, + ) // Save that bundle plus the one that signs the upstream issuer, for test purposes. federationDomainCABundlePath := filepath.Join(t.TempDir(), "test-ca.pem") federationDomainCABundlePEM := federationDomainSelfSignedCA.Bundle() require.NoError(t, os.WriteFile(federationDomainCABundlePath, federationDomainCABundlePEM, 0600)) - // Use the CA to issue a TLS server cert. - certPEM, keyPEM := supervisorIssuer.IssuerServerCert(t, federationDomainSelfSignedCA) - - supervisorClient := testlib.NewSupervisorClientset(t) - temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret( - topSetupCtx, - t, - env.SupervisorNamespace, - env.DefaultTLSCertSecretName(), - supervisorClient, - testlib.NewKubernetesClientset(t), - ) - - var tlsSpecForFederationDomain *supervisorconfigv1alpha1.FederationDomainTLSSpec - if supervisorIssuer.IsIPAddress() { - testlib.CreateTestSecretWithName( - t, - env.SupervisorNamespace, - env.DefaultTLSCertSecretName(), - corev1.SecretTypeTLS, - map[string]string{ - "tls.crt": string(certPEM), - "tls.key": string(keyPEM), - }, - ) - } else { - // Write the serving cert to a secret. - federationDomainTLSServingCertSecret := testlib.CreateTestSecret(t, - env.SupervisorNamespace, - "oidc-provider-tls", - corev1.SecretTypeTLS, - map[string]string{ - "tls.crt": string(certPEM), - "tls.key": string(keyPEM), - }, - ) - tlsSpecForFederationDomain = &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: federationDomainTLSServingCertSecret.Name} - } - // Create the downstream FederationDomain. + // This helper function will nil out spec.TLS if spec.Issuer is an IP address. federationDomain := testlib.CreateTestFederationDomain(topSetupCtx, t, supervisorconfigv1alpha1.FederationDomainSpec{ Issuer: supervisorIssuer.Issuer(), - TLS: tlsSpecForFederationDomain, + TLS: &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: tlsServingCertForSupervisorSecretName}, }, supervisorconfigv1alpha1.FederationDomainPhaseError, // in phase error until there is an IDP created ) @@ -552,6 +531,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) { if runtime.GOOS != "darwin" { // For some unknown reason this breaks the pty library on some macOS machines. // The problem doesn't reproduce for everyone, so this is just a workaround. + var err error kubectlStdoutPipe, err = kubectlCmd.StdoutPipe() require.NoError(t, err) } diff --git a/test/integration/securetls_test.go b/test/integration/securetls_test.go index 9c86385ba..7daf2ba3a 100644 --- a/test/integration/securetls_test.go +++ b/test/integration/securetls_test.go @@ -118,7 +118,15 @@ func TestSecureTLSSupervisor(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) t.Cleanup(cancel) - startKubectlPortForward(ctx, t, "10448", "443", env.SupervisorAppName+"-nodeport", env.SupervisorNamespace) + supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress) + + serviceSuffix := "-nodeport" + if supervisorIssuer.IsIPAddress() { + // Then there's no nodeport service to connect to, it's a load balancer service! + serviceSuffix = "-loadbalancer" + } + + startKubectlPortForward(ctx, t, "10448", "443", env.SupervisorAppName+serviceSuffix, env.SupervisorNamespace) stdout, stderr := testlib.RunNmapSSLEnum(t, "127.0.0.1", 10448) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index a6ead4621..ea9be548e 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -23,6 +23,9 @@ import ( corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + applycorev1 "k8s.io/client-go/applyconfigurations/core/v1" + applymetav1 "k8s.io/client-go/applyconfigurations/meta/v1" + "k8s.io/client-go/kubernetes" "k8s.io/client-go/util/retry" "k8s.io/utils/ptr" @@ -47,16 +50,14 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { client := testlib.NewSupervisorClientset(t) kubeClient := testlib.NewKubernetesClientset(t) - ns := env.SupervisorNamespace - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() - temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, env.DefaultTLSCertSecretName(), client, testlib.NewKubernetesClientset(t)) - defaultCA := createTLSCertificateSecret( + temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, env.SupervisorNamespace, env.DefaultTLSCertSecretName(), client, testlib.NewKubernetesClientset(t)) + defaultCA := createTLSServingCertSecretForSupervisor( ctx, t, - ns, + env, testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress), env.DefaultTLSCertSecretName(), kubeClient, @@ -105,9 +106,9 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { // When FederationDomains are created in sequence they each cause a discovery endpoint to appear only for as long as the FederationDomain exists. config1, jwks1 := requireCreatingFederationDomainCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer1, client) - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config1, client, ns, scheme, addr, caBundle, issuer1) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config1, client, env.SupervisorNamespace, scheme, addr, caBundle, issuer1) config2, jwks2 := requireCreatingFederationDomainCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer2, client) - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config2, client, ns, scheme, addr, caBundle, issuer2) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config2, client, env.SupervisorNamespace, scheme, addr, caBundle, issuer2) // The auto-created JWK's were different from each other. require.NotEqual(t, jwks1.Keys[0]["x"], jwks2.Keys[0]["x"]) require.NotEqual(t, jwks1.Keys[0]["y"], jwks2.Keys[0]["y"]) @@ -121,47 +122,47 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { require.NotEqual(t, jwks3.Keys[0]["y"], jwks4.Keys[0]["y"]) // Editing a FederationDomain to change the issuer URL updates the endpoints that are being served. - updatedConfig4 := editFederationDomainIssuerName(t, config4, client, ns, issuer5) + updatedConfig4 := editFederationDomainIssuerName(t, config4, client, env.SupervisorNamespace, issuer5) requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, issuer4) jwks5 := requireStandardDiscoveryEndpointsAreWorking(t, scheme, addr, caBundle, issuer5, nil) // The JWK did not change when the issuer name was updated. require.Equal(t, jwks4.Keys[0], jwks5.Keys[0]) // When they are deleted they stop serving discovery endpoints. - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config3, client, ns, scheme, addr, caBundle, issuer3) - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, updatedConfig4, client, ns, scheme, addr, caBundle, issuer5) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config3, client, env.SupervisorNamespace, scheme, addr, caBundle, issuer3) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, updatedConfig4, client, env.SupervisorNamespace, scheme, addr, caBundle, issuer5) // When the same issuer URL is added to two FederationDomains, both FederationDomains are marked as duplicates, and neither is serving. config6Duplicate1, _ := requireCreatingFederationDomainCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer6, client) config6Duplicate2 := testlib.CreateTestFederationDomain(ctx, t, supervisorconfigv1alpha1.FederationDomainSpec{Issuer: issuer6}, supervisorconfigv1alpha1.FederationDomainPhaseError) - requireStatus(t, client, ns, config6Duplicate1.Name, supervisorconfigv1alpha1.FederationDomainPhaseError, withFalseConditions([]string{"Ready", "IssuerIsUnique"})) - requireStatus(t, client, ns, config6Duplicate2.Name, supervisorconfigv1alpha1.FederationDomainPhaseError, withFalseConditions([]string{"Ready", "IssuerIsUnique"})) + requireStatus(t, client, env.SupervisorNamespace, config6Duplicate1.Name, supervisorconfigv1alpha1.FederationDomainPhaseError, withFalseConditions([]string{"Ready", "IssuerIsUnique"})) + requireStatus(t, client, env.SupervisorNamespace, config6Duplicate2.Name, supervisorconfigv1alpha1.FederationDomainPhaseError, withFalseConditions([]string{"Ready", "IssuerIsUnique"})) requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, issuer6) // If we delete the first duplicate FederationDomain, the second duplicate FederationDomain starts serving. - requireDelete(t, client, ns, config6Duplicate1.Name) + requireDelete(t, client, env.SupervisorNamespace, config6Duplicate1.Name) requireWellKnownEndpointIsWorking(t, scheme, addr, caBundle, issuer6, nil) - requireStatus(t, client, ns, config6Duplicate2.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady, withAllSuccessfulConditions()) + requireStatus(t, client, env.SupervisorNamespace, config6Duplicate2.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady, withAllSuccessfulConditions()) // When we finally delete all FederationDomains, the discovery endpoints should be down. - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config6Duplicate2, client, ns, scheme, addr, caBundle, issuer6) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config6Duplicate2, client, env.SupervisorNamespace, scheme, addr, caBundle, issuer6) // "Host" headers can be used to send requests to discovery endpoints when the public address is different from the issuer URL. issuer7 := "https://some-issuer-host-and-port-that-doesnt-match-public-supervisor-address.com:2684/issuer7" config7, _ := requireCreatingFederationDomainCausesDiscoveryEndpointsToAppear(ctx, t, scheme, addr, caBundle, issuer7, client) - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config7, client, ns, scheme, addr, caBundle, issuer7) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config7, client, env.SupervisorNamespace, scheme, addr, caBundle, issuer7) // When we create a FederationDomain with an invalid issuer url, the status is set to invalid. badConfig := testlib.CreateTestFederationDomain(ctx, t, supervisorconfigv1alpha1.FederationDomainSpec{Issuer: badIssuer}, supervisorconfigv1alpha1.FederationDomainPhaseError) - requireStatus(t, client, ns, badConfig.Name, supervisorconfigv1alpha1.FederationDomainPhaseError, withFalseConditions([]string{"Ready", "IssuerURLValid"})) + requireStatus(t, client, env.SupervisorNamespace, badConfig.Name, supervisorconfigv1alpha1.FederationDomainPhaseError, withFalseConditions([]string{"Ready", "IssuerURLValid"})) requireDiscoveryEndpointsAreNotFound(t, scheme, addr, caBundle, badIssuer) - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, badConfig, client, ns, scheme, addr, caBundle, badIssuer) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, badConfig, client, env.SupervisorNamespace, scheme, addr, caBundle, badIssuer) issuer8 := fmt.Sprintf("https://%s/issuer8multipleIDP", addr) - config8 := requireIDPsListedByIDPDiscoveryEndpoint(t, env, ctx, kubeClient, ns, scheme, addr, caBundle, issuer8) + config8 := requireIDPsListedByIDPDiscoveryEndpoint(t, env, ctx, kubeClient, env.SupervisorNamespace, scheme, addr, caBundle, issuer8) // requireJWKSEndpointIsWorking() will give us a bit of an idea what to do... - requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config8, client, ns, scheme, addr, caBundle, issuer8) + requireDeletingFederationDomainCausesDiscoveryEndpointsToDisappear(t, config8, client, env.SupervisorNamespace, scheme, addr, caBundle, issuer8) }) } } @@ -172,7 +173,6 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { pinnipedClient := testlib.NewSupervisorClientset(t) kubeClient := testlib.NewKubernetesClientset(t) - ns := env.SupervisorNamespace ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() @@ -182,7 +182,7 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, }, idpv1alpha1.PhaseError) - temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, env.DefaultTLSCertSecretName(), pinnipedClient, kubeClient) + temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, env.SupervisorNamespace, env.DefaultTLSCertSecretName(), pinnipedClient, kubeClient) scheme := "https" supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress) @@ -203,20 +203,31 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuer1) // Create the Secret. - ca1 := createTLSCertificateSecret(ctx, t, ns, supervisorIssuer, certSecretName1, kubeClient) + ca1 := createTLSServingCertSecretForSupervisor( + ctx, + t, + env, + supervisorIssuer, + certSecretName1, + kubeClient, + ) // Now that the Secret exists, we should be able to access the endpoints by hostname using the CA. _ = requireStandardDiscoveryEndpointsAreWorking(t, scheme, address, string(ca1.Bundle()), issuer1, nil) + // Delete the default TLS secret as well + err := kubeClient.CoreV1().Secrets(env.SupervisorNamespace).Delete(ctx, env.DefaultTLSCertSecretName(), metav1.DeleteOptions{}) + require.True(t, err == nil || apierrors.IsNotFound(err), "unexpected error when deleting the default secret: %s", err) + // Update the config to with a new .spec.tls.secretName. certSecretName1update := "integration-test-cert-1-update" require.NoError(t, retry.RetryOnConflict(retry.DefaultRetry, func() error { - federationDomain1LatestVersion, err := pinnipedClient.ConfigV1alpha1().FederationDomains(ns).Get(ctx, federationDomain1.Name, metav1.GetOptions{}) + federationDomain1LatestVersion, err := pinnipedClient.ConfigV1alpha1().FederationDomains(env.SupervisorNamespace).Get(ctx, federationDomain1.Name, metav1.GetOptions{}) if err != nil { return err } federationDomain1LatestVersion.Spec.TLS = &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: certSecretName1update} - _, err = pinnipedClient.ConfigV1alpha1().FederationDomains(ns).Update(ctx, federationDomain1LatestVersion, metav1.UpdateOptions{}) + _, err = pinnipedClient.ConfigV1alpha1().FederationDomains(env.SupervisorNamespace).Update(ctx, federationDomain1LatestVersion, metav1.UpdateOptions{}) return err })) @@ -224,7 +235,14 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuer1) // Create a Secret at the updated name. - ca1update := createTLSCertificateSecret(ctx, t, ns, supervisorIssuer, certSecretName1update, kubeClient) + ca1update := createTLSServingCertSecretForSupervisor( + ctx, + t, + env, + supervisorIssuer, + certSecretName1update, + kubeClient, + ) // Now that the Secret exists at the new name, we should be able to access the endpoints by hostname using the CA. _ = requireStandardDiscoveryEndpointsAreWorking(t, scheme, address, string(ca1update.Bundle()), issuer1, nil) @@ -244,7 +262,14 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { requireStatus(t, pinnipedClient, federationDomain2.Namespace, federationDomain2.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady, withAllSuccessfulConditions()) // Create the Secret. - ca2 := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, issuer2), certSecretName2, kubeClient) + ca2 := createTLSServingCertSecretForSupervisor( + ctx, + t, + env, + testlib.NewSupervisorIssuer(t, issuer2), + certSecretName2, + kubeClient, + ) // Now that the Secret exists, we should be able to access the endpoints by hostname using the CA. _ = requireStandardDiscoveryEndpointsAreWorking(t, scheme, hostname2+":"+hostnamePort2, string(ca2.Bundle()), issuer2, map[string]string{ @@ -258,7 +283,6 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { pinnipedClient := testlib.NewSupervisorClientset(t) kubeClient := testlib.NewKubernetesClientset(t) - ns := env.SupervisorNamespace ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() @@ -268,7 +292,7 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { Client: idpv1alpha1.OIDCClient{SecretName: "this-will-not-exist-but-does-not-matter"}, }, idpv1alpha1.PhaseError) - temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, env.DefaultTLSCertSecretName(), pinnipedClient, kubeClient) + temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, env.SupervisorNamespace, env.DefaultTLSCertSecretName(), pinnipedClient, kubeClient) scheme := "https" supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress) @@ -296,7 +320,14 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuerUsingIPAddress) // Create a Secret at the special name which represents the default TLS cert. - defaultCA := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, issuerUsingIPAddress), env.DefaultTLSCertSecretName(), kubeClient) + defaultCA := createTLSServingCertSecretForSupervisor( + ctx, + t, + env, + testlib.NewSupervisorIssuer(t, issuerUsingIPAddress), + env.DefaultTLSCertSecretName(), + kubeClient, + ) // Now that the Secret exists, we should be able to access the endpoints by IP address using the CA. _ = requireStandardDiscoveryEndpointsAreWorking(t, scheme, ipWithPort, string(defaultCA.Bundle()), issuerUsingIPAddress, nil) @@ -311,57 +342,84 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { requireStatus(t, pinnipedClient, federationDomain2.Namespace, federationDomain2.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady, withAllSuccessfulConditions()) // Create the Secret. - certCA := createTLSCertificateSecret(ctx, t, ns, supervisorIssuer, certSecretName, kubeClient) + certCA := createTLSServingCertSecretForSupervisor( + ctx, + t, + env, + supervisorIssuer, + certSecretName, + kubeClient, + ) // Now that the Secret exists, we should be able to access the endpoints by hostname using the CA from the SNI cert. // Hostnames are case-insensitive, so the request should still work even if the case of the hostname is different // from the case of the issuer URL's hostname. _ = requireStandardDiscoveryEndpointsAreWorking(t, scheme, strings.ToUpper(hostname)+":"+port, string(certCA.Bundle()), issuerUsingHostname, nil) - // And we can still access the other issuer using the default cert. - _ = requireStandardDiscoveryEndpointsAreWorking(t, scheme, ipWithPort, string(defaultCA.Bundle()), issuerUsingIPAddress, nil) + if !supervisorIssuer.IsIPAddress() { + // And we can still access the other issuer using the default cert, + // except when we have an IP address, because in that case we just overwrote the default cert + _ = requireStandardDiscoveryEndpointsAreWorking(t, scheme, ipWithPort, string(defaultCA.Bundle()), issuerUsingIPAddress, nil) + } } -func createTLSCertificateSecret( +func createTLSServingCertSecretForSupervisor( ctx context.Context, t *testing.T, - namespace string, + env *testlib.TestEnv, supervisorIssuer testlib.SupervisorIssuer, secretName string, kubeClient kubernetes.Interface, ) *certauthority.CA { + // If the issuer is an IP address, then we have to create/update the DEFAULT cert, not the given secret + if supervisorIssuer.IsIPAddress() { + secretName = env.DefaultTLSCertSecretName() + } + // Create a CA. ca, err := certauthority.New("Acme Corp", 1000*time.Hour) require.NoError(t, err) - // Using the CA, create a TLS server cert. + // Using the CA, create a TLS serving cert. certPEM, keyPEM := supervisorIssuer.IssuerServerCert(t, ca) - // Write the serving cert to the SNI secret. - secret := corev1.Secret{ - Type: corev1.SecretTypeTLS, - TypeMeta: metav1.TypeMeta{}, - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - Namespace: namespace, + secret := &applycorev1.SecretApplyConfiguration{ + TypeMetaApplyConfiguration: applymetav1.TypeMetaApplyConfiguration{ + Kind: ptr.To("Secret"), + APIVersion: ptr.To("v1"), + }, + Type: ptr.To(corev1.SecretTypeTLS), + ObjectMetaApplyConfiguration: &applymetav1.ObjectMetaApplyConfiguration{ + Name: ptr.To(secretName), + Namespace: ptr.To(env.SupervisorNamespace), }, StringData: map[string]string{ "tls.crt": string(certPEM), "tls.key": string(keyPEM), }, } - _, err = kubeClient.CoreV1().Secrets(namespace).Create(ctx, &secret, metav1.CreateOptions{}) + + _, err = kubeClient.CoreV1().Secrets(env.SupervisorNamespace).Apply(ctx, secret, metav1.ApplyOptions{FieldManager: "pinniped-integration-tests"}) require.NoError(t, err) - t.Logf("wrote TLS cert secret to: %s/%s", namespace, secretName) + t.Logf("wrote TLS cert secret to: %s/%s", env.SupervisorNamespace, secretName) // Delete the Secret when the test ends. t.Cleanup(func() { t.Helper() deleteCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - err := kubeClient.CoreV1().Secrets(namespace).Delete(deleteCtx, secretName, metav1.DeleteOptions{}) - require.NoError(t, err) + // check to see if it exists (so that the default TLS secret doesn't get deleted multiple times) + secret, err := kubeClient.CoreV1().Secrets(env.SupervisorNamespace).Get(deleteCtx, secretName, metav1.GetOptions{}) + require.True(t, err == nil || apierrors.IsNotFound(err), "unexpected error when getting secret %s/%s: %s", + env.SupervisorNamespace, + secretName, + err) + + if err == nil && secret != nil { + err = kubeClient.CoreV1().Secrets(env.SupervisorNamespace).Delete(deleteCtx, secretName, metav1.DeleteOptions{}) + require.NoError(t, err) + } }) return ca @@ -602,7 +660,7 @@ func printServerCert(t *testing.T, address string, dnsOverrides map[string]strin host := addressURL.Host if _, ok := dnsOverrides[host]; ok { - host = dnsOverrides[address] + host = dnsOverrides[host] } conn, err := tls.Dial("tcp", host, conf) diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index e63dfe9e0..582aeea40 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -31,7 +31,6 @@ import ( supervisorconfigv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" - "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/federationdomain/oidc" "go.pinniped.dev/internal/federationdomain/oidcclientvalidator" "go.pinniped.dev/internal/federationdomain/storage" @@ -2940,15 +2939,24 @@ func testSupervisorLogin( supervisorIssuer := env.InferSupervisorIssuerURL(t) + tlsServingCertForSupervisorSecretName := "federation-tls-cert-" + testlib.RandHex(t, 8) + kubeClient := testlib.NewKubernetesClientset(t) + federationDomainSelfSignedCA := createTLSServingCertSecretForSupervisor( + ctx, + t, + env, + supervisorIssuer, + tlsServingCertForSupervisorSecretName, + kubeClient, + ) + // Generate a CA bundle with which to serve this provider. t.Logf("generating test CA") - ca, err := certauthority.New("Downstream Test CA", 1*time.Hour) - require.NoError(t, err) // Create an HTTP client that can reach the downstream discovery endpoint using the CA certs. httpClient := &http.Client{ Transport: &http.Transport{ - TLSClientConfig: &tls.Config{RootCAs: ca.Pool()}, //nolint:gosec // not concerned with TLS MinVersion here + TLSClientConfig: &tls.Config{RootCAs: federationDomainSelfSignedCA.Pool()}, //nolint:gosec // not concerned with TLS MinVersion here Proxy: func(req *http.Request) (*url.URL, error) { if strings.HasPrefix(req.URL.Host, "127.0.0.1") { // don't proxy requests to localhost to avoid proxying calls to our local callback listener @@ -2971,20 +2979,6 @@ func testSupervisorLogin( } oidcHTTPClientContext := coreosoidc.ClientContext(ctx, httpClient) - // Use the CA to issue a TLS server cert. - certPEM, keyPEM := supervisorIssuer.IssuerServerCert(t, ca) - - // Write the serving cert to a secret. - certSecret := testlib.CreateTestSecret(t, - env.SupervisorNamespace, - "oidc-provider-tls", - corev1.SecretTypeTLS, - map[string]string{ - "tls.crt": string(certPEM), - "tls.key": string(keyPEM), - }, - ) - // Create upstream IDP and wait for it to become ready. idpName := createIDP(t) @@ -2996,10 +2990,11 @@ func testSupervisorLogin( } // Create the downstream FederationDomain and expect it to go into the appropriate status condition. + // This helper function will nil out spec.TLS if spec.Issuer is an IP address. federationDomain := testlib.CreateTestFederationDomain(ctx, t, supervisorconfigv1alpha1.FederationDomainSpec{ Issuer: supervisorIssuer.Issuer(), - TLS: &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name}, + TLS: &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: tlsServingCertForSupervisorSecretName}, IdentityProviders: fdIDPSpec, }, // The IDP CR already exists, so even for legacy FederationDomains which do not explicitly list diff --git a/test/integration/supervisor_warnings_test.go b/test/integration/supervisor_warnings_test.go index feec20713..d06ef31e4 100644 --- a/test/integration/supervisor_warnings_test.go +++ b/test/integration/supervisor_warnings_test.go @@ -21,13 +21,11 @@ import ( "github.com/creack/pty" "github.com/stretchr/testify/require" authorizationv1 "k8s.io/api/authorization/v1" - corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" authenticationv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1" supervisorconfigv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1" idpv1alpha1 "go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1" - "go.pinniped.dev/internal/certauthority" "go.pinniped.dev/internal/federationdomain/oidc" "go.pinniped.dev/internal/federationdomain/oidcclientvalidator" "go.pinniped.dev/internal/federationdomain/storage" @@ -49,37 +47,33 @@ func TestSupervisorWarnings_Browser(t *testing.T) { tempDir := t.TempDir() supervisorIssuer := env.InferSupervisorIssuerURL(t) + kubeClient := testlib.NewKubernetesClientset(t) + var err error // Generate a CA bundle with which to serve this provider. t.Logf("generating test CA") - ca, err := certauthority.New("Downstream Test CA", 1*time.Hour) - require.NoError(t, err) + downstreamTLSServingCertName := "oidc-provider-tls-" + testlib.RandHex(t, 8) + federationDomainSelfSignedCA := createTLSServingCertSecretForSupervisor( + ctx, + t, + env, + supervisorIssuer, + downstreamTLSServingCertName, + kubeClient, + ) // Save that bundle plus the one that signs the upstream issuer, for test purposes. testCABundlePath := filepath.Join(tempDir, "test-ca.pem") - testCABundlePEM := []byte(string(ca.Bundle()) + "\n" + env.SupervisorUpstreamOIDC.CABundle) + testCABundlePEM := []byte(string(federationDomainSelfSignedCA.Bundle()) + "\n" + env.SupervisorUpstreamOIDC.CABundle) testCABundleBase64 := base64.StdEncoding.EncodeToString(testCABundlePEM) require.NoError(t, os.WriteFile(testCABundlePath, testCABundlePEM, 0600)) - // Use the CA to issue a TLS server cert. - certPEM, keyPEM := supervisorIssuer.IssuerServerCert(t, ca) - - // Write the serving cert to a secret. - certSecret := testlib.CreateTestSecret(t, - env.SupervisorNamespace, - "oidc-provider-tls", - corev1.SecretTypeTLS, - map[string]string{ - "tls.crt": string(certPEM), - "tls.key": string(keyPEM), - }, - ) - // Create the downstream FederationDomain and expect it to go into the success status condition. + // This helper function will nil out spec.TLS if spec.Issuer is an IP address. downstream := testlib.CreateTestFederationDomain(ctx, t, supervisorconfigv1alpha1.FederationDomainSpec{ Issuer: supervisorIssuer.Issuer(), - TLS: &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name}, + TLS: &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: downstreamTLSServingCertName}, }, supervisorconfigv1alpha1.FederationDomainPhaseError, // in phase error until there is an IDP created ) diff --git a/test/testlib/client.go b/test/testlib/client.go index 90ac06ce5..731e26d21 100644 --- a/test/testlib/client.go +++ b/test/testlib/client.go @@ -367,6 +367,11 @@ func CreateTestFederationDomain( createContext, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() + // If the issuer is an IP address, then we have to update the DEFAULT cert, and there's no secret associated with this FederationDomain + if NewSupervisorIssuer(t, spec.Issuer).IsIPAddress() { + spec.TLS = nil + } + federationDomainsClient := NewSupervisorClientset(t).ConfigV1alpha1().FederationDomains(testEnv.SupervisorNamespace) federationDomain, err := federationDomainsClient.Create(createContext, &supervisorconfigv1alpha1.FederationDomain{ ObjectMeta: TestObjectMeta(t, "oidc-provider"),