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.

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

Proposal: Extensible Admission Control #132

Merged
merged 7 commits into from
May 12, 2017

Conversation

smarterclayton
Copy link
Contributor

This is a very rough first draft of the proposal for how to make admission control extensible. It covers the reasoning and background at a high level but I'm still pulling together the proposed design (one exists, just not in paper).

Basically, add a new "initialization" phase for resources that is opaque to old clients but controllers can opt in to seeing uninitialized objects - the last initializer makes the object visible to everyone else. Also, add non-mutating generic webhooks for doing remotable checks against a server (like SubjectAccessReview). Make both of those dynamically configurable via some core API object.

@kubernetes/sig-api-machinery @bgrant0607

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 2, 2016
@smarterclayton smarterclayton force-pushed the external_admission branch 5 times, most recently from 9060b1e to e8cce45 Compare December 2, 2016 04:04
@smarterclayton
Copy link
Contributor Author

This is also a review of all admission controllers known to exist at the present time, so good reading if you want to know what stuff is being used.

Copy link
Member

@derekwaynecarr derekwaynecarr left a comment

Choose a reason for hiding this comment

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

I think we should identify a handful of existing admission controllers as first candidates to move to this model to prove it out.

I need to think through the quota pieces more. I had thought we could keep it as-is even w/ initializers for the near term. It's possible I am mistaken.

distribution becomes unweildy and limits administrators and the growth of the ecosystem.

This proposal covers changes to the admission control subsystem that allow extension of admission without recompilation
and dynamic adminission control configuration in ways that resemble existing controller behavior.
Copy link
Member

Choose a reason for hiding this comment

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

s/adminission/admission

2. This mechanism would allow dedicated infrastructure to host admission for multiple clusters, and allow some expensive admission to be centralized (like quota which is hard to performantly distribute)
3. There is no way to build initializers for updates without a much more complicated model, but we anticipate initializers to work best on creation.
4. Ordering will probably be necessary on initializers because defaulting in the wild requires ordering. Non-mutating validation on the other hand can be fully parallel.
5. Some admission depends on knowing the identity of the actor - we will likely need to include the **creator** as information to initializers.

Choose a reason for hiding this comment

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

The creator here is referring to User ID/serviceaccount which initiated the object creation or referring to UID of the objects e.g Deployment UID in case of replicaset.

As having User ID/serviceaccount access in admission controller and decorating object creation with label as who created this can be very useful for auditing purpose.

Copy link
Member

Choose a reason for hiding this comment

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

Two thoughts here:

  1. The more opportunity there has been for modification of an object by other users, the less useful the information about the creating user is. I would suggest clearing this information at the end of initialization.
  2. If field-level authorization is implemented using initialization, we have to be aware of the power an initializer has. Consider:
  • cluster admin creates an unprivileged pod
  • initializer A replaces the pod image with a malicious image and changes the pod security context to privileged
  • initializer B checks that the creating user ("cluster admin") is authorized to run a privileged pod (they are)

I think that means that (at least to start) initializers are considered highly trusted and need to be under system control.

Copy link
Member

Choose a reason for hiding this comment

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

The general problem of constraining the order in which initializers can execute is one of the key design issues for this proposal IMO. It's not just a security issue, as @liggitt pointed out, but also a general "not all initializers compose in any arbitrary order sensibly" problem. I don't think it will ever be possible to truly arbitrarily mix-and-match admission controllers. I think this is true if all of your admission controllers are one type (webhook/synchronous or initializer/asynchronous) and even more true if you try to mix the two types (because no initializer style admission controller can run until all of the webhook style admission controllers have run, and no webhook style admission controller can run after any of the initializer style admissoin controllers). We should think carefully about how to spec admission controllers so that administrators know what are the safe and semantically valid combinations (and orders) for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think field level authorization is an admission controller that is earlier in the chain and not necessarily part of this problem statement. It is not necessary to invert the core api server admission to allow a third party to impose field level authorization if we design field level authorization correctly. All other specialized use cases are handled by external ordering.

In general, anything like field level authorization would be a non-mutating admission controller, so those would not be granted the rights to modify anything.

Initializer A in your example has to go through normal admission. So it can't escape anything that a normal update would allow.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Question: are the webhook admission controllers going to be run when an initializer updates an object during the initialization phase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As written, yes. We could define that further as we go.

@brendandburns
Copy link
Contributor

In general, the approach looks ok, though I guarantee you that the first thing someone is going to do is implement a generic one that uses JavaScript/Lua/... so that users don't have to build yet another binary in order to run a custom policy.

Also, devil is in the implementation details here, I'd like to see that fleshed out.

In particular, I'm terrified of a bug where an object gets 'stuck' in a partial state, and a user can't see it via the traditional tools, yet can't create a new one because the stuck one is blocking it.

We need complete implementation details in the proposal before we can really understand the proposal.

Thanks!

@fabiand
Copy link

fabiand commented Dec 16, 2016

@brendandburns … That was actually what polkit (used in fedora and GNOME and others) ended up with. Adding an javascript interpreter to allow writing complex policies. So maybe not such a bad idea if it lands. I'm referring to the generic admission controller here …


### Requirements

1. Easy Initialization
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest Easy Initialization (bold), otherwise the heading runs into the rest of the line (for each item).

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the newline doesn't create a line break when viewing markdown.

occurs as part of creation, and a large chunk of the remaining controllers are for side-effect free
validation of creation and updates. Therefore we propose the following changes to Kubernetes:

1. Allow some controllers to act as "initializers" - watching the API and mutating the object before it is visible to normal clients.
Copy link
Member

Choose a reason for hiding this comment

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


Some admission controller types would not be possible for these extensions:

* Mutating admission webhooks could be a later addition
Copy link
Member

Choose a reason for hiding this comment

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

Is there some reason this is hard, if you don't run them in parallel? Couldn't the webhook reply with a mutated copy of the object it was sent? Or did you mean initializer here (I can see how initializer for the update case might be impossible).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only that there is no compelling need to implement them right now. Out of the 40 some admission controllers, only two had mutate on update requirements.

Choose a reason for hiding this comment

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

I don't think it's fair to say there is no compelling need to implement mutating admission controllers. I get your point that there are only two mutating admission controllers now but what people need out of extensible admission controllers is not directly related to what is already available in the built-in admission controllers. If people only needed what was already available, why have extensible admission controllers at all? I think if people could write extensible admission controllers, we'd very quickly run into the needs for mutation.

To support this train of thought, I realize I'm not everyone but my first to needs for extensible admission controllers were for mutating purposes:

  • As a cluster administrator, I wanted to make sure that all namespaces created had network isolation enabled
  • As a cluster administrator, I wanted to deploy sidecar containers for routing and service discovery purposes (intra-cluster routing, API Management, ...)

I really think we should reconsider this.

Copy link
Member

Choose a reason for hiding this comment

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

@whitlockjc I think the claim is that mutating controllers could be implemented using initializers.

Choose a reason for hiding this comment

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

I don't disagree but to me, "initializer" sounds like it only works with CREATE operations but sometimes you need mutations other operations. For example, if you want to do network isolation at the namespace level, one approach would be to disallow the modification (including deletion) of the annotations at the namespace level that drive network isolation. This is possibly a contrived example since you should be able to do this via authz but you get my drift.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think webhooks may be preferred by some developers due to their simplicity - 'Provided a request, decide what to do with it.' I'm not sure of the wisdom of forcing the controller-based approach if mutation is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, there are definitely people who would prefer a webhook. And as brendan noted, web hooks become more powerful in the age of FaaS.

That being said, our standard pattern is simple mutations of strongly consistent objects, and all of our tools and mechanisms protect that. Admission has always been an "escape hatch". Initializers are not escape hatches, they are designed behavior of the system. I think initializers and hooks can co-exist as mentioned elsewhere in the proposal - initializers as our standard decorator, admission as the "do anything" pattern.

For the scales we are considering for OpenShift, the initializer pattern has several advantages including increased audit and observability, clear security rules, and the potential to see the changes in motion ahead of time.

Copy link
Member

Choose a reason for hiding this comment

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

One nice thing about not having mutating webhooks is that we don't have to have any notion of a dependency graph. Initializers can implement dependencies for themselves if they wish. This greatly simplifies the mechanism. (If not the lives of the initializers.) It might actually be worth explicitly stating this in this proposal somewhere.

