Add a generic admission controller that uses JSONPath for rules.

[brendandburns]

@derekwaynecarr @liggitt @smarterclayton @erictune fyi

Now that I wrote this, it probably already exists upstream...

This allows you to write a config like:

rules:
  # Require all Pods and Services names to start with "Foo"
  - name: FooName
    kindRegexp: "^(Pod)|(Service)$"
    path: ".metadata.name"
    apiVersion: "v1"
    matchRegexp: "^Foo.*"
 # Require all container images to be from gcr.io
 - name: GCRRegistry
   kindRegexp: "^Pod$"
   path: "spec.containers[*].image"
   apiVersion: "v1"
   matchRegexp: "^gcr.io/.*$"

Which will then enforce those rules via admission control. This enables users to easily customize different admission controllers to suit their needs.

---

This change is

[liggitt]

cc @deads2k

[deads2k]

This looks like a limited kind of field level policy that is cluster-wide and doesn't distinguish based on users. Field level policy is on the agenda for the next sig-auth meeting, but I think we'll be talking about how to implement in a way that allows distinction based on users, since the case of "no one in my cluster may set this field that way" is a little narrow.

I noted some points for discussion on the topic here: #27330 (comment).

[deads2k]

You'll need the external representation of this object to be able to evaluate an external json path on it. This is the internal representation.

[smarterclayton]

xref #27129 and #15390

On Tue, Jun 14, 2016 at 8:49 AM, David Eads [email protected] wrote:

In plugin/pkg/admission/jsonpath/admission.go:

• kindRE *regexp.Regexp
• path *util_jsonpath.JSONPath
• matchRE *regexp.Regexp
• acceptNoMatches bool
  +}

+func (r *jsonPathRule) String() string {

• return fmt.Sprintf("%s: %s %s %s %v", r.name, r.kindRE.String(), r.path,
  r.matchRE.String(), r.acceptNoMatches)
  +}

