From fedb9812bdae7ef08dcb011874b93ab63ab85291 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Mon, 4 Nov 2024 17:31:24 -0800 Subject: [PATCH] add SAN to default cert in supervisor_discovery_test.go --- test/integration/supervisor_discovery_test.go | 16 ++-- test/testlib/env.go | 2 +- test/testlib/supervisor_issuer.go | 44 ++++++---- test/testlib/supervisor_issuer_test.go | 80 +++++++++++++------ 4 files changed, 93 insertions(+), 49 deletions(-) diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 03427f36c..dcd92ef49 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/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 { diff --git a/test/testlib/env.go b/test/testlib/env.go index 9e3c1a096..194097db5 100644 --- a/test/testlib/env.go +++ b/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")) diff --git a/test/testlib/supervisor_issuer.go b/test/testlib/supervisor_issuer.go index 5546ac6c7..ee1479978 100644 --- a/test/testlib/supervisor_issuer.go +++ b/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 } diff --git a/test/testlib/supervisor_issuer_test.go b/test/testlib/supervisor_issuer_test.go index 2f44b695e..13286fbf9 100644 --- a/test/testlib/supervisor_issuer_test.go +++ b/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()) })