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

Upstream Openshift Authz #23396

Closed
erictune opened this issue Mar 23, 2016 · 43 comments
Closed

Upstream Openshift Authz #23396

erictune opened this issue Mar 23, 2016 · 43 comments
Assignees
Labels
kind/documentation Categorizes issue or PR as related to documentation. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Milestone

Comments

@erictune
Copy link
Member

in its own api group(s).

@erictune
Copy link
Member Author

I would expect there to be little further development on ABAC, with Openshift Authz addressing many more use cases than ABAC. I expect many users would move to Openshift Authz. But, I envision that we would be to keep supporting ABAC indefinitely, and that even clusters with Openshift Authz in them may still ABAC enabled, and that the ABAC file might be used for "bootstrapping" the authz for basic cluster services.

@erictune
Copy link
Member Author

I'm expecting that the Openshift auth upstreaming would not block on apiserver federation, but would use it when federation is complete. @lavalamp

@erictune
Copy link
Member Author

@deads2k

@erictune
Copy link
Member Author

My summary of Openshift Authz for people not familiar with it:

Design doc

User Docs

Types

Key types to be added are:

  • a Role object lists various actions that someone/thing acting in that role can do. There are predefined roles, and custom ones are possible.
  • the allowed actions of a Role are expressed as a list of PolicyRules.
  • PolicyRules can allow or deny access to all resources of a certain kind, or to specific resources.
  • PolicyRules can allow specific actions ("verbs") on an object. Verbs include: "create", "delete", "get",
    "list", "update", "watch".
  • PolicyRules have an escape valve called an AttributeRestriction which allows arbitrary conditions to be specified if a corresponding "plugin" is plugged into the apiserver. For example, one AttributeRestriction checks that the Host of the requestor (e.g. kubelet) is the same as the Host of the object (e.g. Pod) being requested.
  • A RoleBinding says that a User or Group of users can act in a Role.
  • A Role and a RoleBinding are for namespaced objects and apply to objects in its same namespace.
  • Objects with no namespace and cluster-wide policies that apply regardless of namespace, are
    held in corresponding ClusterRole and ClusterRoleBinding objects.
  • There are half a dozen other types whose purpose is not relevant or I do not understand yet.

The openshiftAuthorizer's Authorize method first checks if an action is globally allowed, and then if it is locally allowed (see func (a *openshiftAuthorizer) authorizeWithNamespaceRules(ctx kapi.Context, passedAttributes AuthorizationAttributes)

@erictune
Copy link
Member Author

@lavalamp

@lavalamp
Copy link
Member

What api group?

@erictune
Copy link
Member Author

@deads2k @liggitt any opinion about what to call the API group for Openshift Auth once it is upstreamed? authorization-policy.openshift.org is descriptive but a bit long.

@deads2k
Copy link
Contributor

deads2k commented Mar 24, 2016

@deads2k @liggitt any opinion about what to call the API group for Openshift Auth once it is upstreamed? authorization-policy.openshift.org

I'm biased towards nesting (policy.authorization.openshift.org), but I'm sure someone will say that's the Java in my trying to get out. :)

@deads2k
Copy link
Contributor

deads2k commented Mar 24, 2016

There are half a dozen other types whose purpose is not relevant or I do not understand yet.

ClusterRole and ClusterRoleBinding are clones of Role and RoleBinding which exist at the cluster scope and which provide authority across items in the cluster, including every namespace. We had to build them because the client and the server couldn't handle the idea of a single type at two scopes or a single Kind attached to two Resources.

Policy and PolicyBinding we talked about removing since they added pain without a lot of value.

@lavalamp
Copy link
Member

I could also see authorization.policy.openshift.org.

Alternatively, if we think anyone other than openshift will use this, a
thematic name might be good. basic.authorization? level2.authorization?
Is there anything unique about this authz system that we could call out in
a name?

On Thu, Mar 24, 2016 at 5:12 AM, David Eads notifications@github.com
wrote:

@deads2k https://github.com/deads2k @liggitt
https://github.com/liggitt any opinion about what to call the API group
for Openshift Auth once it is upstreamed?
authorization-policy.openshift.org

I'm biased towards nesting (policy.authorization.openshift.org), but I'm
sure someone will say that's the Java in my trying to get out. :)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#23396 (comment)

