Skip to content

Commit

Permalink
Ruler: Add support for arbitrary extra URL parameters to Alertmanager…
Browse files Browse the repository at this point in the history
… client's OAuth config (#10030)

* Extend LimitsMap to support strings

* Drive-by fix update bug i noticed

* Add exported way to fetch the map contents

* Add exported way to initialize with data

* Endpoint params support

* make docs, make reference-help

* linter

* fix doc generator

* regen docs

* Extend changelog

* Drop validator

* Update tests

* Drop unused error
  • Loading branch information
alexweav authored Dec 6, 2024
1 parent 96c6ca5 commit 1bf2549
Show file tree
Hide file tree
Showing 11 changed files with 490 additions and 146 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
* [ENHANCEMENT] Distributor: Initialize ha_tracker cache before ha_tracker and distributor reach running state and begin serving writes. #9826 #9976
* [ENHANCEMENT] Ingester: `-ingest-storage.kafka.max-buffered-bytes` to limit the memory for buffered records when using concurrent fetching. #9892
* [ENHANCEMENT] Querier: improve performance and memory consumption of queries that select many series. #9914
* [ENHANCEMENT] Ruler: Support OAuth2 and proxies in Alertmanager client #9945
* [ENHANCEMENT] Ruler: Support OAuth2 and proxies in Alertmanager client #9945 #10030
* [ENHANCEMENT] Ingester: Add `-blocks-storage.tsdb.bigger-out-of-order-blocks-for-old-samples` to build 24h blocks for out-of-order data belonging to the previous days instead of building smaller 2h blocks. This reduces pressure on compactors and ingesters when the out-of-order samples span multiple days in the past. #9844 #10033 #10035
* [ENHANCEMENT] Distributor: allow a different limit for info series (series ending in `_info`) label count, via `-validation.max-label-names-per-info-series`. #10028
* [ENHANCEMENT] Ingester: do not reuse labels, samples and histograms slices in the write request if there are more entries than 10x the pre-allocated size. This should help to reduce the in-use memory in case of few requests with a very large number of labels, samples or histograms. #10040
Expand Down
11 changes: 11 additions & 0 deletions cmd/mimir/config-descriptor.json
Original file line number Diff line number Diff line change
Expand Up @@ -12425,6 +12425,17 @@
"fieldDefaultValue": "",
"fieldFlag": "ruler.alertmanager-client.oauth.scopes",
"fieldType": "string"
},
{
"kind": "field",
"name": "endpoint_params",
"required": false,
"desc": "Optional additional URL parameters to send to the token URL.",
"fieldValue": null,
"fieldDefaultValue": {},
"fieldFlag": "ruler.alertmanager-client.oauth.endpoint-params",
"fieldType": "map of string to string",
"fieldCategory": "advanced"
}
],
"fieldValue": null,
Expand Down
2 changes: 2 additions & 0 deletions cmd/mimir/help-all.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2829,6 +2829,8 @@ Usage of ./cmd/mimir/mimir:
OAuth2 client ID. Enables the use of OAuth2 for authenticating with Alertmanager.
-ruler.alertmanager-client.oauth.client_secret string
OAuth2 client secret.
-ruler.alertmanager-client.oauth.endpoint-params value
Optional additional URL parameters to send to the token URL. (default {})
-ruler.alertmanager-client.oauth.scopes comma-separated-list-of-strings
Optional scopes to include with the token request.
-ruler.alertmanager-client.oauth.token_url string
Expand Down
2 changes: 2 additions & 0 deletions cmd/mimir/help.txt.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,8 @@ Usage of ./cmd/mimir/mimir:
OAuth2 client ID. Enables the use of OAuth2 for authenticating with Alertmanager.
-ruler.alertmanager-client.oauth.client_secret string
OAuth2 client secret.
-ruler.alertmanager-client.oauth.endpoint-params value
Optional additional URL parameters to send to the token URL. (default {})
-ruler.alertmanager-client.oauth.scopes comma-separated-list-of-strings
Optional scopes to include with the token request.
-ruler.alertmanager-client.oauth.token_url string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1972,6 +1972,10 @@ alertmanager_client:
# CLI flag: -ruler.alertmanager-client.oauth.scopes
[scopes: <string> | default = ""]
# (advanced) Optional additional URL parameters to send to the token URL.
# CLI flag: -ruler.alertmanager-client.oauth.endpoint-params
[endpoint_params: <map of string to string> | default = {}]
# (advanced) Optional HTTP, HTTPS via CONNECT, or SOCKS5 proxy URL to route
# requests through. Applies to all requests, including auxiliary traffic, such
# as OAuth token requests.
Expand Down
18 changes: 14 additions & 4 deletions pkg/ruler/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/grafana/mimir/pkg/util"
util_log "github.com/grafana/mimir/pkg/util/log"
"github.com/grafana/mimir/pkg/util/validation"
)

