return forbidden api status object

[deads2k]

Fixes #1421.

By using the API status object, we get a message on the cli that looks like this:

[deads@deads-dev-01 origin]$ osc get nodes
F0325 09:21:42.892396    8611 get.go:67] Error received from API: "/api/v1beta1/nodes?namespace=default" is forbidden because anypassword:asdf cannot list on nodes in default

I think the code will fall out much cleaner for forbidden resource requests once we have full access to the APIInstaller information to properly determine kinds, names, namespaces and codecs, but this is an improvement.

@liggitt

[liggitt]

rather than jumping through hoops to use NewForbidden, can we just do

&kapierror.StatusError{
  kapi.Status{
    Status: kapi.StatusFailure,
    Code:   http.StatusForbidden,
    Reason: kapi.StatusReasonForbidden,
    Message: fmt.Sprintf("%q is forbidden because %s", req.RequestURI, reason),
  },
}

[deads2k]

I think I'd rather stay with the library function and rely on it to set every field except for the message. That keeps us in sync if make changes. It will also alert a rebaser if we end up fixing it upstream.

[liggitt]

is it more correct to have a nil Details field or a Details field with empty name and kind?

[deads2k]

I'd read it as, "a reason forbidden indicates that the StatusDetails MUST be present, and MAY have 'kind' set and MAY have 'id' set". The other reading wouldn't make much sense because it would say, "a reason forbidden indicates that the StatusDetails MUST be present, and MAY have a details stanza. If there is a details stanza, it MUST have both 'kind' and 'id'". That doesn't make much sense since id's aren't always available.

Clearly we should demand RFC style godoc for clarity!

[deads2k]

Also, you could see an argument for including Verb and Namespace in the details. We want to set all those fields in a structured way someday.

[liggitt]

ok

[liggitt]

LGTM

[deads2k]

[merge]

[openshift-bot]

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

[openshift-bot]

Evaluated for origin up to c76aaba