-
Notifications
You must be signed in to change notification settings - Fork 49
docs: How to setup oauth provider Grafana #542
Conversation
75fa113
to
6312dd1
Compare
6312dd1
to
8f8ddd6
Compare
} | ||
``` | ||
|
||
Observe the value of variable `secret_env` it should match the name of variable to be created in [Step 3](#step-3). |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
||
**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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modify the values of Github Auth configuration from | |
Modify the values of the GitHub Auth configuration from |
There was a problem hiding this comment.
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.
8f8ddd6
to
e628290
Compare
e628290
to
3639df5
Compare
a81c626
to
63fb6c6
Compare
@invidian I have overhauled the document and now it is simpler than before. |
There was a problem hiding this 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 ? 😄
What do you mean? For other oauth providers the entire config will change, the env vars will be named differently. |
I mean, we could write something like: "if you would like to use different OAuth provider, |
63fb6c6
to
f3317ef
Compare
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments.
> **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). |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
> `"true"` because Kubernetes expects the key-value pair to be of type `map[string]string` and not | ||
> `map[string]bool`. |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
f3317ef
to
51dde26
Compare
Signed-off-by: Suraj Deshmukh <suraj@kinvolk.io>
51dde26
to
bfb0a74
Compare
No description provided.