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

do not create things that need route if ingress is disabled #860

Merged
merged 1 commit into from
Dec 31, 2024

Conversation

jmazzitelli
Copy link
Contributor

@jmazzitelli
Copy link
Contributor Author

jmazzitelli commented Dec 27, 2024

This needs to skip creating the clusterrole/bindings for the OAuthClient - probably just need to add it to "Process OpenShift OAuth client" task. I'll work on that next

UPDATE: actually, nothing needs to be done. The existing code we have is good enough. Those cluster role/bindings for oauth are not created unless auth.strategy is "openshift" and the only time auth.strategy is allowed to be "openshift" is if the ingress is enabled. If ingress is disabled, the auth.strategy is not allowed to be/will never be openshift, thus if ingress is disabled, then these cluster role/bindings for oauth will never be created.

@jmazzitelli jmazzitelli marked this pull request as ready for review December 29, 2024 03:14
@jmazzitelli
Copy link
Contributor Author

jmazzitelli commented Dec 29, 2024

The molecule tests that should show this doesn't break anything:

  1. Get an OpenShift cluster - make sure you have user credentials kiali/kiali (use hack/crc-openshift.sh to make it easy)
  2. Install Istio (e.g. hack/istio/install-istio-via-istioctl.sh -cp openshift)
  3. cd to your ossmc local repo and build and push the ossmc plugin image (one of the molecule tests installs it): make cluster-push
  4. Build and push server/operator dev images: make build build-ui cluster-push
  5. Run all molecule tests: ./hack/run-molecule-tests.sh --client-exe "$(which oc)" --cluster-type openshift -udi true

Molecule tests pass for me:

=====================
=== TEST RESULTS: ===
=====================

              accessible-namespaces-test... success [8m 38s]
     affinity-tolerations-resources-test... success [4m 22s]
                cluster-wide-access-test... success [10m 2s]
                      config-values-test... success [3m 37s]
                  default-namespace-test... success [2m 36s]
                            grafana-test... success [4m 11s]
                        header-auth-test... skipped
                      instance-name-test... success [3m 33s]
                             jaeger-test... success [3m 16s]
                            metrics-test... success [4m 30s]
                     null-cr-values-test... success [2m 20s]
                only-view-only-mode-test... success [3m 20s]
                             openid-test... skipped
                   os-console-links-test... success [5m 53s]
          ossmconsole-config-values-test... success [3m 4s]
           remote-cluster-resources-test... success [4m 43s]
                              roles-test... success [6m 24s]
                    rolling-restart-test... success [3m 25s]
                              token-test... success [2m 43s]

@jmazzitelli
Copy link
Contributor Author

After you run the test instructions in the previous comment, you can do additional manual testing:

  1. Install operator and server: make operator-create kiali-create
  2. Confirm everything is installed: oc get pods -n istio-system -l app.kubernetes.io/name=kiali
  3. Patch the Kiali CR such that the ingress is disabled: oc patch kiali kiali -n kiali-operator --type=json -p='[{"op": "replace", "path": "/spec/deployment/ingress/enabled", "value": false}]'
  4. Check that the operator aborted the reconciliation with the correct error message:
$ oc get kiali kiali -n kiali-operator -o jsonpath='{.status}' | jq

...
      "lastTransitionTime": "2024-12-29T16:15:22Z",
      "message": "The auth.strategy is 'openshift' which requires a Route, but deployment.ingress.enabled is false. Aborting.",
      "reason": "Failed",
      "status": "True",
      "type": "Failure"
...
  1. Now patch the Kiali CR and turn the auth.strategy to anonymous: oc patch kiali kiali -n kiali-operator --type=json -p='[{"op": "replace", "path": "/spec/auth/strategy", "value": "anonymous"}]'
  2. The install should succeed - you should see no errors in the Kiali CR status and there should be a Kiali server pod that has just been created:
$ oc get kiali kiali -n kiali-operator -o jsonpath='{.status}' | jq
...<there should be no errors>...
$ oc get pods -n istio-system -l app.kubernetes.io/name=kiali
NAME                     READY   STATUS    RESTARTS   AGE
kiali-668c44b795-rkb4z   1/1     Running   0          10s

. abort if ingress is disabled but using openshift auth strategy
Copy link
Contributor

@josunect josunect left a comment

Choose a reason for hiding this comment

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

LGTM:

Molecule tests passed locally in crc:

image

Manual tests with expected results:
Enable ingress:
image

anonymous strategy:
image

kiali pod:
image

@jmazzitelli jmazzitelli merged commit 8d80e98 into kiali:master Dec 31, 2024
1 check passed
@jmazzitelli jmazzitelli deleted the 8023-skip-route branch December 31, 2024 18:49
@jshaughn jshaughn added the test: molecule Molecule test infrastructure and tests themselves label Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test: molecule Molecule test infrastructure and tests themselves
Projects
3 participants