This would reuse the majority of the infrastructure in place for controllers. Because creation is
one-way, the object can be "revealed" to regular clients once a set list of initializers is consumed. These
controllers could run on the cluster as pods.
2. Add a generic **external admission webhook** controller that is non-mutating (thus parallelizable)
Copy link
Member

Choose a reason for hiding this comment

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

You should say "admission controller" instead of "controller" to make it clear you're talking about something that's like today's admission controllers, i.e. part of the synchronous admission path. Also, why can't you put an ordering on these (instead of running them in parallel) and then allow them to be mutating? If they're not mutating, it's why does it have to be non-mutating? Can't the webhook reply with a mutated. BTW is the only reason this approach needed because there's no obvious way to do update admission with the initializer approach?

Copy link

@whitlockjc whitlockjc Dec 23, 2016

Choose a reason for hiding this comment

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

I agree with @davidopp. The main reasons I've needed extensible admission controllers is to do mutations, like locking down namespaces via network isolation or adding sidecar containers. I understand the reasoning behind the limitation but I think there are cases where mutation is desired for webhooks admission controllers. (That desire might be because there is no other alternative right now. I finished kubernetes/kubernetes#34348 today, the first few webhook implementations would be mutating though.) This being said, I wonder if there is a better way? Completely shooting from the hip here but what if admission controllers identified themselves as mutating or not and this data was used to create an execution plan, optimizing where possible?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Out of all of the use cases identified for admission control so far, only a very small set were best solved by admission mutation. The vast majority of admission mutation would either be solved by field level authorization (best), RBAC by type (good), or better API design (better).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we can identify the use cases underlying mutation protection that aren't field level authorization, I'd be happy to add that into the proposal and help find the right solutions to make it possible.

Choose a reason for hiding this comment

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

I mentioned two use cases that are already necessary for cluster administrators (automatic network isolation enablement) and pod sidecar containers. Should these be added to some bullet list somewhere so we can keep a running total?

Copy link

Choose a reason for hiding this comment

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

while I like the idea of being able to parallelize these, I think you'll still want some mechanism in there so that an admin can require some set be run in serial before all others - e.g. auth type of checks so that people don't even get a chance to invoke (and potential get exposure to the inner workings of things) that they shouldn't even know exist. A security concern.

Copy link
Member

Choose a reason for hiding this comment

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

@smarterclayton Is performance the reason to permit parallelism?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplicity is a big one, I think mutating update admission plugins are potentially a sign of bad API design (or other gaps, like field level authorization) so I'm trying to avoid excessively depending on them. But I'm not opposed to solving for the general problem.

2. This mechanism would allow dedicated infrastructure to host admission for multiple clusters, and allow some expensive admission to be centralized (like quota which is hard to performantly distribute)
3. There is no way to build initializers for updates without a much more complicated model, but we anticipate initializers to work best on creation.
4. Ordering will probably be necessary on initializers because defaulting in the wild requires ordering. Non-mutating validation on the other hand can be fully parallel.
5. Some admission depends on knowing the identity of the actor - we will likely need to include the **creator** as information to initializers.
Copy link
Member

Choose a reason for hiding this comment

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

The general problem of constraining the order in which initializers can execute is one of the key design issues for this proposal IMO. It's not just a security issue, as @liggitt pointed out, but also a general "not all initializers compose in any arbitrary order sensibly" problem. I don't think it will ever be possible to truly arbitrarily mix-and-match admission controllers. I think this is true if all of your admission controllers are one type (webhook/synchronous or initializer/asynchronous) and even more true if you try to mix the two types (because no initializer style admission controller can run until all of the webhook style admission controllers have run, and no webhook style admission controller can run after any of the initializer style admissoin controllers). We should think carefully about how to spec admission controllers so that administrators know what are the safe and semantically valid combinations (and orders) for them.

* Limits on performance and reliability
* Not consistent with existing tools and infrastructure
* Requires that masters be updated and has limits on dynamic behavior
3. Direct external call outs for object mutation (RPC to initialize objects)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this is hard. See my previous comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not hard - just not a great design. Controllers work really well, and an initializer is an almost trivial controller.

Copy link
Contributor

Choose a reason for hiding this comment

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

How would supporting RPC be any more costly or complicated vs supporting webhooks?