@liggitt
Copy link
Member

liggitt commented Mar 24, 2016

I would expect it to get used... it's a nice role-based system... rbac.authorization?

@lavalamp
Copy link
Member

That SGTM.

@liggitt
Copy link
Member

liggitt commented Mar 24, 2016

We should also decide whether k8s groups get suffixed with k8s.io, kubernetes.io, or not at all (we have all three in-repo at the moment, and it's like a splinter in my mind)

@deads2k
Copy link
Contributor

deads2k commented Mar 24, 2016

We should also decide whether k8s groups get suffixed with k8s.io, kubernetes.io, or not at all (we have all three in-repo at the moment, and it's like a splinter in my mind)

k8s.io please, with unambiguous prefixes matched on the cmd line.

@liggitt
Copy link
Member

liggitt commented Mar 24, 2016

So what do we do with the fact that the most official ones we have (extensions, autoscaling, batch) all lack that?

@erictune
Copy link
Member Author

@bgrant0607 how do you feel about making this go straight to Beta when it upstreams since it is based on a well-tested API?

@erictune
Copy link
Member Author

So, we have:

  • abac.authorization.kubernetes.io, versions v0 and v1beta1
  • authorization.k8s.io, version v1beta1
  • componentconfig, version v1alpha1
  • extensions, version v1beta1
  • metrics, version v1alpha1
  • autoscaling version v1
  • batch version v1

@erictune
Copy link
Member Author

To move everything to k8s.io or to kubernetes.io, we have to change 6 groups, including two which are v1.
To move all current items to no-suffix, we would have to move 2 groups, which are v1beta1.

The latter seems easier. So, how about if we make it a "graduation to v1 requirement" for abac and authorization, that they drop their suffix, to become abac and authorization.

In terms of a rule for when to use suffix, we could say:

  • all api groups owned by Kubernetes project (github.com/kubernetes/kubernetes), should contain no dots, (or at least, not end in a dot and a TLD name).
  • all api groups owned by other projects should have at least two dots, with the last two or more segments being a domain name owned by the project that owns the group.

We don't have any in the latter category AFIACS.

@erictune
Copy link
Member Author

Would it make sense for this to be called "authorization.openshift.org"? I guess that would not make sense unless all the code was vendored from openshift.org, or if it was in a federated apiserver, which we don't have the ability to do today, right?

@ghost ghost added this to the v1.3 milestone Mar 30, 2016
@ghost ghost added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Mar 30, 2016
@ghost
Copy link

ghost commented Mar 30, 2016

@etune I added priority and milestone based on my understanding. Please feel free to edit as necessary.

@erictune erictune added sig/auth Categorizes an issue or PR as relevant to SIG Auth. and removed area/security labels Apr 12, 2016
@ericchiang
Copy link
Contributor

@erictune noticed in the #sig-auth channel that this is a maybe for 1.3.

Is there active work for this issue and is the milestone correct? Would love to help out if it's just developer time blocking this.

@erictune
Copy link
Member Author

@deads2k

I see you have some big PRs out, like #20573 to add subjectaccessreviews resource, and #23208 to make it easier to work with storage. Are those changes prerequisites for upstreaming openshift authz, or just general improvements.

@erictune
Copy link
Member Author

yes, you can help @ericchiang. @Dead2K said he will help consult on how best to upstream.

@erictune
Copy link
Member Author

erictune commented May 4, 2016

I will update that issue.

@philips
Copy link
Contributor

philips commented May 4, 2016

@erictune sounds good! Maybe you should put that URL in the description for people new to the issue? #23396 (comment)

@erictune
Copy link
Member Author

erictune commented May 6, 2016

@ericchiang

At some stage, we will need to create "bootstrap policy" on the cluster. That is, create canned roles, and bindings for the initial administrator account and for controllers.
On openshift this is stored here: https://github.com/openshift/origin/blob/2fb5c03de1b9a7208a03b9dd28e05a25e5de7d0e/pkg/cmd/server/bootstrappolicy/policy.go

The question is whether to store them as go objects like openshift does or as json or yaml files?

Go has the advantage that it is easier to use go code to generate multiple objects programatically. But YAML is more readable to the general populace, and more directly editable by users, and still supports comments.

So, I am thinking we'd want to store the bootstrap policy as yaml files in git, and have them added by the add-on manager. That probably means:

  • convert OS bootstrap policies to yaml, and to adjust kind, version, and any other differences.
  • git add the yaml files. the files can live under cluster/addons/rbac.policies.
  • when someone does kube-up, addon manager will kubectl create -f those files (this is how addon-manager works, you don't have to do anything).

Nearly identical discussion here: #24600 (comment)

@liggitt
Copy link
Member

liggitt commented May 6, 2016

We also store the yaml versions of the roles and bindings (though it's the programmatically generated ones, not annotated ones, which would be handy):

we have unit tests that makes sure they stay in sync, which also adds an audit step to any changes to the defaults

@ericchiang
Copy link
Contributor

Yeah that sounds good. I figured we'd have a set of reasonable defaults for documentation purposes at the very least.

I prefer yaml since it seems less magical, but either way is fine. Do we plan to turn on RBAC by default? (I remember discussion of making ABAC the default #22813.)

k8s-github-robot pushed a commit that referenced this issue May 12, 2016
Automatic merge from submit-queue

pkg/apis/rbac: Add Openshift authorization API types

This PR updates #23396 by adding the Openshift RBAC types to a new API group.

Changes from Openshift:

* Omission of [ResourceGroups](https://github.com/openshift/origin/blob/458998788337e983fa3e5f9a837664ec00a89204/pkg/authorization/api/types.go#L32-L104) as most of these were Openshift specific. Would like to add the concept back in for a later release of the API.
* Omission of IsPersonalSubjectAccessReview as its implementation relied on Openshift capability.
* Omission of SubjectAccessReview and ResourceAccessReview types. These are defined in `authorization.k8s.io`

~~API group is named `rbac.authorization.openshift.com` as we omitted the AccessReview stuff and that seemed to be the lest controversial based on conversations in #23396. Would be happy to change it if there's a dislike for the name.~~ Edit: API groups is named `rbac`, sorry misread the original thread.

As discussed in #18762, creating a new API group is kind difficult right now and the documentation is very out of date. Got a little help from @soltysh but I'm sure I'm missing some things. Also still need to add validation and a RESTStorage registry interface. Hence "WIP".

Any initial comments welcome.

cc @erictune @deads2k @sym3tri @philips
@davidopp
Copy link
Member

My understanding from @erictune is that the 1.3 bits of this are finished (since #25634 has been merged) but it still needs an integration test before the ship deadline. Is that correct?

@ericchiang
Copy link
Contributor

Yes. I will update this issue tomorrow with TODO for before the release.

@ericchiang
Copy link
Contributor

@erictune I've added a comment[0] to kubernetes/enhancements#2. Do you want me to reproduce that here?

I'm a little unfamiliar with that repo and it's role. Which issue is the best place to track progress?

[0] kubernetes/enhancements#2 (comment)

@fgrzadkowski
Copy link
Contributor

@erictune I see you've self-assigned this 2 days ago. What exactly is missing here? From #23396 (comment) I guess it's just tests. Is that the case?

@erictune
Copy link
Member Author

Yes. Waiting for #26753

@pwittrock
Copy link
Member

@ericchiang
@erictune

Would you provide an update on the status for the documentation for this feature as well as add any PRs as they are created?

Not Started / In Progress / In Review / Done

Thanks

k8s-github-robot pushed a commit that referenced this issue Jun 20, 2016
Automatic merge from submit-queue

add unit and integration tests for rbac authorizer

This PR adds lots of tests for the RBAC authorizer. 

The plan over the next couple days is to add a lot more test cases.

Updates #23396

cc @erictune
@goltermann
Copy link
Contributor

@erictune #26753 is now merged

@erictune
Copy link
Member Author

Waiting just for docs: kubernetes/website#643

Adding docs label.

@erictune erictune added the kind/documentation Categorizes issue or PR as related to documentation. label Jun 20, 2016
@goltermann
Copy link
Contributor

This is an issue marked v1.3 and kind/documentation. Can you get it fixed up this week?

@erictune
Copy link
Member Author

We have Docs. We still need default roles, and moving to beta/GA. That will be tracked in kubernetes/enhancements#2. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Categorizes issue or PR as related to documentation. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/auth Categorizes an issue or PR as relevant to SIG Auth.
Projects
None yet
Development

No branches or pull requests