Skip to content

Commit

Permalink
add url path for admission webhooks
Browse files Browse the repository at this point in the history
  • Loading branch information
deads2k committed Oct 19, 2017
1 parent 244a8fb commit 33deaed
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 30 deletions.
4 changes: 4 additions & 0 deletions pkg/apis/admissionregistration/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@ type AdmissionHookClientConfig struct {
// ports open, port 443 will be used if it is open, otherwise it is an error.
// Required
Service ServiceReference

// URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object.
URLPath string

// CABundle is a PEM encoded CA bundle which will be used to validate webhook's server certificate.
// Required
CABundle []byte
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ func autoConvert_v1alpha1_AdmissionHookClientConfig_To_admissionregistration_Adm
if err := Convert_v1alpha1_ServiceReference_To_admissionregistration_ServiceReference(&in.Service, &out.Service, s); err != nil {
return err
}
out.URLPath = in.URLPath
out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle))
return nil
}
Expand All @@ -76,6 +77,7 @@ func autoConvert_admissionregistration_AdmissionHookClientConfig_To_v1alpha1_Adm
if err := Convert_admissionregistration_ServiceReference_To_v1alpha1_ServiceReference(&in.Service, &out.Service, s); err != nil {
return err
}
out.URLPath = in.URLPath
out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle))
return nil
}
Expand Down
18 changes: 18 additions & 0 deletions pkg/apis/admissionregistration/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,24 @@ func validateExternalAdmissionHook(hook *admissionregistration.ExternalAdmission
if hook.FailurePolicy != nil && !supportedFailurePolicies.Has(string(*hook.FailurePolicy)) {
allErrors = append(allErrors, field.NotSupported(fldPath.Child("failurePolicy"), *hook.FailurePolicy, supportedFailurePolicies.List()))
}

if len(hook.ClientConfig.URLPath) != 0 {
if !strings.HasPrefix(hook.ClientConfig.URLPath, "/") || !strings.HasSuffix(hook.ClientConfig.URLPath, "/") {
allErrors = append(allErrors, field.Invalid(fldPath.Child("clientConfig", "urlPath"), hook.ClientConfig.URLPath, "must start and end with a '/'"))
}
steps := strings.Split(hook.ClientConfig.URLPath[1:len(hook.ClientConfig.URLPath)-1], "/")
for i, step := range steps {
if len(step) == 0 {
allErrors = append(allErrors, field.Invalid(fldPath.Child("clientConfig", "urlPath"), hook.ClientConfig.URLPath, fmt.Sprintf("segment[%d] may not be empty", i)))
continue
}
failures := validation.IsDNS1123Subdomain(step)
for _, failure := range failures {
allErrors = append(allErrors, field.Invalid(fldPath.Child("clientConfig", "urlPath"), hook.ClientConfig.URLPath, fmt.Sprintf("segment[%d]: %v", i, failure)))
}
}
}

return allErrors
}

Expand Down
52 changes: 52 additions & 0 deletions pkg/apis/admissionregistration/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,58 @@ func TestValidateExternalAdmissionHookConfiguration(t *testing.T) {
}),
expectedError: `externalAdmissionHooks[0].failurePolicy: Unsupported value: "other": supported values: "Fail", "Ignore"`,
},
{
name: "URLPath must start with slash",
config: getExternalAdmissionHookConfiguration(
[]admissionregistration.ExternalAdmissionHook{
{
Name: "webhook.k8s.io",
ClientConfig: admissionregistration.AdmissionHookClientConfig{
URLPath: "foo/",
},
},
}),
expectedError: `clientConfig.urlPath: Invalid value: "foo/": must start and end with a '/'`,
},
{
name: "URLPath must end with slash",
config: getExternalAdmissionHookConfiguration(
[]admissionregistration.ExternalAdmissionHook{
{
Name: "webhook.k8s.io",
ClientConfig: admissionregistration.AdmissionHookClientConfig{
URLPath: "/foo",
},
},
}),
expectedError: `clientConfig.urlPath: Invalid value: "/foo": must start and end with a '/'`,
},
{
name: "URLPath no empty step",
config: getExternalAdmissionHookConfiguration(
[]admissionregistration.ExternalAdmissionHook{
{
Name: "webhook.k8s.io",
ClientConfig: admissionregistration.AdmissionHookClientConfig{
URLPath: "/foo//bar/",
},
},
}),
expectedError: `clientConfig.urlPath: Invalid value: "/foo//bar/": segment[1] may not be empty`,
},
{
name: "URLPath no non-subdomain",
config: getExternalAdmissionHookConfiguration(
[]admissionregistration.ExternalAdmissionHook{
{
Name: "webhook.k8s.io",
ClientConfig: admissionregistration.AdmissionHookClientConfig{
URLPath: "/apis/foo.bar/v1alpha1/--bad/",
},
},
}),
expectedError: `clientConfig.urlPath: Invalid value: "/apis/foo.bar/v1alpha1/--bad/": segment[3]: a DNS-1123 subdomain`,
},
}
for _, test := range tests {
errs := ValidateExternalAdmissionHookConfiguration(test.config)
Expand Down
4 changes: 3 additions & 1 deletion plugin/pkg/admission/webhook/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import (
admissioninit "k8s.io/kubernetes/pkg/kubeapiserver/admission"

// install the clientgo admission API for use with api registry
"path"

_ "k8s.io/kubernetes/pkg/apis/admission/install"
)

Expand Down Expand Up @@ -286,7 +288,7 @@ func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.ExternalAdmissionHook)
// TODO: cache these instead of constructing one each time
cfg := &rest.Config{
Host: u.Host,
APIPath: u.Path,
APIPath: path.Join(u.Path, h.ClientConfig.URLPath),
TLSClientConfig: rest.TLSClientConfig{
ServerName: h.ClientConfig.Service.Name + "." + h.ClientConfig.Service.Namespace + ".svc",
CAData: h.ClientConfig.CABundle,
Expand Down
53 changes: 24 additions & 29 deletions plugin/pkg/admission/webhook/admission_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,13 @@ 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 = f.path
return &u, nil
}

Expand Down Expand Up @@ -128,13 +126,17 @@ func TestAdmit(t *testing.T) {
expectAllow bool
errorContains string
}
ccfg := registrationv1alpha1.AdmissionHookClientConfig{
Service: registrationv1alpha1.ServiceReference{
Name: "webhook-test",
Namespace: "default",
},
CABundle: caCert,
ccfg := func(urlPath string) registrationv1alpha1.AdmissionHookClientConfig {
return registrationv1alpha1.AdmissionHookClientConfig{
Service: registrationv1alpha1.ServiceReference{
Name: "webhook-test",
Namespace: "default",
},
URLPath: urlPath,
CABundle: caCert,
}
}

matchEverythingRules := []registrationv1alpha1.RuleWithOperations{{
Operations: []registrationv1alpha1.OperationType{registrationv1alpha1.OperationAll},
Rule: registrationv1alpha1.Rule{
Expand All @@ -152,117 +154,110 @@ func TestAdmit(t *testing.T) {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "nomatch",
ClientConfig: ccfg,
ClientConfig: ccfg("disallow"),
Rules: []registrationv1alpha1.RuleWithOperations{{
Operations: []registrationv1alpha1.OperationType{registrationv1alpha1.Create},
}},
}},
},
path: "disallow",
expectAllow: true,
},
"match & allow": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "allow",
ClientConfig: ccfg,
ClientConfig: ccfg("allow"),
Rules: matchEverythingRules,
}},
},
path: "allow",
expectAllow: true,
},
"match & disallow": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "disallow",
ClientConfig: ccfg,
ClientConfig: ccfg("disallow"),
Rules: matchEverythingRules,
}},
},
path: "disallow",
errorContains: "without explanation",
},
"match & disallow ii": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "disallowReason",
ClientConfig: ccfg,
ClientConfig: ccfg("disallowReason"),
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,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
FailurePolicy: &policyIgnore,
}, {
Name: "internalErr B",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
FailurePolicy: &policyIgnore,
}, {
Name: "internalErr C",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
FailurePolicy: &policyIgnore,
}},
},
path: "internalErr",
expectAllow: true,
},
"match & fail (but allow because fail open on nil)": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "internalErr A",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
}, {
Name: "internalErr B",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
}, {
Name: "internalErr C",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
}},
},
path: "internalErr",
expectAllow: true,
},
"match & fail (but fail because fail closed)": {
hookSource: fakeHookSource{
hooks: []registrationv1alpha1.ExternalAdmissionHook{{
Name: "internalErr A",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
FailurePolicy: &policyFail,
}, {
Name: "internalErr B",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
FailurePolicy: &policyFail,
}, {
Name: "internalErr C",
ClientConfig: ccfg,
ClientConfig: ccfg("internalErr"),
Rules: matchEverythingRules,
FailurePolicy: &policyFail,
}},
},
path: "internalErr",
expectAllow: false,
},
}

for name, tt := range table {
t.Run(name, func(t *testing.T) {
wh.hookSource = &tt.hookSource
wh.serviceResolver = fakeServiceResolver{base: *serverURL, path: tt.path}
wh.serviceResolver = fakeServiceResolver{base: *serverURL}
wh.SetScheme(legacyscheme.Scheme)

err = wh.Admit(admission.NewAttributesRecord(&object, &oldObject, kind, namespace, name, resource, subResource, operation, &userInfo))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ type AdmissionHookClientConfig struct {
// ports open, port 443 will be used if it is open, otherwise it is an error.
// Required
Service ServiceReference `json:"service" protobuf:"bytes,1,opt,name=service"`

// URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object.
URLPath string `json:"urlPath" protobuf:"bytes,3,opt,name=urlPath"`

// CABundle is a PEM encoded CA bundle which will be used to validate webhook's server certificate.
// Required
CABundle []byte `json:"caBundle" protobuf:"bytes,2,opt,name=caBundle"`
Expand Down

0 comments on commit 33deaed

Please sign in to comment.