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

Alerts break authentication to Clickhouse #376

Closed
ertong opened this issue Oct 20, 2021 · 1 comment · Fixed by #387
Closed

Alerts break authentication to Clickhouse #376

ertong opened this issue Oct 20, 2021 · 1 comment · Fixed by #387
Assignees
Milestone

Comments

@ertong
Copy link

ertong commented Oct 20, 2021

For some reason, the quering code for graphs and for alerts are different.
It is not a bug itself, but, as a result, it produces a lot of issues.

Sniffed headers from graphs requests:

[accept application/json, text/plain, */*]
[x-real-ip *****]
[accept-encoding gzip, deflate, br]
[host *****:8124]
[x-grafana-org-id 1]
[sec-fetch-dest empty]
[sec-fetch-mode cors]
[x-forwarded-for *****]
[authorization Basic ZGVmYXVsdDo=]
[sec-fetch-site same-origin]
[user-agent Grafana/8.2.1]

Sniffed headers from alerts requests:

[x-clickhouse-user default]
[content-length 869]
[accept-encoding gzip]
[x-clickhouse-key ]
[user-agent Go-http-client/1.1]

Here, we have clickhouse with default user without password.
Graphs work, alerts - not (http 400 from clickhouse ... I think it relates to empty password)

If we disable "Basic auth" in plugin configuration, alerts work, graphs - not. They does not work due to default grafana authentication header with Bearer token, which clickhouse does not understand and returns 403.

If we enable "Basic auth" with default user and empty password, graphs work, alerts - not. Alerts receive http 400 from clickhouse server (maybe due to empty password header).

Current working workaround: disable Basic auth and set custom http header "Authorization: Basic ZGVmYXVsdDo=" (for default empty password). It replaces Bearer token for graphs, and does not trigger x-clickhouse-* headers for alerts.

@Slach Slach self-assigned this Nov 10, 2021
@Slach
Copy link
Collaborator

Slach commented Nov 10, 2021

@ertong thanks a lot for reporting, issue related with wrong Basic Auth fix
#349 (comment)

@Slach Slach added this to the 2.4.0 milestone Nov 17, 2021
Slach added a commit to Altinity/grafana-plugin-repository that referenced this issue Nov 29, 2021
# 2.4.0 (2021-11-29)

## Enhancement:
* Add support for Grafana 8.x unified alerts, fix Altinity/clickhouse-grafana#380
* Add TLS support for backend alerts part of plugin Altinity/clickhouse-grafana#356 (comment)
* Add $naturalTimeSeries macro, look details in https://github.com/Vertamedia/clickhouse-grafana/pull/89/files#diff-cd9133eda7b58ef9c9264190db4534a1be53216edbda9ac57256fbd800368c03R383-R412
* Update golang-plugin-sdk-go to latest version
* Properly format Value in Table format, look details Altinity/clickhouse-grafana#379
* Remove toDateTime64 casting for column when time column is already DateTime64 to improve performance. Change test to ensure the casting is removed from the query, fix Altinity/clickhouse-grafana#360
* implements `$timeFilter64ByColumn(column_name)` macro, fix Altinity/clickhouse-grafana#343

## Fixes:

* implements properly GET and POST support for alert queries, fix Altinity/clickhouse-grafana#353
* SQL syntax highlight now works always, fix Altinity/clickhouse-grafana#174, fix Altinity/clickhouse-grafana#381
* fix Altinity/clickhouse-grafana#376,
* fix negative behavior for $perSecondColumns Altinity/clickhouse-grafana#337
* fix Altinity/clickhouse-grafana#374, ignore `--` inside quotas as comment
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 a pull request may close this issue.

2 participants