wire up all prometheus proxy config settings for flow and the converters #6306

erikbaranowski · 2024-02-05T18:26:29Z

PR Description

Wires in the 3 additional proxy configurations from promtheus to the http client config and elsewhere. I improved some code along the way and called out discrepancies between docs and code.

Which issue(s) this PR fixes

Closes #6261
Closes #5110

Notes to the Reviewer

PR Checklist

CHANGELOG.md updated
Documentation added
Tests updated
Config converters updated

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

…mented. TBD if this is a docs or code bug. Signed-off-by: erikbaranowski <[email protected]>

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

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

refactor http client config validation Signed-off-by: erikbaranowski <[email protected]>

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

…into prom-proxy-configs

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

erikbaranowski · 2024-02-06T20:44:52Z

component/common/config/types.go

@@ -33,63 +33,44 @@ func (h *HTTPClientConfig) SetToDefault() {

 // Validate returns an error if h is invalid.
 func (h *HTTPClientConfig) Validate() error {


This refactor pushes the responsibility for validating other types to their own Validate funcs except for ProxyConfig (squashed). It also tries to more sanely handle the scenarios where only 1 auth method can be set. I think this code got more and more convoluted over time.

erikbaranowski · 2024-02-06T20:48:32Z

component/common/config/types.go

+	}
+}
+
+func (p *ProxyConfig) Validate() error {


These mirror Prometheus' ProxyConfig validations.

https://github.com/prometheus/common/blob/773d5664eb8d228d2c6ba235b1af12fbf7ee9020/config/http_config.go#L1210

erikbaranowski · 2024-02-06T20:49:28Z

component/common/config/types_test.go

@@ -228,5 +228,5 @@ func TestHTTPClientBadConfig(t *testing.T) {

 	var httpClientConfig HTTPClientConfig
 	err := river.Unmarshal([]byte(exampleRiverConfig), &httpClientConfig)
-	require.ErrorContains(t, err, "at most one of bearer_token & bearer_token_file must be configured")
+	require.ErrorContains(t, err, "at most one of basic_auth password & password_file must be configured")


In some cases, the error changed because the order of validation will be different. I don't think this is harmful.

erikbaranowski · 2024-02-06T20:49:47Z

component/discovery/azure/azure.go

+		return err
+	}
+
+	return a.ProxyConfig.Validate()


Need to explicitly validate squashed properties

erikbaranowski · 2024-02-06T20:50:38Z

component/discovery/docker/docker_test.go

@@ -27,5 +27,5 @@ func TestBadRiverConfig(t *testing.T) {
 	// Make sure the squashed HTTPClientConfig Validate function is being utilized correctly
 	var args Arguments
 	err := river.Unmarshal([]byte(exampleRiverConfig), &args)
-	require.ErrorContains(t, err, "at most one of bearer_token & bearer_token_file must be configured")
+	require.ErrorContains(t, err, "at most one of basic_auth, authorization, oauth2, bearer_token & bearer_token_file must be configured")


These error messages got longer to encapsulate all potential overlapping auths.

erikbaranowski · 2024-02-06T20:51:14Z

component/discovery/linode/linode_test.go

@@ -29,6 +33,10 @@ func TestConvert(t *testing.T) {
 		TagSeparator:    ";",
 		HTTPClientConfig: config.HTTPClientConfig{
 			BearerToken: "FOO",
+			BasicAuth: &config.BasicAuth{


Prove out that an undocumented block could be set on the component.

erikbaranowski · 2024-02-06T20:52:30Z

docs/sources/flow/reference/components/discovery.azure.md

This component brings in portions of http client config and not the whole thing.

docs/sources/flow/reference/components/discovery.dockerswarm.md

docs/sources/flow/reference/components/discovery.hetzner.md

docs/sources/flow/reference/components/discovery.kubelet.md

docs/sources/flow/reference/components/discovery.lightsail.md

docs/sources/flow/reference/components/discovery.linode.md

docs/sources/flow/reference/components/discovery.marathon.md

docs/sources/flow/reference/components/loki.source.podlogs.md

docs/sources/flow/reference/components/prometheus.operator.podmonitors.md

docs/sources/flow/reference/components/prometheus.operator.servicemonitors.md

docs/sources/flow/reference/components/remote.kubernetes.configmap.md

docs/sources/flow/reference/components/remote.kubernetes.secret.md

erikbaranowski · 2024-02-06T21:09:05Z

component/discovery/consul/consul_test.go

 `

 	// Make sure the squashed HTTPClientConfig Validate function is being utilized correctly
 	var args Arguments
 	err := river.Unmarshal([]byte(exampleRiverConfig), &args)
-	require.ErrorContains(t, err, "at most one of bearer_token & bearer_token_file must be configured")
+	require.ErrorContains(t, err, "at most one of basic_auth password & password_file must be configured")


Prove that the cascading validation is happening for properties on HttpClientConfig

component/common/config/types.go

rfratto

LGTM % some final comments and pending a review from @clayton-cornell :)

component/common/config/types.go

converter/internal/promtailconvert/testdata/docker_relabel.river

docs/sources/shared/flow/reference/components/http-client-proxy-config-description.md

Co-authored-by: Robert Fratto <[email protected]>

clayton-cornell

Some comments. Since they apply to all doc files, I commented once with a suggestion. It's a lot easier to search/replace in your local branch vs a large number of identical suggested changes.

docs/sources/flow/reference/components/discovery.azure.md

docs/sources/flow/reference/components/discovery.hetzner.md

docs/sources/shared/flow/reference/components/http-client-proxy-config-description.md

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

docs/sources/flow/reference/components/prometheus.operator.probes.md

mattdurham

Kubelet change seems reasonable though I doubt it actually supports all the auth parameters , but I guess that’s part of sharing. Either way this is better

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

docs/sources/flow/reference/components/discovery.marathon.md

tpaschalis

Huge chunk of work, nice one @erikbaranowski 🙌

…ers (grafana#6306) * wire up all prometheus proxy config settings for flow and the converters Signed-off-by: erikbaranowski <[email protected]> * document new proxy client configs and fix a few undocumented args Signed-off-by: erikbaranowski <[email protected]> * document places where http client config was used for args but undocumented. Signed-off-by: erikbaranowski <[email protected]> * wire in proxy config for oauth2 and document refactor http client config validation Signed-off-by: erikbaranowski <[email protected]> --------- Signed-off-by: erikbaranowski <[email protected]>

erikbaranowski added 10 commits February 2, 2024 16:20

wire up all prometheus proxy config settings for flow and the converters

a00db84

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

document new proxy client configs and fix a few undocumented args

c81af91

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

document places where http client config was used for args but undocu…

eed1602

…mented. TBD if this is a docs or code bug. Signed-off-by: erikbaranowski <[email protected]>

wire up some more places where ProxyConfig is directly used

0e5d351

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

fix up some tests

797782f

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

wire in proxy config for oauth2 and document

5006249

refactor http client config validation Signed-off-by: erikbaranowski <[email protected]>

Merge branch 'main' into prom-proxy-configs

2ac117a

test fixes and never trust an AI to optimize code!

9352074

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

Merge branch 'prom-proxy-configs' of https://github.com/grafana/agent …

480cbe6

…into prom-proxy-configs

Fill in a few more docs

929d6ba

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

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/discovery.dockerswarm.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/discovery.hetzner.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/discovery.kubelet.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/discovery.lightsail.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/discovery.linode.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/discovery.marathon.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/loki.source.podlogs.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/prometheus.operator.podmonitors.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/prometheus.operator.servicemonitors.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/remote.kubernetes.configmap.md Outdated Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

docs/sources/flow/reference/components/remote.kubernetes.secret.md Outdated Show resolved Hide resolved

erikbaranowski marked this pull request as ready for review February 6, 2024 21:04

erikbaranowski requested a review from a team as a code owner February 6, 2024 21:04

erikbaranowski requested a review from clayton-cornell as a code owner February 6, 2024 21:04

erikbaranowski requested review from rfratto and tpaschalis February 6, 2024 21:04

erikbaranowski assigned rfratto, tpaschalis and clayton-cornell Feb 6, 2024

erikbaranowski commented Feb 6, 2024

View reviewed changes

component/common/config/types.go Show resolved Hide resolved

erikbaranowski commented Feb 6, 2024

View reviewed changes

component/common/config/types.go Show resolved Hide resolved

rfratto approved these changes Feb 6, 2024

View reviewed changes

Update component/common/config/types.go

d9fe712

Co-authored-by: Robert Fratto <[email protected]>

clayton-cornell reviewed Feb 6, 2024

View reviewed changes

erikbaranowski added 2 commits February 7, 2024 16:03

fix up docs from feedback

162139f

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

add TODO where Validate is modifying objects

c760f74

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

erikbaranowski commented Feb 7, 2024

View reviewed changes

docs/sources/flow/reference/components/prometheus.operator.probes.md Outdated Show resolved Hide resolved

mattdurham approved these changes Feb 7, 2024

View reviewed changes

clean up empty blocks for http_client_config in the converter output

54dda38

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

erikbaranowski requested a review from clayton-cornell February 7, 2024 22:08

wildum reviewed Feb 8, 2024

View reviewed changes

docs/sources/flow/reference/components/discovery.marathon.md Show resolved Hide resolved

clayton-cornell approved these changes Feb 8, 2024

View reviewed changes

clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Feb 8, 2024

tpaschalis approved these changes Feb 8, 2024

View reviewed changes

erikbaranowski merged commit 47ce94b into main Feb 9, 2024
11 checks passed

erikbaranowski deleted the prom-proxy-configs branch February 9, 2024 16:15

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 Mar 11, 2024

github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wire up all prometheus proxy config settings for flow and the converters #6306

wire up all prometheus proxy config settings for flow and the converters #6306

erikbaranowski commented Feb 5, 2024 •

edited

Loading

erikbaranowski Feb 6, 2024

erikbaranowski Feb 6, 2024

erikbaranowski Feb 6, 2024

erikbaranowski Feb 6, 2024

erikbaranowski Feb 6, 2024

erikbaranowski Feb 6, 2024

erikbaranowski Feb 6, 2024

erikbaranowski Feb 6, 2024

rfratto left a comment

clayton-cornell left a comment

mattdurham left a comment

tpaschalis left a comment

		@@ -33,63 +33,44 @@ func (h *HTTPClientConfig) SetToDefault() {

		// Validate returns an error if h is invalid.
		func (h *HTTPClientConfig) Validate() error {

wire up all prometheus proxy config settings for flow and the converters #6306

wire up all prometheus proxy config settings for flow and the converters #6306

Conversation

erikbaranowski commented Feb 5, 2024 • edited Loading

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rfratto left a comment

Choose a reason for hiding this comment

clayton-cornell left a comment

Choose a reason for hiding this comment

mattdurham left a comment

Choose a reason for hiding this comment

tpaschalis left a comment

Choose a reason for hiding this comment

erikbaranowski commented Feb 5, 2024 •

edited

Loading