Skip to content
This repository has been archived by the owner on May 31, 2023. It is now read-only.

Cleaner and more general handling of auth options #219

Merged
merged 1 commit into from
May 27, 2020

Conversation

faxm0dem
Copy link
Contributor

@faxm0dem faxm0dem commented May 6, 2020

Before this change, the keys of grafana_auth's value were tested for a
static list of names like 'disable_login_form'. If they didn't match it
would suppose they're a mapping and thus descent into it.
This would work as long as the key was in the list, but for example
using 'login_maximum_inactive_lifetime_days' would fail as it's not a
hash, but it is a valid grafana option.

This change makes the test more elegant, robust and easier to maintain.

Fixes #218

@github-actions github-actions bot added the area/jinja Templates label May 6, 2020
@paulfantom
Copy link
Member

paulfantom commented May 6, 2020

This was done to prevent including incorrect options, without this anything can be put into configuration which in turn can render grafana inoperational. But I think it would be ok to change it now.


You also included binary swap file from vim. TIP: add a global .gitignore file and exlude common files there, more https://help.github.com/en/github/using-git/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer

Before this change, the keys of grafana_auth's value were tested for a
static list of names like 'disable_login_form'. If they didn't match it
would suppose they're a mapping and thus descent into it.
This would work as long as the key was in the list, but for example
using 'login_maximum_inactive_lifetime_days' would fail as it's not a
hash, but it is a valid grafana option.

This change makes the test more elegant, robust and easier to maintain.

Fixes cloudalchemy#218
@faxm0dem faxm0dem force-pushed the fix/additional-auth-options branch from c23a74b to 506bbd2 Compare May 6, 2020 13:32
@faxm0dem
Copy link
Contributor Author

faxm0dem commented May 6, 2020

sorry about the swap file, I just removed it

@faxm0dem
Copy link
Contributor Author

Sorry to poke you, but (when) do you plan to merge this ?

@paulfantom paulfantom merged commit 93a041e into cloudalchemy:master May 27, 2020
@faxm0dem
Copy link
Contributor Author

thanks for merging this !

cosandr pushed a commit to cosandr/ansible-grafana that referenced this pull request Aug 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/jinja Templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration fails for unlisted options in template for grafana_auth
2 participants