@mfischer-zd
Copy link

Should mount policies be enforceable by the admission controller as well? See, e.g., kubernetes/kubernetes#29326

Copy link
Member

@thockin thockin left a comment

Choose a reason for hiding this comment

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

This is a very important - and very subtle - proposal. As much as I want it now-now-now, I think we need to REALLY think it through.

3. Backwards Compatible
Existing API clients must see no change in behavior to external admission other than increased latency
4. Easy Installation
Administrators should be able to easily write a new admission plugin and deploy it in the cluster
Copy link
Member

Choose a reason for hiding this comment

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

probably need to clarify intent for situations like GKE. E.g. "copy a file to the master" is not a sufficient form of installation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be worth a side discussion on the general config approach - several groups are independently dancing around a common center, and we should probably ensure kubeadm, David's generic work, this, and the kubelet work all share a common thread. Can someone take "dynamic config czar" title for two weeks in the beginning of January and get everyone pointing the same way?

Copy link

Choose a reason for hiding this comment

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

👍 on discussing a general configuration mechanism.

I do like the TPR approach, and wonder if it makes sense to move further in that direction an either have different entites to extend existing resources i.e. kind: KubeletPlugin, AdmissionPlugin or have a single resource to ship extensions to different components. At least the form factor would be the same in both cases - we'd be using containers to ship the stuff.

Just my 2ct

Copy link
Member

Choose a reason for hiding this comment

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

Reiterating comment I made during the meeting:

We probably need integrators, such as Openshift and GKE, to be able to easily add new plugins, also. For instance, does it make sense to allow cluster admins to disable OriginResourceQuota? That would just cause an API feature to not work.

one-way, the object can be "revealed" to regular clients once a set list of initializers is consumed. These
controllers could run on the cluster as pods.
2. Add a generic **external admission webhook** controller that is non-mutating (thus parallelizable)
This generic webhook API would resemble `admission.Interface` and be given the input object (for create) and the
Copy link
Member

Choose a reason for hiding this comment

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

How do we do API versioning for webhooks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as we do today, except your endpoint has to accept multiple objects and at some point we have to cut over if we change the schema. This more than our other APIs just looks like an RPC endpoint.

Copy link
Member

Choose a reason for hiding this comment

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

Would this mean that the webhook receiver release rollout must be synchronized with the rollout of the apiserver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Custom admission controllers, then masters, then controllers, then nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a section