var (
Expand All @@ -51,17 +52,22 @@ func (cfg *NotifierConfig) RegisterFlags(f *flag.FlagSet) {
}

type OAuth2Config struct {
ClientID string `yaml:"client_id"`
ClientSecret flagext.Secret `yaml:"client_secret"`
TokenURL string `yaml:"token_url"`
Scopes flagext.StringSliceCSV `yaml:"scopes,omitempty"`
ClientID string `yaml:"client_id"`
ClientSecret flagext.Secret `yaml:"client_secret"`
TokenURL string `yaml:"token_url"`
Scopes flagext.StringSliceCSV `yaml:"scopes,omitempty"`
EndpointParams validation.LimitsMap[string] `yaml:"endpoint_params" category:"advanced"`
}

func (cfg *OAuth2Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) {
f.StringVar(&cfg.ClientID, prefix+"client_id", "", "OAuth2 client ID. Enables the use of OAuth2 for authenticating with Alertmanager.")
f.Var(&cfg.ClientSecret, prefix+"client_secret", "OAuth2 client secret.")
f.StringVar(&cfg.TokenURL, prefix+"token_url", "", "Endpoint used to fetch access token.")
f.Var(&cfg.Scopes, prefix+"scopes", "Optional scopes to include with the token request.")
if !cfg.EndpointParams.IsInitialized() {
cfg.EndpointParams = validation.NewLimitsMap[string](nil)
}
f.Var(&cfg.EndpointParams, prefix+"endpoint-params", "Optional additional URL parameters to send to the token URL.")
}

func (cfg *OAuth2Config) IsEnabled() bool {
Expand Down Expand Up @@ -226,6 +232,10 @@ func amConfigWithSD(rulerConfig *Config, url *url.URL, sdConfig discovery.Config
Scopes: rulerConfig.Notifier.OAuth2.Scopes,
}

if rulerConfig.Notifier.OAuth2.EndpointParams.IsInitialized() {
amConfig.HTTPClientConfig.OAuth2.EndpointParams = rulerConfig.Notifier.OAuth2.EndpointParams.Read()
}

if rulerConfig.Notifier.ProxyURL != "" {
url, err := url.Parse(rulerConfig.Notifier.ProxyURL)
if err != nil {
Expand Down
86 changes: 86 additions & 0 deletions pkg/ruler/notifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@
package ruler

import (
"bytes"
"errors"
"flag"
"maps"
"net/url"
"testing"
"time"
Expand All @@ -17,9 +20,11 @@ import (
"github.com/prometheus/common/model"
"github.com/prometheus/prometheus/config"
"github.com/prometheus/prometheus/discovery"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/grafana/mimir/pkg/util"
"github.com/grafana/mimir/pkg/util/validation"
)

func TestBuildNotifierConfig(t *testing.T) {
Expand Down Expand Up @@ -373,6 +378,51 @@ func TestBuildNotifierConfig(t *testing.T) {
},
},
},
{
name: "with OAuth2 and optional endpoint params",
cfg: &Config{
AlertmanagerURL: "dnssrv+https://_http._tcp.alertmanager-0.default.svc.cluster.local/alertmanager",
Notifier: NotifierConfig{
OAuth2: OAuth2Config{
ClientID: "oauth2-client-id",
ClientSecret: flagext.SecretWithValue("test"),
TokenURL: "https://oauth2-token-endpoint.local/token",
EndpointParams: validation.NewLimitsMapWithData[string](
map[string]string{
"param1": "value1",
"param2": "value2",
},
nil,
),
},
},
},
ncfg: &config.Config{
AlertingConfig: config.AlertingConfig{
AlertmanagerConfigs: []*config.AlertmanagerConfig{
{
HTTPClientConfig: config_util.HTTPClientConfig{
OAuth2: &config_util.OAuth2{
ClientID: "oauth2-client-id",
ClientSecret: "test",
TokenURL: "https://oauth2-token-endpoint.local/token",
EndpointParams: map[string]string{"param1": "value1", "param2": "value2"},
},
},
APIVersion: "v2",
Scheme: "https",
PathPrefix: "/alertmanager",
ServiceDiscoveryConfigs: discovery.Configs{
dnsServiceDiscovery{
Host: "_http._tcp.alertmanager-0.default.svc.cluster.local",
QType: dns.SRV,
},
},
},
},
},
},
},
{
name: "with OAuth2 and proxy_url simultaneously, inheriting proxy",
cfg: &Config{
Expand Down Expand Up @@ -498,6 +548,42 @@ func TestBuildNotifierConfig(t *testing.T) {
}
}

func TestOAuth2Config_ValidateEndpointParams(t *testing.T) {
for name, tc := range map[string]struct {
args []string
expected validation.LimitsMap[string]
error string
}{
"basic test": {
args: []string{"-map-flag", "{\"param1\": \"value1\" }"},
expected: validation.NewLimitsMapWithData(map[string]string{
"param1": "value1",
}, nil),
},
"parsing error": {
args: []string{"-map-flag", "{\"hello\": ..."},
error: "invalid value \"{\\\"hello\\\": ...\" for flag -map-flag: invalid character '.' looking for beginning of value",
},
} {
t.Run(name, func(t *testing.T) {
v := validation.NewLimitsMap[string](nil)

fs := flag.NewFlagSet("test", flag.ContinueOnError)
fs.SetOutput(&bytes.Buffer{}) // otherwise errors would go to stderr.
fs.Var(v, "map-flag", "Map flag, you can pass JSON into this")
err := fs.Parse(tc.args)

if tc.error != "" {
require.NotNil(t, err)
assert.Equal(t, tc.error, err.Error())
} else {
assert.NoError(t, err)
assert.True(t, maps.Equal(tc.expected.Read(), v.Read()))
}
})
}
}

func urlMustParse(t *testing.T, raw string) *url.URL {
t.Helper()
u, err := url.Parse(raw)
Expand Down
6 changes: 3 additions & 3 deletions pkg/util/validation/limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ func (o *Overrides) RulerTenantShardSize(userID string) int {
func (o *Overrides) RulerMaxRulesPerRuleGroup(userID, namespace string) int {
u := o.getOverridesForUser(userID)

if namespaceLimit, ok := u.RulerMaxRulesPerRuleGroupByNamespace.data[namespace]; ok {
if namespaceLimit, ok := u.RulerMaxRulesPerRuleGroupByNamespace.Read()[namespace]; ok {
return namespaceLimit
}

Expand All @@ -914,7 +914,7 @@ func (o *Overrides) RulerMaxRulesPerRuleGroup(userID, namespace string) int {
func (o *Overrides) RulerMaxRuleGroupsPerTenant(userID, namespace string) int {
u := o.getOverridesForUser(userID)

if namespaceLimit, ok := u.RulerMaxRuleGroupsPerTenantByNamespace.data[namespace]; ok {
if namespaceLimit, ok := u.RulerMaxRuleGroupsPerTenantByNamespace.Read()[namespace]; ok {
return namespaceLimit
}

Expand Down Expand Up @@ -990,7 +990,7 @@ func (o *Overrides) AlertmanagerReceiversBlockPrivateAddresses(user string) bool
// 4. default limits
func (o *Overrides) getNotificationLimitForUser(user, integration string) float64 {
u := o.getOverridesForUser(user)
if n, ok := u.NotificationRateLimitPerIntegration.data[integration]; ok {
if n, ok := u.NotificationRateLimitPerIntegration.Read()[integration]; ok {
return n
}

Expand Down
17 changes: 13 additions & 4 deletions pkg/util/validation/limits_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,19 @@ import (
"gopkg.in/yaml.v3"
)

// LimitsMap is a generic map that can hold either float64 or int as values.
type LimitsMap[T float64 | int] struct {
// LimitsMap is a flag.Value implementation that looks like a generic map, holding float64s, ints, or strings as values.
type LimitsMap[T float64 | int | string] struct {
data map[string]T
validator func(k string, v T) error
}

func NewLimitsMap[T float64 | int](validator func(k string, v T) error) LimitsMap[T] {
func NewLimitsMap[T float64 | int | string](validator func(k string, v T) error) LimitsMap[T] {
return NewLimitsMapWithData(make(map[string]T), validator)
}

func NewLimitsMapWithData[T float64 | int | string](data map[string]T, validator func(k string, v T) error) LimitsMap[T] {
return LimitsMap[T]{
data: make(map[string]T),
data: data,
validator: validator,
}
}
Expand Down Expand Up @@ -45,6 +49,10 @@ func (m LimitsMap[T]) Set(s string) error {
return m.updateMap(newMap)
}

func (m LimitsMap[T]) Read() map[string]T {
return m.data
}

// Clone returns a copy of the LimitsMap.
func (m LimitsMap[T]) Clone() LimitsMap[T] {
newMap := make(map[string]T, len(m.data))
Expand Down Expand Up @@ -73,6 +81,7 @@ func (m LimitsMap[T]) updateMap(newMap map[string]T) error {
}
}

clear(m.data)
for k, v := range newMap {
m.data[k] = v
}
Expand Down
Loading

0 comments on commit 1bf2549

Please sign in to comment.