OCPBUGS-59909: Don't cache server errors when checking for password grant support #781
Conversation
openshift-ci-robot
added
jira/severity-important
|
@jacobsee: This pull request references Jira Issue OCPBUGS-59909, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/retest-required |
1 similar comment
|
/retest-required |
liouk
left a comment
liouk
left a comment
There was a problem hiding this comment.
A couple of comments on the commit structure:
- we don't need the new caching mechanism, so no point in adding it and then removing it in separate commits; can you please drop those and keep only the commit for the 5xx?
- please don't prefix commits with the jira; we have this info in the PR. Use a component prefix instead, i.e.
configobservation/oauth:
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| if resp.StatusCode >= 500 && resp.StatusCode < 600 { |
There was a problem hiding this comment.
nit: 600 reads weird in http 😅 Would you mind using <=599 instead?
…hile checking for password grant support.
jacobsee
force-pushed
the
OCPBUGS-59909/dont-cache-non-decodable-oidc-forever
branch
from
eb4fa5c to
d0e360b
Compare
jacobsee
changed the title
|
@jacobsee: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jacobsee, liouk The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/retest-required |
|
/jira refresh |
|
@jacobsee: This pull request references Jira Issue OCPBUGS-59909, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@jacobsee: This pull request references Jira Issue OCPBUGS-59909, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-merge-bot
bot
merged commit b0bfaf3
into
openshift:master
|
@jacobsee: Jira Issue OCPBUGS-59909: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-59909 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[ART PR BUILD NOTIFIER] Distgit: ose-cluster-authentication-operator |
4 participants
If we encounter an OIDC server which is not properly responding using JSON, we should occasionally retry pinging it for the purpose of checking password grant support rather than assuming that it doesn't support password grant forever.Now: Reject and don't cache 5xx errors outright while checking for password grant support.