From 450ce6a4aae63f24cb1e78adda5c91876faf6042 Mon Sep 17 00:00:00 2001 From: Matt Moyer Date: Tue, 25 May 2021 17:44:25 -0500 Subject: [PATCH] Switch impersonatorconfig to new endpointaddr package. Signed-off-by: Matt Moyer --- .../impersonatorconfig/impersonator_config.go | 57 ++++--------------- .../impersonator_config_test.go | 50 ---------------- 2 files changed, 11 insertions(+), 96 deletions(-) diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index 3d3bcd3c..ffabd619 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -12,7 +12,6 @@ import ( "fmt" "net" "reflect" - "strconv" "strings" "time" @@ -41,6 +40,7 @@ import ( "go.pinniped.dev/internal/controller/issuerconfig" "go.pinniped.dev/internal/controllerlib" "go.pinniped.dev/internal/dynamiccert" + "go.pinniped.dev/internal/endpointaddr" "go.pinniped.dev/internal/plog" ) @@ -761,13 +761,13 @@ func (c *impersonatorConfigController) findDesiredTLSCertificateName(config *v1a } func (c *impersonatorConfigController) findTLSCertificateNameFromEndpointConfig(config *v1alpha1.ImpersonationProxySpec) *certNameInfo { - endpointMaybeWithPort := config.ExternalEndpoint - endpointWithoutPort := strings.Split(endpointMaybeWithPort, ":")[0] - parsedAsIP := net.ParseIP(endpointWithoutPort) - if parsedAsIP != nil { - return &certNameInfo{ready: true, selectedIP: parsedAsIP, clientEndpoint: endpointMaybeWithPort} + addr, _ := endpointaddr.Parse(config.ExternalEndpoint, 443) + endpoint := strings.TrimSuffix(addr.Endpoint(), ":443") + + if ip := net.ParseIP(addr.Host); ip != nil { + return &certNameInfo{ready: true, selectedIP: ip, clientEndpoint: endpoint} } - return &certNameInfo{ready: true, selectedHostname: endpointWithoutPort, clientEndpoint: endpointMaybeWithPort} + return &certNameInfo{ready: true, selectedHostname: addr.Host, clientEndpoint: endpoint} } func (c *impersonatorConfigController) findTLSCertificateNameFromLoadBalancer() (*certNameInfo, error) { @@ -1017,46 +1017,11 @@ func validateCredentialIssuerSpec(spec *v1alpha1.ImpersonationProxySpec) error { return fmt.Errorf("externalEndpoint must be set when service.type is None") } - if err := validateExternalEndpoint(spec.ExternalEndpoint); err != nil { - return fmt.Errorf("invalid ExternalEndpoint %q: %w", spec.ExternalEndpoint, err) + if spec.ExternalEndpoint != "" { + if _, err := endpointaddr.Parse(spec.ExternalEndpoint, 443); err != nil { + return fmt.Errorf("invalid ExternalEndpoint %q: %w", spec.ExternalEndpoint, err) + } } return nil } - -func validateExternalEndpoint(endpoint string) error { - // Empty string is valid (no external endpoint, default to service name) - if endpoint == "" { - return nil - } - - // Try parsing it both with and without an implicit port 443 at the end. - host, port, err := net.SplitHostPort(endpoint) - - // If we got an error parsing the raw input, try adding an implicit port 443. - if err != nil { - host, port, err = net.SplitHostPort(net.JoinHostPort(endpoint, "443")) - } - - // If there's still an error, fail now. - if err != nil { - return err - } - - portInt, _ := strconv.Atoi(port) - if len(validation.IsValidPortNum(portInt)) > 0 { - return fmt.Errorf("invalid port %q", port) - } - - // Check if the host part is a valid IP address. - if len(validation.IsValidIP(host)) == 0 { - return nil - } - - // Check if the host part is a valid hostname according to RFC 1123. - if len(validation.IsDNS1123Subdomain(host)) == 0 { - return nil - } - - return fmt.Errorf("host %q is not a valid hostname or IP address", host) -} diff --git a/internal/controller/impersonatorconfig/impersonator_config_test.go b/internal/controller/impersonatorconfig/impersonator_config_test.go index fc8e346b..6f0c0fa1 100644 --- a/internal/controller/impersonatorconfig/impersonator_config_test.go +++ b/internal/controller/impersonatorconfig/impersonator_config_test.go @@ -3535,53 +3535,3 @@ func (q *testQueue) AddRateLimited(key controllerlib.Key) { q.key = key } - -func TestValidateExternalEndpoint(t *testing.T) { - t.Parallel() - - for _, tt := range []struct { - input string - expectErr string - }{ - {input: ""}, - {input: "127.0.0.1"}, - {input: "127.0.0.1:8443"}, - {input: "[127.0.0.1]:8443"}, - {input: "2001:db8::ffff"}, - {input: "[2001:db8::ffff]:8443"}, - {input: "host.example.com"}, - {input: "host-dev.example.com"}, - {input: "host.example.com:8443"}, - {input: "[host.example.com]:8443"}, - { - input: "https://host.example.com", - expectErr: `invalid port "//host.example.com"`, - }, - { - input: "host.example.com/some/path", - expectErr: `host "host.example.com/some/path" is not a valid hostname or IP address`, - }, - { - input: "[host.example.com", - expectErr: "address [host.example.com:443: missing ']' in address", - }, - { - input: "___.example.com:1234", - expectErr: `host "___.example.com" is not a valid hostname or IP address`, - }, - { - input: "HOST.EXAMPLE.COM", - expectErr: `host "HOST.EXAMPLE.COM" is not a valid hostname or IP address`, - }, - } { - tt := tt - t.Run(fmt.Sprintf("parse %q", tt.input), func(t *testing.T) { - got := validateExternalEndpoint(tt.input) - if tt.expectErr == "" { - require.NoError(t, got) - } else { - require.EqualError(t, got, tt.expectErr) - } - }) - } -}