Skip to content

Commit b47349a

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#82252 from liggitt/webhook-client-auth-test
Match webhook client auth with ports consistently
2 parents f71cfdf + e734c70 commit b47349a

File tree

6 files changed

+443
-30
lines changed

6 files changed

+443
-30
lines changed

‎staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/testing/authentication_info_resolver.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ type authenticationInfoResolver struct {
5252
cacheMisses *int32
5353
}
5454

55-
func (a *authenticationInfoResolver) ClientConfigFor(server string) (*rest.Config, error) {
55+
func (a *authenticationInfoResolver) ClientConfigFor(hostPort string) (*rest.Config, error) {
5656
atomic.AddInt32(a.cacheMisses, 1)
5757
return a.restConfig, nil
5858
}
5959

60-
func (a *authenticationInfoResolver) ClientConfigForService(serviceName, serviceNamespace string) (*rest.Config, error) {
60+
func (a *authenticationInfoResolver) ClientConfigForService(serviceName, serviceNamespace string, servicePort int) (*rest.Config, error) {
6161
atomic.AddInt32(a.cacheMisses, 1)
6262
return a.restConfig, nil
6363
}

‎staging/src/k8s.io/apiserver/pkg/util/webhook/authentication.go‎

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ package webhook
1919
import (
2020
"fmt"
2121
"io/ioutil"
22+
"net"
2223
"net/http"
24+
"strconv"
2325
"strings"
2426
"time"
2527

@@ -40,17 +42,17 @@ func NewDefaultAuthenticationInfoResolverWrapper(
4042

4143
webhookAuthResolverWrapper := func(delegate AuthenticationInfoResolver) AuthenticationInfoResolver {
4244
return &AuthenticationInfoResolverDelegator{
43-
ClientConfigForFunc: func(server string) (*rest.Config, error) {
44-
if server == "kubernetes.default.svc" {
45+
ClientConfigForFunc: func(hostPort string) (*rest.Config, error) {
46+
if hostPort == "kubernetes.default.svc:443" {
4547
return kubeapiserverClientConfig, nil
4648
}
47-
return delegate.ClientConfigFor(server)
49+
return delegate.ClientConfigFor(hostPort)
4850
},
49-
ClientConfigForServiceFunc: func(serviceName, serviceNamespace string) (*rest.Config, error) {
50-
if serviceName == "kubernetes" && serviceNamespace == corev1.NamespaceDefault {
51+
ClientConfigForServiceFunc: func(serviceName, serviceNamespace string, servicePort int) (*rest.Config, error) {
52+
if serviceName == "kubernetes" && serviceNamespace == corev1.NamespaceDefault && servicePort == 443 {
5153
return kubeapiserverClientConfig, nil
5254
}
53-
ret, err := delegate.ClientConfigForService(serviceName, serviceNamespace)
55+
ret, err := delegate.ClientConfigForService(serviceName, serviceNamespace, servicePort)
5456
if err != nil {
5557
return nil, err
5658
}
@@ -67,27 +69,27 @@ func NewDefaultAuthenticationInfoResolverWrapper(
6769
// AuthenticationInfoResolver builds rest.Config base on the server or service
6870
// name and service namespace.
6971
type AuthenticationInfoResolver interface {
70-
// ClientConfigFor builds rest.Config based on the server.
71-
ClientConfigFor(server string) (*rest.Config, error)
72+
// ClientConfigFor builds rest.Config based on the hostPort.
73+
ClientConfigFor(hostPort string) (*rest.Config, error)
7274
// ClientConfigForService builds rest.Config based on the serviceName and
7375
// serviceNamespace.
74-
ClientConfigForService(serviceName, serviceNamespace string) (*rest.Config, error)
76+
ClientConfigForService(serviceName, serviceNamespace string, servicePort int) (*rest.Config, error)
7577
}
7678

7779
// AuthenticationInfoResolverDelegator implements AuthenticationInfoResolver.
7880
type AuthenticationInfoResolverDelegator struct {
79-
ClientConfigForFunc func(server string) (*rest.Config, error)
80-
ClientConfigForServiceFunc func(serviceName, serviceNamespace string) (*rest.Config, error)
81+
ClientConfigForFunc func(hostPort string) (*rest.Config, error)
82+
ClientConfigForServiceFunc func(serviceName, serviceNamespace string, servicePort int) (*rest.Config, error)
8183
}
8284

83-
// ClientConfigFor returns client config for given server.
84-
func (a *AuthenticationInfoResolverDelegator) ClientConfigFor(server string) (*rest.Config, error) {
85-
return a.ClientConfigForFunc(server)
85+
// ClientConfigFor returns client config for given hostPort.
86+
func (a *AuthenticationInfoResolverDelegator) ClientConfigFor(hostPort string) (*rest.Config, error) {
87+
return a.ClientConfigForFunc(hostPort)
8688
}
8789

8890
// ClientConfigForService returns client config for given service.
89-
func (a *AuthenticationInfoResolverDelegator) ClientConfigForService(serviceName, serviceNamespace string) (*rest.Config, error) {
90-
return a.ClientConfigForServiceFunc(serviceName, serviceNamespace)
91+
func (a *AuthenticationInfoResolverDelegator) ClientConfigForService(serviceName, serviceNamespace string, servicePort int) (*rest.Config, error) {
92+
return a.ClientConfigForServiceFunc(serviceName, serviceNamespace, servicePort)
9193
}
9294

9395
type defaultAuthenticationInfoResolver struct {
@@ -113,12 +115,12 @@ func NewDefaultAuthenticationInfoResolver(kubeconfigFile string) (Authentication
113115
return &defaultAuthenticationInfoResolver{kubeconfig: clientConfig}, nil
114116
}
115117

116-
func (c *defaultAuthenticationInfoResolver) ClientConfigFor(server string) (*rest.Config, error) {
117-
return c.clientConfig(server)
118+
func (c *defaultAuthenticationInfoResolver) ClientConfigFor(hostPort string) (*rest.Config, error) {
119+
return c.clientConfig(hostPort)
118120
}
119121

120-
func (c *defaultAuthenticationInfoResolver) ClientConfigForService(serviceName, serviceNamespace string) (*rest.Config, error) {
121-
return c.clientConfig(serviceName + "." + serviceNamespace + ".svc")
122+
func (c *defaultAuthenticationInfoResolver) ClientConfigForService(serviceName, serviceNamespace string, servicePort int) (*rest.Config, error) {
123+
return c.clientConfig(net.JoinHostPort(serviceName+"."+serviceNamespace+".svc", strconv.Itoa(servicePort)))
122124
}
123125

124126
func (c *defaultAuthenticationInfoResolver) clientConfig(target string) (*rest.Config, error) {
@@ -136,8 +138,25 @@ func (c *defaultAuthenticationInfoResolver) clientConfig(target string) (*rest.C
136138
}
137139
}
138140

141+
// If target included the default https port (443), search again without the port
142+
if target, port, err := net.SplitHostPort(target); err == nil && port == "443" {
143+
// exact match without port
144+
if authConfig, ok := c.kubeconfig.AuthInfos[target]; ok {
145+
return restConfigFromKubeconfig(authConfig)
146+
}
147+
148+
// star prefixed match without port
149+
serverSteps := strings.Split(target, ".")
150+
for i := 1; i < len(serverSteps); i++ {
151+
nickName := "*." + strings.Join(serverSteps[i:], ".")
152+
if authConfig, ok := c.kubeconfig.AuthInfos[nickName]; ok {
153+
return restConfigFromKubeconfig(authConfig)
154+
}
155+
}
156+
}
157+
139158
// if we're trying to hit the kube-apiserver and there wasn't an explicit config, use the in-cluster config
140-
if target == "kubernetes.default.svc" {
159+
if target == "kubernetes.default.svc:443" {
141160
// if we can find an in-cluster-config use that. If we can't, fall through.
142161
inClusterConfig, err := rest.InClusterConfig()
143162
if err == nil {

‎staging/src/k8s.io/apiserver/pkg/util/webhook/authentication_test.go‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,90 @@ func TestAuthenticationDetection(t *testing.T) {
109109
},
110110
expected: rest.Config{BearerToken: "first"},
111111
},
112+
{
113+
name: "exact match with default https port",
114+
serverName: "one.two.three.com:443",
115+
kubeconfig: clientcmdapi.Config{
116+
AuthInfos: map[string]*clientcmdapi.AuthInfo{
117+
"one.two.three.com:443": {Token: "exact"},
118+
"*.two.three.com": {Token: "first"},
119+
"*.three.com": {Token: "second"},
120+
"*.com": {Token: "third"},
121+
"*": {Token: "fallback"},
122+
},
123+
},
124+
expected: rest.Config{BearerToken: "exact"},
125+
},
126+
{
127+
name: "wildcard match with default https port",
128+
serverName: "one.two.three.com:443",
129+
kubeconfig: clientcmdapi.Config{
130+
AuthInfos: map[string]*clientcmdapi.AuthInfo{
131+
"*.two.three.com:443": {Token: "first-with-port"},
132+
"*.two.three.com": {Token: "first"},
133+
"*.three.com": {Token: "second"},
134+
"*.com": {Token: "third"},
135+
"*": {Token: "fallback"},
136+
},
137+
},
138+
expected: rest.Config{BearerToken: "first-with-port"},
139+
},
140+
{
141+
name: "wildcard match without default https port",
142+
serverName: "one.two.three.com:443",
143+
kubeconfig: clientcmdapi.Config{
144+
AuthInfos: map[string]*clientcmdapi.AuthInfo{
145+
"*.two.three.com": {Token: "first"},
146+
"*.three.com": {Token: "second"},
147+
"*.com": {Token: "third"},
148+
"*": {Token: "fallback"},
149+
},
150+
},
151+
expected: rest.Config{BearerToken: "first"},
152+
},
153+
{
154+
name: "exact match with non-default https port",
155+
serverName: "one.two.three.com:8443",
156+
kubeconfig: clientcmdapi.Config{
157+
AuthInfos: map[string]*clientcmdapi.AuthInfo{
158+
"one.two.three.com:8443": {Token: "exact"},
159+
"*.two.three.com": {Token: "first"},
160+
"*.three.com": {Token: "second"},
161+
"*.com": {Token: "third"},
162+
"*": {Token: "fallback"},
163+
},
164+
},
165+
expected: rest.Config{BearerToken: "exact"},
166+
},
167+
{
168+
name: "wildcard match with non-default https port",
169+
serverName: "one.two.three.com:8443",
170+
kubeconfig: clientcmdapi.Config{
171+
AuthInfos: map[string]*clientcmdapi.AuthInfo{
172+
"*.two.three.com:8443": {Token: "first-with-port"},
173+
"one.two.three.com": {Token: "first-without-port"},
174+
"*.two.three.com": {Token: "first"},
175+
"*.three.com": {Token: "second"},
176+
"*.com": {Token: "third"},
177+
"*": {Token: "fallback"},
178+
},
179+
},
180+
expected: rest.Config{BearerToken: "first-with-port"},
181+
},
182+
{
183+
name: "wildcard match without non-default https port",
184+
serverName: "one.two.three.com:8443",
185+
kubeconfig: clientcmdapi.Config{
186+
AuthInfos: map[string]*clientcmdapi.AuthInfo{
187+
"one.two.three.com": {Token: "first-without-port"},
188+
"*.two.three.com": {Token: "first"},
189+
"*.three.com": {Token: "second"},
190+
"*.com": {Token: "third"},
191+
"*": {Token: "fallback"},
192+
},
193+
},
194+
expected: rest.Config{BearerToken: "fallback"},
195+
},
112196
}
113197

114198
for _, tc := range tests {

‎staging/src/k8s.io/apiserver/pkg/util/webhook/client.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"fmt"
2424
"net"
2525
"net/url"
26+
"strconv"
2627

2728
"github.com/hashicorp/golang-lru"
2829
"k8s.io/apimachinery/pkg/runtime"
@@ -151,13 +152,20 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
151152
}
152153

153154
if cc.Service != nil {
154-
restConfig, err := cm.authInfoResolver.ClientConfigForService(cc.Service.Name, cc.Service.Namespace)
155+
port := cc.Service.Port
156+
if port == 0 {
157+
// Default to port 443 if no service port is specified
158+
port = 443
159+
}
160+
161+
restConfig, err := cm.authInfoResolver.ClientConfigForService(cc.Service.Name, cc.Service.Namespace, int(port))
155162
if err != nil {
156163
return nil, err
157164
}
158165
cfg := rest.CopyConfig(restConfig)
159166
serverName := cc.Service.Name + "." + cc.Service.Namespace + ".svc"
160-
host := serverName + ":443"
167+
168+
host := net.JoinHostPort(serverName, strconv.Itoa(int(port)))
161169
cfg.Host = "https://" + host
162170
cfg.APIPath = cc.Service.Path
163171
// Set the server name if not already set
@@ -172,10 +180,6 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
172180
}
173181
cfg.Dial = func(ctx context.Context, network, addr string) (net.Conn, error) {
174182
if addr == host {
175-
port := cc.Service.Port
176-
if port == 0 {
177-
port = 443
178-
}
179183
u, err := cm.serviceResolver.ResolveEndpoint(cc.Service.Namespace, cc.Service.Name, port)
180184
if err != nil {
181185
return nil, err
@@ -197,7 +201,13 @@ func (cm *ClientManager) HookClient(cc ClientConfig) (*rest.RESTClient, error) {
197201
return nil, &ErrCallingWebhook{WebhookName: cc.Name, Reason: fmt.Errorf("Unparsable URL: %v", err)}
198202
}
199203

200-
restConfig, err := cm.authInfoResolver.ClientConfigFor(u.Host)
204+
hostPort := u.Host
205+
if len(u.Port()) == 0 {
206+
// Default to port 443 if no port is specified
207+
hostPort = net.JoinHostPort(hostPort, "443")
208+
}
209+
210+
restConfig, err := cm.authInfoResolver.ClientConfigFor(hostPort)
201211
if err != nil {
202212
return nil, err
203213
}

‎test/integration/apiserver/admissionwebhook/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ go_test(
55
srcs = [
66
"admission_test.go",
77
"broken_webhook_test.go",
8+
"client_auth_test.go",
89
"load_balance_test.go",
910
"main_test.go",
1011
"reinvocation_test.go",

0 commit comments

Comments
 (0)