feat(exporter/elasticsearch): add Basic Auth support #5814

hainenber · 2023-11-20T16:25:28Z

PR Description

Which issue(s) this PR fixes

Fixes #5740

Notes to the Reviewer

List of AIs need to be done before out of draft:

Test with local ES cluster
Add documentation

PR Checklist

CHANGELOG.md updated
Documentation added
Tests updated
Config converters updated

hainenber · 2023-11-21T16:26:32Z

Test results

[Before] Unauthorized request when trying to reach secure ES cluster without `Basic-Auth` block.

Config

prometheus.exporter.elasticsearch "LABEL" {
    address = "https://localhost:9200"
    ca = "/Users/hainenber/Projects/labs/http_ca.crt"
}

Agent logs

ts=2023-11-21T16:20:21.904464Z level=info msg="triggering initial cluster info call" component=prometheus.exporter.elasticsearch.LABEL
ts=2023-11-21T16:20:21.90448Z level=info msg="now listening for http traffic" service=http addr=127.0.0.1:12345
ts=2023-11-21T16:20:21.904495Z level=info msg="providing consumers with updated cluster info label" component=prometheus.exporter.elasticsearch.LABEL
ts=2023-11-21T16:20:21.943395Z level=error msg="failed to retrieve cluster info from ES" component=prometheus.exporter.elasticsearch.LABEL err="HTTP Request failed with code 401"
ts=2023-11-21T16:20:31.904826Z level=info msg="initial cluster info call timed out" component=prometheus.exporter.elasticsearch.LABEL

[After] Authorized request when trying to reach secure ES cluster with `Basic-Auth` block.

Config

prometheus.exporter.elasticsearch "LABEL" {
    address = "https://localhost:9200"
    ca = "/Users/hainenber/Projects/labs/http_ca.crt"
    basic_auth {
        username = "elastic"
        password = env("ELASTIC_PASSWORD")
    }
}

Agent logs

ts=2023-11-21T16:25:39.898612Z level=info msg="scheduling loaded components and services"
ts=2023-11-21T16:25:39.899237Z level=info msg="starting cluster node" peers="" advertise_addr=127.0.0.1:12345
ts=2023-11-21T16:25:39.899895Z level=info msg="peers changed" new_peers=hainenber.local
ts=2023-11-21T16:25:39.899912Z level=info msg="triggering initial cluster info call" component=prometheus.exporter.elasticsearch.LABEL
ts=2023-11-21T16:25:39.899938Z level=info msg="providing consumers with updated cluster info label" component=prometheus.exporter.elasticsearch.LABEL
ts=2023-11-21T16:25:39.900179Z level=info msg="now listening for http traffic" service=http addr=127.0.0.1:12345
ts=2023-11-21T16:25:39.941752Z level=info msg="started cluster info retriever" component=prometheus.exporter.elasticsearch.LABEL interval=5m0s

clayton-cornell · 2023-11-21T17:35:02Z

@grafana/grafana-agent-maintainers This needs your review as well.

rfratto · 2023-11-23T15:35:11Z

Thanks @hainenber!

@thepalbi Can you take a look at this once, since the exporter integration was originally contributed by you?

thepalbi · 2023-11-23T18:24:43Z

pkg/integrations/elasticsearch_exporter/elasticsearch_exporter.go

