Integration tests should expect that the Supervisor hostname might be an IP address

test/integration/concierge_tls_spec_test.go

@@ -506,7 +506,7 @@ func TestTLSSpecValidationConcierge_Parallel(t *testing.T) {
 			})
 
 			t.Run("apply jwt authenticator", func(t *testing.T) {
-				_, supervisorIssuer := env.InferSupervisorIssuerURL(t)
+				supervisorIssuer := env.InferSupervisorIssuerURL(t)
 
 				resourceName := "test-jwt-authenticator-" + testlib.RandHex(t, 7)
 
@@ -519,7 +519,7 @@ func TestTLSSpecValidationConcierge_Parallel(t *testing.T) {
 				)
 
 				yamlBytes := []byte(fmt.Sprintf(jwtAuthenticatorYamlTemplate,
-					env.APIGroupSuffix, resourceName, supervisorIssuer,
+					env.APIGroupSuffix, resourceName, supervisorIssuer.Issuer(),
 					indentForHeredoc(tc.tlsYAML(secretOrConfigmapResourceName))))
 
 				stdOut, stdErr, err := performKubectlApply(t, resourceName, yamlBytes)

test/integration/e2e_test.go

@@ -11,7 +11,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"net"
 	"net/url"
 	"os"
 	"os/exec"
@@ -71,12 +70,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	// Build pinniped CLI.
 	pinnipedExe := testlib.PinnipedCLIPath(t)
 
-	issuerURL, _ := env.InferSupervisorIssuerURL(t)
-	isIssuerAnIPAddress := net.ParseIP(issuerURL.Hostname()) != nil
-	var issuerIPs []net.IP
-	if isIssuerAnIPAddress {
-		issuerIPs = append(issuerIPs, net.ParseIP(issuerURL.Hostname()))
-	}
+	supervisorIssuer := env.InferSupervisorIssuerURL(t)
 
 	// Generate a CA bundle with which to serve this provider.
 	t.Logf("generating test CA")
@@ -89,12 +83,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	require.NoError(t, os.WriteFile(federationDomainCABundlePath, federationDomainCABundlePEM, 0600))
 
 	// Use the CA to issue a TLS server cert.
-	t.Logf("issuing test certificate")
-	federationDomainTLSServingCert, err := federationDomainSelfSignedCA.IssueServerCert(
-		[]string{issuerURL.Hostname()}, issuerIPs, 1*time.Hour)
-	require.NoError(t, err)
-	federationDomainTLSServingCertPEM, federationDomainTLSServingCertKeyPEM, err := certauthority.ToPEM(federationDomainTLSServingCert)
-	require.NoError(t, err)
+	certPEM, keyPEM := supervisorIssuer.IssuerServerCert(t, federationDomainSelfSignedCA)
 
 	supervisorClient := testlib.NewSupervisorClientset(t)
 	temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(
@@ -107,15 +96,15 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	)
 
 	var tlsSpecForFederationDomain *supervisorconfigv1alpha1.FederationDomainTLSSpec
-	if isIssuerAnIPAddress {
+	if supervisorIssuer.IsIPAddress() {
 		testlib.CreateTestSecretWithName(
 			t,
 			env.SupervisorNamespace,
 			env.DefaultTLSCertSecretName(),
 			corev1.SecretTypeTLS,
 			map[string]string{
-				"tls.crt": string(federationDomainTLSServingCertPEM),
-				"tls.key": string(federationDomainTLSServingCertKeyPEM),
+				"tls.crt": string(certPEM),
+				"tls.key": string(keyPEM),
 			},
 		)
 	} else {
@@ -125,8 +114,8 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 			"oidc-provider-tls",
 			corev1.SecretTypeTLS,
 			map[string]string{
-				"tls.crt": string(federationDomainTLSServingCertPEM),
-				"tls.key": string(federationDomainTLSServingCertKeyPEM),
+				"tls.crt": string(certPEM),
+				"tls.key": string(keyPEM),
 			},
 		)
 		tlsSpecForFederationDomain = &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: federationDomainTLSServingCertSecret.Name}
@@ -135,7 +124,7 @@ func TestE2EFullIntegration_Browser(t *testing.T) {
 	// Create the downstream FederationDomain.
 	federationDomain := testlib.CreateTestFederationDomain(topSetupCtx, t,
 		supervisorconfigv1alpha1.FederationDomainSpec{
-			Issuer: issuerURL.String(),
+			Issuer: supervisorIssuer.Issuer(),
 			TLS:    tlsSpecForFederationDomain,
 		},
 		supervisorconfigv1alpha1.FederationDomainPhaseError, // in phase error until there is an IDP created

test/integration/supervisor_discovery_test.go

@@ -53,16 +53,19 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) {
 	defer cancel()
 
 	httpsAddress := env.SupervisorHTTPSAddress
-	var ips []net.IP
 	if host, _, err := net.SplitHostPort(httpsAddress); err == nil {
 		httpsAddress = host
 	}
-	if ip := net.ParseIP(httpsAddress); ip != nil {
-		ips = append(ips, ip)
-	}
 
 	temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, env.DefaultTLSCertSecretName(), client, testlib.NewKubernetesClientset(t))
-	defaultCA := createTLSCertificateSecret(ctx, t, ns, httpsAddress, ips, env.DefaultTLSCertSecretName(), kubeClient)
+	defaultCA := createTLSCertificateSecret(
+		ctx,
+		t,
+		ns,
+		testlib.NewSupervisorIssuer(t, httpsAddress),
+		env.DefaultTLSCertSecretName(),
+		kubeClient,
+	)
 
 	tests := []struct {
 		Name     string
@@ -203,7 +206,7 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) {
 	requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuer1)
 
 	// Create the Secret.
-	ca1 := createTLSCertificateSecret(ctx, t, ns, hostname1, nil, certSecretName1, kubeClient)
+	ca1 := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, hostname1), 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)
@@ -224,7 +227,7 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) {
 	requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuer1)
 
 	// Create a Secret at the updated name.
