mirrors/pinniped
1
0
Fork 0
mirror of https://github.com/vmware-tanzu/pinniped.git synced 2026-01-03 11:45:45 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

also use port number when checking https proxy for WebhookAuthenticator

Browse Source
This commit is contained in:
Ryan Richard
2024-10-11 14:49:46 -07:00
parent 4d2bbac674
commit dc195536d0
4 changed files with 121 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
internal/controller/authenticator/webhookcachefiller/webhookcachefiller.go
View File

@@ -510,7 +510,7 @@ func (c *webhookCacheFillerController) validateEndpoint(endpoint string, conditi
return nil, conditions, false, false
}
usingProxyForHost, err := c.proxyDetector.UsingProxyForHost(endpointHostPort.Host)
usingProxyForHost, err := c.proxyDetector.UsingProxyForHost(endpointHostPort.Endpoint())
if err != nil {
msg := fmt.Sprintf("%s: %s", "spec.endpoint URL error", err.Error())
conditions = append(conditions, &metav1.Condition{

20
internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go
View File

@@ -1068,7 +1068,10 @@ func TestController(t *testing.T) {
fakeProxyDetect := fakeproxydetect.New(true, nil)
t.Cleanup(func() {
require.Equal(t, 1, fakeProxyDetect.NumberOfInvocations())
require.Equal(t, "127.0.0.1", fakeProxyDetect.ReceivedHostDuringMostRecentInvocation())
require.Equal(t,
strings.TrimPrefix(goodWebhookAuthenticatorSpecWithCA.Endpoint, "https://"),
fakeProxyDetect.ReceivedHostDuringMostRecentInvocation(),
)
})
return fakeProxyDetect
},
@@ -1117,7 +1120,10 @@ func TestController(t *testing.T) {
fakeProxyDetect := fakeproxydetect.New(false, nil)
t.Cleanup(func() {
require.Equal(t, 1, fakeProxyDetect.NumberOfInvocations())
require.Equal(t, "127.0.0.1", fakeProxyDetect.ReceivedHostDuringMostRecentInvocation())
require.Equal(t,
strings.TrimPrefix(goodWebhookAuthenticatorSpecWithCA.Endpoint, "https://"),
fakeProxyDetect.ReceivedHostDuringMostRecentInvocation(),
)
})
return fakeProxyDetect
},
@@ -1160,7 +1166,10 @@ func TestController(t *testing.T) {
fakeProxyDetect := fakeproxydetect.New(true, nil)
t.Cleanup(func() {
require.Equal(t, 1, fakeProxyDetect.NumberOfInvocations())
require.Equal(t, "127.0.0.1", fakeProxyDetect.ReceivedHostDuringMostRecentInvocation())
require.Equal(t,
strings.TrimPrefix(goodWebhookAuthenticatorSpecWithCA.Endpoint, "https://"),
fakeProxyDetect.ReceivedHostDuringMostRecentInvocation(),
)
})
return fakeProxyDetect
},
@@ -1208,7 +1217,10 @@ func TestController(t *testing.T) {
fakeProxyDetect := fakeproxydetect.New(false, errors.New("fake proxy detector error"))
t.Cleanup(func() {
require.Equal(t, 1, fakeProxyDetect.NumberOfInvocations())
require.Equal(t, "127.0.0.1", fakeProxyDetect.ReceivedHostDuringMostRecentInvocation())
require.Equal(t,
strings.TrimPrefix(goodWebhookAuthenticatorSpecWithCA.Endpoint, "https://"),
fakeProxyDetect.ReceivedHostDuringMostRecentInvocation(),
)
})
return fakeProxyDetect
},

10
internal/proxydetect/proxydetect.go
View File

@@ -8,6 +8,8 @@ import (
"fmt"
"net/http"
"net/url"
"k8s.io/apimachinery/pkg/util/net"
)
type ProxyDetect interface {
@@ -26,7 +28,13 @@ type detector struct {
var _ ProxyDetect = (*detector)(nil)
func New() ProxyDetect {
return &detector{proxyFromEnvironmentFunc: http.ProxyFromEnvironment}
return &detector{
// Because this is intended for use with the WebhookAuthenticator, let's
// use the same proxy function that we use for the WebhookAuthenticator.
// Refer to how webhookcachefiller.go constructs the webhook authenticator,
// and you'll find how it sets the proxy function in kubeclient.go to be this...
proxyFromEnvironmentFunc: net.NewProxierWithNoProxyCIDR(http.ProxyFromEnvironment),
}
}
func (d *detector) UsingProxyForHost(host string) (bool, error) {

99
internal/proxydetect/proxydetect_test.go
View File

@@ -12,9 +12,100 @@ import (
"github.com/stretchr/testify/require"
)
func TestProxyDetect(t *testing.T) {
t.Parallel()
func TestProxyDetectWithoutMock(t *testing.T) {
// Setting these real env vars means that we cannot run this test in parallel.
t.Setenv("HTTPS_PROXY", "http://proxy.pinniped.dev")
t.Setenv("NO_PROXY", "1.2.3.4,1.2.3.5:3333,4.4.4.0/28,example1.pinniped.dev:8443,example2.pinniped.dev")
subject := New()
tests := []struct {
name string
host string
wantProxied bool
wantErr string
}{
// This does not test all permutations of how HTTPS_PROXY and NO_PROXY work.
// Some basic tests to understand how these settings work are included below.
// See https://pkg.go.dev/golang.org/x/net/http/httpproxy for docs.
{
name: "any host not included in NO_PROXY should use the proxy",
host: "www.pinniped.dev",
wantProxied: true,
},
{
name: "a port different from the one specified in NO_PROXY should use the proxy, for the default HTTPS port",
host: "example1.pinniped.dev",
wantProxied: true,
},
{
name: "a port different from the one specified in NO_PROXY should use the proxy, for an explicit port",
host: "example1.pinniped.dev:994",
wantProxied: true,
},
{
name: "same port as the one specified in NO_PROXY should skip the proxy",
host: "example1.pinniped.dev:8443",
wantProxied: false,
},
{
name: "any host included in NO_PROXY should skip the proxy, with default ports",
host: "example2.pinniped.dev",
wantProxied: false,
},
{
name: "an IP specified in NO_PROXY should skip the proxy",
host: "1.2.3.4",
wantProxied: false,
},
{
name: "an IP specified in NO_PROXY should skip the proxy, with matching explicit ports",
host: "1.2.3.5:3333",
wantProxied: false,
},
{
name: "an IP specified in NO_PROXY should use the proxy when the ports don't match",
host: "1.2.3.5:1234",
wantProxied: true,
},
{
name: "an IP included in a NO_PROXY CIDR should skip the proxy",
host: "4.4.4.4",
wantProxied: false,
},
{
name: "an IP outside a NO_PROXY CIDR should use the proxy",
host: "4.4.4.16",
wantProxied: true,
},
{
name: "as a special case in the Go documentation, localhost never uses the proxy, regardless of NO_PROXY settings",
host: "localhost",
wantProxied: false,
},
{
name: "a bad hostname returns an error",
host: "bad hostname",
wantProxied: false,
wantErr: `could not determine if requests will be proxied for host "bad hostname": parse "https://bad hostname": invalid character " " in host name`,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
usingProxyForHost, err := subject.UsingProxyForHost(tt.host)
if tt.wantErr != "" {
require.EqualError(t, err, tt.wantErr)
} else {
require.NoError(t, err)
}
require.Equal(t, tt.wantProxied, usingProxyForHost)
})
}
}
func TestProxyDetectWithMock(t *testing.T) {
proxyURL, err := url.Parse("http://myproxy.com")
require.NoError(t, err)
@@ -66,12 +157,12 @@ func TestProxyDetect(t *testing.T) {
proxied, err := subject.UsingProxyForHost(tt.host)
require.Equal(t, tt.wantProxied, proxied)
if tt.wantErr != "" {
require.Equal(t, tt.wantErr, err.Error())
require.EqualError(t, err, tt.wantErr)
} else {
require.NoError(t, err)
}
require.Equal(t, tt.wantProxied, proxied)
})
}
}