RepoMirrors
/
alertmanager
mirror of https://github.com/prometheus/alertmanager
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix inconsistent defaults in UTF-8 behavior (#3668) 

This commit fixes inconsistent UTF-8 behavior if the compat package is
not initialized and feature flags are not passed to the API. This can
happen when Alertmanager is used as a package in software such
as Cortex or Mimir.

The inconsistent behavior is that Alertmanager will accept UTF-8 alerts
but reject UTF-8 configurations.

Since feature flags are optional via api.Options, we cannot force them
to be passed to api.New at compile time. Instead, it's better to defer
back to the compat package which is consistent even when not initialized.

Signed-off-by: George Robinson <george.robinson@grafana.com>
This commit is contained in:
George Robinson 2024-01-15 10:03:51 +00:00 committed by GitHub
parent 9ed52df5a7
commit fa6a7e6dd6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 52 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
api/api.go
View File

@ -29,7 +29,6 @@ import (
"github.com/prometheus/alertmanager/cluster"
"github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/dispatch"
"github.com/prometheus/alertmanager/featurecontrol"
"github.com/prometheus/alertmanager/provider"
"github.com/prometheus/alertmanager/silence"
"github.com/prometheus/alertmanager/types"
@ -68,9 +67,6 @@ type Options struct {
Concurrency int
// Logger is used for logging, if nil, no logging will happen.
Logger log.Logger
// FeatureFlags contains the set of feature flags. If nil, NoopFlags are used,
// and all controlled features are disabled.
FeatureFlags featurecontrol.Flagger
// Registry is used to register Prometheus metrics. If nil, no metrics
// registration will happen.
Registry prometheus.Registerer
@ -121,7 +117,6 @@ func New(opts Options) (*API, error) {
opts.Silences,
opts.Peer,
log.With(l, "version", "v2"),
opts.FeatureFlags,
opts.Registry,
)
if err != nil {

9
api/v2/api.go
View File

@ -45,7 +45,6 @@ import (
"github.com/prometheus/alertmanager/cluster"
"github.com/prometheus/alertmanager/config"
"github.com/prometheus/alertmanager/dispatch"
"github.com/prometheus/alertmanager/featurecontrol"
"github.com/prometheus/alertmanager/matchers/compat"
"github.com/prometheus/alertmanager/pkg/labels"
"github.com/prometheus/alertmanager/provider"
@ -73,7 +72,6 @@ type API struct {
logger log.Logger
m *metrics.Alerts
ff featurecontrol.Flagger
Handler http.Handler
}
@ -92,12 +90,8 @@ func NewAPI(
silences *silence.Silences,
peer cluster.ClusterPeer,
l log.Logger,
ff featurecontrol.Flagger,
r prometheus.Registerer,
) (*API, error) {
if ff == nil {
ff = featurecontrol.NoopFlags{}
}
api := API{
alerts: alerts,
getAlertStatus: sf,
@ -106,7 +100,6 @@ func NewAPI(
silences: silences,
logger: l,
m: metrics.NewAlerts(r),
ff: ff,
uptime: time.Now(),
}
@ -356,7 +349,7 @@ func (api *API) postAlertsHandler(params alert_ops.PostAlertsParams) middleware.
for _, a := range alerts {
removeEmptyLabels(a.Labels)
if err := a.Validate(api.ff); err != nil {
if err := a.Validate(); err != nil {
validationErrs.Add(err)
api.m.Invalid().Inc()
continue

19
cmd/alertmanager/main.go
View File

@ -320,16 +320,15 @@ func run() int {
}
api, err := api.New(api.Options{
Alerts: alerts,
Silences: silences,
StatusFunc: marker.Status,
Peer: clusterPeer,
Timeout: *httpTimeout,
Concurrency: *getConcurrency,
Logger: log.With(logger, "component", "api"),
FeatureFlags: ff,
Registry: prometheus.DefaultRegisterer,
GroupFunc: groupFn,
Alerts: alerts,
Silences: silences,
StatusFunc: marker.Status,
Peer: clusterPeer,
Timeout: *httpTimeout,
Concurrency: *getConcurrency,
Logger: log.With(logger, "component", "api"),
Registry: prometheus.DefaultRegisterer,
GroupFunc: groupFn,
})
if err != nil {
level.Error(logger).Log("err", fmt.Errorf("failed to create API: %w", err))

47
silence/silence.go
View File

@ -27,7 +27,6 @@ import (
"sort"
"sync"
"time"
"unicode/utf8"
"github.com/benbjohnson/clock"
"github.com/go-kit/log"
@ -39,6 +38,7 @@ import (
"github.com/prometheus/alertmanager/cluster"
"github.com/prometheus/alertmanager/featurecontrol"
"github.com/prometheus/alertmanager/matchers/compat"
"github.com/prometheus/alertmanager/pkg/labels"
pb "github.com/prometheus/alertmanager/silence/silencepb"
"github.com/prometheus/alertmanager/types"
@ -193,7 +193,6 @@ type Silences struct {
logger log.Logger
metrics *metrics
ff featurecontrol.Flagger
retention time.Duration
mtx sync.RWMutex
@ -340,7 +339,6 @@ func New(o Options) (*Silences, error) {
clock: clock.New(),
mc: matcherCache{},
logger: log.NewNopLogger(),
ff: featurecontrol.NoopFlags{},
retention: o.Retention,
broadcast: func([]byte) {},
st: state{},
@ -351,10 +349,6 @@ func New(o Options) (*Silences, error) {
s.logger = o.Logger
}
if o.FeatureFlags != nil {
s.ff = o.FeatureFlags
}
if o.SnapshotFile != "" {
if r, err := os.Open(o.SnapshotFile); err != nil {
if !os.IsNotExist(err) {
@ -477,9 +471,8 @@ func (s *Silences) GC() (int, error) {
return n, nil
}
// validateClassicMatcher validates the matcher against the classic rules.
func validateClassicMatcher(m *pb.Matcher) error {
if !model.LabelName(m.Name).IsValid() {
func validateMatcher(m *pb.Matcher) error {
if !compat.IsValidLabelName(model.LabelName(m.Name)) {
return fmt.Errorf("invalid label name %q", m.Name)
}
switch m.Type {
@ -497,29 +490,6 @@ func validateClassicMatcher(m *pb.Matcher) error {
return nil
}
// validateUTF8Matcher validates the matcher against the UTF-8 rules.
func validateUTF8Matcher(m *pb.Matcher) error {
if !utf8.ValidString(m.Name) {
return fmt.Errorf("invalid label name %q", m.Name)
}
switch m.Type {
case pb.Matcher_EQUAL, pb.Matcher_NOT_EQUAL:
if !utf8.ValidString(m.Pattern) {
return fmt.Errorf("invalid label value %q", m.Pattern)
}
case pb.Matcher_REGEXP, pb.Matcher_NOT_REGEXP:
if !utf8.ValidString(m.Pattern) {
return fmt.Errorf("invalid regular expression %q", m.Pattern)
}
if _, err := regexp.Compile(m.Pattern); err != nil {
return fmt.Errorf("invalid regular expression %q: %w", m.Pattern, err)
}
default:
return fmt.Errorf("unknown matcher type %q", m.Type)
}
return nil
}
func matchesEmpty(m *pb.Matcher) bool {
switch m.Type {
case pb.Matcher_EQUAL:
@ -532,7 +502,7 @@ func matchesEmpty(m *pb.Matcher) bool {
}
}
func validateSilence(s *pb.Silence, ff featurecontrol.Flagger) error {
func validateSilence(s *pb.Silence) error {
if s.Id == "" {
return errors.New("ID missing")
}
@ -541,13 +511,8 @@ func validateSilence(s *pb.Silence, ff featurecontrol.Flagger) error {
}
allMatchEmpty := true
validateFunc := validateUTF8Matcher
if ff.ClassicMode() {
validateFunc = validateClassicMatcher
}
for i, m := range s.Matchers {
if err := validateFunc(m); err != nil {
if err := validateMatcher(m); err != nil {
return fmt.Errorf("invalid label matcher %d: %w", i, err)
}
allMatchEmpty = allMatchEmpty && matchesEmpty(m)
@ -588,7 +553,7 @@ func (s *Silences) setSilence(sil *pb.Silence, now time.Time, skipValidate bool)
sil.UpdatedAt = now
if !skipValidate {
if err := validateSilence(sil, s.ff); err != nil {
if err := validateSilence(sil); err != nil {
return fmt.Errorf("silence invalid: %w", err)
}
}

23
silence/silence_test.go
View File

@ -33,6 +33,7 @@ import (
"go.uber.org/atomic"
"github.com/prometheus/alertmanager/featurecontrol"
"github.com/prometheus/alertmanager/matchers/compat"
pb "github.com/prometheus/alertmanager/silence/silencepb"
"github.com/prometheus/alertmanager/types"
)
@ -1060,7 +1061,7 @@ func TestSilenceExpireInvalid(t *testing.T) {
UpdatedAt: now.Add(-time.Hour),
}
// Assert that this silence is invalid.
require.EqualError(t, validateSilence(&silence, featurecontrol.NoopFlags{}), "invalid label matcher 0: unknown matcher type \"-1\"")
require.EqualError(t, validateSilence(&silence), "invalid label matcher 0: unknown matcher type \"-1\"")
s.st = state{"active": &pb.MeshSilence{Silence: &silence}}
@ -1241,7 +1242,7 @@ func TestValidateClassicMatcher(t *testing.T) {
}
for _, c := range cases {
checkErr(t, c.err, validateClassicMatcher(c.m))
checkErr(t, c.err, validateMatcher(c.m))
}
}
@ -1330,8 +1331,18 @@ func TestValidateUTF8Matcher(t *testing.T) {
},
}
// Change the mode to UTF-8 mode.
ff, err := featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureUTF8StrictMode)
require.NoError(t, err)
compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
// Restore the mode to classic at the end of the test.
ff, err = featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureClassicMode)
require.NoError(t, err)
defer compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
for _, c := range cases {
checkErr(t, c.err, validateUTF8Matcher(c.m))
checkErr(t, c.err, validateMatcher(c.m))
}
}
@ -1455,11 +1466,7 @@ func TestValidateSilence(t *testing.T) {
},
}
for _, c := range cases {
ff, err := featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureClassicMode)
if err != nil {
t.Fatal("unexpected err", err)
}
checkErr(t, c.err, validateSilence(c.s, ff))
checkErr(t, c.err, validateSilence(c.s))
}
}

28
types/types.go
View File

@ -18,12 +18,11 @@ import (
"strings"
"sync"
"time"
"unicode/utf8"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/common/model"
"github.com/prometheus/alertmanager/featurecontrol"
"github.com/prometheus/alertmanager/matchers/compat"
"github.com/prometheus/alertmanager/pkg/labels"
)
@ -305,24 +304,9 @@ type Alert struct {
Timeout bool
}
// validateLs validates the label set against either the classic rules
// or the UTF-8 rules depending on the feature flag.
func validateLs(ls model.LabelSet, ff featurecontrol.Flagger) error {
if ff.ClassicMode() {
return validateClassicLs(ls)
}
return validateUTF8Ls(ls)
}
// validateClassicLs validates the label set against the classic rules.
func validateClassicLs(ls model.LabelSet) error {
return ls.Validate()
}
// validateUTF8Ls validates the label set against the UTF-8 rules.
func validateUTF8Ls(ls model.LabelSet) error {
func validateLs(ls model.LabelSet) error {
for ln, lv := range ls {
if len(ln) == 0 || !utf8.ValidString(string(ln)) {
if !compat.IsValidLabelName(ln) {
return fmt.Errorf("invalid name %q", ln)
}
if !lv.IsValid() {
@ -334,7 +318,7 @@ func validateUTF8Ls(ls model.LabelSet) error {
// Validate overrides the same method in model.Alert to allow UTF-8 labels.
// This can be removed once prometheus/common has support for UTF-8.
func (a *Alert) Validate(ff featurecontrol.Flagger) error {
func (a *Alert) Validate() error {
if a.StartsAt.IsZero() {
return fmt.Errorf("start time missing")
}
@ -344,10 +328,10 @@ func (a *Alert) Validate(ff featurecontrol.Flagger) error {
if len(a.Labels) == 0 {
return fmt.Errorf("at least one label pair required")
}
if err := validateLs(a.Labels, ff); err != nil {
if err := validateLs(a.Labels); err != nil {
return fmt.Errorf("invalid label set: %w", err)
}
if err := validateLs(a.Annotations, ff); err != nil {
if err := validateLs(a.Annotations); err != nil {
return fmt.Errorf("invalid annotations: %w", err)
}
return nil

16
types/types_test.go
View File

@ -20,9 +20,13 @@ import (
"testing"
"time"
"github.com/go-kit/log"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/common/model"
"github.com/stretchr/testify/require"
"github.com/prometheus/alertmanager/featurecontrol"
"github.com/prometheus/alertmanager/matchers/compat"
)
func TestMemMarker_Count(t *testing.T) {
@ -334,9 +338,19 @@ func TestValidateUTF8Ls(t *testing.T) {
err: "invalid name \"\\xff\"",
}}
// Change the mode to UTF-8 mode.
ff, err := featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureUTF8StrictMode)
require.NoError(t, err)
compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
// Restore the mode to classic at the end of the test.
ff, err = featurecontrol.NewFlags(log.NewNopLogger(), featurecontrol.FeatureClassicMode)
require.NoError(t, err)
defer compat.InitFromFlags(log.NewNopLogger(), compat.RegisteredMetrics, ff)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
err := validateUTF8Ls(test.ls)
err := validateLs(test.ls)
if err != nil && err.Error() != test.err {
t.Errorf("unexpected err for %s: %s", test.ls, err)
} else if err == nil && test.err != "" {