-	ca1update := createTLSCertificateSecret(ctx, t, ns, hostname1, nil, certSecretName1update, kubeClient)
+	ca1update := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, hostname1), 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 +247,7 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) {
 	requireStatus(t, pinnipedClient, federationDomain2.Namespace, federationDomain2.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady, withAllSuccessfulConditions())
 
 	// Create the Secret.
-	ca2 := createTLSCertificateSecret(ctx, t, ns, hostname2, nil, certSecretName2, kubeClient)
+	ca2 := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, hostname2), 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{
@@ -284,10 +287,12 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) {
 	ips, err := testlib.LookupIP(ctx, hostname)
 	require.NoError(t, err)
 	require.NotEmpty(t, ips)
-	ipWithPort := ips[0].String() + ":" + port
+	ipAsHostname := ips[0].String()
+	ipWithPort := ipAsHostname + ":" + port
 
+	// Use different paths just in case the hostname is the IP address!
 	issuerUsingIPAddress := fmt.Sprintf("%s://%s/issuer1", scheme, ipWithPort)
-	issuerUsingHostname := fmt.Sprintf("%s://%s/issuer1", scheme, address)
+	issuerUsingHostname := fmt.Sprintf("%s://%s/issuer2", scheme, address)
 
 	// Create an FederationDomain without a spec.tls.secretName.
 	federationDomain1 := testlib.CreateTestFederationDomain(ctx, t, supervisorconfigv1alpha1.FederationDomainSpec{Issuer: issuerUsingIPAddress}, supervisorconfigv1alpha1.FederationDomainPhaseReady)
@@ -297,7 +302,7 @@ 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, "cert-hostname-doesnt-matter", []net.IP{ips[0]}, env.DefaultTLSCertSecretName(), kubeClient)
+	defaultCA := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, ipAsHostname), 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)
@@ -312,7 +317,7 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) {
 	requireStatus(t, pinnipedClient, federationDomain2.Namespace, federationDomain2.Name, supervisorconfigv1alpha1.FederationDomainPhaseReady, withAllSuccessfulConditions())
 
 	// Create the Secret.
-	certCA := createTLSCertificateSecret(ctx, t, ns, hostname, nil, certSecretName, kubeClient)
+	certCA := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, hostname), 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
@@ -327,8 +332,7 @@ func createTLSCertificateSecret(
 	ctx context.Context,
 	t *testing.T,
 	namespace string,
-	hostname string,
-	ips []net.IP,
+	supervisorIssuer testlib.SupervisorIssuer,
 	secretName string,
 	kubeClient kubernetes.Interface,
 ) *certauthority.CA {
@@ -337,12 +341,9 @@ func createTLSCertificateSecret(
 	require.NoError(t, err)
 
 	// Using the CA, create a TLS server cert.
-	tlsCert, err := ca.IssueServerCert([]string{hostname}, ips, 1000*time.Hour)
-	require.NoError(t, err)
+	certPEM, keyPEM := supervisorIssuer.IssuerServerCert(t, ca)
 
 	// Write the serving cert to the SNI secret.
-	tlsCertChainPEM, tlsPrivateKeyPEM, err := certauthority.ToPEM(tlsCert)
-	require.NoError(t, err)
 	secret := corev1.Secret{
 		Type:     corev1.SecretTypeTLS,
 		TypeMeta: metav1.TypeMeta{},
@@ -351,8 +352,8 @@ func createTLSCertificateSecret(
 			Namespace: namespace,
 		},
 		StringData: map[string]string{
-			"tls.crt": string(tlsCertChainPEM),
-			"tls.key": string(tlsPrivateKeyPEM),
+			"tls.crt": string(certPEM),
+			"tls.key": string(keyPEM),
 		},
 	}
 	_, err = kubeClient.CoreV1().Secrets(namespace).Create(ctx, &secret, metav1.CreateOptions{})

test/integration/supervisor_login_test.go

@@ -2938,7 +2938,7 @@ func testSupervisorLogin(
 	ctx, cancel := context.WithTimeout(context.Background(), 7*time.Minute)
 	defer cancel()
 
-	issuerURL, _ := env.InferSupervisorIssuerURL(t)
+	supervisorIssuer := env.InferSupervisorIssuerURL(t)
 
 	// Generate a CA bundle with which to serve this provider.
 	t.Logf("generating test CA")
@@ -2972,18 +2972,17 @@ func testSupervisorLogin(
 	oidcHTTPClientContext := coreosoidc.ClientContext(ctx, httpClient)
 
 	// Use the CA to issue a TLS server cert.
-	t.Logf("issuing test certificate")
-	tlsCert, err := ca.IssueServerCert([]string{issuerURL.Hostname()}, nil, 1*time.Hour)
-	require.NoError(t, err)
-	certPEM, keyPEM, err := certauthority.ToPEM(tlsCert)
-	require.NoError(t, err)
+	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)},
+		map[string]string{
+			"tls.crt": string(certPEM),
+			"tls.key": string(keyPEM),
+		},
 	)
 
 	// Create upstream IDP and wait for it to become ready.
@@ -2999,7 +2998,7 @@ func testSupervisorLogin(
 	// Create the downstream FederationDomain and expect it to go into the appropriate status condition.
 	federationDomain := testlib.CreateTestFederationDomain(ctx, t,
 		supervisorconfigv1alpha1.FederationDomainSpec{
-			Issuer:            issuerURL.String(),
+			Issuer:            supervisorIssuer.Issuer(),
 			TLS:               &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name},
 			IdentityProviders: fdIDPSpec,
 		},
@@ -3015,7 +3014,7 @@ func testSupervisorLogin(
 	requestJWKSEndpoint, err := http.NewRequestWithContext(
 		ctx,
 		http.MethodGet,
-		fmt.Sprintf("%s/jwks.json", issuerURL.String()),
+		fmt.Sprintf("%s/jwks.json", supervisorIssuer.Issuer()),
 		nil,
 	)
 	require.NoError(t, err)

test/integration/supervisor_warnings_test.go

@@ -48,7 +48,7 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 	pinnipedExe := testlib.PinnipedCLIPath(t)
 	tempDir := t.TempDir()
 
-	issuerURL, _ := env.InferSupervisorIssuerURL(t)
+	supervisorIssuer := env.InferSupervisorIssuerURL(t)
 
 	// Generate a CA bundle with which to serve this provider.
 	t.Logf("generating test CA")
@@ -62,24 +62,23 @@ func TestSupervisorWarnings_Browser(t *testing.T) {
 	require.NoError(t, os.WriteFile(testCABundlePath, testCABundlePEM, 0600))
 
 	// Use the CA to issue a TLS server cert.
-	t.Logf("issuing test certificate")
-	tlsCert, err := ca.IssueServerCert([]string{issuerURL.Hostname()}, nil, 1*time.Hour)
-	require.NoError(t, err)
-	certPEM, keyPEM, err := certauthority.ToPEM(tlsCert)
-	require.NoError(t, err)
+	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)},
+		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.
 	downstream := testlib.CreateTestFederationDomain(ctx, t,
 		supervisorconfigv1alpha1.FederationDomainSpec{
-			Issuer: issuerURL.String(),
+			Issuer: supervisorIssuer.Issuer(),
 			TLS:    &supervisorconfigv1alpha1.FederationDomainTLSSpec{SecretName: certSecret.Name},
 		},
 		supervisorconfigv1alpha1.FederationDomainPhaseError, // in phase error until there is an IDP created

test/testlib/browsertest/browsertest.go

@@ -365,8 +365,8 @@ func LoginToUpstreamOIDC(t *testing.T, b *Browser, upstream testlib.TestOIDCUpst
 		},
 		{
 			Name:                "Dex",
-			IssuerPattern:       regexp.MustCompile(`\Ahttps://dex\.tools\.svc\.cluster\.local/dex.*\z`),
-			LoginPagePattern:    regexp.MustCompile(`\Ahttps://dex\.tools\.svc\.cluster\.local/dex/auth/local.+\z`),
+			IssuerPattern:       regexp.MustCompile(`\Ahttps://.*/dex.*\z`),
+			LoginPagePattern:    regexp.MustCompile(`\Ahttps://.*/dex/auth/local.+\z`),
 			UsernameSelector:    "input#login",
 			PasswordSelector:    "input#password",
 			LoginButtonSelector: "button#submit-login",
@@ -378,7 +378,7 @@ func LoginToUpstreamOIDC(t *testing.T, b *Browser, upstream testlib.TestOIDCUpst
 		}
 	}
 	if cfg == nil {
-		require.Failf(t, "could not find login provider for issuer %q", upstream.Issuer)
+		require.Failf(t, "failure message goes here", "could not find login provider for issuer %q", upstream.Issuer)
 		return
 	}
 

test/testlib/env.go

@@ -5,17 +5,20 @@ package testlib
 
 import (
 	"encoding/base64"
+	"net"
 	"net/url"
 	"os"
 	"sort"
 	"strings"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 	"sigs.k8s.io/yaml"
 
 	authenticationv1alpha1 "go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1"
+	"go.pinniped.dev/internal/certauthority"
 )
 
 type Capability string
@@ -84,18 +87,73 @@ type TestOIDCUpstream struct {
 	ExpectedGroups   []string `json:"expectedGroups"`
 }
 
-// InferSupervisorIssuerURL infers the downstream issuer URL from the callback associated with the upstream test client registration.
-func (e *TestEnv) InferSupervisorIssuerURL(t *testing.T) (*url.URL, string) {
+type SupervisorIssuer struct {
+	issuerURL *url.URL
+}
+
+func NewSupervisorIssuer(t *testing.T, issuer string) SupervisorIssuer {
 	t.Helper()
-	issuerURL, err := url.Parse(e.SupervisorUpstreamOIDC.CallbackURL)
+
+	t.Logf("NewSupervisorIssuer: %s", issuer)
+
+	issuerURL, err := url.Parse(issuer)
 	require.NoError(t, err)
-	require.True(t, strings.HasSuffix(issuerURL.Path, "/callback"))
-	issuerURL.Path = strings.TrimSuffix(issuerURL.Path, "/callback")
 
-	issuerAsString := issuerURL.String()
-	t.Logf("testing with downstream issuer URL %s", issuerAsString)
+	return SupervisorIssuer{
+		issuerURL: issuerURL,
+	}
+}
 
-	return issuerURL, issuerAsString
+func (s SupervisorIssuer) Issuer() string {
+	return s.issuerURL.String()
+}
+
+func (s SupervisorIssuer) Hostnames() []string {
+	// TODO: Why does this happen?
+	if s.issuerURL.Hostname() == "" {
+		return []string{"localhost"}
+	}
+	return []string{s.issuerURL.Hostname()}
+}
+
+func (s SupervisorIssuer) IPs() []net.IP {
+	var ips []net.IP
+	if ip := net.ParseIP(s.issuerURL.Hostname()); ip != nil {
+		ips = append(ips, ip)
+	}
+	return ips
+}
+
+func (s SupervisorIssuer) IssuerServerCert(
+	t *testing.T,
+	ca *certauthority.CA,
+) ([]byte, []byte) {
+	t.Helper()
+
+	t.Logf("issuing server cert for Supervisor: hostname=%+v, ips=%+v",
+		s.Hostnames(), s.IPs())
+
+	cert, err := ca.IssueServerCert(s.Hostnames(), s.IPs(), 24*time.Hour)
+	require.NoError(t, err)
+	certPEM, keyPEM, err := certauthority.ToPEM(cert)
+	require.NoError(t, err)
+	return certPEM, keyPEM
+}
+
+func (s SupervisorIssuer) IsIPAddress() bool {
+	return len(s.IPs()) > 0
+}
+
+// InferSupervisorIssuerURL infers the downstream issuer URL from the callback associated with the upstream test client registration.
+func (e *TestEnv) InferSupervisorIssuerURL(t *testing.T) SupervisorIssuer {
+	t.Helper()
+	supervisorIssuer := NewSupervisorIssuer(t, e.SupervisorUpstreamOIDC.CallbackURL)
+	require.True(t, strings.HasSuffix(supervisorIssuer.issuerURL.Path, "/callback"))
+	supervisorIssuer.issuerURL.Path = strings.TrimSuffix(supervisorIssuer.issuerURL.Path, "/callback")
+
+	t.Logf("InferSupervisorIssuerURL(): testing with downstream issuer URL %s", supervisorIssuer.Issuer())
+
+	return supervisorIssuer
 }
 
 func (e *TestEnv) DefaultTLSCertSecretName() string {