@deads2k @mkulke PTAL
@mkulke can you comment to appease the cla bot?

@kubernetes/kube-iam this should give us sufficient authz power to allow API discovery and version negotiation while still constraining users to namespaces. I'd like to update the ABAC file format to a versioned format (falling back to the current JSONL format on deserialize errors for backwards compatibility), but will do that in another PR

Labelling this PR as size/L

@liggitt the sticking point last time was whether non-resource access was default allow or default deny. Which did you choose?

@liggitt LGTM

i built and tested the branch, seems to work fine:

$ ../../tmp/kubernetes/_output/dockerized/bin/linux/amd64/kubectl version
Client Version: version.Info{Major:"1", Minor:"2+", GitVersion:"v1.2.0-alpha.1.1513+e162b71ae80901", GitCommit:"e162b71ae809019a631c0704f18c3a1de631fb86", GitTreeState:"clean"}
Server Version: version.Info{Major:"1", Minor:"2+", GitVersion:"v1.2.0-alpha.1.1513+e162b71ae80901", GitCommit:"e162b71ae809019a631c0704f18c3a1de631fb86", GitTreeState:"clean"}

$ ../../tmp/kubernetes/_output/dockerized/bin/linux/amd64/kubectl --context=orange run nginx --image=nginx
replicationcontroller "nginx" created

$ ../../tmp/kubernetes/_output/dockerized/bin/linux/amd64/kubectl --context=black run nginx --image=nginx
replicationcontroller "nginx" created

$ ../../tmp/kubernetes/_output/dockerized/bin/linux/amd64/kubectl --context=black get po
NAME          READY     STATUS    RESTARTS   AGE
nginx-b80ks   1/1       Running   0          5m

$ ../../tmp/kubernetes/_output/dockerized/bin/linux/amd64/kubectl --context=liggitt --all-namespaces=true get po
NAMESPACE   NAME          READY     STATUS    RESTARTS   AGE
black       nginx-b80ks   1/1       Running   0          6m
orange      nginx-ayprx   1/1       Running   0          6m

flakes, only one that looked like it could be relevant (kubectl test) passed locally:

[Fail] Kubectl client Kubectl describe [It] should check if kubectl describe prints relevant information for rc and pods [Conformance] 
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/kubectl.go:936

[Fail] Probing container [It] with readiness probe should not be ready before initial delay and never restart [Conformance] 
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/container_probe.go:59

[Fail] Pods [It] should have their container restart back-off timer increase exponentially 
/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/pods.go:859

@k8s-bot test this

maybe... do you care if I squash to a single commit?

@deads2k the sticking point last time was whether non-resource access was default allow or default deny. Which did you choose?

I chose default deny, because default allow seems crazy when a field name typo in the policy file means the rule would be empty, allowing all. Granted, that's how namespace and resource behave now... but I was hoping to avoid propagating that.

Thinking further, though, {"user":"admin"} has to grant access to everything (resource and non-resource) to preserve compatibility. I'll add tests to make sure that doesn't break.

To get this looking the way I want internally, I may bite the bullet and add a versioned schema for ABACPolicy, and start requiring "*" for namespace and resource to match. Something like this:

{
  "apiVersion":"v1",
  "kind":"ABACPolicyList",
  "items":[
    {"user":"admin","nonResourcePath":"*","namespace":"*",resource:"*"},
  ]
}

Deserialize failures on old ABAC files could attempt to read the JSONL format and convert to the versioned form like this:

{"user":"u"}                  -> {"user":"u","nonResourcePath":"*","namespace":"*","resource":"*"}

{"user":"u","namespace":"ns"} -> {"user":"u","namespace":"ns","resource":"*"}

{"user":"u","resource":"r"}   -> {"user":"u","namespace":"*","resource":"r"}

Before I start on that, any objections from @kubernetes/kube-iam on the approach?

or, while bullets are being bitten, this:

{
  "apiVersion":"v1",
  "kind":"ABACPolicyList",
  "items":[
    {
      "user":"u",
      "groups":["g1","g2"],
      "verbs":["get","list","watch"], // or ["*"]
      "namespaces":["ns1","ns2"], // or ["*"]
      "resources":["pods","replicationcontrollers"], // or ["*"]
      "nonResourcePaths":["/api","/api/*","/apis","/apis/*"] // or ["*"]
    },
    ...
  ]
}

Being able to grant a user or group permission across namespaces, verbs, and resources in a single rule improves readability quite a bit. Depends on how real we envision this being, and whether we see this as a progression toward dynamic authz.

or, while bullets are being bitten, this:

No. That makes verbs for nonResourcePaths weird. What would a watch on a nonResourcePath mean?

@liggitt: sure, go ahead.

I like default denies and other than allowing both non-resource and resource rules in a single statement, I like the format of #16148 (comment).

If you want a new format, would it be easier to simply have a second authorizer to implement it?

/cc @erictune (This is the kubectl namespacing patch I was talking about)

Labelling this PR as size/XXL

GCE e2e test build/test passed for commit 0ad102799d267992f65df5505a50eea44aa90a56.

• Build Log
• Test Artifacts
• Internal Jenkins Results

GCE e2e test build/test passed for commit bb20bead6cf93cf3f5faf5adff6ca0c1d6591a97.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Thank you for humoring me, no more objections here!

lgtm.

Squash and tag at will.

@lavalamp thinking further, was there a specific reason for making the version v1beta1 rather than v1? It's a very granular api group, so bumping versions wouldn't be hard, I just wondered

Yeah-- if we call it v1beta1 then I don't have to be picky about the
contents of the API types :) :)

On Wed, Dec 2, 2015 at 10:37 AM, Jordan Liggitt notifications@github.com
wrote:

@lavalamp https://github.com/lavalamp thinking further, was there a
specific reason for making the version v1beta1 rather than v1? It's a
very granular api group, so bumping versions wouldn't be hard, I just
wondered

—
Reply to this email directly or view it on GitHub
#16148 (comment)
.

GCE e2e test build/test passed for commit dc4506ecd57e2f53a4ec7fad31f9a99914a71e77.

• Build Log
• Test Artifacts
• Internal Jenkins Results

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

GCE e2e test build/test passed for commit dc4506ecd57e2f53a4ec7fad31f9a99914a71e77.

• Build Log
• Test Artifacts
• Internal Jenkins Results

rebased

@k8s-bot ok to test
@k8s-bot test this

pr builder appears to be missing, activating due to 'lgtm' label.

Travis continuous integration appears to have missed, closing and re-opening to trigger it

GCE e2e test build/test passed for commit 2321651.

• Build Log
• Test Artifacts
• Internal Jenkins Results

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

GCE e2e test build/test passed for commit 2321651.

• Build Log
• Test Artifacts
• Internal Jenkins Results

Automatic merge from submit-queue