provide a way to tell the operator to create only the remote cluster resources

[jmazzitelli]

This adds a new deployment flag in Kiali CR:

spec:
  deployment:
    remote_cluster_resources_only: false

The default is false, which means it all behaves like before. But when set to true, only the remote cluster resources are created by the operator (only SA, role/bindings, configmap - no secrets, deployment/pod, ingress/route, or service resources are created; also, no OpenShift specific resources (ConsoleLink, OAuthClient) are created).

The ConfigMap, while not needed by the server when accessing the remote cluster, it is needed by the operator for future reconciliation of the CR, hence that is why the ConfigMap is created along with the SA and roles/bindings.

This supports the typical deployment.discovery_selectors.default and deployment.cluster_wide_access behavior - they determine which roles/bindings are created - if CWA is false the accessible namespaces defined by discovery selectors will get Role / RoleBindings created. If CWA is true, the cluster role/binding pair are created. This is no different than when deploying the server normally (it's all the same code).

fixes: kiali/kiali#7861

[jmazzitelli]

this really is a draft right now - strictly PoC. It's just some noodling in my head and me trying something.

[jmazzitelli]

Some quick smoke testing shows this is (surprisingly :-) working pretty good; very small amount of code changes, too.

I will write some molecule tests to exercise this functionality more completely before marking this PR as "ready for review".

[jmazzitelli]

Test procedures (it is basically the same for both OpenShift and minikube)

Quick Test

If you can't run molecule tests for whatever reason, or just want to run a quick test, you can do the following (I'll assume you have an OpenShift or minikube cluster with Istio installed and the dev image of the operator pushed to your cluster via the make target "cluster-push"):

1. Create a Kiali CR with deployment.remote_cluster_resources_only set to true and deployment.cluster_wide_access set to true (I'll assume you are going to use auth.strategy=anonymous - if not, you have to set auth.openshift.redirect_uris)
2. After the operator reconciliation is done, look in istio-system (or where ever you put the CR or set deployment.namespace) and see that you only have the SA, cluster role/binding, and the ConfigMap - ensure there IS NO kiali deployment, kiali pod, kiali service, or any kiali secrets: kubectl get role,clusterrole,rolebinding,clusterrolebinding,deployment,pod,sa,secret,cm -n istio-system -l app.kubernetes.io/name=kiali
3. Set the Kiali CR deployment.cluster_wide_access to false and see the cluster roles/bindings go away and the role/rolebinding in the accessible namespaces get created (if you don't set any discovery selectors, should at least go in the deployment namespace; but you can set discovery selectors to see the roles go in whatever you select).
4. Delete the Kiali CR and see that everything deletes successfully.

OpenShift

1. Start OpenShift (crc) and install Istio

hack/crc-openshift.sh start -p ~/bin/downloads/openshift-pull-secret.txt && \
oc login -u kubeadmin -p kiali https://api.crc.testing:6443 --insecure-skip-tls-verify && \
podman login --tls-verify=false -u kiali -p $(oc whoami -t) default-route-openshift-image-registry.apps-crc.testing && \
hack/istio/install-istio-via-istioctl.sh -cp openshift

2. Push the dev images

make cluster-push

3. Run the the new molecule test and then another test (using the dev images (-udi)) to confirm the operator can clean up the deleted CR properly and a new one can be reconciled successfully

hack/run-molecule-tests.sh --client-exe "$(which oc)" -dorp podman --cluster-type openshift -udi true -at "accessible-namespaces-test remote-cluster-resources-test rolling-restart-test"

Minikube

1. Start minikube and install Istio

hack/k8s-minikube.sh start && hack/k8s-minikube.sh istio

2. Push the dev images

make CLUSTER_TYPE=minikube cluster-push

3. Run the the new molecule test and then another test (using the dev images (-udi)) to confirm the operator can clean up the deleted CR properly and a new one can be reconciled successfully

hack/run-molecule-tests.sh --client-exe "$(which kubectl)" -dorp podman --cluster-type minikube -udi true -at "accessible-namespaces-test remote-cluster-resources-test rolling-restart-test"

[jmazzitelli]

I ran the full suite of molecule tests and things look good.

[nrfox]

no OpenShift specific resources (ConsoleLink, OAuthClient) are created).

For OpenShift, we will still need an OAuthClient in the remote cluster: https://github.com/kiali/kiali/blob/31bcc777ea265c4ec971f25608ecbbf52c291ca8/hack/istio/multicluster/deploy-kiali.sh#L201-L202

And we need a way of passing a redirectURI to the OAuthClient on the remote cluster.

[jmazzitelli]

no OpenShift specific resources (ConsoleLink, OAuthClient) are created).

For OpenShift, we will still need an OAuthClient in the remote cluster: https://github.com/kiali/kiali/blob/31bcc777ea265c4ec971f25608ecbbf52c291ca8/hack/istio/multicluster/deploy-kiali.sh#L201-L202

And we need a way of passing a redirectURI to the OAuthClient on the remote cluster.

OK. Here's the OAuthClient that is created when deploying Kiali Server normally. We'll want to reuse that (just as this PR reuses all the other templates). This looks doable, because the only thing that looks like we need to be able to set (that isn't already configurable in the Kiali CR) is that kiali_route_url which the operator auto-detects.

Interestingly, I think at one point in the past we considered making that configurable in the auth.openshift section in order to allow the user to circumvent the operator auto-detection just in case for whatever reason the auto-detection was not able to find it. We never did implement that. I think we need to do that now.

I think things will work if we add this to the Kiali CR (inside the auth.openshift section):

So we'd now have:

spec:
  auth:
    openshift:
      kiali_route_url: http://blahblah

That would not be defined by default, thus retaining backward compat and enabling the operator's auto-discovery. But if the user defines that, the operator would skip auto-discovery and use spec.auth.openshift.kiali_route_url for the value of that kiali_route_url that normally got auto-discovered. This will be how the user would configure it on the remote side, but could also be used when normally installing Kiali in case (for whatever reason) the operator is not able to auto-discover the route after it is installed (probably would never be used that way but, hey, you never know).

(side note: kiali_route_url is also used in one other place, and is unrelated to auth -- in the ConsoleLink. That's why I did not propose the new config name to be "redirect_uri" and whose value is different than the root route URL - because the console link would also want to use it. Console links are not created in the remote cluster, BTW.

[nrfox]

kiali_route_url and redirectURI are two potentially different things. I'm confused as to why it's not spec.auth.openshift.redirectURI[s]?

[jmazzitelli]

kiali_route_url and redirectURI are two potentially different things. I'm confused as to why it's not spec.auth.openshift.redirectURI[s]?

Because:

(side note: kiali_route_url is also used in one other place, and is unrelated to auth -- in the ConsoleLink. That's why I did not propose the new config name to be "redirect_uri" and whose value is different than the root route URL - because the console link would also want to use it.

I'm torn between the name because if we set it to redirect_uri, I can't use this setting for the ConsoleLink objects (even though the user will know the url because he's specifying it here, with some extra stuff for the redirect) which means I'll have to do the discovery anyway, even if the user specified the redirect_uri. But then again, the user most likely won't have to specify the route url if deploying normally because the route will be created and the operator should discovery it.

I'm know that's confusing, but it makes sense to me :)

[jmazzitelli]

spec.auth.openshift.redirectURI[s]

You indicate this might be plural (more than one URI) -- is that true? If so, then we might just make this a separate thing (separate from the ConsoleLink).

So, would this actually be a valid thing to want to do?

spec:
  auth:
    openshift:
      redirect_uri:
      - http://first
      - http://second

[jmazzitelli]

The current code in this PR as of now passes the new molecule tests and has the new OAuthClient functionality. But need to think more about the name of the new setting and how we want it to be configurable; more changes might be coming

[jmazzitelli]

@nrfox OK, I just realized that OAuthClient is able to be configured with multiple redirectURIs (as of our latest release, we only define one - using the kiali route). If we are going to be able to allow a user to provide custom redirect URI, we might as well support them being able to give us multiple ones.

So, I will change this implementation slightly so spec.auth.openshift no longer has kiali_route_url but instead has a new list setting redirect_uris such that this would be possible:

spec:
  auth:
    openshift:
      redirect_uris:
      - http://foo
      - http://bar

Is that reasonable?

[jmazzitelli]

I changed the code to now be able to set auth.openshift.redirect_uris as my previous comment described. Molecule tests in place.

[nrfox]

@nrfox OK, I just realized that OAuthClient is able to be configured with multiple redirectURIs (as of our latest release, we only define one - using the kiali route). If we are going to be able to allow a user to provide custom redirect URI, we might as well support them being able to give us multiple ones.

The way that the openshift oauth service is implemented is that when Kiali connects to the remote cluster, it will just choose the first redirect_uri: https://github.com/kiali/kiali/blob/a9f4d727243affd113809bc3d398403ee174f3b4/business/openshift_oauth.go#L142

That being said, I think it makes sense for the field to support multiple values because if you want to set this outside of the multi-cluster use case then you might want multiple values there and it's just a pass through from the Kiali CR to the OAuthClient resource.

[jmazzitelli]

molecule tests all pass on OpenShift:

              accessible-namespaces-test... success [7m 58s]
     affinity-tolerations-resources-test... success [3m 43s]
                cluster-wide-access-test... success [8m 25s]
                      config-values-test... success [3m 43s]
                  default-namespace-test... success [1m 55s]
                            grafana-test... success [3m 32s]
                        header-auth-test... skipped
                      instance-name-test... success [2m 21s]
                             jaeger-test... success [2m 38s]
                            metrics-test... success [3m 45s]
                     null-cr-values-test... success [1m 43s]
                only-view-only-mode-test... success [2m 35s]
                             openid-test... skipped
                   os-console-links-test... success [4m 42s]
          ossmconsole-config-values-test... success [3m 7s]
           remote-cluster-resources-test... success [4m 1s]
                              roles-test... success [5m 20s]
                    rolling-restart-test... success [2m 36s]
                              token-test... success [2m 4s]

[nrfox]

Now I think we'll need to update the kiali-prepare-remote-cluster.sh hack script to have an option to only create the remote secret and not deploy the remote resources so that you can create a Kiali in each cluster with one being a "full" kiali and one being a "remote" kiali. Otherwise when you create the "remote" kiali it overrides the resources created by the hack script.

[jmazzitelli]

Now I think we'll need to update the kiali-prepare-remote-cluster.sh hack script to have an option to only create the remote secret and not deploy the remote resources

That is already supported: --process-kiali-secret true --process-remote-resources false

See: https://github.com/kiali/kiali/blob/master/hack/istio/multicluster/kiali-prepare-remote-cluster.sh#L466-L479

[jmazzitelli]

@nrfox approved these changes 24 minutes ago

Are you also going to look at the 3 other PRs for this issue, or should we get others to review?

PRs are listed here: kiali/kiali#7861 (comment)