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

Validation webhook plugin converts objects to the external version before sending to webhooks #55127

Merged
merged 1 commit into from
Nov 14, 2017
Merged

Validation webhook plugin converts objects to the external version before sending to webhooks #55127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions hack/.golint_failures
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,8 @@ staging/src/k8s.io/apiserver/pkg/apis/audit/v1beta1
staging/src/k8s.io/apiserver/pkg/apis/audit/validation
staging/src/k8s.io/apiserver/pkg/apis/example
staging/src/k8s.io/apiserver/pkg/apis/example/v1
staging/src/k8s.io/apiserver/pkg/apis/example2
staging/src/k8s.io/apiserver/pkg/apis/example2/v1
staging/src/k8s.io/apiserver/pkg/audit
staging/src/k8s.io/apiserver/pkg/audit/policy
staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory
Expand Down
2 changes: 1 addition & 1 deletion hack/local-up-cluster.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ function start_apiserver {
fi

# Admission Controllers to invoke prior to persisting objects in cluster
ADMISSION_CONTROL=Initializers,NamespaceLifecycle,LimitRanger,ServiceAccount${security_admission},DefaultStorageClass,DefaultTolerationSeconds,ResourceQuota,GenericAdmissionWebhook
ADMISSION_CONTROL=Initializers,NamespaceLifecycle,LimitRanger,ServiceAccount${security_admission},DefaultStorageClass,DefaultTolerationSeconds,GenericAdmissionWebhook,ResourceQuota
# This is the default dir and filename where the apiserver will generate a self-signed cert
# which should be able to be used as the CA to verify itself

Expand Down
1 change: 1 addition & 0 deletions hack/update-generated-protobuf-dockerized.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ PACKAGES=(
k8s.io/metrics/pkg/apis/custom_metrics/v1beta1
k8s.io/apiserver/pkg/apis/audit/v1alpha1
k8s.io/apiserver/pkg/apis/audit/v1beta1
k8s.io/apiserver/pkg/apis/example2/v1
)

# requires the 'proto' tag to build (will remove when ready)
Expand Down
1 change: 1 addition & 0 deletions pkg/generated/openapi/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ openapi_library(
"k8s.io/apiserver/pkg/apis/audit/v1alpha1",
"k8s.io/apiserver/pkg/apis/audit/v1beta1",
"k8s.io/apiserver/pkg/apis/example/v1",
"k8s.io/apiserver/pkg/apis/example2/v1",
"k8s.io/client-go/pkg/version",
"k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1",
"k8s.io/metrics/pkg/apis/custom_metrics/v1beta1",
Expand Down
1 change: 1 addition & 0 deletions staging/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ filegroup(
"//staging/src/k8s.io/apiserver/pkg/apis/apiserver:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/apis/audit:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/apis/example:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/apis/example2:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/audit:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:all-srcs",
"//staging/src/k8s.io/apiserver/pkg/authentication/authenticatorfactory:all-srcs",
Expand Down
12 changes: 10 additions & 2 deletions staging/src/k8s.io/apiextensions-apiserver/test/integration/testserver/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,11 @@ func NewCurletInstance(namespace, name string) *unstructured.Unstructured {
}
}

func CreateNewCustomResourceDefinition(crd *apiextensionsv1beta1.CustomResourceDefinition, apiExtensionsClient clientset.Interface, clientPool dynamic.ClientPool) (dynamic.Interface, error) {
// CreateNewCustomResourceDefinitionWatchUnsafe creates the CRD and makes sure
// the apiextension apiserver has installed the CRD. But it's not safe to watch
// the created CR. Please call CreateNewCustomResourceDefinition if you need to
// watch the CR.
func CreateNewCustomResourceDefinitionWatchUnsafe(crd *apiextensionsv1beta1.CustomResourceDefinition, apiExtensionsClient clientset.Interface, clientPool dynamic.ClientPool) (dynamic.Interface, error) {
_, err := apiExtensionsClient.Apiextensions().CustomResourceDefinitions().Create(crd)
if err != nil {
return nil, err
Expand All @@ -169,7 +173,11 @@ func CreateNewCustomResourceDefinition(crd *apiextensionsv1beta1.CustomResourceD
return nil, err
}

dynamicClient, err := clientPool.ClientForGroupVersionResource(schema.GroupVersionResource{Group: crd.Spec.Group, Version: crd.Spec.Version, Resource: crd.Spec.Names.Plural})
return clientPool.ClientForGroupVersionResource(schema.GroupVersionResource{Group: crd.Spec.Group, Version: crd.Spec.Version, Resource: crd.Spec.Names.Plural})
}

func CreateNewCustomResourceDefinition(crd *apiextensionsv1beta1.CustomResourceDefinition, apiExtensionsClient clientset.Interface, clientPool dynamic.ClientPool) (dynamic.Interface, error) {
dynamicClient, err := CreateNewCustomResourceDefinitionWatchUnsafe(crd, apiExtensionsClient, clientPool)
if err != nil {
return nil, err
}
Expand Down
6 changes: 6 additions & 0 deletions staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library",
Expand All @@ -46,6 +47,7 @@ go_test(
"admission_test.go",
"authentication_test.go",
"certs_test.go",
"conversion_test.go",
"rules_test.go",
"serviceresolver_test.go",
],
Expand All @@ -58,11 +60,15 @@ go_test(
"//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/labels:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
"//vendor/k8s.io/apiserver/pkg/apis/example:go_default_library",
"//vendor/k8s.io/apiserver/pkg/apis/example/v1:go_default_library",
"//vendor/k8s.io/apiserver/pkg/apis/example2/v1:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
"//vendor/k8s.io/client-go/rest:go_default_library",
"//vendor/k8s.io/client-go/tools/clientcmd/api:go_default_library",
Expand Down
112 changes: 95 additions & 17 deletions staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/runtime/serializer"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
Expand Down Expand Up @@ -134,6 +135,8 @@ type GenericAdmissionWebhook struct {
negotiatedSerializer runtime.NegotiatedSerializer
namespaceLister corelisters.NamespaceLister
client clientset.Interface
convertor runtime.ObjectConvertor
creator runtime.ObjectCreater

authInfoResolver AuthenticationInfoResolver
cache *lru.Cache
Expand Down Expand Up @@ -169,6 +172,8 @@ func (a *GenericAdmissionWebhook) SetScheme(scheme *runtime.Scheme) {
a.negotiatedSerializer = serializer.NegotiatedSerializerWrapper(runtime.SerializerInfo{
Serializer: serializer.NewCodecFactory(scheme).LegacyCodec(admissionv1alpha1.SchemeGroupVersion),
})
a.convertor = scheme
a.creator = scheme
}
}

Expand Down Expand Up @@ -219,6 +224,37 @@ func (a *GenericAdmissionWebhook) loadConfiguration(attr admission.Attributes) (
return hookConfig, nil
}

type versionedAttributes struct {
admission.Attributes
oldObject runtime.Object
object runtime.Object
}

func (v versionedAttributes) GetObject() runtime.Object {
return v.object
}

func (v versionedAttributes) GetOldObject() runtime.Object {
return v.oldObject
}

func (a *GenericAdmissionWebhook) convertToGVK(obj runtime.Object, gvk schema.GroupVersionKind) (runtime.Object, error) {
// Unlike other resources, custom resources do not have internal version, so
// if obj is a custom resource, it should not need conversion.
if obj.GetObjectKind().GroupVersionKind() == gvk {
return obj, nil
}
out, err := a.creator.New(gvk)
if err != nil {
return nil, err
}
err = a.convertor.Convert(obj, out, nil)
if err != nil {
return nil, err
}
return out, nil
}

// Admit makes an admission decision based on the request attributes.
func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) error {
hookConfig, err := a.loadConfiguration(attr)
Expand All @@ -228,14 +264,49 @@ func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) error {
hooks := hookConfig.Webhooks
ctx := context.TODO()

errCh := make(chan error, len(hooks))
wg := sync.WaitGroup{}
wg.Add(len(hooks))
var relevantHooks []*v1alpha1.Webhook
for i := range hooks {
call, err := a.shouldCallHook(&hooks[i], attr)
if err != nil {
return err
}
if call {
relevantHooks = append(relevantHooks, &hooks[i])
}
}

if len(relevantHooks) == 0 {
// no matching hooks
return nil
}

// convert the object to the external version before sending it to the webhook
versionedAttr := versionedAttributes{
Attributes: attr,
}
if oldObj := attr.GetOldObject(); oldObj != nil {
out, err := a.convertToGVK(oldObj, attr.GetKind())
if err != nil {
return apierrors.NewInternalError(err)
}
versionedAttr.oldObject = out
}
if obj := attr.GetObject(); obj != nil {
out, err := a.convertToGVK(obj, attr.GetKind())
if err != nil {
return apierrors.NewInternalError(err)
}
versionedAttr.object = out
}

wg := sync.WaitGroup{}
errCh := make(chan error, len(relevantHooks))
wg.Add(len(relevantHooks))
for i := range relevantHooks {
go func(hook *v1alpha1.Webhook) {
defer wg.Done()

err := a.callHook(ctx, hook, attr)
err := a.callHook(ctx, hook, versionedAttr)
if err == nil {
return
}
Expand All @@ -256,7 +327,7 @@ func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) error {

glog.Warningf("rejected by webhook %q: %#v", hook.Name, err)
errCh <- err
}(&hooks[i])
}(relevantHooks[i])
}
wg.Wait()
close(errCh)
Expand Down Expand Up @@ -313,7 +384,7 @@ func (a *GenericAdmissionWebhook) getNamespaceLabels(attr admission.Attributes)

// whether the request is exempted by the webhook because of the
// namespaceSelector of the webhook.
func (a *GenericAdmissionWebhook) exemptedByNamespaceSelector(h *v1alpha1.Webhook, attr admission.Attributes) (bool, error) {
func (a *GenericAdmissionWebhook) exemptedByNamespaceSelector(h *v1alpha1.Webhook, attr admission.Attributes) (bool, *apierrors.StatusError) {
namespaceName := attr.GetNamespace()
if len(namespaceName) == 0 && attr.GetResource().Resource != "namespaces" {
// If the request is about a cluster scoped resource, and it is not a
Expand All @@ -323,8 +394,14 @@ func (a *GenericAdmissionWebhook) exemptedByNamespaceSelector(h *v1alpha1.Webhoo
return true, nil
}
namespaceLabels, err := a.getNamespaceLabels(attr)
// this means the namespace is not found, for backwards compatibility,
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there a namespace checker admission plugin? Maybe it needs to be placed in the list before webhooks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't there a namespace checker admission plugin?

The checker always allow DELETE request:

kubernetes/staging/src/k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle/admission.go

Line 113 in 1418f98

return nil

Also, we need this webhook to function correctly even if the namespace plugin is turned off.

Maybe it needs to be placed in the list before webhooks?

It is :)

// return a 404
if apierrors.IsNotFound(err) {
return false, err
status, ok := err.(apierrors.APIStatus)
if !ok {
return false, apierrors.NewInternalError(err)
}
return false, &apierrors.StatusError{status.Status()}
}
if err != nil {
return false, apierrors.NewInternalError(err)
Expand All @@ -337,15 +414,8 @@ func (a *GenericAdmissionWebhook) exemptedByNamespaceSelector(h *v1alpha1.Webhoo
return !selector.Matches(labels.Set(namespaceLabels)), nil
}

func (a *GenericAdmissionWebhook) callHook(ctx context.Context, h *v1alpha1.Webhook, attr admission.Attributes) error {
excluded, err := a.exemptedByNamespaceSelector(h, attr)
if err != nil {
return err
}
if excluded {
return nil
}
matches := false
func (a *GenericAdmissionWebhook) shouldCallHook(h *v1alpha1.Webhook, attr admission.Attributes) (bool, *apierrors.StatusError) {
var matches bool
for _, r := range h.Rules {
m := RuleMatcher{Rule: r, Attr: attr}
if m.Matches() {
Expand All @@ -354,9 +424,17 @@ func (a *GenericAdmissionWebhook) callHook(ctx context.Context, h *v1alpha1.Webh
}
}
if !matches {
return nil
return false, nil
}

excluded, err := a.exemptedByNamespaceSelector(h, attr)
if err != nil {
return false, err
}
return !excluded, nil
}

func (a *GenericAdmissionWebhook) callHook(ctx context.Context, h *v1alpha1.Webhook, attr admission.Attributes) error {
// Make the webhook request
request := createAdmissionReview(attr)
client, err := a.hookClient(h)
Expand Down