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

ruler: Support OAuth2 and proxies in Alertmanager client #9945

Merged
merged 13 commits into from
Nov 26, 2024

Conversation

alexweav
Copy link
Contributor

@alexweav alexweav commented Nov 18, 2024

What this PR does

The AM client already supports several TLS options + basic auth for talking to Alertmanager.
Extend this to also support OAuth2 Client Credentials Workflow, plus a URL for talking to the Alertmanager through a proxy.

These are all supported in upstream Prometheus's Alertmanager client: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#alertmanager_config
via `http_config: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#alertmanager_config

Supports HTTP, HTTP with CONNECT, or SOCKS5 proxies per https://pkg.go.dev/net/http#Transport. Under the hood, prometheus/common doesn't implement anything special for proxies and just wires the configuration up to http.Transport's proxy (1, 2)

Which issue(s) this PR fixes or relates to

n/a

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

@alexweav
Copy link
Contributor Author

Still in draft state as I finalize testing.

@alexweav alexweav marked this pull request as ready for review November 20, 2024 20:54
@alexweav alexweav requested review from a team and tacole02 as code owners November 20, 2024 20:54
Copy link
Contributor

@tacole02 tacole02 left a comment

Choose a reason for hiding this comment

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

Looks good!

@alexweav alexweav force-pushed the alexweav/ruler-proxy-oauth branch from 1499f5b to 1be45db Compare November 25, 2024 17:38
ClientID string `yaml:"client_id"`
ClientSecret flagext.Secret `yaml:"client_secret"`
TokenURL string `yaml:"token_url"`
Scopes flagext.StringSlice `yaml:"scopes,omitempty"`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: Moving this to StringSliceCSV. Nearly every other field that behaves this way across the project uses the CSV version, the regular version looks like it doesn't work as cleanly.

@francoposa
Copy link
Member

per discussion we said we would leave adding the support for specifying token audience via client credentials config EndpointParams for a subsequent PR.

@alexweav alexweav merged commit c178c83 into main Nov 26, 2024
29 checks passed
@alexweav alexweav deleted the alexweav/ruler-proxy-oauth branch November 26, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants