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

Fix idempotent issue in grafana_datasource provider #297

Conversation

tobias-urdin
Copy link

This fixes the idempotent issue mentioned in the issue.

Fixes #289

@joernott
Copy link

joernott commented Oct 4, 2022

I actually noded, that there are two fields (depending on the datasource)

Notice: /Stage[main]/Profiles_monitoring::Grafana::Icinga_config/Grafana_datasource[icinga2_influxdb]/password: changed [redacted] to [redacted]
Notice: /Stage[main]/Profiles_monitoring::Grafana::Icinga_config/Grafana_datasource[icinga2_influxdb]/basic_auth_password: changed [redacted] to [redacted]

so, the patch should be applied to the "password" field as well

@alexjfisher
Copy link
Member

@joernott Good spot. I've did some manual testing on @tobias-urdin's change and it looked good.

I'll see if I can add some acceptance tests for both types of datasource.

@alexjfisher
Copy link
Member

This doesn't seem to work if basic_auth_password is something other than an empty string (the default). I think I've seen the issue. Will test a fix shortly.

@@ -63,6 +63,12 @@ def datasources

datasource = JSON.parse(response.body)

basic_auth_password = if datasource.key?('secureJsonData') && datasource['secureJsonData'].key?('basicAuthPassword')
Copy link
Member

Choose a reason for hiding this comment

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

secureJsonData is never part of the response. There's secureJsonFields but this just returns booleans. See grafana/grafana#20274 (comment) :(

Copy link
Author

@tobias-urdin tobias-urdin Oct 4, 2022

Choose a reason for hiding this comment

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

I see :(

Copy link
Author

Choose a reason for hiding this comment

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

We don't use basic auth so I didn't verify that part, but that's bummer. We are using this PR internally until something can be figured out here.

Copy link
Member

Choose a reason for hiding this comment

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

#301 might be that something. Still not much use for Grafana 9 users who need to configure prometheus basic auth or influxdb password, but if you're not using any auth, should remove the problem for you.

@tobias-urdin
Copy link
Author

Closing in favor of #301

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

grafana_datasource keeps updating basic_auth_password, no longer idempotent since grafana 9.0
3 participants