type ObjectMeta struct {
...
// Initializers is a list of initializers that must run prior to this object being visible to
// normal clients. Initializers are executed in order. This field is nil if initialization is not
Copy link
Member

Choose a reason for hiding this comment

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

Finalizers are not run in-order, though ACK that this is not the same. Is it unreasonable to demand that initializers be order-independent, and rely on resourceVersion to arbitrate conflicts? I fear I know the answer, but the I think it bears thinking about (and maybe documenting why, if not possible)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's benefit in order dependency in a few of the use cases discussed, and no real benefit from order independence. If all initializers must finish for the object to be initialized, it means initialization logic is more complex and there's more for someone to debug. A controller can always speculatively initialize if performance is an issue. Out of order is also a lot harder for an initializer author to reason about.

Copy link
Member

Choose a reason for hiding this comment

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

@thockin I think initializers have to run in order.

}
```

On creation, an admission controller defaults the initializers field (if nil) to a value from the system
Copy link
Member

Choose a reason for hiding this comment

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

I strongly object to distinguishing nil from empty slice, based on my previous attempts to make it work sanely. It's a bad precedent. Why do we need to allow manual setting of this field?

If we really need to allow it, I'd rather a sibling field ManualInitializers bool or something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I disagree with you, as evidenced by that thread. JSON makes the distinction, we have support for it (except in a couple of cases). We can queue that up for a discussion in API machinery.

Copy link
Member

Choose a reason for hiding this comment

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

I think this isn't the only case where we need to distinguish nil from empty. Do we need to make it a pointer to a slice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to make initializers a struct with nested fields, so that we have a place to backpropogate information. In order for this addition to be backwards compatible an object without initialization must be easily distinguished, and we may want to hang other data off this struct in the future. So I'm envisioning:

type Initialization struct {
  Initializers []string
  Status *metav1.Status
}

and keep the meta struct having *Initialization. Will add that to proposal.

the admission controller will check whether the user has the ability to run the `initialize` verb on
the current resource type, and reject the entry with 403 if not.

Once created, an object is not visible to clients unless the following conditions are satisfied:
Copy link
Member

Choose a reason for hiding this comment

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

Later you discuss idempotency, and blocking. Maybe a forward ref here?


Because admission is performance critical, the following considerations are taken:

1. The caller may pass the received body bytes AS-IS to the admission controller, so defaulting may not be performed.
Copy link
Member

Choose a reason for hiding this comment

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

"may not" is too vague to be useful. Can we say "will not" categorically, or is there some case in which it will be defaulted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm mostly worried about performance. Defaulting is expensive and requires a full deserialization / serialization, and I'm looking for opportunities to shave fat off this flow.

Copy link
Member

@lavalamp lavalamp Mar 20, 2017

Choose a reason for hiding this comment

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

The result of this is likely that the admission controller would do some of its own defaulting, which I'm against. The admission controller would not be aware of default changes and may build in assumptions. I guess it's not the end of the world as long as this remains non-mutating, but I bet it will cause admission controller authors some heartache.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps amend to state that we'll have to revisit this before we allow mutating webhooks.

}

type ResourceDefaultInitializer struct {
// Resource is the resource that should be initialized
Copy link
Member

Choose a reason for hiding this comment

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

I assume this includes apiversion - maybe an example?

Copy link
Member

Choose a reason for hiding this comment

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

+1 to this question. This "resource" is specified how?

configuration, but may opt for alternate mechanisms.

The initializer admission controller and the generic webhook admission controller should dynamically load config
from a `ConfigMap` holding the following configuration schema:
Copy link
Member

Choose a reason for hiding this comment

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

How does the apiserver find the config map? Fixed name or a flag or selector?

from a `ConfigMap` holding the following configuration schema:

```
type AdmissionControlConfiguration struct {
Copy link
Member

Choose a reason for hiding this comment

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

Rather than a single struct, why not use the fact that CMs have multiple keys? Minor point, I guess

from a `ConfigMap` holding the following configuration schema:

```
type AdmissionControlConfiguration struct {
Copy link
Member

Choose a reason for hiding this comment

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

This schema needs to be versioned and probably self-identifying. ObjectMeta?

@smarterclayton
Copy link
Contributor Author

Quota does not require changes under this model. All quota is local to its apiserver - only the resources the api server supports should service quota.

@smarterclayton
Copy link
Contributor Author

@brendandburns

though I guarantee you that the first thing someone is going to do is implement a generic one that uses JavaScript/Lua/... so that users don't have to build yet another binary in order to run a custom policy.

You can implement that today in Kube - I think that's orthogonal to this issue. This is more concerned with production level admission - any admission controller that wraps generic logic can be used as an external admission controller. I will selectively call that out - any sort of in-process generic transformation is possible, but is ultimately limited by two things:

  1. Performance (caching and access to other informers) and complexity (unable to invoke method X on object Y in the language)
  2. Supporting all possible variants of API objects - admission controllers need to deal with multiple versions

I'm in favor of someone writing a Lua or JS or insert scripting language here admission controller - I think it should be possible to configure that dynamically (via the mechanism described in this PR, possibly) so that admission not be required to be extended.

@smarterclayton
Copy link
Contributor Author

I want to highlight that the point of this is not to replace the current admission control subsystem. It is to add two pragmatic new admission controllers that address the majority of production use cases from OpenShift and select examples from outside (I need to do another sweep to ensure I catch those). The goal is to allow out-of-tree admission to start being part of a Kubernetes system, not to drive it to completion. Because those use cases are production based, it's focused on solutions that enable existing admission controllers to be adapted.

  1. Initializers is part admission control and part special API logic. The admission controller bit enforces the rules regarding mutation.
  2. Generic external admission controllers is the simplest possible way to take an existing admission controller and move it out of tree.
  3. Dynamic configuration allows an out of tree extension to provide a new admission controller, avoiding the need to recompile the master.

The current path we are on is to allow the core to function mostly independent of external pieces, except hopefully for extension. I.e. you bring up kube-apiserver, then a cluster administrator installs myco-superpolicyplugin as pod, which involves registering a custom admission controller, which can then begin enforcing rules about pods.


It should be possible to perform holistic policy enforcement in Kubernetes without the recompilation of the
core project as plugins that can be added and removed to a stock Kubernetes release. That extension
of admission control should leverage similar mechanisms to our existing controller frameworks where possible
Copy link
Contributor

Choose a reason for hiding this comment

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

What does it mean by "leverage similar mechanism to our existing controller frameworks", "otherwise be performant and reliable"?

This is confusing..


Name | Code | Description
---- | ---- | -----------
AlwaysPullImages | alwayspullimages/admission.go | Forces the Kubelet to pull images to prevent pods from from accessing private images that another user with credentials has already pulled to the node.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/from from/from/

@0xmichalis
Copy link
Contributor

The grouping of the admission plugins at the start of this doc is neat. http://kubernetes.io/docs/admin/admission-controllers/ could make use of the structure cc: @kubernetes/sig-docs-misc

@justinsb
Copy link
Member

justinsb commented Jan 5, 2017

Can you explain why this is better than "just" converting the admission Interface to be GRPC based?

1. Allow some controllers to act as "initializers" - watching the API and mutating the object before it is visible to normal clients.
This would reuse the majority of the infrastructure in place for controllers. Because creation is
one-way, the object can be "revealed" to regular clients once a set list of initializers is consumed. These
controllers could run on the cluster as pods.
Copy link

Choose a reason for hiding this comment

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

its not clear, but maybe its stated later on, but is this first bullet still compiled into the system or something external? I'm hoping its external but since you explicitly call out "webhooks" in the next one I had to ask.

Copy link
Member

Choose a reason for hiding this comment

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

Both internal and external controllers would be supported.

Copy link
Member

@bgrant0607 bgrant0607 left a comment

Choose a reason for hiding this comment

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

I made a lot of comments, but I like this overall.

I think the main issue is that of whether defaulting can be deferred until after initialization -- I don't see how that could work.


The four core systems in Kubernetes are:

1. API servers with persistent storage, providing basic object validation and CRUD operations
Copy link
Member

Choose a reason for hiding this comment

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

The API servers also perform defaulting and API version conversion. Defaulting is relevant here, because we need to specify whether it happens before or after admission control, as is conversion, for similar reasons (e.g., do admission controllers act upon versioned or internal resources?).

3. [Admission controller layers](admission_control.md) that can control and limit the CRUD operations clients perform synchronously.
4. Controllers which watch the API and react to changes made by other users asynchronously (scheduler, replication controller, kubelet, kube-proxy, and ingress are all examples of controllers).

Admission control supports a wide range of policy and behavior enforcement for administrators.
Copy link
Member

Choose a reason for hiding this comment

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

for cluster administrators and integrators. We have a need for behaviors that are configurable by cluster administrators and ones that are imposed by the integrator (e.g., Openshift, GKE).


While admission controllers can operate on all verbs, resources, and sub resource types, in practice they
mostly deal with create and update on primary resources. Most sub resources are highly privileged operations
and so are typically covered by policy. Other controllers like quota tend to be per apiserver and therefore
Copy link
Member

Choose a reason for hiding this comment

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

covered by authorization policy?

Name | Code | Description
---- | ---- | -----------
InitialResources | initialresources/admission.go | Default the resources for a container based on past usage
LimitRanger | limitranger/admission.go | Set defaults for container requests and limits, or enforce upper bounds on certain resources (no more than 2GB of memory, default to 512MB)
Copy link
Member

Choose a reason for hiding this comment

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

I'd note that this implements a v1 (core) API.

---- | ---- | -----------
InitialResources | initialresources/admission.go | Default the resources for a container based on past usage
LimitRanger | limitranger/admission.go | Set defaults for container requests and limits, or enforce upper bounds on certain resources (no more than 2GB of memory, default to 512MB)
ResourceQuota | resourcequota/admission.go | Calculate and deny number of objects (pods, rc, service load balancers) or total consumed resources (cpu, memory, disk) in a namespace
Copy link
Member

Choose a reason for hiding this comment

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

I'd note that this implements a v1 API.

that must terminate initialization, the `Status` field on the initializer should be set instead of removing
the initializer entry and then the initializer should delete the object.

During initialization, resources may have relaxed validation requirements, which means initializers must
Copy link
Member

Choose a reason for hiding this comment

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

I think it can't happen before defaulting. The initializer controllers could access the resource via a different api version than the creator.

initializers run. The initial object creation must still be validated - resource types that wish to support
deep initialization must design their objects to accept those.

To allow naive clients to avoid having to deal with uninitialized objects, the API will automatically
Copy link
Member

Choose a reason for hiding this comment

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

When reading this, I expected this information much earlier in the proposal.


To allow naive clients to avoid having to deal with uninitialized objects, the API will automatically
filter uninitialized objects out LIST and WATCH. Explicit GETs to that object should return the
appropriate status code `202 Accepted` indicating that the resource is reserved and including a `Status`
Copy link
Member

Choose a reason for hiding this comment

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

Any idea whether old versions of kubectl would handle this gracefully?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They will, some client libraries might not, but get to find a resource that doesn't exist yet is an in between state.


There is no current error case for a timeout that exactly matches existing behavior except a 5xx
timeout if etcd does not respond quickly. We should return that error, but return an appropriate
cause that lets a client determine what the outcome was.
Copy link
Member

Choose a reason for hiding this comment

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

We don't need it initially, but might want a TTL for resources that are never fully initialized.

}

type ResourceDefaultInitializer struct {
// Resource is the resource that should be initialized
Copy link
Member

Choose a reason for hiding this comment

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

+1 to this question. This "resource" is specified how?

@smarterclayton
Copy link
Contributor Author

Updated with almost all comments in most recent commit.

##### Failure

If the apiserver crashes before the initialization is complete, it may be necessary for the
apiserver or another controller to complete deletion.
Copy link
Member

Choose a reason for hiding this comment

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

Like the PodGC controller.

@lavalamp
Copy link
Member

/lgtm

We gotta execute on this. Future changes to this can come via other PRs :)

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 12, 2017
@lavalamp lavalamp merged commit cbce2cd into kubernetes:master May 12, 2017
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this pull request May 20, 2017
Automatic merge from submit-queue (batch tested with PRs 45996, 46121, 45707, 46011, 45564)

add "admission" API group

This commit is an initial pass at providing an admission API group.
The API group is required by the webhook admission controller being
developed as part of kubernetes/community#132
and could be used more as that proposal comes to fruition.

**Note:** This PR was created by following the [Adding an API Group](https://github.com/kubernetes/community/blob/master/contributors/devel/adding-an-APIGroup.md) documentation.

cc @smarterclayton
lavalamp pushed a commit to lavalamp/kubernetes that referenced this pull request May 31, 2017
To properly register the types in the admission API group we need to
create an "install" package and wire it up.  This is required by the
webhook admission controller being developed as part of
kubernetes/community#132
lavalamp pushed a commit to lavalamp/kubernetes that referenced this pull request May 31, 2017
As part of kubernetes/community#132, thsi commit
adds a generic webhook admission controller.  This plugin allows for a
completely declarative approach for filtering/matching admission requests
and for matching admission requests, calls out to an external webhook for
handling admission requests.
mrIncompetent pushed a commit to kubermatic/kubernetes that referenced this pull request Jun 6, 2017
To properly register the types in the admission API group we need to
create an "install" package and wire it up.  This is required by the
webhook admission controller being developed as part of
kubernetes/community#132
mrIncompetent pushed a commit to kubermatic/kubernetes that referenced this pull request Jun 6, 2017
As part of kubernetes/community#132, thsi commit
adds a generic webhook admission controller.  This plugin allows for a
completely declarative approach for filtering/matching admission requests
and for matching admission requests, calls out to an external webhook for
handling admission requests.
MadhavJivrajani pushed a commit to MadhavJivrajani/community that referenced this pull request Nov 30, 2021
* Proposal: Extensible Admission Control

* More clarification

* Design details for initializer

* Describe external admission and dynamic configuration

* Move review comments, refine initializers

* Review comments and clarifications

* Review comments
rmohr pushed a commit to rmohr/community that referenced this pull request May 4, 2022
…es#132)

Signed-off-by: Nahshon Unna-Tsameret <nunnatsa@redhat.com>
danehans pushed a commit to danehans/community that referenced this pull request Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.