@@ -66,6 +70,21 @@ type Config struct {
 	ExportDataStreams bool `yaml:"data_stream,omitempty"`
 	// Export stats for Snapshot Lifecycle Management
 	ExportSLM bool `yaml:"slm,omitempty"`
+	// BasicAuth block allows secure connection with Elasticsearch cluster via Basic-Auth
+	BasicAuth *commoncfg.BasicAuth `yaml:"basic_auth,omitempty"`


given that this also touches the integration implementation itself, we should also update the static configs doc https://grafana.com/docs/agent/latest/static/configuration/integrations/elasticsearch-exporter-config/

Also, I think we should use in here the prometheus BasicAuth type, which is yaml serializable, in contrast with the river definition of it. If not, someone trying to use this feature in a static config agent won't be able to.

See that the river BasicAuth type has a Convert method that translates it to the prometheus type in here.

@rfratto wdyt?

Hi there, thanks for the review! Makes sense to continue supporting the static config :D. I've made amends in latest commit. PTAL if you have time.

Signed-off-by: hainenber <[email protected]>

PR grafana#5802 accidentally added the changelog entry to a published release.

Signed-off-by: hainenber <[email protected]>

erikbaranowski · 2023-12-15T17:04:43Z

Can we also add a mapping from static to flow for this new property in the converter code?

agent/converter/internal/staticconvert/internal/build/elasticsearch_exporter.go

Line 14 in fd12c48

    
           func toElasticsearchExporter(config *elasticsearch_exporter.Config) *elasticsearch.Arguments {

This can be optionally validated in the testing in converter/internal/staticconvert/testdata/integrations.*

Signed-off-by: hainenber <[email protected]>

hainenber · 2023-12-17T08:40:34Z

Can we also add a mapping from static to flow for this new property in the converter code?

agent/converter/internal/staticconvert/internal/build/elasticsearch_exporter.go

Line 14 in fd12c48

func toElasticsearchExporter(config *elasticsearch_exporter.Config) *elasticsearch.Arguments {

This can be optionally validated in the testing in converter/internal/staticconvert/testdata/integrations.*

Done :D

Signed-off-by: hainenber <[email protected]>

tpaschalis · 2023-12-18T11:16:07Z

cc @thepalbi in case he can take a final look here.

thepalbi · 2023-12-18T14:23:25Z

pkg/integrations/elasticsearch_exporter/elasticsearch_exporter.go

+}
+
+// Custom http.Transport struct for Basic Auth-secured communication with ES cluster
+type BasicAuthHTTPTransport struct {


nit: can we rename this to AuthenticatedHTTPTransport or sth like this, since it just takes a generic header value?

thepalbi

LGTM, just left a small nit

Signed-off-by: erikbaranowski <[email protected]>

rfratto · 2024-01-02T13:34:54Z

Merging since there's two approvals and no further commits in two weeks. Thanks @hainenber!

* feat(exporter/elasticsearch): add Basic Auth support Signed-off-by: hainenber <[email protected]> * misc: fix changelog entry (grafana#5812) PR grafana#5802 accidentally added the changelog entry to a published release. * fix(exporter/elasticsearch): only add Auth header when needed Signed-off-by: hainenber <[email protected]> * doc(exporter/elasticsearch): add Basic Auth block desc Signed-off-by: hainenber <[email protected]> * feat(exporter/elasticsearch): add Basic Auth support Signed-off-by: hainenber <[email protected]> * feat(exporter/elasticsearch): support Basic Auth for static config Signed-off-by: hainenber <[email protected]> * chore(CHANGELOG): remove merge conflict artifact Signed-off-by: hainenber <[email protected]> * misc: fix changelog entry Signed-off-by: hainenber <[email protected]> * feat(converter): map ES exporter's BasicAuth flow from static to flow Signed-off-by: hainenber <[email protected]> * fix(converter): nil check for ES exporter's Basic Auth Signed-off-by: hainenber <[email protected]> * lint Signed-off-by: erikbaranowski <[email protected]> --------- Signed-off-by: hainenber <[email protected]> Signed-off-by: erikbaranowski <[email protected]> Co-authored-by: Robert Fratto <[email protected]> Co-authored-by: erikbaranowski <[email protected]>

hainenber marked this pull request as ready for review November 21, 2023 16:35

hainenber requested a review from clayton-cornell as a code owner November 21, 2023 16:35

clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Nov 21, 2023

clayton-cornell requested a review from a team November 21, 2023 17:34

rfratto requested a review from thepalbi November 23, 2023 15:35

rfratto assigned rfratto and thepalbi and unassigned thepalbi Nov 23, 2023

thepalbi requested changes Nov 23, 2023

View reviewed changes

hainenber requested a review from a team as a code owner November 25, 2023 08:06

hainenber force-pushed the basic-auth-for-elasticsearch-exporter branch from 1151711 to 67b236f Compare November 25, 2023 08:12

hainenber and others added 7 commits November 25, 2023 15:13

feat(exporter/elasticsearch): add Basic Auth support

159ec6c

Signed-off-by: hainenber <[email protected]>

misc: fix changelog entry (grafana#5812)

3f3a021

PR grafana#5802 accidentally added the changelog entry to a published release.

fix(exporter/elasticsearch): only add Auth header when needed

e8baca6

Signed-off-by: hainenber <[email protected]>

doc(exporter/elasticsearch): add Basic Auth block desc

6a7adf0

Signed-off-by: hainenber <[email protected]>

feat(exporter/elasticsearch): add Basic Auth support

270bc7d

Signed-off-by: hainenber <[email protected]>

feat(exporter/elasticsearch): support Basic Auth for static config

94daa6e

Signed-off-by: hainenber <[email protected]>

chore(CHANGELOG): remove merge conflict artifact

520608d

Signed-off-by: hainenber <[email protected]>

hainenber force-pushed the basic-auth-for-elasticsearch-exporter branch from 67b236f to 520608d Compare November 25, 2023 08:15

misc: fix changelog entry

2dcc506

Signed-off-by: hainenber <[email protected]>

erikbaranowski self-assigned this Dec 15, 2023

hainenber and others added 2 commits December 17, 2023 15:36

feat(converter): map ES exporter's BasicAuth flow from static to flow

9acca89

Signed-off-by: hainenber <[email protected]>

Merge branch 'main' into basic-auth-for-elasticsearch-exporter

ab6544a

fix(converter): nil check for ES exporter's Basic Auth

13b58f1

Signed-off-by: hainenber <[email protected]>

tpaschalis requested a review from thepalbi December 18, 2023 11:16

thepalbi reviewed Dec 18, 2023

View reviewed changes

thepalbi approved these changes Dec 18, 2023

View reviewed changes

erikbaranowski approved these changes Dec 18, 2023

View reviewed changes

lint

a24bc88

Signed-off-by: erikbaranowski <[email protected]>

rfratto merged commit 6ef7b8b into grafana:main Jan 2, 2024
10 checks passed

hainenber deleted the basic-auth-for-elasticsearch-exporter branch January 2, 2024 16:43

github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 21, 2024

github-actions bot locked as resolved and limited conversation to collaborators Feb 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(exporter/elasticsearch): add Basic Auth support #5814

feat(exporter/elasticsearch): add Basic Auth support #5814

hainenber commented Nov 20, 2023 •

edited

Loading

hainenber commented Nov 21, 2023 •

edited

Loading

clayton-cornell commented Nov 21, 2023

rfratto commented Nov 23, 2023

thepalbi Nov 23, 2023

thepalbi Nov 23, 2023

hainenber Nov 25, 2023

erikbaranowski commented Dec 15, 2023 •

edited

Loading

hainenber commented Dec 17, 2023

tpaschalis commented Dec 18, 2023

thepalbi Dec 18, 2023

thepalbi left a comment

rfratto commented Jan 2, 2024

feat(exporter/elasticsearch): add Basic Auth support #5814

feat(exporter/elasticsearch): add Basic Auth support #5814

Conversation

hainenber commented Nov 20, 2023 • edited Loading

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

hainenber commented Nov 21, 2023 • edited Loading

Test results

[Before] Unauthorized request when trying to reach secure ES cluster without Basic-Auth block.

[After] Authorized request when trying to reach secure ES cluster with Basic-Auth block.

clayton-cornell commented Nov 21, 2023

rfratto commented Nov 23, 2023

thepalbi Nov 23, 2023

Choose a reason for hiding this comment

thepalbi Nov 23, 2023

Choose a reason for hiding this comment

hainenber Nov 25, 2023

Choose a reason for hiding this comment

erikbaranowski commented Dec 15, 2023 • edited Loading

hainenber commented Dec 17, 2023

tpaschalis commented Dec 18, 2023

thepalbi Dec 18, 2023

Choose a reason for hiding this comment

thepalbi left a comment

Choose a reason for hiding this comment

rfratto commented Jan 2, 2024

hainenber commented Nov 20, 2023 •

edited

Loading

hainenber commented Nov 21, 2023 •

edited

Loading

[Before] Unauthorized request when trying to reach secure ES cluster without `Basic-Auth` block.

[After] Authorized request when trying to reach secure ES cluster with `Basic-Auth` block.

erikbaranowski commented Dec 15, 2023 •

edited

Loading