-
Notifications
You must be signed in to change notification settings - Fork 4.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
make webhook admission kind of work #16861
Changes from all commits
d7cdcba
0db40a6
ea12582
099f600
e8061fe
b8e3c19
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 |
---|---|---|
|
@@ -36,6 +36,7 @@ import ( | |
kubeclientgoinformers "k8s.io/client-go/informers" | ||
kubeclientgoclient "k8s.io/client-go/kubernetes" | ||
"k8s.io/client-go/rest" | ||
aggregatorapiserver "k8s.io/kube-aggregator/pkg/apiserver" | ||
kclientsetexternal "k8s.io/kubernetes/pkg/client/clientset_generated/clientset" | ||
kclientsetinternal "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" | ||
kexternalinformers "k8s.io/kubernetes/pkg/client/informers/informers_generated/externalversions" | ||
|
@@ -144,6 +145,14 @@ func NewPluginInitializer( | |
cloudConfig, | ||
restMapper, | ||
quotaRegistry) | ||
// upstream broke this, so we can't use their mechanism. We need to get an actual client cert and practically speaking privileged loopback will always have one | ||
kubePluginInitializer.SetClientCert(privilegedLoopbackConfig.TLSClientConfig.CertData, privilegedLoopbackConfig.TLSClientConfig.KeyData) | ||
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. Where can a webhook find the CA that signs this cert? 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.
This cert is signed by the client CA for the apiservers. Since the our webhook apiservers are run as aggregated API servers, they automatically trust that CA (though a user could wire it differently). That means that normal cluster authentication and authorization (rbac) rules apply. |
||
// this is a really problematic thing, because it breaks DNS resolution and IP routing, but its for an alpha feature that | ||
// I need to work cluster-up | ||
kubePluginInitializer.SetServiceResolver(aggregatorapiserver.NewClusterIPServiceResolver( | ||
informers.GetClientGoKubeInformers().Core().V1().Services().Lister(), | ||
)) | ||
|
||
openshiftPluginInitializer := &oadmission.PluginInitializer{ | ||
OpenshiftInternalAuthorizationClient: authorizationClient, | ||
OpenshiftInternalBuildClient: buildClient, | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
broke what?
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.
They wire to the wrong client cert: kubernetes/kubernetes#53828 . Also, it's built such that using the "normal" constructor produces a plugin initializer that will panic on a "normal" admission chain.
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.
That link helps. Next time I read your PR description.