add rule to allow self-subject access reviews

[deads2k]

This adds a fine-grained policy rule that checks to see if a subject access review request is for "self". If it is, then the additional check allows it.

@liggitt It fell out pretty clean. Almost exactly as planned, except for needing to parse the content.

[liggitt]

if we don't have attribute restrictions, we can return true
if we do have attribute restrictions, and they aren't a recognized type, we should return false (or an error)

[deads2k]

That was bad. Fixed and a test added.

[deads2k]

rebased.

[liggitt]

I think IsPersonalSubjectAccessReview is a better name for the restriction marker type itself

[liggitt]

nit about the marker type name and squash, then LGTM

[deads2k]

Reminder to myself to update the serialization to omitempty

[deads2k]

[merge]

[openshift-bot]

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_openshift3/1257/) (Image: devenv-fedora_1106)

[openshift-bot]

Evaluated for origin up to 8e71ce9

[deads2k]

@pravisankar This should fix your problem with personal subject access reviews from the docker registry. Remember you'll need to delete the contents of your etcd to get the new default policy.

[smarterclayton]

This doesn't work anymore. I'm going to have to rip this and replace it with standard objects. You can't put interfaces into our objects right now.

[liggitt]

That shouldn't be using anything unusual. What wasn't working?

[smarterclayton]

runtime.EmbeddedObject{} has a runtime.Object in its internal version, which means its an interface, which means gob.Encode/Decode (used by conversion.DeepCopy) can't round trip it.

We didn't really need EmbeddedObject here - because we can just define things by type.

[liggitt]

Hmm, didn't that break other things that used runtime.EmbeddedObject?

[smarterclayton]

Nothing else uses EmbeddedObject

----- Original Message -----

Hmm, didn't that break other things that used runtime.EmbeddedObject?

---

Reply to this email directly or view it on GitHub:
#1392 (comment)

[liggitt]

@deads2k awesome… apparently we were the only ones using this. Trouble for config as well?

[smarterclayton]

We were using this in config???

----- Original Message -----

@deads2k awesome… apparently we were the only ones using this. Trouble for
config as well?

---

Reply to this email directly or view it on GitHub:
#1392 (comment)

[liggitt]

EmbeddedObject, yes

[smarterclayton]

We should probably not.

On May 4, 2015, at 7:17 PM, Jordan Liggitt [email protected] wrote:

EmbeddedObject, yes

—
Reply to this email directly or view it on GitHub.

[deads2k]

We didn't really need EmbeddedObject here - because we can just define things by type

What do you mean by, "define things by type"? There are other rules we want to write in code, things like kubelet rules. I'd rather not list every possibility as a pointer and name them all. You can also see the usage inside of the kubeconfig objects. That is the point where openshift can extend a kubeconfig file in an easy way. We also make use of these in the openshift start config objects for dealing with places where are there are many potentially valid types. Rather than trying to enforce "at most one" logic on a set of pointers.

I'd rather not end up trying to enforce a type that looks like this: https://github.com/openshift/origin/blob/master/pkg/build/api/types.go#L107. Adding anything other than git there is going to start looking crazy with partially completed objects. It'll start to look like this: https://github.com/openshift/origin/blob/master/pkg/build/api/types.go#L154 only a lot worse.

[smarterclayton]

On May 5, 2015, at 7:38 AM, David Eads [email protected] wrote:

We didn't really need EmbeddedObject here - because we can just define things by type

What do you mean by, "define things by type"? There are other rules we want to write in code, things like kubelet rules. I'd rather not list every possibility as a pointer and name them all.

If you have less than X where X is reasonably small the switch is easier to document and use as a client. Large X is where the unknown object becomes useful. Will sync with you in person and we can close this.
You can also see the usage inside of the kubeconfig objects. That is the point where openshift can extend a kubeconfig file in an easy way. We also make use of these in the openshift start config objects for dealing with places where are there are many potentially valid types. Rather than trying to enforce "at most one" logic on a set of pointers.

I'd rather not end up trying to enforce a type that looks like this: https://github.com/openshift/origin/blob/master/pkg/build/api/types.go#L107. Adding anything other than git there is going to start looking crazy with partially completed objects. It'll start to look like this: https://github.com/openshift/origin/blob/master/pkg/build/api/types.go#L154 only a lot worse.

—
Reply to this email directly or view it on GitHub.