add SAN to default cert in supervisor_discovery_test.go

test/integration/supervisor_discovery_test.go

@@ -53,12 +53,18 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute)
 	defer cancel()
 
-	temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, env.SupervisorNamespace, env.DefaultTLSCertSecretName(), client, testlib.NewKubernetesClientset(t))
-	defaultCA := createTLSServingCertSecretForSupervisor(
-		ctx,
+	temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx,
+		t, env.SupervisorNamespace, env.DefaultTLSCertSecretName(), client, kubeClient)
+
+	supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress)
+	if env.SupervisorHTTPSIngressAddress != "" {
+		// Since this cert could be used for either server name in the tests below, make it valid for both names.
+		supervisorIssuer.AddAlternativeName(env.SupervisorHTTPSIngressAddress)
+	}
+	defaultCA := createTLSServingCertSecretForSupervisor(ctx,
 		t,
 		env,
-		testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress),
+		supervisorIssuer,
 		env.DefaultTLSCertSecretName(),
 		kubeClient,
 	)
@@ -367,7 +373,7 @@ func createTLSServingCertSecretForSupervisor(
 	ctx context.Context,
 	t *testing.T,
 	env *testlib.TestEnv,
-	supervisorIssuer testlib.SupervisorIssuer,
+	supervisorIssuer *testlib.SupervisorIssuer,
 	secretName string,
 	kubeClient kubernetes.Interface,
 ) *certauthority.CA {

test/testlib/env.go

@@ -85,7 +85,7 @@ type TestOIDCUpstream struct {
 }
 
 // InferSupervisorIssuerURL infers the downstream issuer URL from the callback associated with the upstream test client registration.
-func (e *TestEnv) InferSupervisorIssuerURL(t *testing.T) SupervisorIssuer {
+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"))

test/testlib/supervisor_issuer.go

@@ -15,11 +15,12 @@ import (
 )
 
 type SupervisorIssuer struct {
-	issuerURL *url.URL
-	ip        net.IP
+	issuerURL        *url.URL
+	ip               net.IP
+	alternativeNames []string
 }
 
-func NewSupervisorIssuer(t *testing.T, issuer string) SupervisorIssuer {
+func NewSupervisorIssuer(t *testing.T, issuer string) *SupervisorIssuer {
 	t.Helper()
 
 	t.Logf("NewSupervisorIssuer: %s", issuer)
@@ -30,25 +31,30 @@ func NewSupervisorIssuer(t *testing.T, issuer string) SupervisorIssuer {
 
 	ip := net.ParseIP(issuerURL.Hostname())
 
-	return SupervisorIssuer{
+	return &SupervisorIssuer{
 		issuerURL: issuerURL,
 		ip:        ip,
 	}
 }
 
-func (s SupervisorIssuer) Issuer() string {
+// AddAlternativeName adds a SAN for the cert. It is not intended to take an IP address as its argument.
+func (s *SupervisorIssuer) AddAlternativeName(san string) {
+	s.alternativeNames = append(s.alternativeNames, san)
+}
+
+func (s *SupervisorIssuer) Issuer() string {
 	return s.issuerURL.String()
 }
 
-func (s SupervisorIssuer) Address() string {
+func (s *SupervisorIssuer) Address() string {
 	return s.issuerURL.Host
 }
 
-func (s SupervisorIssuer) Hostname() string {
+func (s *SupervisorIssuer) Hostname() string {
 	return s.issuerURL.Hostname()
 }
 
-func (s SupervisorIssuer) Port(defaultPort string) string {
+func (s *SupervisorIssuer) Port(defaultPort string) string {
 	port := s.issuerURL.Port()
 	if port == "" {
 		return defaultPort
@@ -56,36 +62,40 @@ func (s SupervisorIssuer) Port(defaultPort string) string {
 	return s.issuerURL.Port()
 }
 
-func (s SupervisorIssuer) Hostnames() []string {
-	if s.IsIPAddress() {
-		return nil // don't want DNS records in the cert when using IP address
+func (s *SupervisorIssuer) hostnamesForCert() []string {
+	var hostnames []string
+	if !s.IsIPAddress() {
+		hostnames = append(hostnames, s.issuerURL.Hostname())
 	}
-	return []string{s.issuerURL.Hostname()}
+	if s.alternativeNames != nil {
+		hostnames = append(hostnames, s.alternativeNames...)
+	}
+	return hostnames
 }
 
-func (s SupervisorIssuer) IPs() []net.IP {
+func (s *SupervisorIssuer) ipsForCert() []net.IP {
 	if !s.IsIPAddress() {
 		return nil
 	}
 	return []net.IP{s.ip}
 }
 
-func (s SupervisorIssuer) IssuerServerCert(
+func (s *SupervisorIssuer) IssuerServerCert(
 	t *testing.T,
 	ca *certauthority.CA,
 ) ([]byte, []byte) {
 	t.Helper()
 
-	cert, err := ca.IssueServerCert(s.Hostnames(), s.IPs(), 24*time.Hour)
+	cert, err := ca.IssueServerCert(s.hostnamesForCert(), s.ipsForCert(), 24*time.Hour)
 	require.NoError(t, err)
 	certPEM, keyPEM, err := certauthority.ToPEM(cert)
 	require.NoError(t, err)
 	t.Logf("issued server cert for Supervisor: hostname=%+v, ips=%+v\n%s",
-		s.Hostnames(), s.IPs(),
+		s.hostnamesForCert(), s.ipsForCert(),
 		certPEM)
 	return certPEM, keyPEM
 }
 
-func (s SupervisorIssuer) IsIPAddress() bool {
+func (s *SupervisorIssuer) IsIPAddress() bool {
 	return s.ip != nil
 }

test/testlib/supervisor_issuer_test.go

@@ -12,41 +12,43 @@ import (
 
 func TestSupervisorIssuer(t *testing.T) {
 	tests := []struct {
-		name            string
-		issuer          string
-		wantHostname    string
+		name             string
+		issuer           string
+		alternativeNames []string
+
+		wantHostnames   []string
 		wantAddress     string
 		wantIP          net.IP
 		wantIsIPAddress bool
 	}{
 		{
-			name:         "works for localhost",
-			issuer:       "https://localhost:443",
-			wantHostname: "localhost",
-			wantAddress:  "localhost:443",
+			name:          "works for localhost",
+			issuer:        "https://localhost:443",
+			wantHostnames: []string{"localhost"},
+			wantAddress:   "localhost:443",
 		},
 		{
-			name:         "works for localhost with path",
-			issuer:       "https://localhost:443/some/path",
-			wantHostname: "localhost",
-			wantAddress:  "localhost:443",
+			name:          "works for localhost with path",
+			issuer:        "https://localhost:443/some/path",
+			wantHostnames: []string{"localhost"},
+			wantAddress:   "localhost:443",
 		},
 		{
-			name:         "works for domain",
-			issuer:       "https://example.com:443",
-			wantHostname: "example.com",
-			wantAddress:  "example.com:443",
+			name:          "works for domain",
+			issuer:        "https://example.com:443",
+			wantHostnames: []string{"example.com"},
+			wantAddress:   "example.com:443",
 		},
 		{
-			name:         "works for domain with path",
-			issuer:       "https://example.com:443/some/path",
-			wantHostname: "example.com",
-			wantAddress:  "example.com:443",
+			name:          "works for domain with path",
+			issuer:        "https://example.com:443/some/path",
+			wantHostnames: []string{"example.com"},
+			wantAddress:   "example.com:443",
 		},
 		{
 			name:            "works for IPv4",
 			issuer:          "https://1.2.3.4:443",
-			wantHostname:    "", // don't want DNS records in the cert when using IP address
+			wantHostnames:   nil, // don't want DNS records in the cert when using IP address without SANs
 			wantAddress:     "1.2.3.4:443",
 			wantIP:          net.ParseIP("1.2.3.4"),
 			wantIsIPAddress: true,
@@ -54,27 +56,53 @@ func TestSupervisorIssuer(t *testing.T) {
 		{
 			name:            "works for IPv4 with path",
 			issuer:          "https://1.2.3.4:443/some/path",
-			wantHostname:    "", // don't want DNS records in the cert when using IP address
+			wantHostnames:   nil, // don't want DNS records in the cert when using IP address without SANs
 			wantAddress:     "1.2.3.4:443",
 			wantIP:          net.ParseIP("1.2.3.4"),
 			wantIsIPAddress: true,
 		},
+		{
+			name:             "works with one SAN",
+			issuer:           "https://example.com:443",
+			alternativeNames: []string{"alt.example.com"},
+			wantHostnames:    []string{"example.com", "alt.example.com"},
+			wantAddress:      "example.com:443",
+		},
+		{
+			name:             "works with two SANs",
+			issuer:           "https://example.com:443",
+			alternativeNames: []string{"alt1.example.com", "alt2.example.com"},
+			wantHostnames:    []string{"example.com", "alt1.example.com", "alt2.example.com"},
+			wantAddress:      "example.com:443",
+		},
+		{
+			name:             "IP works with SANs",
+			issuer:           "https://1.2.3.4:443",
+			alternativeNames: []string{"alt1.example.com", "alt2.example.com"},
+			wantHostnames:    []string{"alt1.example.com", "alt2.example.com"},
+			wantAddress:      "1.2.3.4:443",
+			wantIP:           net.ParseIP("1.2.3.4"),
+			wantIsIPAddress:  true,
+		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			supervisorIssuer := NewSupervisorIssuer(t, test.issuer)
+			for _, n := range test.alternativeNames {
+				supervisorIssuer.AddAlternativeName(n)
+			}
 			require.Equal(t, test.issuer, supervisorIssuer.Issuer())
 			require.Equal(t, test.wantAddress, supervisorIssuer.Address())
-			if test.wantHostname != "" {
-				require.Equal(t, []string{test.wantHostname}, supervisorIssuer.Hostnames())
+			if test.wantHostnames != nil {
+				require.Equal(t, test.wantHostnames, supervisorIssuer.hostnamesForCert())
 			} else {
-				require.Nil(t, supervisorIssuer.Hostnames())
+				require.Nil(t, supervisorIssuer.hostnamesForCert())
 			}
 			if test.wantIP != nil {
-				require.Equal(t, []net.IP{test.wantIP}, supervisorIssuer.IPs())
+				require.Equal(t, []net.IP{test.wantIP}, supervisorIssuer.ipsForCert())
 			} else {
-				require.Nil(t, supervisorIssuer.IPs())
+				require.Nil(t, supervisorIssuer.ipsForCert())
 			}
 			require.Equal(t, test.wantIsIPAddress, supervisorIssuer.IsIPAddress())
 		})