Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

operator-route-playbook: add check for auth.strategey if running on openshift (OSSM8665) #859

Closed
wants to merge 1 commit into from

Conversation

michaelalang
Copy link

The Kiali operator does not honor a custom route even when

spec:
  auth:
    strategy: !openshift

with reconciliation errors

TASK [v1.73/kiali-deploy : Get the Kiali Route URL] ****************************
included: /opt/ansible/roles/v1.73/kiali-deploy/tasks/openshift/os-get-kiali-route-url.yml for localhost

TASK [v1.73/kiali-deploy : Detect Kiali route on OpenShift] ********************
FAILED - RETRYING: Detect Kiali route on OpenShift (30 retries left).

PLAY RECAP *********************************************************************
localhost                  : ok=76   changed=8    unreachable=0    failed=1    skipped=50   rescued=0    ignored=0   

{"level":"error","ts":"2024-12-25T19:53:44Z","msg":"Reconciler error","controller":"kiali-controller","object":{"name":"kiali","namespace":"istio-system"},"namespace":"istio-system","name":"kiali","reconcileID":"c242cc58-3a09-4331-9310-652ca7947a9b","error":"event runner on failed","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\toperator-sdk/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:329\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\toperator-sdk/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:274\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\toperator-sdk/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:235"} 

Reason for that is the playbook only checks for is_openshift but does not account for a different auth strategy.

Copy link
Contributor

@jmazzitelli jmazzitelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

auth.strategy does not matter when determining is_openshift. The internal setting is_openshift is ONLY ever determined by what cluster it is. We never look at auth.strategy anywhere to determine if is_openshift is true. So this is an invalid PR change.

@jmazzitelli
Copy link
Contributor

Can you explain more what you mean "does not honor custom route"?

@jmazzitelli
Copy link
Contributor

The reason why I say this is invalid is because we still need kiali_route_url for the ConsoleLinks (and those are created regardless of auth.strategy. See https://github.com/kiali/kiali-operator/blob/v2.3.0/roles/default/kiali-deploy/templates/openshift/console-links.yaml#L9 )

If you, for example, set auth.strategy to anonymous, and this PR is in effect, you will get invalid ConsoleLinks created (and most likely get an operator abort since kiali_route_url won't be defined).

@michaelalang
Copy link
Author

The reason why I say this is invalid is because we still need kiali_route_url for the ConsoleLinks (and those are created regardless of auth.strategy. See https://github.com/kiali/kiali-operator/blob/v2.3.0/roles/default/kiali-deploy/templates/openshift/console-links.yaml#L9 )

If you, for example, set auth.strategy to anonymous, and this PR is in effect, you will get invalid ConsoleLinks created (and most likely get an operator abort since kiali_route_url won't be defined).

@jmazzitelli setting auth.strategy to anonymous and adding a custom route protected by oauth proxy leaves the Kiali operator in the reconciliation loop. Reason in the logs as mentioned is that it never finishes for the route task.
That the OpenShift Console links are not created only matters in case of installing the plugin which isn't happening anyway with multi-cluster CR's as the auth strategy never works with auth.strategy openshift in that regards.

According to the documentation and when setting deployment.ingress.enabled: false will render the operator always into this state.
I am not asking for moving this scenario into a supported configuration but to give the possibilities to people to use the Kiali CR's to deploy in a way it fits their infrastructure with a working reconciliation loop of the operator.

@jmazzitelli
Copy link
Contributor

when setting deployment.ingress.enabled: false

Ah!! That's the missing piece! OK. I see now. This isn't directly related to the strategy, but if ingress is disabled, we need to:

  1. skip the check for the Route
  2. skip creation of OAuthClient (because it needs the route)
  3. skip creation of ConsoleLinks (because they need the route).
  4. IF the user has auth.strategy set to "openshift" (which is the default if is_openshift==true) then the operator needs to abort the reconciliation because that strategy requires the Route

I created a github issue for this: kiali/kiali#8023

@jmazzitelli
Copy link
Contributor

This is what we need (this isn't everything - I need to figure out the best way to skip creating the clusterrole/bindings for the OAuthClient - probably just need to add it to "Process OpenShift OAuth client" task)

#860

@jmazzitelli
Copy link
Contributor

@michaelalang I'm going to close this PR in lieu of this PR: #860

In that new PR, I think I have this fixed, along with added molecule test code to make sure it doesn't break anything else. I'm still testing, but I think that PR has everything covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants