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

add subject access review types #18722

Merged
merged 1 commit into from
Jan 21, 2016
Merged

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Dec 15, 2015

Ref #12209

This adds SubjectAccessReview kinds.

  1. SubjectAccessReview is cluster-scoped and allows lookups for arbitrary users and groups
  2. LocalSubjectAccessReview is namespace-scoped and allows lookups for arbitrary users and groups. This is separate to make it very easy to grant permissions to inspect permissions for users in a particular namespace
  3. SelfSubjectAccessReview is cluster-scoped and allows lookups only for the current user.

@liggitt @smarterclayton @erictune
@kubernetes/kube-iam @kubernetes/kube-api

@k8s-github-robot
Copy link

Labelling this PR as size/XL

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 15, 2015
@k8s-bot
Copy link

k8s-bot commented Dec 15, 2015

GCE e2e build/test failed for commit 7d6cc022425c3b30ada62535ab3f306b427fa93b.

unversioned.TypeMeta

// Spec holds information about the request being evaluated. spec.namespace must be equal to the namespace
// you made the request against. If emtpy, it is defaulted.
Copy link
Member

Choose a reason for hiding this comment

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

typo

@k8s-bot
Copy link

k8s-bot commented Dec 15, 2015

GCE e2e build/test failed for commit d2f13d981056606bb1f844f922b7bf0bd6573c87.

type ResourceAuthorizationAttributes struct {
// Namespace is the namespace of the action being requested. Currently, there is no distinction between no namespace and all namespaces
Namespace string
// Verb is one of: get, list, watch, create, update, delete
Copy link
Member

Choose a reason for hiding this comment

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

proxy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

proxy?

I'll tweak the comment to indicate that its "one like" and probably mention that you have to pass "*" to mean "all verbs", not empty string.

@deads2k
Copy link
Contributor Author

deads2k commented Dec 15, 2015

api types updated, I'll get the rest of the ripples tomorrow.

@k8s-bot
Copy link

k8s-bot commented Dec 15, 2015

GCE e2e build/test failed for commit 7bc5ec644459b637833876139f29b42a4d2c3200.

@deads2k
Copy link
Contributor Author

deads2k commented Dec 16, 2015

All the generated files up to date again.

@k8s-github-robot
Copy link

Labelling this PR as size/XXL

@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 16, 2015
@k8s-bot
Copy link

k8s-bot commented Dec 16, 2015

GCE e2e test build/test passed for commit 1d8e319b411f10155928d4fc0e88e69cf2eb9e72.

@deads2k
Copy link
Contributor Author

deads2k commented Dec 18, 2015

@erictune @smarterclayton. Comments?

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 24, 2015
@deads2k deads2k assigned erictune and unassigned smarterclayton Jan 8, 2016
@deads2k
Copy link
Contributor Author

deads2k commented Jan 8, 2016

@lavalamp fyi.

@k8s-bot
Copy link

k8s-bot commented Jan 14, 2016

GCE e2e test build/test passed for commit cb3379714e32ac857a4a1ce9bfb6d1ab8e5ab87d.

@deads2k
Copy link
Contributor Author

deads2k commented Jan 15, 2016

@k8s-bot unit test this please

@erictune
Copy link
Member

unit test says FAIL k8s.io/kubernetes/pkg/apis/authorization/validation 0.123s. Seems like it might be related.

@erictune
Copy link
Member

Okay, I've decided not to block this on comments from @lavalamp and @bgrant0607 .
So, LGTM.

@erictune erictune added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 20, 2016
@erictune
Copy link
Member

@k8s-bot unit test this please

@deads2k
Copy link
Contributor Author

deads2k commented Jan 20, 2016

rebased, validation typo fixed (unit test), and squashed.

@deads2k deads2k added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Jan 20, 2016
@k8s-bot
Copy link

k8s-bot commented Jan 20, 2016

GCE e2e test build/test passed for commit 14396fc.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jan 21, 2016

GCE e2e build/test failed for commit 14396fc.

@deads2k
Copy link
Contributor Author

deads2k commented Jan 21, 2016

@k8s-bot e2e test this please

@k8s-bot
Copy link

k8s-bot commented Jan 21, 2016

GCE e2e test build/test passed for commit 14396fc.

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jan 21, 2016

GCE e2e test build/test passed for commit 14396fc.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Jan 21, 2016
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit dda29be into kubernetes:master Jan 21, 2016
unversioned.TypeMeta `json:",inline"`

// Spec holds information about the request being evaluated
Spec SubjectAccessReviewSpec `json:"spec"`
Copy link
Member

Choose a reason for hiding this comment

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

It's useful for spec to be optional (omitempty) when posting to /status.

@erictune
Copy link
Member

Suggest in follow up PR make Spec and Status of each object be pointers so that Status can be empty on the POST and Spec can be empty in the response?

@smarterclayton
Copy link
Contributor

smarterclayton commented Jan 22, 2016 via email

@erictune
Copy link
Member

Fine. But can the specs be omitempty so that they don't have to be duplicated on the respose?

@deads2k deads2k deleted the add-sar branch January 25, 2016 17:50
@deads2k
Copy link
Contributor Author

deads2k commented Jan 25, 2016

Fine. But can the specs be omitempty so that they don't have to be duplicated on the respose?

I don't object. I'll update that when I start the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants