Design: Dynamic registration of admission controllers and initializers #611

caesarxuchao · 2017-05-10T00:31:12Z

A sub-proposal of #132, proposal on Extensible Admission Control.

@smarterclayton @lavalamp @whitlockjc as initial reviewers.

nikhiljindal · 2017-05-10T17:44:39Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+
+Instead of having the two controllers do consistent read of the entire
+`AdmissionControlConfiguration` object, we let the registry store the
+resourceVersion of the `AdmissionControlConfiguration` (perhaps in a configMap),


There is no atomic update guarantee right? Apiserver can crash after updating the configmap but before updating AdmissionControlConfiguration.
We should be fine if we always update the configmap first.

Yeah, apiserver should always update the configmap first.

No, apiserver would check for changes and rebuild the configmap on startup if necessary.

lavalamp

the substantive point here is to reconcile how we locate the webhooks with how the aggregator locates user apiservers.

lavalamp · 2017-05-10T21:04:24Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+type ExternalAdmissionHook struct {
+    // Operations is the list of operations this hook will be invoked on - Create, Update, or *
+    // for all operations. Defaults to '*'.
+    Operations []string


should be an enum, not a string?

lavalamp · 2017-05-10T21:04:59Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    Operations []string
+    // Resources are the resources this hook should be invoked on. '*' is all resources.
+    Resources []string
+    // Subresources are the list of subresources this hook should be invoked on. '*' is all resources.


a subresource does not make sense without a resource? how does this work?

lavalamp · 2017-05-10T21:06:29Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    ClientConfig AdmissionHookClientConfig
+
+    // FailurePolicy defines how unrecognized errors from the admission endpoint are handled -
+    // allowed values are Ignore, Retry, Fail. Default value is Fail


should retry be in at all? if yes, should it be in our first implementation?

lavalamp · 2017-05-10T21:07:29Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+type AdmissionHookClientConfig struct {
+	// Address of the external admission hook, could be a host string, 
+    // a host:port pair, or a URL.
+	Address string


the aggregator uses a service instead of directly listing an address.

lavalamp · 2017-05-10T21:09:41Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+	// Address of the external admission hook, could be a host string, 
+    // a host:port pair, or a URL.
+	Address string
+	// ClientCertificate is the path to a client cert file for TLS.


This cannot include a client cert. Not as a file path, that doesn't make sense in a dynamic config object (won't work in envs where users don't have access to apiserver disk, which is half of the point of the dynamic config).

Instead, we have to make available a CA that the webhook trusts.

lavalamp · 2017-05-10T21:11:53Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+	// CertificateAuthority is the path to a cert file for the certificate authority.
+	CertificateAuthority string
+	// CertificateAuthorityData contains PEM-encoded certificate authority certificates. Overrides CertificateAuthority
+	CertificateAuthorityData []byte


Keep this. Take a look at the aggregation registration api types.

lavalamp · 2017-05-10T21:12:49Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // If timeout is reached, the intializer is removed from the resource's
+    // initializer list by the apiserver.
+    // Default to XXX seconds.
+    Timeout *int64


a per-resource per-initializer timeout seems extreme. Better to have a global timeout?

lavalamp · 2017-05-10T21:14:18Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+If the `initializer admission controller` and the `generic webhook admission
+controller` watch the `AdmissionControlConfiguration` and act upon deltas, their
+cached version of the configuration might be arbitrarily delayed. This makes it
+impossible to predicate what initializer/hooks will be applied to newly created


s/predicate/predict/

lavalamp · 2017-05-10T21:16:20Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+
+Instead of having the two controllers do consistent read of the entire
+`AdmissionControlConfiguration` object, we let the registry store the
+resourceVersion of the `AdmissionControlConfiguration` (perhaps in a configMap),


No, apiserver would check for changes and rebuild the configmap on startup if necessary.

lavalamp · 2017-05-10T21:20:16Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+We considered a few ways to make the behavior of the `initializer admission
+controller` and the `generic webhook admission controller` predictable.
+
+(I prefer #2. #1 is inefficient, #3 requires complex schema and is not intuitive)


Thanks for the clear specification of the options. IMO 2 is an optimization of 1, we should see if 1 is really problematic before doing the extra work. I would guess that an extra trip to etcd is going to be an insignificant amount of latency compared to the initializers/webhooks themselves. @smarterclayton agree?

@wojtek-t This is as good a place as any to ask about how this is going to impact our SLO, whether the 1 second return SLO will still make sense for create requests.

@lavalamp - thanks for looping me in.

So I added a question about #2: https://github.com/kubernetes/community/pull/611/files#r116245850 which I think I don't understand.

The additional read from etcd for every create request sounds problematic, but we need to do some math to compute how many QPS it will generate.
I think that the only thing we are afraid about it latency of the call to etcd. We probably should care mostly about larger clusters. So let's say we are targeting 100pods/s throughput. So there will be O(100) create requests then.
In 5000-node clusters, we have 500 writes/s of just Node updates, so it's not that in such clusters we will have 10x more requests - it will be more like 10-20%. Still a lot though.

caesarxuchao · 2017-05-10T23:29:57Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    Operations []OperationType
+    // Resources are the resources this hook should be invoked on. '*' is all resources.
+    Resources []string
+    // Subresources is list of subresources. If non-empty, this hook should be invoked on 


@smarterclayton i changed the comment. Does it match what you have in mind?

liggitt · 2017-05-11T00:39:20Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // for all operations. Defaults to '*'.
+    Operations []OperationType
+    // Resources are the resources this hook should be invoked on. '*' is all resources.
+    Resources []string


How is API group specified?

Updated to include the group name.

caesarxuchao · 2017-05-11T22:07:16Z

@lavalamp @smarterclayton ptal. Thanks.

lavalamp · 2017-05-11T23:03:16Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+type AdmissionHookClientConfig struct {
+	// Address of the external admission hook, could be a host string, 
+    // a host:port pair, or a URL.
+	Address string


OK, I still think this needs to be the same as the api here https://github.com/mbohlool/kubernetes/blob/54fb3a6e7a3dc88bfb4f9cd1e9ec6d7290d016d6/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/types.go

…ok's address

smarterclayton · 2017-05-12T01:26:57Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+modifications.
+
+```golang
+type AdmissionControlConfiguration struct {


I had assumed that we would want to allow components to register initializers independently. I.e. the component that provides "foo" initializer might be completely decoupled from initializer "bar", and therefore want to register in a decoupled way. That however assumes that two components can lazily define "before" and "after".

I think in general, most initializers don't care about before or after, and simply want a chance to fire. For those that do... what should we do?

I thought we would never trust initializers to register themselves. I thought it would be a human admin who wrote ACC. A single careless initializer could destroy the carefully ordered list of initializers, right?

I'm just thinking that most initializers are going to be like 3rd party controllers. They're independently delivered. If we have one resource, the install steps for those controllers are going to be ugly patch statements. If we have multiple resources, we'll have to declare dependencies.

Sure. In the future, will we want to enforce order among webhooks? If not, we can make the webhook part of ACC multiple instances.

I think webhook "phases" or "before or after" are inevitable.

I think a single ACC is much easier if both intilaizers and webhooks will have order.

If the initializer and webhook wish to do self registration, they can specify partial order with the latest strategic merge patch syntax.

What would that look like?

-1 for using PATCH for installation.

I think we probably eventually want the user to POST all the initializers/webhooks, and then a controller assembles the ordered list. However I think the thing Chao has here will be a lot easier to implement in the remaining time. Having this list here also lets us push off modeling the dependency graph into the future. So I propose to keep it as is for 1.7, even though it means installing these things will be a bit harder.

According to https://github.com/kubernetes/community/pull/537/files, it would look like:

resourceInitializers: - resource $setElementOrder/initializers: - existing_initialier_a - existing_initialier_b - new_initializer - existing_initialier_c initilizers: - name: new_initiailizer failurePolicy: "ignore"

I'm ok with harder for now.

smarterclayton · 2017-05-12T01:45:57Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+before applying the configuration to the incoming objects. This adds latency to
+every CREATE request. Because the two admission controllers are in the same
+process as the apiserver, the latency mainly consists of the consistent read
+latency of the backend storage (etcd), and the proto unmarshalling.


Fairly expensive today.

Thinking about this... watch from within the same etcd server is effectively free. We need to guarantee that the clock time for any given ACC is within some bound (because an admission controller can be arbitrarily delayed between when the data is read from etcd and when it applies the rules, so a hard boundary is probably too hard).

The scenario of greatest concern is when an apiservers watch client is delayed relative to the mutation operations being executed, perhaps due to a network hang. If we had a single etcd watch on a set of key ranges, our minimum bound is:

The most recent admission control configuration delivered on the same watch channel as any returned event on that watch channel must be no older than X seconds after the last update operation on this server was performed, otherwise a consistent read must be performed.

That can degrade to just performing a consistent read from each api server every X seconds and delaying / rejecting requests until the read returns.

Said another way, our etcd client simply has to ensure that the open watch channel from the server that also watches ACC has delivered ANY event within the time window we care about.

I don't know if the etcd client and server preserve the guarantee that would be necessary for the optimization there though. The client has to construct a watch, and the etcd server has to guarantee a "delivered before" behavior over the connection (event on ACC key has to be delivered before another key with higher index). gRPC substreams (one for each watch) might not support that if two watches are in different sub streams because the packets for the one stream can be delayed.

Or the api server write to the endpoints object has to track the member observed high etcd index and delay writes if other members of the cluster have higher observed indices (if the watch cache value is higher than the read value of the GET on endpoints)

I kind of like the upper bound and the degrading thing. Will make e2e tests slow though.

So from first principles:

admin must be able to know when config is applied

admin can always restart an apiserver and guarantee it sees the latest config (fallback)

admin needs to be able to ensure config eventually applies within some bound

Those are good goals for an alpha api. To just do 3, could we:

do a consistent read every 30s

if the consistent read fails, block the admission plugin (fail) until it succeeds

Tests would have to get clever about verifying the config is working. If we're concerned, we can simply add a watch on the config objects and always use the latest, but the consistent read must succeed every 30s. That would allow tests to work most of the time.

Why is 2 an explicit goal? That sounds very easy to achieve.

Sure. I'll add this option to the doc, and let's see if we are on the same page.

I just mean as - it's the thing that they have to do today, and it's worked so far. So it's ok to say "you must restart your masters to guarantee the latest config applied", which is probably reasonable.

@smarterclayton - got it. Thanks a lot for explanation. That makes perfect sense to me.
I added a potential improvement for it in #611 (comment)

wojtek-t · 2017-05-12T14:36:41Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+Instead of having the two controllers do consistent read of the entire
+`AdmissionControlConfiguration` object, we let the registry store the
+resourceVersion of the `AdmissionControlConfiguration` (perhaps in a configMap),
+and let the two controllers always do consistent read of the resourceVersion and


Sorry, I think I don't understand this option. Where is the "resourceVersion of the ACC" coming from? Do we still get it from etcd?
If not (just from local cache), I don't understand how it would work e.g. in HA setup.

The apiserver that handles the ACC update will persist the updated ACC's rv in a configmap. When updating the rv, the apiserver needs to ensure its increasing.

caesarxuchao · 2017-05-12T21:10:08Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+We propose two ways to make the behavior of the `initializer admission
+controller` and the `generic webhook admission controller` predictable.
+
+#### 1. Do consistent read of AdmissionControlConfiguration periodically


@wojtek-t how do you like this approach suggested by @smarterclayton ? It should be much lighter.

Let's start out with this option. Seems way easier. It is not super important that these things get applied instantly.

@caesarxuchao - I'm completely fine with this option.
I would even go step further, and do:

let's do consistent read every 1s

if there wasn't any successful read over last 5s (or 10s) then block everythin

1qps will not be visible and this will give us some tolerance on unsuccessful reads (which may happen from time to time).

WDYT?

@wojtek-t it sounds good. I'll incorporate it to the doc.

caesarxuchao · 2017-05-12T21:11:05Z

@smarterclayton i incorporated your comments to the doc, ptal, thanks.

lavalamp

looking good, just a couple points

lavalamp · 2017-05-13T00:13:15Z

contributors/design-proposals/dynamic-admission-control-configuration.md

-	// Address of the external admission hook, could be a host string, 
-    // a host:port pair, or a URL.
-	Address string
+    // Service is a reference to the service for this webhook. It must communicate


I would say-- if multiple ports are exposed, it will use 443 if available and be an error condition otherwise. If only a single port is exposed it can just use that.

lavalamp · 2017-05-13T00:14:54Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+We propose two ways to make the behavior of the `initializer admission
+controller` and the `generic webhook admission controller` predictable.
+
+#### 1. Do consistent read of AdmissionControlConfiguration periodically


Let's start out with this option. Seems way easier. It is not super important that these things get applied instantly.

lavalamp · 2017-05-13T00:15:14Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+   to introduce the additional concept of "readiness".
+
+
+## Considered bug REJECTED synchronization mechinism:


lavalamp · 2017-05-13T00:18:16Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+the configuration to an incoming request. If the configmap has changed, then
+they do a consistent read of the `AdmissionControlConfiguration`.
+
+## What if an initializer controller/webhook is not ready after registered? (**optional for alpha implement**)


this is unclear to me-- is the first step going to create things fail-open or fail-closed?

(I think we should do fail-open for 1.7 and add fail-closed behavior later when this feature grows up)

As long as it is explicitly specified that it is fail open

caesarxuchao · 2017-05-15T19:36:54Z

@smarterclayton ptal. I'll start the implementation after your approval.

caesarxuchao · 2017-05-15T19:38:03Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // apiserver removes initilizer from the initializers list of the resource
+    // if the timeout is reached; If "Fail" is set, apiserver returns timeout
+    // error if the timeout is reached.
+    FailurePolicy FailurePolicyType


@smarterclayton do you agree with adding FailurePolicy to initializers? It's not in your original proposal #132.

Use case here is "the system doesn't grind to a halt"?

Who in this scenario reaps these? Without this, initializers don't need a controller (or we can avoid it). With it, we must have one.

Use case here is "the system doesn't grind to a halt"?

Yes.

Yes, we'll need a controller. I thought we could let apiserver do the reaping, but lavalamp pointed out apiserver could crash.

Or we can say initializers always fail closed for 1.7.

smarterclayton · 2017-05-15T21:24:18Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+The `initializer admission controller` and the `generic webhook admission
+controller` do a consistent read of the configmap *everytime* before applying
+the configuration to an incoming request. If the configmap has changed, then
+they do a consistent read of the `AdmissionControlConfiguration`.


Section for future work here?

lavalamp · 2017-05-15T21:26:58Z

fail-closed seems more dangerous to me than fail-open does...

…

On Mon, May 15, 2017 at 2:24 PM, Chao Xu ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In contributors/design-proposals/dynamic-admission-control- configuration.md <#611 (comment)>: > + // initializer list by the apiserver. + // Default to XXX seconds. + Timeout *int64 +} + +type Initializer struct { + // Name is the string that will be registered to the resource that needs + // initialization. + Name string + + // FailurePolicy defines what happens if there is no initializer controller + // takes action. Allowed values are Ignore, or Fail. If "Ignore" is set, + // apiserver removes initilizer from the initializers list of the resource + // if the timeout is reached; If "Fail" is set, apiserver returns timeout + // error if the timeout is reached. + FailurePolicy FailurePolicyType Or we can say initializers always fail closed for 1.7. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#611 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAnglrN7dFctUU0CZ3M3zK7sZJ968TOiks5r6MKkgaJpZM4NWCu8> .

lavalamp · 2017-05-15T21:32:13Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // If timeout is reached, an intializer is removed from the resource's
+    // initializer list by the apiserver.
+    // Default to XXX seconds.
+    Timeout *int64


Does a per-resource timeout make sense?

Does the timeout need to be customizable at all? What bad thing happens if we hard-code 30s?

Will we ever have an initializer that takes longer than 30s?

We can start with a global default of 30s. In Clayton's proposal i don't see any existing mutating admission controller that's going to take longer than 30s.

whitlockjc · 2017-05-19T22:27:50Z

I'll give the new version a peek over the weekend.

caesarxuchao · 2017-05-22T17:08:48Z

@whitlockjc @lavalamp @smarterclayton @deads2k ptal. Thanks!

deads2k · 2017-05-22T19:27:43Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // ExternalAdmissionHooks is a list of external admission webhooks and the
+    // affected resources and operations.
+    // +optional
+    ExternalAdmissionHooks []ExternalAdmissionHook `json:"externalAdmissionHooks,omitempty" patchStrategy:"merge" patchMergeKey:"name"`


webhooks are never ordered with respect to initializers, so why are these in the same object?

deads2k · 2017-05-22T19:28:52Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // fails to takes action. Allowed values are Ignore, or Fail. If "Ignore" is
+    // set, initializer is removed from the initializers list of an object if
+    // the timeout is reached; If "Fail" is set, apiserver returns timeout error
+    // if the timeout is reached.


What is the default value?

deads2k · 2017-05-22T19:30:13Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // Name is the identifier of the initializer. It will be added to the
+    // object that needs to be initialized.
+    // Required
+    Name string `json:"name"`


Let's start with forcing these to be fully qualified names. At least two dots: openshift.io should be illegal, but scc.openshift.io should be legal

Do you mean enforcing the rule via validation?

deads2k · 2017-05-22T19:30:27Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+type ExternalAdmissionHook struct {
+    // The name of the external admission webhook.
+    // Required.
+    Name string `json:"name"`


same qualified name comment.

caesarxuchao · 2017-05-22T21:27:56Z

Addressed @deads2k 's comments.

lavalamp

/lgtm

lavalamp · 2017-05-22T21:44:48Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+type Initializer struct {
+    // Name is the identifier of the initializer. It will be added to the
+    // object that needs to be initialized.
+    // Name should be fully qualified.


give example?

lavalamp · 2017-05-22T21:46:10Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // APIVersions is the API versions the resources belong to. '*' is all versions.
+    // If '*' is present, the length of the slice must be one.
+    // Required.
+    APIVersions []string `json:"apiVersions,omitempty"`


What if I set groups to [apps, batch, core] and versions to [v1, v2alpha1, v1beta1], and not all versions are in all groups?

It's recommended to separate them to different Rules, and make sure in each Rule all the tuple expansions make sense. The best practice is documented in the struct comment.

Also, even if the best practice is not followed, the Rule still works, the invalid tuple expansion is ignored.

lavalamp · 2017-05-22T22:33:17Z

/lgtm

smarterclayton · 2017-05-23T05:00:33Z

Lgtm as well On May 22, 2017, at 6:33 PM, Daniel Smith <notifications@github.com> wrote: /lgtm — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#611 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_pw1X0fz2j9WuxTiTd-oPFU83govJks5r8g0vgaJpZM4NWCu8> .

whitlockjc · 2017-05-23T13:41:33Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    // for all verbs.
+    // If '*' is present, the length of the slice must be one.
+    // Required.
+    Verbs []OperationType `json:"verbs,omitempty"`


We are calling these Verbs but they are not, they are admission.Operations. I think we should be consistent with admission.Attributes and use Operations as the struct member name.

Also, why isn't Operations just a member of Rule instead of being wrapped in another type?

rbac called it verbs. I think being consistent with rbac is more important, because it's user-facing.

The rule doesn't contain verbs because it's used by initializers which only cares about CREATE.

I don't think this is the case here. If someone were writing a Go-based AC, they would be operating with admission.Attributes and since our rules will filter/match based on admission.Attributes, I think that is the contract. I get that RBAC consistency is nice but I think this is one case where it's more confusing to do so.

/cc @deads2k @smarterclayton

@whitlockjc you are right. I thought authorizer interface and admission interface would use the same term, but the are not, authorizer interface's method is called GetVerb, while the admission interface's method is called GetOperation. Is that intentional?

whitlockjc · 2017-05-23T13:42:42Z

contributors/design-proposals/dynamic-admission-control-configuration.md

+    Rule `json:",inline"`
+}
+
+type OperationType string


I really don't see the purpose of mirroring admission.Operation here. I don't see it happening but what happens if an admission.Operation is modified or added or removed? Why not just use admission.Operation as the type?

Let the api depends on the admission package sounds wrong. For example, if we decide to move the API to k8s.io/api, i don't want to make k8s.io/api depends on k8s.io/apiserver.

But since we're building against/for admission controllers, why is depending on admission wrong?

/cc @deads2k @smarterclayton

It will create import cycle.

Well, that would depend on where we put these new types.

I see you are already doing it in pkg/apis/admission: https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/admission/types.go#L52

I think reversing the dependency is better, i.e. putting the definition in the API definition, because the API packages are usually the leaves in the dependency graph.

I'm still confused as to what goes into an API group vs. what ends up elsewhere. I could see some of these things in plugin/pkg/admission ending up in pkg/apis/admission myself. As for dependency stuff, I'd rather a stable object model than one that has multiple sources of truth. I realize that the chances of this are low right now but that doesn't mean impossible. Having admission.Operation being used by the internal admission stuff and having a shadow type to reflect it seems like one link that's easy to break. Just my two cents.

whitlockjc · 2017-05-23T13:48:09Z

I had a couple of consistency issues where our new types deviate from the types in admission. Since we are working with admission.Attributes objects, I think we should have consistent naming and types instead of new members/types that do not match their admission equivalent.

smarterclayton · 2017-05-24T03:50:40Z

We should be consistent with admission interface unless we have a good reason not to.

…

On Tue, May 23, 2017 at 5:00 PM, Chao Xu ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In contributors/design-proposals/dynamic-admission-control- configuration.md <#611 (comment)>: > + Rules []RuleWithVerbs `json:"rules,omitempty"` + + // FailurePolicy defines how unrecognized errors from the admission endpoint are handled - + // allowed values are Ignore or Fail. Defaults to Ignore. + // +optional + FailurePolicy *FailurePolicyType +} + +// RuleWithVerbs is a tuple of Verbs and Resources. It is recommended to make +// sure that all the tuple expansions are valid. +type RuleWithVerbs struct { + // Verbs is the verbs the admission hook cares about - CREATE, UPDATE, or * + // for all verbs. + // If '*' is present, the length of the slice must be one. + // Required. + Verbs []OperationType `json:"verbs,omitempty"` @whitlockjc <https://github.com/whitlockjc> you are right. I thought authorizer interface and admission interface would use the same term, but the are not, authorizer interface's method is called GetVerb <https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/authorization/authorizer/interfaces.go#L33>, while the admission interface's method is called GetOperation <https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/admission/interfaces.go#L43>. Is that intentional? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#611 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p1FZDiiUa6MKFtY1COLeLb3a4_eDks5r80kCgaJpZM4NWCu8> .

…ation-prototype Automatic merge from submit-queue (batch tested with PRs 46383, 45645, 45923, 44884, 46294) Dynamic registration prototype Implementing the api proposed in kubernetes/community#611. Wiring the code to serve the api via apiserver. ```release-note Adding admissionregistration API group which enables dynamic registration of initializers and external admission webhooks. It is an alpha feature. ```

@smarterclayton

Automatic merge from submit-queue Configuration manager for dynamic admission control registration Implementing this [section](https://github.com/kubernetes/community/blob/master/contributors/design-proposals/dynamic-admission-control-configuration.md#synchronization-of-admission-control-configurations) of kubernetes/community#611 Adding a configuration manager that reads the ExternalAdmissionHookConfigurations and InitializerConfigurations periodically, and returns the merged configuration. cc @smarterclayton @whitlockjc

…er-admission Design: Dynamic registration of admission controllers and initializers

* add the release manager for Istio 1.11 * sort the release manager name according to alpha order

caesarxuchao assigned smarterclayton May 10, 2017

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 10, 2017

proposal for dynamic admission controller configuration

b81ec0a

caesarxuchao force-pushed the dynamic-initializer-admission branch from 4b6389a to b81ec0a Compare May 10, 2017 00:33

nikhiljindal reviewed May 10, 2017

View reviewed changes

lavalamp reviewed May 10, 2017

View reviewed changes

address lavalamp's comments

9d02a4a

caesarxuchao force-pushed the dynamic-initializer-admission branch from e9479cb to 9d02a4a Compare May 10, 2017 23:29

caesarxuchao commented May 10, 2017

View reviewed changes

liggitt reviewed May 11, 2017

View reviewed changes

qualify Resource with Group

b976d26

lavalamp reviewed May 11, 2017

View reviewed changes

caesarxuchao changed the title ~~Proposal: Dynamic registration of admission controllers and initializers~~ Design: Dynamic registration of admission controllers and initializers May 12, 2017

register the service for the webhook instead of registering the webho…

72dbdf7

…ok's address

smarterclayton reviewed May 12, 2017

View reviewed changes

wojtek-t reviewed May 12, 2017

View reviewed changes

address clayton's comments

2102ef9

caesarxuchao commented May 12, 2017

View reviewed changes

lavalamp reviewed May 13, 2017

View reviewed changes

wojtek-t mentioned this pull request May 13, 2017

Design proposal for adding priority to Kubernetes API #604

Merged

caesarxuchao commented May 15, 2017

View reviewed changes

address more comments; incorporate wojtek-t's suggestion

b3ada68

caesarxuchao force-pushed the dynamic-initializer-admission branch from e6f03ee to b3ada68 Compare May 15, 2017 19:45

smarterclayton reviewed May 15, 2017

View reviewed changes

lavalamp reviewed May 15, 2017

View reviewed changes

address comments

950375f

caesarxuchao force-pushed the dynamic-initializer-admission branch from dd9d024 to 950375f Compare May 20, 2017 00:02

deads2k reviewed May 22, 2017

View reviewed changes

address deads2k's comments

ac735ba

lavalamp reviewed May 22, 2017

View reviewed changes

address lavalamp's comment

2b1e4a3

lavalamp self-assigned this May 22, 2017

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2017

smarterclayton merged commit f929dac into kubernetes:master May 23, 2017

whitlockjc reviewed May 23, 2017

View reviewed changes

whitlockjc mentioned this pull request May 23, 2017

add generic webhook admission controller kubernetes/kubernetes#46316

Closed

This was referenced May 24, 2017

Dynamic registration prototype kubernetes/kubernetes#46294

Merged

Configuration manager for dynamic admission control registration kubernetes/kubernetes#46302

Merged

pires mentioned this pull request Jun 28, 2017

Proposal: Google Cloud IAM Auth Backend hashicorp/vault#2813

Closed

caesarxuchao mentioned this pull request Nov 28, 2017

Continuous polling of webhook admission registration objects kubernetes/kubernetes#56357

Closed

MadhavJivrajani pushed a commit to MadhavJivrajani/community that referenced this pull request Nov 30, 2021

Merge pull request kubernetes#611 from caesarxuchao/dynamic-initializ…

cc5d821

…er-admission Design: Dynamic registration of admission controllers and initializers

danehans pushed a commit to danehans/community that referenced this pull request Jul 18, 2023

add the release manager for Istio 1.11 (kubernetes#611)

582369e

* add the release manager for Istio 1.11 * sort the release manager name according to alpha order

		to introduce the additional concept of "readiness".


		## Considered bug REJECTED synchronization mechinism:

Design: Dynamic registration of admission controllers and initializers #611

Design: Dynamic registration of admission controllers and initializers #611

Conversation

caesarxuchao commented May 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lavalamp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented May 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarterclayton May 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented May 12, 2017

lavalamp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented May 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lavalamp commented May 15, 2017 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whitlockjc commented May 19, 2017

caesarxuchao commented May 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao commented May 22, 2017

lavalamp left a comment

smarterclayton May 12, 2017 •

edited

Loading