Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Plumbing the proxy dialer to the webhook admission plugin; Fix the tls config #50476

Merged
merged 2 commits into from
Sep 15, 2017
Merged

Plumbing the proxy dialer to the webhook admission plugin; Fix the tls config #50476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
186a068
plumb the proxyTransport to the webhook admission plugin;
Aug 10, 2017
856a1db
fix the webhook unit test; the server cert needs to have a valid CN;
Sep 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions cmd/kube-apiserver/app/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func CreateNodeDialer(s *options.ServerRunOptions) (tunneler.Tunneler, *http.Tra
}

// CreateKubeAPIServerConfig creates all the resources for running the API server, but runs none of them
func CreateKubeAPIServerConfig(s *options.ServerRunOptions, nodeTunneler tunneler.Tunneler, proxyTransport http.RoundTripper) (*master.Config, informers.SharedInformerFactory, clientgoinformers.SharedInformerFactory, *kubeserver.InsecureServingInfo, aggregatorapiserver.ServiceResolver, error) {
func CreateKubeAPIServerConfig(s *options.ServerRunOptions, nodeTunneler tunneler.Tunneler, proxyTransport *http.Transport) (*master.Config, informers.SharedInformerFactory, clientgoinformers.SharedInformerFactory, *kubeserver.InsecureServingInfo, aggregatorapiserver.ServiceResolver, error) {
// set defaults in the options before trying to create the generic config
if err := defaultOptions(s); err != nil {
return nil, nil, nil, nil, nil, err
Expand All @@ -269,8 +269,7 @@ func CreateKubeAPIServerConfig(s *options.ServerRunOptions, nodeTunneler tunnele
return nil, nil, nil, nil, nil, err
}
}

genericConfig, sharedInformers, versionedInformers, insecureServingOptions, serviceResolver, err := BuildGenericConfig(s)
genericConfig, sharedInformers, versionedInformers, insecureServingOptions, serviceResolver, err := BuildGenericConfig(s, proxyTransport)
if err != nil {
return nil, nil, nil, nil, nil, err
}
Expand Down Expand Up @@ -354,7 +353,7 @@ func CreateKubeAPIServerConfig(s *options.ServerRunOptions, nodeTunneler tunnele
}

// BuildGenericConfig takes the master server options and produces the genericapiserver.Config associated with it
func BuildGenericConfig(s *options.ServerRunOptions) (*genericapiserver.Config, informers.SharedInformerFactory, clientgoinformers.SharedInformerFactory, *kubeserver.InsecureServingInfo, aggregatorapiserver.ServiceResolver, error) {
func BuildGenericConfig(s *options.ServerRunOptions, proxyTransport *http.Transport) (*genericapiserver.Config, informers.SharedInformerFactory, clientgoinformers.SharedInformerFactory, *kubeserver.InsecureServingInfo, aggregatorapiserver.ServiceResolver, error) {
genericConfig := genericapiserver.NewConfig(api.Codecs)
if err := s.GenericServerRunOptions.ApplyTo(genericConfig); err != nil {
return nil, nil, nil, nil, nil, err
Expand Down Expand Up @@ -460,6 +459,7 @@ func BuildGenericConfig(s *options.ServerRunOptions) (*genericapiserver.Config,
sharedInformers,
genericConfig.Authorizer,
serviceResolver,
proxyTransport,
)
if err != nil {
return nil, nil, nil, nil, nil, fmt.Errorf("failed to create admission plugin initializer: %v", err)
Expand All @@ -476,7 +476,7 @@ func BuildGenericConfig(s *options.ServerRunOptions) (*genericapiserver.Config,
}

// BuildAdmissionPluginInitializer constructs the admission plugin initializer
func BuildAdmissionPluginInitializer(s *options.ServerRunOptions, client internalclientset.Interface, externalClient clientset.Interface, sharedInformers informers.SharedInformerFactory, apiAuthorizer authorizer.Authorizer, serviceResolver aggregatorapiserver.ServiceResolver) (admission.PluginInitializer, error) {
func BuildAdmissionPluginInitializer(s *options.ServerRunOptions, client internalclientset.Interface, externalClient clientset.Interface, sharedInformers informers.SharedInformerFactory, apiAuthorizer authorizer.Authorizer, serviceResolver aggregatorapiserver.ServiceResolver, proxyTransport *http.Transport) (admission.PluginInitializer, error) {
var cloudConfig []byte

if s.CloudProvider.CloudConfigFile != "" {
Expand Down Expand Up @@ -510,6 +510,7 @@ func BuildAdmissionPluginInitializer(s *options.ServerRunOptions, client interna
}

pluginInitializer = pluginInitializer.SetServiceResolver(serviceResolver)
pluginInitializer = pluginInitializer.SetProxyTransport(proxyTransport)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was kinda hoping to combine the service resolver with this proxy transport. But maybe it's not important. hm.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want them to be combined because both of them are needed iff an admission plugin wants to contact a service provided by the user? I can put them into a single struct if you think it's necessary.


return pluginInitializer, nil
}
Expand Down
22 changes: 20 additions & 2 deletions pkg/kubeapiserver/admission/initializer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package admission

import (
"net/http"
"net/url"

"k8s.io/apimachinery/pkg/api/meta"
Expand Down Expand Up @@ -88,6 +89,12 @@ type ServiceResolver interface {
ResolveEndpoint(namespace, name string) (*url.URL, error)
}

// WantsProxyTransport defines a fuction that accepts a proxy transport for admission
// plugins that need to make calls to pods.
type WantsProxyTransport interface {
SetProxyTransport(proxyTransport *http.Transport)
}

type PluginInitializer struct {
internalClient internalclientset.Interface
externalClient clientset.Interface
Expand All @@ -99,8 +106,9 @@ type PluginInitializer struct {
serviceResolver ServiceResolver

// for proving we are apiserver in call-outs
clientCert []byte
clientKey []byte
clientCert []byte
clientKey []byte
proxyTransport *http.Transport
}

var _ admission.PluginInitializer = &PluginInitializer{}
Expand Down Expand Up @@ -142,6 +150,12 @@ func (i *PluginInitializer) SetClientCert(cert, key []byte) *PluginInitializer {
return i
}

// SetProxyTransport sets the proxyTransport which is needed by some plugins.
func (i *PluginInitializer) SetProxyTransport(proxyTransport *http.Transport) *PluginInitializer {
i.proxyTransport = proxyTransport
return i
}

// Initialize checks the initialization interfaces implemented by each plugin
// and provide the appropriate initialization data
func (i *PluginInitializer) Initialize(plugin admission.Interface) {
Expand Down Expand Up @@ -186,4 +200,8 @@ func (i *PluginInitializer) Initialize(plugin admission.Interface) {
}
wants.SetClientCert(i.clientCert, i.clientKey)
}

if wants, ok := plugin.(WantsProxyTransport); ok {
wants.SetProxyTransport(i.proxyTransport)
}
}
20 changes: 17 additions & 3 deletions plugin/pkg/admission/webhook/admission.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"context"
"fmt"
"io"
"net"
"net/http"
"sync"
"time"

Expand Down Expand Up @@ -106,6 +108,7 @@ type GenericAdmissionWebhook struct {
negotiatedSerializer runtime.NegotiatedSerializer
clientCert []byte
clientKey []byte
proxyTransport *http.Transport
}

var (
Expand All @@ -114,6 +117,10 @@ var (
_ = admissioninit.WantsExternalKubeClientSet(&GenericAdmissionWebhook{})
)

func (a *GenericAdmissionWebhook) SetProxyTransport(pt *http.Transport) {
a.proxyTransport = pt
}

func (a *GenericAdmissionWebhook) SetServiceResolver(sr admissioninit.ServiceResolver) {
a.serviceResolver = sr
}
Expand Down Expand Up @@ -242,20 +249,27 @@ func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.ExternalAdmissionHook)
return nil, err
}

var dial func(network, addr string) (net.Conn, error)
if a.proxyTransport != nil && a.proxyTransport.Dial != nil {
dial = a.proxyTransport.Dial
}

// TODO: cache these instead of constructing one each time
cfg := &rest.Config{
Host: u.Host,
APIPath: u.Path,
TLSClientConfig: rest.TLSClientConfig{
CAData: h.ClientConfig.CABundle,
CertData: a.clientCert,
KeyData: a.clientKey,
ServerName: h.ClientConfig.Service.Name + "." + h.ClientConfig.Service.Namespace + ".svc",
CAData: h.ClientConfig.CABundle,
CertData: a.clientCert,
KeyData: a.clientKey,
},
UserAgent: "kube-apiserver-admission",
Timeout: 30 * time.Second,
ContentConfig: rest.ContentConfig{
NegotiatedSerializer: a.negotiatedSerializer,
},
Dial: dial,
}
return rest.UnversionedRESTClientFor(cfg)
}
42 changes: 24 additions & 18 deletions plugin/pkg/admission/webhook/admission_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,15 @@ func (f *fakeHookSource) Run(stopCh <-chan struct{}) {}

type fakeServiceResolver struct {
base url.URL
path string
}

func (f fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) {
if namespace == "failResolve" {
return nil, fmt.Errorf("couldn't resolve service location")
}
u := f.base
u.Path = name
u.Path = f.path
return &u, nil
}

Expand Down Expand Up @@ -89,7 +90,6 @@ func TestAdmit(t *testing.T) {
if err != nil {
t.Fatal(err)
}
wh.serviceResolver = fakeServiceResolver{*serverURL}
wh.clientCert = clientCert
wh.clientKey = clientKey

Expand Down Expand Up @@ -123,16 +123,16 @@ func TestAdmit(t *testing.T) {

type test struct {
hookSource fakeHookSource
path string
expectAllow bool
errorContains string
}
ccfg := func(result string) registrationv1alpha1.AdmissionHookClientConfig {
return registrationv1alpha1.AdmissionHookClientConfig{
Service: registrationv1alpha1.ServiceReference{
Name: result,
},
CABundle: caCert,
}
ccfg := registrationv1alpha1.AdmissionHookClientConfig{
Service: registrationv1alpha1.ServiceReference{
Name: "webhook-test",
Namespace: "default",
},
CABundle: caCert,
}
matchEverythingRules := []registrationv1alpha1.RuleWithOperations{{
Operations: []registrationv1alpha1.OperationType{registrationv1alpha1.OperationAll},
Expand All @@ -148,66 +148,72 @@ func TestAdmit(t *testing.T) {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "nomatch",
ClientConfig: ccfg("disallow"),
ClientConfig: ccfg,
Rules: []registrationv1alpha1.RuleWithOperations{{
Operations: []registrationv1alpha1.OperationType{registrationv1alpha1.Create},
}},
}},
},
path: "disallow",
expectAllow: true,
},
"match & allow": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "allow",
ClientConfig: ccfg("allow"),
ClientConfig: ccfg,
Rules: matchEverythingRules,
}},
},
path: "allow",
expectAllow: true,
},
"match & disallow": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "disallow",
ClientConfig: ccfg("disallow"),
ClientConfig: ccfg,
Rules: matchEverythingRules,
}},
},
path: "disallow",
errorContains: "without explanation",
},
"match & disallow ii": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "disallowReason",
ClientConfig: ccfg("disallowReason"),
ClientConfig: ccfg,
Rules: matchEverythingRules,
}},
},
path: "disallowReason",
errorContains: "you shall not pass",
},
"match & fail (but allow because fail open)": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "internalErr A",
ClientConfig: ccfg("internalErr"),
ClientConfig: ccfg,
Rules: matchEverythingRules,
}, {
Name: "invalidReq B",
ClientConfig: ccfg("invalidReq"),
Name: "internalErr B",
ClientConfig: ccfg,
Rules: matchEverythingRules,
}, {
Name: "invalidResp C",
ClientConfig: ccfg("invalidResp"),
Name: "internalErr C",
ClientConfig: ccfg,
Rules: matchEverythingRules,
}},
},
path: "internalErr",
expectAllow: true,
},
}

for name, tt := range table {
wh.hookSource = &tt.hookSource
wh.serviceResolver = fakeServiceResolver{base: *serverURL, path: tt.path}

err = wh.Admit(admission.NewAttributesRecord(&object, &oldObject, kind, namespace, name, resource, subResource, operation, &userInfo))
if tt.expectAllow != (err == nil) {
Expand Down
Loading