Use common interface to fetch secrets in HTTP client config #538

TheSpiritXIII · 2023-11-30T00:41:10Z

Part 1 for secret provider support within common. See: https://groups.google.com/g/prometheus-developers/c/WKOej_pnhXg

This change consolidates fetching secrets across all authorization types. Authorization types which previously had different structs per secret type now have a single type.

The unification helps with adding secret manager support which part 2 will include. There are two ways we could do this.

With a secret "reference" (or "handle"), e.g. a name to a secret which is configured somewhere else. This PR implements this approach: Add support for secret refs via a secret manager #572
Inline. This would not be backwards-compatible. The only way I could think of serialization to work with the type defined outside of this package (secret provider configs would be part of the main prometheus repository since they're plugins), is to use a generic secret config type.

Signed-off-by: Daniel Hrabovcak <thespiritxiii@gmail.com>

bwplotka

LGTM, thanks!

Exciting for prometheus/prometheus#13955 (comment) which this PR enables! ❤️

Disclaimer: @TheSpiritXIII is in my Google team. I tried to get more hands on this review for weeks, but no response. The secret provider feature was generally accepted 1, 2, so allowing myself to move forward after the careful review

bwplotka · 2024-05-09T13:00:54Z

config/http_config.go

+	return "inline"
+}
+
+func (s *inlineSecret) immutable() bool {


nice, this will optimize things 👍🏽

alanprot · 2024-06-05T17:32:30Z

Should we export the secret types implementations or the secretFrom methods?

Downstream projects like thanos/cortex uses the methods that now receives the secret interface but both interface and the implementations methods are not being exported:

Ex:

https://github.com/thanos-io/thanos/blob/58447e1a6e354470f746cfd5d37dc080acb569a3/pkg/clientconfig/http.go#L164-L175

Ex of how we changed on this PR:

https://github.com/prometheus/common/pull/538/files#diff-82fa1ce44db0403c7b0bac6e1fa6030746f8ef9e3958ad4f74637dd621d3cda3L724

nicolasparada · 2024-06-07T23:56:45Z

config/http_config.go

+// NewAuthorizationCredentialsRoundTripper adds the authorization credentials
+// read from the provided secret to a request unless the authorization header
+// has already been set.
+func NewAuthorizationCredentialsRoundTripper(authType string, authCredentials secret, rt http.RoundTripper) http.RoundTripper {


Breaking change. Also secret is not exported.

Ah, fixed already #650

yup, plus the lib is 0.x.z so we can break it. We try to do it rarely and only if there is a good reason - here it was useful to make it more consistent if we can, sorry for pain though.

TheSpiritXIII force-pushed the http-secret-interface branch 2 times, most recently from 0d08b50 to 95e1bd7 Compare November 30, 2023 19:06

TheSpiritXIII force-pushed the http-secret-interface branch 2 times, most recently from 377cde8 to aa94e76 Compare December 13, 2023 22:31

TheSpiritXIII force-pushed the http-secret-interface branch 3 times, most recently from 3c3e1cc to 4a41b93 Compare February 2, 2024 13:53

TheSpiritXIII mentioned this pull request Feb 2, 2024

Add support for secret refs via a secret manager #572

Merged

TheSpiritXIII force-pushed the http-secret-interface branch from 4a41b93 to 160c10d Compare February 12, 2024 14:08

TheSpiritXIII marked this pull request as ready for review February 12, 2024 20:23

Use common interface to fetch secrets in HTTP client config

18ad275

Signed-off-by: Daniel Hrabovcak <thespiritxiii@gmail.com>

TheSpiritXIII force-pushed the http-secret-interface branch from 160c10d to 18ad275 Compare April 18, 2024 19:54

TheSpiritXIII mentioned this pull request Apr 18, 2024

Add secret provider interface under feature flag prometheus/prometheus#13955

Open

bwplotka approved these changes May 9, 2024

View reviewed changes

config/http_config.go

return "inline"

}

func (s *inlineSecret) immutable() bool {

Copy link

Member

bwplotka May 9, 2024

Choose a reason for hiding this comment

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

nice, this will optimize things 👍🏽

bwplotka merged commit 260bf03 into prometheus:main May 9, 2024
7 checks passed

This was referenced Jun 5, 2024

[Chore] Update Prometheus thanos-io/thanos#7416

Merged

Expose secret as SecretReader and InlineSecret from config package #650

Merged

Expose secret as FileSecret from config package #653

Merged

nicolasparada reviewed Jun 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use common interface to fetch secrets in HTTP client config #538

Use common interface to fetch secrets in HTTP client config #538

TheSpiritXIII commented Nov 30, 2023 •

edited

Loading

bwplotka left a comment

bwplotka May 9, 2024

alanprot commented Jun 5, 2024

nicolasparada Jun 7, 2024

nicolasparada Jun 7, 2024

bwplotka Jul 29, 2024

Use common interface to fetch secrets in HTTP client config #538

Use common interface to fetch secrets in HTTP client config #538

Conversation

TheSpiritXIII commented Nov 30, 2023 • edited Loading

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka May 9, 2024

Choose a reason for hiding this comment

alanprot commented Jun 5, 2024

nicolasparada Jun 7, 2024

Choose a reason for hiding this comment

nicolasparada Jun 7, 2024

Choose a reason for hiding this comment

bwplotka Jul 29, 2024

Choose a reason for hiding this comment

TheSpiritXIII commented Nov 30, 2023 •

edited

Loading