Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

docs: How to setup oauth provider Grafana #542

Merged
merged 1 commit into from
Sep 2, 2020

Conversation

surajssd
Copy link
Member

@surajssd surajssd commented Jun 3, 2020

No description provided.

@surajssd surajssd requested a review from johananl as a code owner June 3, 2020 08:49
@surajssd surajssd force-pushed the surajssd/add-how-to-gh-oauth branch 5 times, most recently from 75fa113 to 6312dd1 Compare June 3, 2020 09:05
@surajssd surajssd requested review from invidian and ipochi June 3, 2020 11:27
@surajssd surajssd marked this pull request as draft June 4, 2020 09:28
@surajssd surajssd force-pushed the surajssd/add-how-to-gh-oauth branch from 6312dd1 to 8f8ddd6 Compare June 18, 2020 07:36
@surajssd surajssd marked this pull request as ready for review July 9, 2020 11:34
@surajssd surajssd requested a review from iaguis July 9, 2020 11:35
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
}
```

Observe the value of variable `secret_env` it should match the name of variable to be created in [Step 3](#step-3).
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move this to Step 3 after defining the variable so the attention doesn't jump back and forth too much.

Copy link
Member Author

Choose a reason for hiding this comment

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

Step 3 is loaded with lot of information, I think keeping it in step2 and since it is in vicinity of step3 there won't be a lot of jumping.

docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved

**NOTE**: In above configs the boolean value is set to `"'true'"` instead of plain `"true"` because Kubernetes expects the key value pair to be of type string and not boolean.

Modify the values of Github Auth configuration from
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused from this point, where should users change this? How does changing "ini" syntax to yaml work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Modify the values of Github Auth configuration from
Modify the values of the GitHub Auth configuration from

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a bit confused from this point, where should users change this?

If users refer to the Grafana docs then they should change the values specified in the ini file to look like as shown in our docs.

How does changing "ini" syntax to yaml work?

It is a manual work to convert the ini file to terraform key value pairs.

docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
@surajssd surajssd force-pushed the surajssd/add-how-to-gh-oauth branch from 8f8ddd6 to e628290 Compare August 3, 2020 10:21
@surajssd surajssd force-pushed the surajssd/add-how-to-gh-oauth branch from e628290 to 3639df5 Compare August 25, 2020 10:29
@surajssd surajssd force-pushed the surajssd/add-how-to-gh-oauth branch 5 times, most recently from a81c626 to 63fb6c6 Compare August 27, 2020 09:09
@surajssd
Copy link
Member Author

@invidian I have overhauled the document and now it is simpler than before.

invidian
invidian previously approved these changes Aug 27, 2020
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

Looks nice and crispy @surajssd, thanks!

One thing we could consider adding somewhere is how to go from GF_AUTH_GITHUB_SCOPES to other oauth provider. Perhaps one more note in Step 2 ? 😄

@surajssd
Copy link
Member Author

One thing we could consider adding somewhere is how to go from GF_AUTH_GITHUB_SCOPES to other oauth provider

What do you mean? For other oauth providers the entire config will change, the env vars will be named differently.

@invidian
Copy link
Member

I mean, we could write something like: "if you would like to use different OAuth provider, GF_AUTH_GITHUB_SCOPES should become e.g. GF_AUTH_GOOGLE_FOO, as described in ".

@surajssd
Copy link
Member Author

The parameters change from one OAuth provider to another. So I wouldn't just say change from GITHUB to GOOGLE and things will work. So I have added additional line in the note above Step 1, which points user to the Grafana docs that explain how to convert ini config to env var: https://grafana.com/docs/grafana/latest/administration/configuration/#configure-with-environment-variables

invidian
invidian previously approved these changes Aug 28, 2020
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@iaguis iaguis left a comment

Choose a reason for hiding this comment

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

Some comments.

docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
Comment on lines +100 to +103
> **NOTE**: On Packet, you either need to create a DNS entry for `grafana.<cluster name>.<DNS zone>`
> and point it to the Packet external IP for the contour service (see the [Packet ingress guide for
> more details](./ingress-with-contour-metallb.md)) or use the [External DNS
> component](../configuration-reference/components/external-dns.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this still make sense if we ask for the External DNS component in the prerequisites?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to keep it just in case someone is not using external dns and would want to have setup done manually.

Comment on lines +106 to +107
> `"true"` because Kubernetes expects the key-value pair to be of type `map[string]string` and not
> `map[string]bool`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Mentioning map[string]string and such seems too detailed for a howto, but I don't have a better suggestion...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I thought how best to put it but I think as programmers this makes most sense to convey why it does not work.

docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
docs/how-to-guides/setup-thirdparty-auth-for-grafana.md Outdated Show resolved Hide resolved
invidian
invidian previously approved these changes Sep 2, 2020
iaguis
iaguis previously approved these changes Sep 2, 2020
Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
@surajssd surajssd dismissed stale reviews from iaguis and invidian via bfb0a74 September 2, 2020 14:50
@surajssd surajssd requested review from johananl and invidian and removed request for ipochi September 2, 2020 14:51
@surajssd surajssd merged commit 0fc3d59 into master Sep 2, 2020
@surajssd surajssd deleted the surajssd/add-how-to-gh-oauth branch September 2, 2020 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants