WIP: AUTH-543: OIDC/OAuth resource configuration #740
Conversation
|
@liouk: This pull request references AUTH-543 which is a valid jira issue. 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-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: liouk The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
liouk
force-pushed
the
oidc-oauth-cleanup
branch
22 times, most recently
from
e99d56a to
46287b6
Compare
liouk
force-pushed
the
oidc-oauth-cleanup
branch
from
46287b6 to
ae635c0
Compare
liouk
changed the title
If OIDC is available then set replicas to 0, otherwise fall back to the default behaviour of counting nodes.
This allows the controller to proceed with the sync and eventually scale down the deployment to 0 replicas.
… external OIDC config is available
So that it can be stopped when auth type is OIDC.
liouk
force-pushed
the
oidc-oauth-cleanup
branch
from
ae635c0 to
5198aea
Compare
| return false, err | ||
| } else if oidcAvailable { | ||
| // the route is no longer a pre-requisite | ||
| return true, nil |
There was a problem hiding this comment.
The route is gone. But we can still fulfill the precondition without the need for a deployment of the oauth-server?
There was a problem hiding this comment.
I am currently reworking the way the workload/deployment controllers clean up the deployment, so this will likely change. I'll report back to this thread once this is done.
| } else if oidcAvailable { | ||
| return common.ApplyControllerConditions(ctx, c.operatorClient, c.controllerInstanceName, knownConditionNames, nil) | ||
| } | ||
|
|
||
| foundConditions := []operatorv1.OperatorCondition{} | ||
|
|
||
| workers, err := c.nodeLister.List(labels.SelectorFromSet(labels.Set{"node-role.kubernetes.io/worker": ""})) |
There was a problem hiding this comment.
Isn't it odd, that we check pod availability for the ingress pods primarily on worker nodes, while the oauth-servers are on the master node?
If the ingress dies, the oauth-server becomes unreachable, no?
…atus if external OIDC config is available
|
Pre-merge testing has revealed some issues with this PR; turning this into WIP until fixed. /retitle WIP: AUTH-543: OIDC/OAuth resource configuration |
openshift-ci
bot
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
@liouk: 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. |
This PR adjusts all OAuth related controllers to remove (or not) their operands depending on whether authentication is external OIDC or not.