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

Improve Install via Helm. #765

Merged
merged 3 commits into from
Mar 19, 2024
Merged

Conversation

chickenchickenlove
Copy link
Contributor

Background

Hi, I'm new in kiali and this is my first PR!
When I first tried to install the Kiali operator and access the Kiali UI, I encountered a problem where the token could not be found, and it was difficult to find a solution through searching the documentation.
The problem occurred with the commands below.

$ helm install \
    --set cr.create=true \
    --set cr.namespace=istio-system \
    --namespace kiali-operator \
    --create-namespace \
    kiali-operator \
    kiali/kiali-operator

Thus, IMO, it will be better to add --set cr.auth.strategy="anonymous" for newbie in kiali to started with kiali easily.

Changes

  • Add --set cr.auth.strategy="anonymous" to install via helm page.
  • Add some comments to describe accessing kiali UI at install via helm page.

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.

This is OK. The reason we did not specify anonymous setting before is because this will open up the Kiali UI to anyone. So it isn't secure. Because of this, just add a blurb at the end to call this out to the reader so they are aware running that command is not secure, and if they want to make it secure, there are options to do so. See my suggestion (make sure that link works :).

@jmazzitelli
Copy link
Contributor

The netlify link to go to in order to see the change is found here: https://deploy-preview-765--kiali.netlify.app/docs/installation/installation-guide/install-with-helm/#install-with-operator

Co-authored-by: John Mazzitelli <mazz@redhat.com>
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.

Check the netlify page - https://deploy-preview-765--kiali.netlify.app/docs/installation/installation-guide/install-with-helm/#install-with-operator

As I suspected ("See my suggestion (make sure that link works :)") , the link doesn't work :) I think you need to prefix it with /doc. So /doc/configuration/authentication/.

@jmazzitelli jmazzitelli merged commit 2f28068 into kiali:staging Mar 19, 2024
5 checks passed
@@ -69,6 +69,7 @@ command:
$ helm install \
--set cr.create=true \
--set cr.namespace=istio-system \
--set cr.auth.strategy="anonymous" \
Copy link
Contributor

Choose a reason for hiding this comment

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

This is my fault. I did not properly review this PR.

This is wrong and does nothing to affect the strategy. It should have been:

    --set cr.spec.auth.strategy="anonymous" \

(note the missing .spec)

the Kiali operator starts, it will process it to deploy Kiali. After Kiali has started,
you can access Kiali UI through 'http://localhost:20001' by executing
`kubectl port-forward service/kiali -n istio-system 20001:20001`
because of `--set cr.auth.strategy="anonymous"`. But realize that anonymous mode will allow anyone to be able to see and use Kiali. If you wish to require users to authenticate themselves by logging into Kiali, use one of the other [auth strategies](/docs/configuration/authentication/).
Copy link
Contributor

Choose a reason for hiding this comment

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

and here, too - should be "cr.spec.auth.strategy"

Copy link
Contributor Author

@chickenchickenlove chickenchickenlove Mar 26, 2024

Choose a reason for hiding this comment

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

@jmazzitelli sorry to bother you.
you right, i missed it. I'm really sorry.
From this, as you said, cr.auth.strategy="anonymous" should be change to cr.spec.auth.strategy="anonymous", i think.

May i create new PR to fix it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.
Thanks for your help. 🙇‍♂️

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