+func (r *jsonPathRule) admit(a admission.Attributes) error {

• if !r.kindRE.MatchString(a.GetKind().Kind) {

•    return nil
  

• }

• vals, err := r.path.FindResults(a.GetObject())

You'll need the external representation of this object to be able to
evaluate an external json path on it. This is the internal representation.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[brendandburns]

@deads2k @smarterclayton is there an easy way to externalize? From scanning the code it looks like I'd need to determine if its a core object or an extension, and then do object conversion? It seems horrible and hacky, I feel like there should be a library function to do it, but I can't seem to find it... Any pointers?

[brendandburns]

@deads2k @erictune

Regarding refinements (per-user, per-namespace) it seems like the best way to achieve this is to have a SpecificUserAdmissionController or SpecificNamespceAdmissionController which wraps any arbitrary instance of admission.Interface

That way you can re-use the restrict/targeting controllers (for lack of a better word) with any arbitrary admission controller and not have to re-implement for every controller that might want it.

I'd be happy to discuss this at sig-auth or in a formal proposal.

As it stands, I think that there is utility in this admission controller by itself (e.g. the restrict images issue that #27129) that Clayton references, so I'd prefer to see this get in, and then write the targeting admission controllers separately. (I'm happy to write them as well)

[derekwaynecarr]

FYI I think you would need to handle generated names from the prefix here as well to really have this use case.

[brendandburns]

I'm not sure I quite follow? What is the prefix, what is the generated name?

[derekwaynecarr]

I was calling out that many creates do not include ObjectMeta.Name and instead just specify ObjectMeta.GenerateName (https://github.com/kubernetes/kubernetes/blob/master/pkg/api/types.go#L78). The actual expansion of the generated name into ObjectMeta.Name happens AFTER admission logic today (unfortunately).

[brendandburns]

ah, this is just an example, I changed it so as not to confuse people.

[derekwaynecarr]

I see utility in this but users would need to be careful about fields that are defaulted after the admission sequence. So for your example using name, name is defaulted after admission right now. I agree with David's comments on your specific scenario but also think blunt hammers are useful when you need them absent a more particularly focused plug-in. I need to think if there are other scenarios where I would find this useful. Is image restriction your primary scenario now or are there others?

[brendandburns]

@derekwaynecarr one of the others I have in mind is requiring that every Pod have a specific annotation:

e.g. owners-email: [email protected]

To enforce system-wide metadata that could be used for reporting, etc.

Another example annotation to require might be the githash the container was built from, etc.

[olegshaldybin]

@brendandburns Please take a look at #27330 (comment): @tcahill and I have been recently thinking about adding some policy and compliance mechanisms to k8s, but we were approaching it from a slightly different angle; the comment I've linked describes that approach. We think it's important to be able to continuously monitor the compliance of a k8s cluster, and we've been looking at labels and annotations as a glue between different domain-specific policies and authorization.

We were thinking of preparing a proposal on handling policy and compliance in k8s for the next sig-auth meeting, does that make sense?

/cc @smarterclayton

[brendandburns]

@olegshaldybin I do think that monitoring is essential. (and I think that enabling users to do more sophisticated admission control [e.g. call out to some user-provided service] is also going to be critical).

However, I do think there is also a role for admission control like the one in the PR for two reasons:

1. Fail-fast is a better user interface. It's way better for a user to get an "Error that Pod can't be added because the registry is invalid" error when they try to create Pod, rather than have some compliance bot come along and blow away the pod later on, when the user isn't looking. (of course you want to have the compliance bot also but hopefully its just for defense in depth.

2. I think there are a class of users who just want to enable some degree of simplistic security, and for them, and admission controller like this one is a good blend of ease of use, with reasonable dynamic configurability.

What do you think?
--brendan

[brendandburns]

@derekwaynecarr one comment addressed, one clarification requested
@deads2k one question about (hopefully) a library function.

[deads2k]

@deads2k one question about (hopefully) a library function.

I think its something like api.Codecs.SerializerForMediaType("application/json").EncodeToStream(obj, stream, <version specified by user that matches the json path in his file>). Check to make sure you get the right apiVersion field; kube still has json on internal versions.

[olegshaldybin]

@brendandburns Agreed with you on both points: resources that don't satisfy the set of existing policies should be rejected at admission time. There also needs to be some monitoring agent that makes sure that changes to these policies apply to the existing resources (maybe they could just send the notification and make sure the compliance information is available via some dashboard). Would be great if that controller could use the same set of rules as the admission controller: this implies not directly depending on the identity of the requester, but rather applying different policies based on resources' metadata (labels/annotations seem like a good fit, especially for dev/test/prod workloads collocation).

Also agreed that some users want a simple security system that just makes sure that object fields match some predefined rules: that makes sense for single-tenant systems, or systems where all tenants have the same set of permissions. There's definitely space for both approaches, as Kubernetes is flexible in that regard.

[derekwaynecarr]

have this embed *admission.Handler, see example here:

https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/resourcequota/admission.go#L68

Then you don't need to implement Handles (or anything else that may come in the future)

[brendandburns]

done.

[brendandburns]

@derekwaynecarr @deads2k
comments addressed, please re-check.

thanks!
--brnedan

[derekwaynecarr]

this looks fine to me, will let @lavalamp look at how it uses the rest of the machinery. thanks for the updates @brendandburns

[brendandburns]

Ping to @lavalamp on this one.

Thanks!
--brendan

[brendandburns]

Ping to @lavalamp for a post 1.3 craziness review.

Thanks
--brendan

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[lavalamp]

FieldPath?

[lavalamp]

May this specify multiple fields?

[lavalamp]

This is going to be hard to use. It'll require a restart of apiserver. It won't be different per-namespace, so it's not useful for GKE or OpenShift users. It won't be obvious what the net effect of 50 rules is unless you're really good at doing regexps in your head.

This needs significant documentation somewhere, which should cover the above points.

I think we can take it if you can add some documentation and @erictune is OK with it.

[erictune]

We had planned to discuss this at SIG-Auth today, but ran out of time. It will be raised again at the 7/27 SIG-auth meeting

[deads2k]

From sig-auth:

1. If the goal of this is to provide validation for third-party resources (that would very handy), then having to restart the API server is a significant management burden. It seems like a webhook in the thirdpartyresource definition would be easier to use. Slower, but easier to manage, gives greater power (very few of our validators could be expressed as regexes), ensures that validation happens after all defaulting, and keeps separate roles separate (you don't have to have power to restart the API server to create a thirdpartyresource).
2. If the goal is to act as a sort of field level enforcement, then applying it unconditionally across namespaces and users severely limits its utility. The most probable use-case (restricting images), seems more likely to be covered by Container Image Policy enhancements#59. Is there another specific use-case this is trying to solve?
3. Currently the admission chain is flat. Creating a nested admission chain seems ripe for confusion and the current admission configuration (single file for all plugins) can't accommodate this scenario where different rules would apply at different nesting levels.

[deads2k]

@kubernetes/sig-auth summary from our discussion above.

[erictune]

@deads2k Good summary. Missed sig-auth meeting, but I agree with those points.

[brendandburns]

@deads2k @erictune

I think that there are a number of cluster wide field-level policies that you might want to enforce:

• Every object needs to have an 'email' annotation.
• Every container needs to have a git commit signature annotation.
• Every service needs to have a 'content-type' annotation.
• ...

Basically there are lots of different kinds of object meta info I want to require for parts of my cluster.

[googlebot]

CLAs look good, thanks!

[brendandburns]

@deads2k @erictune
Though, fwiw, if adding namespace support is what is required to land this PR, I'm happy to do that

[k8s-bot]

GCE e2e build/test passed for commit 6bfa322.

• Test Results
• Build Log
• Test Artifacts
• Internal Jenkins Results

[bgrant0607]

OPA presented to SIG auth, also:
https://github.com/open-policy-agent/opa

[k8s-github-robot]

@brendandburns PR needs rebase

[k8s-github-robot]

[APPROVALNOTIFIER] Needs approval from an approver in each of these OWNERS Files:

• cmd/kube-apiserver/OWNERS
• pkg/OWNERS
• plugin/pkg/admission/OWNERS
• test/OWNERS

We suggest the following people:
cc @smarterclayton @ixdy @derekwaynecarr
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[k8s-github-robot]

This PR hasn't been active in 90 days. Closing this PR. Please reopen if you would like to work towards merging this change, if/when the PR is ready for the next round of review.

cc @brendandburns @erictune

You can add 'keep-open' label to prevent this from happening again, or add a comment to keep it open another 90 days