diff --git a/hack/prepare-for-integration-tests.sh b/hack/prepare-for-integration-tests.sh index 711f0dc0c..338f212b1 100755 --- a/hack/prepare-for-integration-tests.sh +++ b/hack/prepare-for-integration-tests.sh @@ -414,7 +414,7 @@ export PINNIPED_TEST_WEBHOOK_CA_BUNDLE=${webhook_ca_bundle} export PINNIPED_TEST_SUPERVISOR_NAMESPACE=${supervisor_namespace} export PINNIPED_TEST_SUPERVISOR_APP_NAME=${supervisor_app_name} export PINNIPED_TEST_SUPERVISOR_CUSTOM_LABELS='${supervisor_custom_labels}' -export PINNIPED_TEST_SUPERVISOR_HTTPS_ADDRESS="localhost:12344" +export PINNIPED_TEST_SUPERVISOR_HTTPS_ADDRESS="https://localhost:12344" export PINNIPED_TEST_PROXY=http://127.0.0.1:12346 export PINNIPED_TEST_LDAP_HOST=ldap.tools.svc.cluster.local export PINNIPED_TEST_LDAP_STARTTLS_ONLY_HOST=ldapstarttls.tools.svc.cluster.local diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index fd314cc19..1d2c7d4be 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -52,17 +52,12 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() - httpsAddress := env.SupervisorHTTPSAddress - if host, _, err := net.SplitHostPort(httpsAddress); err == nil { - httpsAddress = host - } - temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, env.DefaultTLSCertSecretName(), client, testlib.NewKubernetesClientset(t)) defaultCA := createTLSCertificateSecret( ctx, t, ns, - testlib.NewSupervisorIssuer(t, httpsAddress), + testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress), env.DefaultTLSCertSecretName(), kubeClient, ) @@ -88,6 +83,8 @@ func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { t.Skip("no address defined") } + addr, _ = strings.CutPrefix(addr, "https://") + // Create any IDP so that any FederationDomain created later by this test will see that exactly one IDP exists. testlib.CreateTestOIDCIdentityProvider(t, idpv1alpha1.OIDCIdentityProviderSpec{ Issuer: "https://example.cluster.local/fake-issuer-url-does-not-matter", @@ -188,9 +185,9 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, env.DefaultTLSCertSecretName(), pinnipedClient, kubeClient) scheme := "https" - address := env.SupervisorHTTPSAddress // hostname and port for direct access to the supervisor's port 8443 + supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress) + address := supervisorIssuer.Address() // hostname and port WITHOUT SCHEME for direct access to the supervisor's port 8443 - hostname1 := strings.Split(address, ":")[0] issuer1 := fmt.Sprintf("%s://%s/issuer1", scheme, address) certSecretName1 := "integration-test-cert-1" @@ -206,7 +203,7 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuer1) // Create the Secret. - ca1 := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, hostname1), certSecretName1, kubeClient) + ca1 := createTLSCertificateSecret(ctx, t, ns, 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) @@ -227,7 +224,7 @@ func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { requireEndpointHasBootstrapTLSErrorBecauseCertificatesAreNotReady(t, issuer1) // Create a Secret at the updated name. - ca1update := createTLSCertificateSecret(ctx, t, ns, testlib.NewSupervisorIssuer(t, hostname1), certSecretName1update, kubeClient) + ca1update := createTLSCertificateSecret(ctx, t, ns, 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) @@ -247,7 +244,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, testlib.NewSupervisorIssuer(t, hostname2), certSecretName2, kubeClient) + ca2 := createTLSCertificateSecret(ctx, t, ns, 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{ @@ -274,15 +271,12 @@ func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { temporarilyRemoveAllFederationDomainsAndDefaultTLSCertSecret(ctx, t, ns, env.DefaultTLSCertSecretName(), pinnipedClient, kubeClient) scheme := "https" - address := env.SupervisorHTTPSAddress // hostname and port for direct access to the supervisor's port 8443 + supervisorIssuer := testlib.NewSupervisorIssuer(t, env.SupervisorHTTPSAddress) + address := supervisorIssuer.Address() // hostname and port WITHOUT SCHEME for direct access to the supervisor's port 8443 - hostAndPortSegments := strings.Split(address, ":") // hostnames are case-insensitive, so test mis-matching the case of the issuer URL and the request URL - hostname := strings.ToLower(hostAndPortSegments[0]) - port := "8443" - if len(hostAndPortSegments) > 1 { - port = hostAndPortSegments[1] - } + hostname := strings.ToLower(supervisorIssuer.Hostname()) + port := supervisorIssuer.Port("8443") ips, err := testlib.LookupIP(ctx, hostname) require.NoError(t, err) @@ -302,7 +296,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, testlib.NewSupervisorIssuer(t, ipAsHostname), env.DefaultTLSCertSecretName(), kubeClient) + defaultCA := createTLSCertificateSecret(ctx, t, ns, 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) @@ -317,7 +311,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, testlib.NewSupervisorIssuer(t, hostname), certSecretName, kubeClient) + certCA := createTLSCertificateSecret(ctx, t, ns, 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 diff --git a/test/integration/supervisor_healthz_test.go b/test/integration/supervisor_healthz_test.go index a84ba8534..4e746acc9 100644 --- a/test/integration/supervisor_healthz_test.go +++ b/test/integration/supervisor_healthz_test.go @@ -37,10 +37,10 @@ func TestSupervisorHealthzBootstrap_Disruptive(t *testing.T) { const badTLSConfigBody = "pinniped supervisor has invalid TLS serving certificate configuration\n" - httpGet(ctx, t, httpClient, fmt.Sprintf("https://%s/healthz", env.SupervisorHTTPSAddress), http.StatusOK, "ok") - httpGet(ctx, t, httpClient, fmt.Sprintf("https://%s", env.SupervisorHTTPSAddress), http.StatusInternalServerError, badTLSConfigBody) - httpGet(ctx, t, httpClient, fmt.Sprintf("https://%s/nothealthz", env.SupervisorHTTPSAddress), http.StatusInternalServerError, badTLSConfigBody) - httpGet(ctx, t, httpClient, fmt.Sprintf("https://%s/healthz/something", env.SupervisorHTTPSAddress), http.StatusInternalServerError, badTLSConfigBody) + httpGet(ctx, t, httpClient, fmt.Sprintf("%s/healthz", env.SupervisorHTTPSAddress), http.StatusOK, "ok") + httpGet(ctx, t, httpClient, env.SupervisorHTTPSAddress, http.StatusInternalServerError, badTLSConfigBody) + httpGet(ctx, t, httpClient, fmt.Sprintf("%s/nothealthz", env.SupervisorHTTPSAddress), http.StatusInternalServerError, badTLSConfigBody) + httpGet(ctx, t, httpClient, fmt.Sprintf("%s/healthz/something", env.SupervisorHTTPSAddress), http.StatusInternalServerError, badTLSConfigBody) } func httpGet(ctx context.Context, t *testing.T, client *http.Client, url string, expectedStatus int, expectedBody string) { diff --git a/test/testlib/env.go b/test/testlib/env.go index 8d78e3310..68d1c8e5c 100644 --- a/test/testlib/env.go +++ b/test/testlib/env.go @@ -5,20 +5,16 @@ 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 @@ -87,63 +83,6 @@ type TestOIDCUpstream struct { ExpectedGroups []string `json:"expectedGroups"` } -type SupervisorIssuer struct { - issuerURL *url.URL -} - -func NewSupervisorIssuer(t *testing.T, issuer string) SupervisorIssuer { - t.Helper() - - t.Logf("NewSupervisorIssuer: %s", issuer) - - issuerURL, err := url.Parse(issuer) - require.NoError(t, err) - - return SupervisorIssuer{ - issuerURL: issuerURL, - } -} - -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() diff --git a/test/testlib/supervisor_issuer.go b/test/testlib/supervisor_issuer.go new file mode 100644 index 000000000..ac4f37e9f --- /dev/null +++ b/test/testlib/supervisor_issuer.go @@ -0,0 +1,91 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package testlib + +import ( + "net" + "net/url" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "go.pinniped.dev/internal/certauthority" +) + +type SupervisorIssuer struct { + issuerURL *url.URL + ip net.IP +} + +func NewSupervisorIssuer(t *testing.T, issuer string) SupervisorIssuer { + t.Helper() + + t.Logf("NewSupervisorIssuer: %s", issuer) + + issuerURL, err := url.Parse(issuer) + require.NoError(t, err) + require.NotEmpty(t, issuerURL.Hostname(), "hostname cannot be empty, usually this happens when the scheme is empty. issuer=%q", issuer) + + ip := net.ParseIP(issuerURL.Hostname()) + + return SupervisorIssuer{ + issuerURL: issuerURL, + ip: ip, + } +} + +func (s SupervisorIssuer) Issuer() string { + return s.issuerURL.String() +} + +func (s SupervisorIssuer) Address() string { + return s.issuerURL.Host +} + +func (s SupervisorIssuer) Hostname() string { + return s.issuerURL.Hostname() +} + +func (s SupervisorIssuer) Port(defaultPort string) string { + port := s.issuerURL.Port() + if port == "" { + return defaultPort + } + 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 + } + return []string{s.issuerURL.Hostname()} +} + +func (s SupervisorIssuer) IPs() []net.IP { + if !s.IsIPAddress() { + return nil + } + return []net.IP{s.ip} +} + +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 s.ip != nil +} diff --git a/test/testlib/supervisor_issuer_test.go b/test/testlib/supervisor_issuer_test.go new file mode 100644 index 000000000..2f44b695e --- /dev/null +++ b/test/testlib/supervisor_issuer_test.go @@ -0,0 +1,82 @@ +// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package testlib + +import ( + "net" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSupervisorIssuer(t *testing.T) { + tests := []struct { + name string + issuer string + wantHostname 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 with path", + issuer: "https://localhost:443/some/path", + wantHostname: "localhost", + wantAddress: "localhost:443", + }, + { + name: "works for domain", + issuer: "https://example.com:443", + wantHostname: "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 IPv4", + issuer: "https://1.2.3.4:443", + wantHostname: "", // don't want DNS records in the cert when using IP address + wantAddress: "1.2.3.4:443", + wantIP: net.ParseIP("1.2.3.4"), + wantIsIPAddress: true, + }, + { + 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 + 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) + 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()) + } else { + require.Nil(t, supervisorIssuer.Hostnames()) + } + if test.wantIP != nil { + require.Equal(t, []net.IP{test.wantIP}, supervisorIssuer.IPs()) + } else { + require.Nil(t, supervisorIssuer.IPs()) + } + require.Equal(t, test.wantIsIPAddress, supervisorIssuer.IsIPAddress()) + }) + } +}