Enable osin internal error logging

[mrogers950]

osin now has an interface for internal error logging that provides some additional context on failure.

Example output:

=== RUN   TestClientCredentialFlow
--- PASS: TestClientCredentialFlow (0.00s)
=== RUN   TestAuthorizeStartFlow
I0206 14:58:22.818652   22392 access.go:561] osin: error=unauthorized_client, internal_error=<nil> get_client=client check failed, client_id=test
--- FAIL: TestAuthorizeStartFlow (0.00s)
osinserver_test.go:95: unexpected error: The client is not authorized to request a token using this method.
osinserver_test.go:135: unexpected error: Get http://127.0.0.1:39369/assert?code=KWhLRoflbPdPCNYNP6T3MpqZn4mv0zxKLU6TRg6QEAc&state=: EOF

@openshift/sig-security

[mrogers950]

/retest

[enj]

I feel like we always want to log these. They are rare and point to "stuff is broken". Maybe V(1) in case we are really worried about spam. @sdodson @jupierce @smarterclayton any concerns? What log level do we normally run online at?

[php-coder]

What log level do we normally run online at?

It's a loglevel 2 AFAIR.

[enj]

It's a loglevel 2 AFAIR.

Confirm this with @jupierce

[enj]

Info depth of 3 so we see the correct caller. Error depth is probably more appropriate (pretty sure every place we log leads to a failed OAuth flow).

[mrogers950]

@enj I rebased and turned the V level down to 2, and changed InfoDepth to an ErrorDepth of 3.

[mrogers950]

/retest

[enj]

/lgtm

@deads2k can you tag?

[deads2k]

So bumps require determination of whether you should actually bump the packages in question. Separating the repo dependencies into yours, mine and ours buckets, we do not pin "yours" (kube's). We always pin "mine" (but we have identified the set). We sometimes pin "ours" depending on whether we really care what it is and who the other dependant is.

We are actively trying to make this better (see #18543), but it isn't going to make 3.9.

[deads2k]

Oh, and syncing out from our vendor folder is currently in progress. Rules merged early this morning and @mfojtik is trying to enable the bot.

[mrogers950]

@deads2k I bumped only the osin dependency manually. I'm not sure how the vendor sync-out you mentioned affects this, so let me know if there is something I might need to do different here...

[deads2k]

@deads2k I bumped only the osin dependency manually. I'm not sure how the vendor sync-out you mentioned affects this, so let me know if there is something I might need to do different here...

A manual change just leaves a time bomb for the next guy. I know it's annoying, I know it can be tedious, I know it's not something you'd like to do. I know this far more than the average person on the team. Understanding how our product manages golang dependencies must not remain an arcane skill. Please take the time to learn how to manage dependencies.

[mrogers950]

@deads2k after rebasing, this time running update-deps resulted in only updating osin (a "mine" now pinned at 2dc1b431) and k8s.io/kubernetes. PTAL

[mrogers950]

/retest

[mrogers950]

@deads2k nevermind, this dep update was wrong again. specifically, it was just reverting upstream commits that have gone in to origin since the last kube sync (at 0f3cef4), so I think I need to wait until kube is caught up.

[mrogers950]

@deads2k thanks for syncing kube! the dep bump is clean now. PTAL.

[deads2k]

/approve

[enj]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, enj, mrogers950

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• OWNERS [deads2k]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[mrogers950]

/retest

[openshift-merge-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[openshift-merge-robot]

Automatic merge from submit-queue (batch tested with PRs 18505, 18617, 18604).