-
Notifications
You must be signed in to change notification settings - Fork 38.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cache admission webhook restClient #54861
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,13 +19,15 @@ package webhook | |
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
"fmt" | ||
"io" | ||
"net" | ||
"net/url" | ||
"path" | ||
"sync" | ||
|
||
"github.com/golang/glog" | ||
lru "github.com/hashicorp/golang-lru" | ||
|
||
admissionv1alpha1 "k8s.io/api/admission/v1alpha1" | ||
"k8s.io/api/admissionregistration/v1alpha1" | ||
|
@@ -45,7 +47,8 @@ import ( | |
|
||
const ( | ||
// Name of admission plug-in | ||
PluginName = "GenericAdmissionWebhook" | ||
PluginName = "GenericAdmissionWebhook" | ||
defaultCacheSize = 200 | ||
) | ||
|
||
type ErrCallingWebhook struct { | ||
|
@@ -96,6 +99,11 @@ func NewGenericAdmissionWebhook(configFile io.Reader) (*GenericAdmissionWebhook, | |
return nil, err | ||
} | ||
|
||
cache, err := lru.New(defaultCacheSize) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return &GenericAdmissionWebhook{ | ||
Handler: admission.NewHandler( | ||
admission.Connect, | ||
|
@@ -105,6 +113,7 @@ func NewGenericAdmissionWebhook(configFile io.Reader) (*GenericAdmissionWebhook, | |
), | ||
authInfoResolver: authInfoResolver, | ||
serviceResolver: defaultServiceResolver{}, | ||
cache: cache, | ||
}, nil | ||
} | ||
|
||
|
@@ -116,6 +125,7 @@ type GenericAdmissionWebhook struct { | |
negotiatedSerializer runtime.NegotiatedSerializer | ||
|
||
authInfoResolver AuthenticationInfoResolver | ||
cache *lru.Cache | ||
} | ||
|
||
// serviceResolver knows how to convert a service reference into an actual location. | ||
|
@@ -300,23 +310,48 @@ func toStatusErr(name string, result *metav1.Status) *apierrors.StatusError { | |
} | ||
|
||
func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.Webhook) (*rest.RESTClient, error) { | ||
serverName := h.ClientConfig.Service.Name + "." + h.ClientConfig.Service.Namespace + ".svc" | ||
u, err := a.serviceResolver.ResolveEndpoint(h.ClientConfig.Service.Namespace, h.ClientConfig.Service.Name) | ||
cacheKey, err := json.Marshal(h.ClientConfig) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if client, ok := a.cache.Get(string(cacheKey)); ok { | ||
return client.(*rest.RESTClient), nil | ||
} | ||
|
||
// TODO: cache these instead of constructing one each time | ||
serverName := h.ClientConfig.Service.Name + "." + h.ClientConfig.Service.Namespace + ".svc" | ||
restConfig, err := a.authInfoResolver.ClientConfigFor(serverName) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
cfg := rest.CopyConfig(restConfig) | ||
cfg.Host = u.Host | ||
cfg.APIPath = path.Join(u.Path, h.ClientConfig.URLPath) | ||
host := serverName + ":443" | ||
cfg.Host = "https://" + host | ||
cfg.APIPath = h.ClientConfig.URLPath | ||
cfg.TLSClientConfig.ServerName = serverName | ||
cfg.TLSClientConfig.CAData = h.ClientConfig.CABundle | ||
cfg.ContentConfig.NegotiatedSerializer = a.negotiatedSerializer | ||
cfg.ContentConfig.ContentType = runtime.ContentTypeJSON | ||
return rest.UnversionedRESTClientFor(cfg) | ||
|
||
delegateDialer := cfg.Dial | ||
if delegateDialer == nil { | ||
delegateDialer = net.Dial | ||
} | ||
|
||
cfg.Dial = func(network, addr string) (net.Conn, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am rebasing on top of this right now-- why did this get moved to a dialer instead of being done up-front? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure what you mean by up-front... you need to resolve every time you connect in case the resolution has changed, otherwise you're stuck with the initially resolved address for the lifetime of the client. Dial is the right place to do connect-time name->IP resolution There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, that makes sense, thanks. |
||
if addr == host { | ||
u, err := a.serviceResolver.ResolveEndpoint(h.ClientConfig.Service.Namespace, h.ClientConfig.Service.Name) | ||
if err != nil { | ||
return nil, err | ||
} | ||
addr = u.Host | ||
} | ||
return delegateDialer(network, addr) | ||
} | ||
|
||
client, err := rest.UnversionedRESTClientFor(cfg) | ||
if err == nil { | ||
a.cache.Add(string(cacheKey), client) | ||
} | ||
return client, err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve the existing dialer set on the config, if present
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
understand