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

support remote_cluster_resources_only=true and support redirect_uris for openshift oauthclient #295

Merged
merged 2 commits into from
Nov 7, 2024

Conversation

jmazzitelli
Copy link
Contributor

@jmazzitelli jmazzitelli commented Nov 1, 2024

this refactor the way we determine if it is an OpenShift cluster (for easier testing and maintenance)

part of kiali/kiali#7861

To test quickly run this 4-part command:

echo -e "\nK8s full install:\n" && helm template -n istio-system --set deployment.ingress.enabled=true --set isOpenShift=false kiali-server _output/charts/kiali-server-*.tgz | grep "^kind:" && echo -e "\nOpenShift full install\n" && helm template -n istio-system --set deployment.ingress.enabled=true --set auth.openshift.redirect_uris[0]=foo --set isOpenShift=true kiali-server _output/charts/kiali-server-*.tgz | grep "^kind:" && echo -e "\nK8s remote only install:\n" && helm template -n istio-system --set deployment.ingress.enabled=true --set deployment.remote_cluster_resources_only=true --set isOpenShift=false kiali-server _output/charts/kiali-server-*.tgz | grep "^kind:" && echo -e "\nOpenShift remote only install\n" && helm template -n istio-system --set deployment.remote_cluster_resources_only=true --set deployment.ingress.enabled=true --set auth.openshift.redirect_uris[0]=foo --set isOpenShift=true kiali-server _output/charts/kiali-server-*.tgz | grep "^kind:"

and see that only some resources are created when remote_cluster_resources_only is true:


K8s full install:

kind: ServiceAccount
kind: ConfigMap
kind: ClusterRole
kind: ClusterRoleBinding
kind: Service
kind: Deployment
kind: Ingress

OpenShift full install

kind: ServiceAccount
kind: ConfigMap
kind: ConfigMap
kind: ClusterRole
kind: ClusterRoleBinding
kind: Service
kind: Deployment
kind: OAuthClient
kind: Route

K8s remote only install:

kind: ServiceAccount
kind: ConfigMap
kind: ClusterRole
kind: ClusterRoleBinding

OpenShift remote only install

kind: ServiceAccount
kind: ConfigMap
kind: ClusterRole
kind: ClusterRoleBinding
kind: OAuthClient

Notice things like the ingress/route, service, and deployment are not installed when we just want the remote cluster resources created.

…for openshift oauth client

refactor the way we determine if it is an OpenShift cluster (for easier testing and maintainence)
@jmazzitelli jmazzitelli self-assigned this Nov 1, 2024
@jmazzitelli
Copy link
Contributor Author

jmazzitelli commented Nov 1, 2024

another test is to see the OAuthClient is not created in any case when auth.strategy is not openshift:

full install:

$ helm template -n istio-system --set auth.strategy=anonymous --set isOpenShift=true kiali-server _output/charts/kiali-server-*.tgz | grep "^kind:"
kind: ServiceAccount
kind: ConfigMap
kind: ConfigMap
kind: ClusterRole
kind: ClusterRoleBinding
kind: Service
kind: Deployment
kind: Route

remote cluster resources only:

$ helm template -n istio-system --set auth.strategy=anonymous --set deployment.remote_cluster_resources_only=true --set isOpenShift=true kiali-server _output/charts/kiali-server-*.tgz | grep "^kind:"
kind: ServiceAccount
kind: ConfigMap
kind: ClusterRole
kind: ClusterRoleBinding

Comment on lines 19 to 28
{{/*
Determine if on OpenShift (when debugging the chart for OpenShift use-cases, set "simulateOpenShift")
*/}}
{{- define "kiali-server.isOpenShift" -}}
{{- if .Values.simulateOpenShift -}}
true
{{- else }}
{{- .Capabilities.APIVersions.Has "operator.openshift.io/v1" -}}
{{- end -}}
{{- end }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of having another simulateOpenShift variable, we could just make isOpenShift (or maybe something like platform: openshift?) a variable and we only do the .Capabilities.APIVersions.Has "operator.openshift.io/v1" check if it's not defined?

Copy link
Contributor Author

@jmazzitelli jmazzitelli Nov 7, 2024

Choose a reason for hiding this comment

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

See latest commit.

There really isn't any other option for "platform" .. it's either OpenShift or it's not. So I just made it look for "isOpenShift" and only do the .Capabilities check if its not defined.

I updated the test procedures in earlier comments to now use --set isOpenShift=true"and --set isOpenShift=false... you can omit the --set isOpenShift=false since this will work without having to tell "helm template" that (because the .Capabilities will always return false anyway with "helm template").

@jmazzitelli jmazzitelli merged commit 98802c2 into kiali:master Nov 7, 2024
1 check passed
@jmazzitelli jmazzitelli deleted the 7861-remote-cluster-resources branch November 7, 2024 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants