add subjectaccessreviews resource #20573
Conversation
k8s-github-robot
added
the
size/L
|
Looks good so far. |
|
PR needs rebase |
k8s-github-robot
added
the
needs-rebase
deads2k
changed the title
|
spec and status made into pointers and a simple test added to verify the plumbing. @lavalamp I can't get the client to generate properly. It seems to built around the idea that all types will have names. I was following directions here: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/generating-clientset.md . |
k8s-github-robot
added
kind/api-change
|
@caesarxuchao can you take a look at why the client generation is unhappy? |
@deads2k not sure what you mean by "all types will have names". I don't see anything special in your types.go. How can I reproduce the problem you encountered? |
|
this is cool. 👍 |
|
@caesarxuchao I updated this branch with the failing code. You can't get these objects, so there's no list and no name. I end up with failures like this: |
k8s-github-robot
added
size/XXL
|
@deads2k I see how the client-gen fails. What client methods to you need? It seems you can only have Create(), DeleteCollection(), and Watch(), because the object doesn't have ObjectMeta and doesn't have a list. Are they the methods you need? Or Do you need to some special methods to operate on |
I only need |
|
I'll fix client-gen to allow user express what client methods to generate. Opened issue #23547. In the meantime, if you don't want to get blocked by the client, you can use the "noMethods=true" tag (https://github.com/kubernetes/kubernetes/pull/19499/files#diff-e1c628c34c42840b0392c48481d41320R52), and manually write your create method as an expansion, you can take the pod_expansion.go as an example: https://github.com/kubernetes/kubernetes/blob/master/pkg/client/typed/generated/core/unversioned/pod_expansion.go. How does this sound? |
I'll give it a go. |
|
@deads2k Could you explain why the authorization API only needs a Create() method? Is there a design doc that I should read? I'm trying to understand if this is a general problem for other API as well. Thanks. |
Yes, I'm filling in the implementation for this: https://github.com/kubernetes/kubernetes/blob/master/docs/design/enhance-pluggable-policy.md#subjectaccessreviews . |
The short version is that we need to provide a request body to hold an API object and we want to return the system's view of the world back to the caller. We don't care about storing these, so we haven't built any infrastructure to actually persist them. |
IIUC, the return value of the |
deads2k
added
release-note
k8s-github-robot
removed
the
needs-rebase
|
restricted validation and updated generators for versioned clients |
|
@k8s-bot test this: issue #IGNORE "Unable to load build details from gs://kubernetes-jenkins/pr-logs/pull/20573/node-pull-build-e2e-test/17411" |
|
What did he say that persuaded you that something which is not persisted On Fri, Aug 5, 2016 at 9:24 AM, Kubernetes Bot notifications@github.com
|
We have generic cachers that key based on name and resourceVersion. An access review check has a logical resource version that corresponds to the current state used to make the authorization decision. Some authorizers (rbac for instance), can tie that to an etcd resource version. If we were to careful control the shape of the name (we do this for some resources in validation based on field value), we'd be able to create a combination of name and resourceVersion that could effectively cache a policy decision. Not saying I'd go forth an do it now, but it doesn't seem like an entirely unreasonable step. |
|
@k8s-bot test this: issue #IGNORE |
|
man, node e2e is angry "Unable to load build details from" again: https://k8s-gubernator.appspot.com/build/kubernetes-jenkins/pr-logs/pull/20573/node-pull-build-e2e-test/17412/ |
|
Caching is great. But that is stretching my concept of resourceVersion. On Fri, Aug 5, 2016 at 10:06 AM, David Eads notifications@github.com
|
If object1 is a virtual view into object2, object1 can be modeled as having a resourceVersion== object2.resourceVersion. An accessreview check follows the same model, but viewing the entire list of resources as its resourceVersion, like ListMeta. @erictune Given the tight validation (must be empty) on objectmeta, you're still comfortable with this, right? |
|
fine. On Fri, Aug 5, 2016 at 10:49 AM, David Eads notifications@github.com
|
deads2k
added
the
lgtm
|
ResourceVersion is a consistent identifier for a state - if we allow
list to have resource version, whether that list is generated from a
consistent cache or underlying store, the version applies to the view.
So I don't think it's a stretch to say resourceVersions can be applied
to arbitrary views of data
|
|
GCE e2e build/test passed for commit d9a2034. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit d9a2034. |
|
Automatic merge from submit-queue |
Automatic merge from submit-queue Client-gen: Add fake clients to testoutput; fix the imports in fake clients Start generating fake client in the testoutput, so that we can check the fake client is generated correctly under extreme conditions. Fix the problem reported kubernetes/kubernetes#20573 (comment), where the import names contain dots when the group name contains dots. cc @deads2k
Automatic merge from submit-queue Remove dot in generated deepcopy package name ref kubernetes/kubernetes#20573 (comment) cc @wojtek-t I tested with #20573 to verify hack/codegen.sh worked and the generated code compiled.
Adds a subjectaccessreviews endpoint that uses the API server's authorizer to determine if a subject is allowed to perform an action.
Part of kubernetes/enhancements#37