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

Let webhook controller uses a local scheme that understand admissionReview #60995

Merged
merged 1 commit into from Mar 12, 2018
Merged

Let webhook controller uses a local scheme that understand admissionReview #60995

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
1 change: 1 addition & 0 deletions staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config/BUILD
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//vendor/github.com/hashicorp/golang-lru:go_default_library",
"//vendor/k8s.io/api/admission/v1beta1:go_default_library",
"//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
Expand Down
12 changes: 7 additions & 5 deletions staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/config/client.go
View file
Open in desktop
Expand Up @@ -24,8 +24,10 @@ import (
"net/url"

lru "github.com/hashicorp/golang-lru"
admissionv1beta1 "k8s.io/api/admission/v1beta1"
"k8s.io/api/admissionregistration/v1beta1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
webhookerrors "k8s.io/apiserver/pkg/admission/plugin/webhook/errors"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -54,8 +56,13 @@ func NewClientManager() (ClientManager, error) {
if err != nil {
return ClientManager{}, err
}
admissionScheme := runtime.NewScheme()
admissionv1beta1.AddToScheme(admissionScheme)
Copy link
Member

Choose a reason for hiding this comment

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

This can return an error

Copy link
Member

Choose a reason for hiding this comment

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

@ash2k Not necessary, because admissionv1beta1.AddToScheme will never return an error.

Copy link
Member

Choose a reason for hiding this comment

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

If it will never return an error why does the function return an error? :) Tomorrow the implementation will change and something will not be right. E.g. some other code would be added before this function is called and it will cause it to error out. The worst thing about such situations is that it will be impossible to see the problem directly.
Russian proverb: "Even a stick shoots once a year" :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that we should at least wrap those calls and trigger a panic. But this must be a pattern through the whole code base.

return ClientManager{
cache: cache,
negotiatedSerializer: serializer.NegotiatedSerializerWrapper(runtime.SerializerInfo{
Serializer: serializer.NewCodecFactory(admissionScheme).LegacyCodec(admissionv1beta1.SchemeGroupVersion),
}),
}, nil
}

Expand All @@ -79,11 +86,6 @@ func (cm *ClientManager) SetServiceResolver(sr ServiceResolver) {
}
}

// SetNegotiatedSerializer sets the NegotiatedSerializer.
func (cm *ClientManager) SetNegotiatedSerializer(n runtime.NegotiatedSerializer) {
cm.negotiatedSerializer = n
}

// Validate checks if ClientManager is properly set up.
func (cm *ClientManager) Validate() error {
var errs []error
Expand Down
1 change: 0 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD
View file
Open in desktop
Expand Up @@ -15,7 +15,6 @@ go_library(
"//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//vendor/k8s.io/apiserver/pkg/admission:go_default_library",
Expand Down
4 changes: 0 additions & 4 deletions staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go
View file
Open in desktop
Expand Up @@ -31,7 +31,6 @@ import (
"k8s.io/api/admissionregistration/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
"k8s.io/apimachinery/pkg/runtime/serializer/json"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apiserver/pkg/admission"
Expand Down Expand Up @@ -131,9 +130,6 @@ func (a *MutatingWebhook) SetServiceResolver(sr config.ServiceResolver) {
// SetScheme sets a serializer(NegotiatedSerializer) which is derived from the scheme
func (a *MutatingWebhook) SetScheme(scheme *runtime.Scheme) {
if scheme != nil {
a.clientManager.SetNegotiatedSerializer(serializer.NegotiatedSerializerWrapper(runtime.SerializerInfo{
Serializer: serializer.NewCodecFactory(scheme).LegacyCodec(admissionv1beta1.SchemeGroupVersion),
}))
a.convertor.Scheme = scheme
a.defaulter = scheme
a.jsonSerializer = json.NewSerializer(json.DefaultMetaFactory, scheme, scheme, false)
Expand Down
1 change: 0 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD
View file
Open in desktop
Expand Up @@ -14,7 +14,6 @@ go_library(
"//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime: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/apiserver/pkg/admission:go_default_library",
"//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library",
Expand Down
4 changes: 0 additions & 4 deletions staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go
View file
Open in desktop
Expand Up @@ -31,7 +31,6 @@ import (
"k8s.io/api/admissionregistration/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/admission/configuration"
Expand Down Expand Up @@ -128,9 +127,6 @@ func (a *ValidatingAdmissionWebhook) SetServiceResolver(sr config.ServiceResolve
// SetScheme sets a serializer(NegotiatedSerializer) which is derived from the scheme
func (a *ValidatingAdmissionWebhook) SetScheme(scheme *runtime.Scheme) {
if scheme != nil {
a.clientManager.SetNegotiatedSerializer(serializer.NegotiatedSerializerWrapper(runtime.SerializerInfo{
Serializer: serializer.NewCodecFactory(scheme).LegacyCodec(admissionv1beta1.SchemeGroupVersion),
}))
a.convertor.Scheme = scheme
}
}
Expand Down