From cbeaf203496ab8df606b88911167a7a4c8401b8c Mon Sep 17 00:00:00 2001 From: Bisman-Singh Date: Tue, 1 Sep 2026 05:03:24 +0530 Subject: [PATCH 1/3] fix(config): accumulate validation errors across receivers and notifier configs Config validation previously stopped at the first invalid notifier config: each notifier's UnmarshalYAML returned its Validate() error, which aborted the whole YAML decode, so only one error was reported per run even when several receivers or notifier configs were independently broken. Move the Validate() calls out of the notifier UnmarshalYAML methods (split out for this purpose in #4992) into Config.UnmarshalYAML, and accumulate all receiver-level and notifier-level validation errors with errors.Join before returning, so config loading and 'amtool check-config' report every error at once. Individual error messages are unchanged. Tests that unmarshaled a single notifier config directly now call Validate() explicitly, since validation no longer runs during unmarshaling. Fixes #4990 Fixes #4991 Signed-off-by: Bisman-Singh --- config/config.go | 187 +++++++++++++++++++++++++------ config/config_test.go | 130 ++++++++++++++++++++- config/notifiers.go | 20 +++- config/notifiers_test.go | 21 ++++ notify/discord/config.go | 4 +- notify/incidentio/config.go | 4 +- notify/jira/config.go | 4 +- notify/mattermost/config.go | 4 +- notify/mattermost/config_test.go | 3 + notify/msteams/config.go | 4 +- notify/msteamsv2/config.go | 4 +- notify/opsgenie/config.go | 4 +- notify/opsgenie/config_test.go | 3 + notify/pagerduty/config.go | 4 +- notify/pagerduty/config_test.go | 12 ++ notify/pushover/config.go | 4 +- notify/pushover/config_test.go | 15 +++ notify/rocketchat/config.go | 4 +- notify/sns/config.go | 4 +- notify/sns/config_test.go | 3 + notify/telegram/config.go | 4 +- notify/telegram/config_test.go | 3 + notify/webhook/config.go | 4 +- notify/webhook/config_test.go | 6 + 24 files changed, 398 insertions(+), 57 deletions(-) diff --git a/config/config.go b/config/config.go index 487ec83e4a..ce2d49a4f0 100644 --- a/config/config.go +++ b/config/config.go @@ -382,28 +382,45 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { names := map[string]struct{}{} + // Validation errors are accumulated across all receivers and notifier + // configs so that a single invalid entry does not hide the others. + var errs error + for _, rcv := range c.Receivers { if _, ok := names[rcv.Name]; ok { - return fmt.Errorf("notification config name %q is not unique", rcv.Name) + errs = errors.Join(errs, fmt.Errorf("notification config name %q is not unique", rcv.Name)) + continue } for _, wh := range rcv.WebhookConfigs { if wh == nil { - return errors.New("missing webhook config") + errs = errors.Join(errs, errors.New("missing webhook config")) + continue + } + if err := wh.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } wh.HTTPConfig = cmp.Or(wh.HTTPConfig, c.Global.HTTPConfig) } for _, ec := range rcv.EmailConfigs { if ec == nil { - return errors.New("missing email config") + errs = errors.Join(errs, errors.New("missing email config")) + continue + } + if err := ec.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } ec.TLSConfig = cmp.Or(ec.TLSConfig, c.Global.SMTPTLSConfig) ec.Smarthost = cmp.Or(ec.Smarthost, c.Global.SMTPSmarthost) if ec.Smarthost.String() == "" { - return errors.New("no global SMTP smarthost set") + errs = errors.Join(errs, errors.New("no global SMTP smarthost set")) + continue } ec.From = cmp.Or(ec.From, c.Global.SMTPFrom) if ec.From == "" { - return errors.New("no global SMTP from set") + errs = errors.Join(errs, errors.New("no global SMTP from set")) + continue } ec.Hello = cmp.Or(ec.Hello, c.Global.SMTPHello) ec.AuthUsername = cmp.Or(ec.AuthUsername, c.Global.SMTPAuthUsername) @@ -427,10 +444,14 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { for _, sc := range rcv.SlackConfigs { if sc == nil { sc = &SlackConfig{} + } else if err := sc.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } sc.AppURL = cmp.Or(sc.AppURL, c.Global.SlackAppURL) if sc.AppURL == nil { - return errors.New("no global Slack App URL set") + errs = errors.Join(errs, errors.New("no global Slack App URL set")) + continue } // we only want to set the app token from global if there's no local authorization or webhook url if sc.AppToken == "" && len(sc.AppTokenFile) == 0 && (sc.HTTPConfig == nil || sc.HTTPConfig.Authorization == nil) && sc.APIURL == nil { @@ -442,7 +463,8 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { sc.APIURLFile = c.Global.SlackAPIURLFile } if sc.APIURL == nil && len(sc.APIURLFile) == 0 && sc.AppToken == "" && len(sc.AppTokenFile) == 0 { - return errors.New("no Slack API URL nor App token set either inline or in a file") + errs = errors.Join(errs, errors.New("no Slack API URL nor App token set either inline or in a file")) + continue } if sc.HTTPConfig == nil { // we don't want to change the global http config when setting the receiver's http config, do we do a copy @@ -451,7 +473,8 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } if sc.AppToken != "" || len(sc.AppTokenFile) != 0 { if sc.HTTPConfig.Authorization != nil { - return errors.New("http authorization can't be set when using Slack App tokens") + errs = errors.Join(errs, errors.New("http authorization can't be set when using Slack App tokens")) + continue } sc.HTTPConfig.Authorization = &commoncfg.Authorization{ Type: "Bearer", @@ -463,41 +486,62 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, poc := range rcv.PushoverConfigs { if poc == nil { - return errors.New("missing pushover config") + errs = errors.Join(errs, errors.New("missing pushover config")) + continue + } + if err := poc.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } poc.HTTPConfig = cmp.Or(poc.HTTPConfig, c.Global.HTTPConfig) } for _, pdc := range rcv.PagerdutyConfigs { if pdc == nil { - return errors.New("missing pagerduty config") + errs = errors.Join(errs, errors.New("missing pagerduty config")) + continue + } + if err := pdc.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } pdc.HTTPConfig = cmp.Or(pdc.HTTPConfig, c.Global.HTTPConfig) pdc.URL = cmp.Or(pdc.URL, c.Global.PagerdutyURL) if pdc.URL == nil { - return errors.New("no global PagerDuty URL set") + errs = errors.Join(errs, errors.New("no global PagerDuty URL set")) + continue } } for _, iio := range rcv.IncidentioConfigs { if iio == nil { - return errors.New("missing incidentio config") + errs = errors.Join(errs, errors.New("missing incidentio config")) + continue + } + if err := iio.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } iio.HTTPConfig = cmp.Or(iio.HTTPConfig, c.Global.HTTPConfig) } for _, ogc := range rcv.OpsGenieConfigs { if ogc == nil { ogc = &opsgenie.OpsGenieConfig{} + } else if err := ogc.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } ogc.HTTPConfig = cmp.Or(ogc.HTTPConfig, c.Global.HTTPConfig) ogc.APIURL = cmp.Or(ogc.APIURL, c.Global.OpsGenieAPIURL) if ogc.APIURL == nil { - return errors.New("no global OpsGenie URL set") + errs = errors.Join(errs, errors.New("no global OpsGenie URL set")) + continue } if !strings.HasSuffix(ogc.APIURL.Path, "/") { ogc.APIURL.Path += "/" } if ogc.APIKey == "" && len(ogc.APIKeyFile) == 0 { if c.Global.OpsGenieAPIKey == "" && len(c.Global.OpsGenieAPIKeyFile) == 0 { - return errors.New("no global OpsGenie API Key set either inline or in a file") + errs = errors.Join(errs, errors.New("no global OpsGenie API Key set either inline or in a file")) + continue } ogc.APIKey = c.Global.OpsGenieAPIKey ogc.APIKeyFile = c.Global.OpsGenieAPIKeyFile @@ -506,16 +550,21 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { for _, wcc := range rcv.WechatConfigs { if wcc == nil { wcc = &WechatConfig{} + } else if err := wcc.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } wcc.HTTPConfig = cmp.Or(wcc.HTTPConfig, c.Global.HTTPConfig) wcc.APIURL = cmp.Or(wcc.APIURL, c.Global.WeChatAPIURL) if wcc.APIURL == nil { - return errors.New("no global Wechat URL set") + errs = errors.Join(errs, errors.New("no global Wechat URL set")) + continue } if wcc.APISecret == "" && len(wcc.APISecretFile) == 0 { if c.Global.WeChatAPISecret == "" && len(c.Global.WeChatAPISecretFile) == 0 { - return errors.New("no global Wechat Api Secret set either inline or in a file") + errs = errors.Join(errs, errors.New("no global Wechat Api Secret set either inline or in a file")) + continue } wcc.APISecret = c.Global.WeChatAPISecret wcc.APISecretFile = c.Global.WeChatAPISecretFile @@ -523,7 +572,8 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { wcc.CorpID = cmp.Or(wcc.CorpID, c.Global.WeChatAPICorpID) if wcc.CorpID == "" { - return errors.New("no global Wechat CorpID set") + errs = errors.Join(errs, errors.New("no global Wechat CorpID set")) + continue } if !strings.HasSuffix(wcc.APIURL.Path, "/") { @@ -532,19 +582,26 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, voc := range rcv.VictorOpsConfigs { if voc == nil { - return errors.New("missing victorops config") + errs = errors.Join(errs, errors.New("missing victorops config")) + continue + } + if err := voc.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } voc.HTTPConfig = cmp.Or(voc.HTTPConfig, c.Global.HTTPConfig) voc.APIURL = cmp.Or(voc.APIURL, c.Global.VictorOpsAPIURL) if voc.APIURL == nil { - return errors.New("no global VictorOps URL set") + errs = errors.Join(errs, errors.New("no global VictorOps URL set")) + continue } if !strings.HasSuffix(voc.APIURL.Path, "/") { voc.APIURL.Path += "/" } if voc.APIKey == "" && len(voc.APIKeyFile) == 0 { if c.Global.VictorOpsAPIKey == "" && len(c.Global.VictorOpsAPIKeyFile) == 0 { - return errors.New("no global VictorOps API Key set") + errs = errors.Join(errs, errors.New("no global VictorOps API Key set")) + continue } voc.APIKey = c.Global.VictorOpsAPIKey voc.APIKeyFile = c.Global.VictorOpsAPIKeyFile @@ -552,20 +609,31 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, sns := range rcv.SNSConfigs { if sns == nil { - return errors.New("missing sns config") + errs = errors.Join(errs, errors.New("missing sns config")) + continue + } + if err := sns.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } sns.HTTPConfig = cmp.Or(sns.HTTPConfig, c.Global.HTTPConfig) } for _, telegram := range rcv.TelegramConfigs { if telegram == nil { - return errors.New("missing telegram config") + errs = errors.Join(errs, errors.New("missing telegram config")) + continue + } + if err := telegram.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } telegram.HTTPConfig = cmp.Or(telegram.HTTPConfig, c.Global.HTTPConfig) telegram.APIUrl = cmp.Or(telegram.APIUrl, c.Global.TelegramAPIUrl) if telegram.BotToken == "" && len(telegram.BotTokenFile) == 0 { if c.Global.TelegramBotToken == "" && len(c.Global.TelegramBotTokenFile) == 0 { - return errors.New("missing bot_token or bot_token_file on telegram_config") + errs = errors.Join(errs, errors.New("missing bot_token or bot_token_file on telegram_config")) + continue } telegram.BotToken = c.Global.TelegramBotToken telegram.BotTokenFile = c.Global.TelegramBotTokenFile @@ -573,35 +641,58 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, discord := range rcv.DiscordConfigs { if discord == nil { - return errors.New("missing discord config") + errs = errors.Join(errs, errors.New("missing discord config")) + continue + } + if err := discord.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } discord.HTTPConfig = cmp.Or(discord.HTTPConfig, c.Global.HTTPConfig) if discord.WebhookURL == nil && len(discord.WebhookURLFile) == 0 { - return errors.New("no discord webhook URL or URLFile provided") + errs = errors.Join(errs, errors.New("no discord webhook URL or URLFile provided")) + continue } } for _, webex := range rcv.WebexConfigs { if webex == nil { - return errors.New("missing webex config") + errs = errors.Join(errs, errors.New("missing webex config")) + continue + } + if err := webex.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } webex.HTTPConfig = cmp.Or(webex.HTTPConfig, c.Global.HTTPConfig) webex.APIURL = cmp.Or(webex.APIURL, c.Global.WebexAPIURL) if webex.APIURL == nil { - return errors.New("no global Webex URL set") + errs = errors.Join(errs, errors.New("no global Webex URL set")) + continue } } for _, msteams := range rcv.MSTeamsConfigs { if msteams == nil { - return errors.New("missing msteams config") + errs = errors.Join(errs, errors.New("missing msteams config")) + continue + } + if err := msteams.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } msteams.HTTPConfig = cmp.Or(msteams.HTTPConfig, c.Global.HTTPConfig) if msteams.WebhookURL == nil && len(msteams.WebhookURLFile) == 0 { - return errors.New("no msteams webhook URL or URLFile provided") + errs = errors.Join(errs, errors.New("no msteams webhook URL or URLFile provided")) + continue } } for _, msteamsv2 := range rcv.MSTeamsV2Configs { if msteamsv2 == nil { - return errors.New("missing msteamsv2 config") + errs = errors.Join(errs, errors.New("missing msteamsv2 config")) + continue + } + if err := msteamsv2.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } if msteamsv2.HTTPConfig == nil { // copy the global config so receiver-level mutations don't affect it @@ -614,35 +705,47 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { msteamsv2.HTTPConfig.ProxyURL = c.Global.HTTPConfig.ProxyURL } if msteamsv2.WebhookURL == nil && len(msteamsv2.WebhookURLFile) == 0 { - return errors.New("no msteamsv2 webhook URL or URLFile provided") + errs = errors.Join(errs, errors.New("no msteamsv2 webhook URL or URLFile provided")) + continue } } for _, jira := range rcv.JiraConfigs { if jira == nil { - return errors.New("missing jira config") + errs = errors.Join(errs, errors.New("missing jira config")) + continue + } + if err := jira.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } jira.HTTPConfig = cmp.Or(jira.HTTPConfig, c.Global.HTTPConfig) jira.APIURL = cmp.Or(jira.APIURL, c.Global.JiraAPIURL) if jira.APIURL == nil { - return errors.New("no global Jira Cloud URL set") + errs = errors.Join(errs, errors.New("no global Jira Cloud URL set")) + continue } } for _, rocketchatcfg := range rcv.RocketchatConfigs { if rocketchatcfg == nil { rocketchatcfg = &rocketchat.RocketchatConfig{} + } else if err := rocketchatcfg.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } rocketchatcfg.HTTPConfig = cmp.Or(rocketchatcfg.HTTPConfig, c.Global.HTTPConfig) rocketchatcfg.APIURL = cmp.Or(rocketchatcfg.APIURL, c.Global.RocketchatAPIURL) if rocketchatcfg.TokenID == nil && len(rocketchatcfg.TokenIDFile) == 0 { if c.Global.RocketchatTokenID == nil && len(c.Global.RocketchatTokenIDFile) == 0 { - return errors.New("no global Rocketchat TokenID set either inline or in a file") + errs = errors.Join(errs, errors.New("no global Rocketchat TokenID set either inline or in a file")) + continue } rocketchatcfg.TokenID = c.Global.RocketchatTokenID rocketchatcfg.TokenIDFile = c.Global.RocketchatTokenIDFile } if rocketchatcfg.Token == nil && len(rocketchatcfg.TokenFile) == 0 { if c.Global.RocketchatToken == nil && len(c.Global.RocketchatTokenFile) == 0 { - return errors.New("no global Rocketchat Token set either inline or in a file") + errs = errors.Join(errs, errors.New("no global Rocketchat Token set either inline or in a file")) + continue } rocketchatcfg.Token = c.Global.RocketchatToken rocketchatcfg.TokenFile = c.Global.RocketchatTokenFile @@ -650,12 +753,18 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, mattermost := range rcv.MattermostConfigs { if mattermost == nil { - return errors.New("missing mattermost config") + errs = errors.Join(errs, errors.New("missing mattermost config")) + continue + } + if err := mattermost.Validate(); err != nil { + errs = errors.Join(errs, err) + continue } mattermost.HTTPConfig = cmp.Or(mattermost.HTTPConfig, c.Global.HTTPConfig) if mattermost.WebhookURL == nil && len(mattermost.WebhookURLFile) == 0 { if c.Global.MattermostWebhookURL == nil && len(c.Global.MattermostWebhookURLFile) == 0 { - return errors.New("missing webhook_url or webhook_url_file on mattermost_config") + errs = errors.Join(errs, errors.New("missing webhook_url or webhook_url_file on mattermost_config")) + continue } mattermost.WebhookURL = c.Global.MattermostWebhookURL mattermost.WebhookURLFile = c.Global.MattermostWebhookURLFile @@ -665,6 +774,10 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { names[rcv.Name] = struct{}{} } + if errs != nil { + return errs + } + // The root route must not have any matchers as it is the fallback node // for all alerts. if c.Route == nil { diff --git a/config/config_test.go b/config/config_test.go index c70fdaf3d2..3bd41e0dbd 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -109,6 +109,130 @@ receivers: } } +func TestReceiverValidationErrorsAccumulate(t *testing.T) { + tests := []struct { + name string + in string + + expectedErrs []string + }{ + { + // https://github.com/prometheus/alertmanager/issues/4990 + name: "multiple invalid notifier configs in a single receiver", + in: ` +route: + receiver: team-X + +receivers: +- name: 'team-X' + webhook_configs: + - send_resolved: true + - url: 'http://example.com/' + url_file: '/etc/secrets/webhook-url' +`, + expectedErrs: []string{ + "one of url or url_file must be configured", + "at most one of url & url_file must be configured", + }, + }, + { + name: "invalid notifier configs of different types in a single receiver", + in: ` +route: + receiver: team-X + +receivers: +- name: 'team-X' + webhook_configs: + - send_resolved: true + pagerduty_configs: + - url: 'https://example.com/' +`, + expectedErrs: []string{ + "one of url or url_file must be configured", + "missing service or routing key in PagerDuty config", + }, + }, + { + // https://github.com/prometheus/alertmanager/issues/4991 + name: "invalid notifier configs across multiple receivers", + in: ` +route: + receiver: team-A + +receivers: +- name: 'team-A' + webhook_configs: + - send_resolved: true +- name: 'team-B' + email_configs: + - smarthost: 'smtp.example.com:587' + from: 'alertmanager@example.com' +- name: 'team-C' + pagerduty_configs: + - url: 'https://example.com/' +`, + expectedErrs: []string{ + "one of url or url_file must be configured", + "missing to address in email config", + "missing service or routing key in PagerDuty config", + }, + }, + { + name: "missing global fallbacks are accumulated across receivers", + in: ` +route: + receiver: team-A + +receivers: +- name: 'team-A' + email_configs: + - to: 'team-A@example.com' +- name: 'team-B' + webhook_configs: + - url: 'http://example.com/' + url_file: '/etc/secrets/webhook-url' +`, + expectedErrs: []string{ + "no global SMTP smarthost set", + "at most one of url & url_file must be configured", + }, + }, + { + name: "duplicate receiver name is accumulated with notifier config errors", + in: ` +route: + receiver: team-X + +receivers: +- name: 'team-X' + webhook_configs: + - send_resolved: true +- name: 'team-X' +`, + expectedErrs: []string{ + "one of url or url_file must be configured", + "notification config name \"team-X\" is not unique", + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := Load(tc.in) + + expected := strings.Join(tc.expectedErrs, "\n") + + if err == nil { + t.Fatalf("no error returned, expected:\n%v", expected) + } + if err.Error() != expected { + t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) + } + }) + } +} + func TestReceiverExists(t *testing.T) { in := ` route: @@ -1582,8 +1706,10 @@ func TestRocketchatNoToken(t *testing.T) { if err == nil { t.Fatalf("Expected an error parsing %s: %s", "testdata/conf.rocketchat-no-token.yml", err) } - if err.Error() != "no global Rocketchat Token set either inline or in a file" { - t.Errorf("Expected: %s\nGot: %s", "no global Rocketchat Token set either inline or in a file", err.Error()) + // Both receivers in the fixture lack a token, so the error is reported once per receiver. + expected := "no global Rocketchat Token set either inline or in a file\nno global Rocketchat Token set either inline or in a file" + if err.Error() != expected { + t.Errorf("Expected: %s\nGot: %s", expected, err.Error()) } } diff --git a/config/notifiers.go b/config/notifiers.go index 8ae478eca7..f19ab51208 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -107,7 +107,9 @@ func (c *WebexConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } func (c *WebexConfig) Validate() error { @@ -177,7 +179,9 @@ func (c *EmailConfig) UnmarshalYAML(unmarshal func(any) error) error { } c.Headers = normalizedHeaders - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } func (c *EmailConfig) Validate() error { @@ -348,7 +352,9 @@ func (c *SlackConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } func (c *SlackConfig) Validate() error { @@ -403,7 +409,9 @@ func (c *WechatConfig) UnmarshalYAML(unmarshal func(any) error) error { c.MessageType = "text" } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } func (c *WechatConfig) Validate() error { @@ -442,7 +450,9 @@ func (c *VictorOpsConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } func (c *VictorOpsConfig) Validate() error { diff --git a/config/notifiers_test.go b/config/notifiers_test.go index 0ab362dc00..7a931b5c34 100644 --- a/config/notifiers_test.go +++ b/config/notifiers_test.go @@ -29,6 +29,9 @@ to: '' ` var cfg EmailConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "missing to address in email config" @@ -120,6 +123,9 @@ routing_key: '' ` var cfg VictorOpsConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "missing Routing key in VictorOps config" @@ -139,6 +145,9 @@ api_key_file: /global_file ` var cfg VictorOpsConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "at most one of api_key & api_key_file must be configured" @@ -159,6 +168,9 @@ custom_fields: ` var cfg VictorOpsConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "victorOps config contains custom field entity_state which cannot be used as it conflicts with the fixed/static fields" @@ -294,6 +306,9 @@ api_url: 'https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXX for _, rt := range tests { var cfg SlackConfig err := yaml.UnmarshalStrict([]byte(rt.in), &cfg) + if err == nil { + err = cfg.Validate() + } // Check if an error occurred when it was NOT expected to. if rt.expectedErr == "" && err != nil { @@ -550,6 +565,9 @@ http_config: t.Run(tt.name, func(t *testing.T) { var cfg WebexConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) + if err == nil { + err = cfg.Validate() + } require.Equal(t, tt.expected, err) }) @@ -608,6 +626,9 @@ headers: {X-Custom-Header: CustomValue, X-CUSTOM-HEADER: AnotherValue} t.Run(tt.name, func(t *testing.T) { var cfg EmailConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) + if err == nil { + err = cfg.Validate() + } require.Equal(t, tt.expected, err) }) diff --git a/notify/discord/config.go b/notify/discord/config.go index 63caaa7976..b872fe1d5e 100644 --- a/notify/discord/config.go +++ b/notify/discord/config.go @@ -53,7 +53,9 @@ func (c *DiscordConfig) UnmarshalYAML(unmarshal func(any) error) error { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the DiscordConfig for correctness. diff --git a/notify/incidentio/config.go b/notify/incidentio/config.go index 12ee5e64d0..c6eef1fbe1 100644 --- a/notify/incidentio/config.go +++ b/notify/incidentio/config.go @@ -66,7 +66,9 @@ func (c *IncidentioConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the IncidentioConfig for correctness. diff --git a/notify/jira/config.go b/notify/jira/config.go index c7cccf2c73..c18a7f2d38 100644 --- a/notify/jira/config.go +++ b/notify/jira/config.go @@ -94,7 +94,9 @@ func (c *JiraConfig) UnmarshalYAML(unmarshal func(any) error) error { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the JiraConfig for correctness. diff --git a/notify/mattermost/config.go b/notify/mattermost/config.go index 52c9119928..1c5b999e77 100644 --- a/notify/mattermost/config.go +++ b/notify/mattermost/config.go @@ -135,7 +135,9 @@ func (c *MattermostConfig) UnmarshalYAML(unmarshal func(any) error) error { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the MattermostConfig for correctness. diff --git a/notify/mattermost/config_test.go b/notify/mattermost/config_test.go index 15660d6011..bc05387e9a 100644 --- a/notify/mattermost/config_test.go +++ b/notify/mattermost/config_test.go @@ -124,6 +124,9 @@ attachments: t.Run(tt.name, func(t *testing.T) { var cfg MattermostConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) + if err == nil { + err = cfg.Validate() + } require.Equal(t, tt.expected, err) }) diff --git a/notify/msteams/config.go b/notify/msteams/config.go index 782479567c..85c5b0bddf 100644 --- a/notify/msteams/config.go +++ b/notify/msteams/config.go @@ -48,7 +48,9 @@ func (c *MSTeamsConfig) UnmarshalYAML(unmarshal func(any) error) error { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the MSTeamsConfig for correctness. diff --git a/notify/msteamsv2/config.go b/notify/msteamsv2/config.go index 55f0dd6a42..182c503440 100644 --- a/notify/msteamsv2/config.go +++ b/notify/msteamsv2/config.go @@ -46,7 +46,9 @@ func (c *MSTeamsV2Config) UnmarshalYAML(unmarshal func(any) error) error { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the MSTeamsV2Config for correctness. diff --git a/notify/opsgenie/config.go b/notify/opsgenie/config.go index 2c37cebf7c..70521b4af1 100644 --- a/notify/opsgenie/config.go +++ b/notify/opsgenie/config.go @@ -68,7 +68,9 @@ func (c *OpsGenieConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the OpsGenieConfig for correctness. diff --git a/notify/opsgenie/config_test.go b/notify/opsgenie/config_test.go index 87e0868766..201435c5a7 100644 --- a/notify/opsgenie/config_test.go +++ b/notify/opsgenie/config_test.go @@ -90,6 +90,9 @@ api_url: http://example.com var cfg OpsGenieConfig err := yaml.UnmarshalStrict([]byte(tc.in), &cfg) + if err == nil { + err = cfg.Validate() + } if tc.err { if err == nil { t.Fatalf("expected error but got none") diff --git a/notify/pagerduty/config.go b/notify/pagerduty/config.go index 694a3ff541..8cd2064ed8 100644 --- a/notify/pagerduty/config.go +++ b/notify/pagerduty/config.go @@ -100,7 +100,9 @@ func (c *PagerdutyConfig) UnmarshalYAML(unmarshal func(any) error) error { c.Details[k] = v } } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the PagerdutyConfig for correctness. diff --git a/notify/pagerduty/config_test.go b/notify/pagerduty/config_test.go index 3d31bd0017..9b2d2a0b24 100644 --- a/notify/pagerduty/config_test.go +++ b/notify/pagerduty/config_test.go @@ -27,6 +27,9 @@ routing_key: '' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "missing service or routing key in PagerDuty config" @@ -45,6 +48,9 @@ routing_key_file: 'xyz' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "at most one of routing_key & routing_key_file must be configured" @@ -64,6 +70,9 @@ service_key: '' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "missing service or routing key in PagerDuty config" @@ -82,6 +91,9 @@ service_key_file: 'xyz' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "at most one of service_key & service_key_file must be configured" diff --git a/notify/pushover/config.go b/notify/pushover/config.go index 08074a12f2..41d35a5d82 100644 --- a/notify/pushover/config.go +++ b/notify/pushover/config.go @@ -80,7 +80,9 @@ func (c *PushoverConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the PushoverConfig for correctness. diff --git a/notify/pushover/config_test.go b/notify/pushover/config_test.go index 36185d71d4..230bb6d052 100644 --- a/notify/pushover/config_test.go +++ b/notify/pushover/config_test.go @@ -25,6 +25,9 @@ user_key: '' ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "one of user_key or user_key_file must be configured" @@ -43,6 +46,9 @@ user_key_file: /pushover/user_key ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "at most one of user_key & user_key_file must be configured" @@ -61,6 +67,9 @@ token: '' ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "one of token or token_file must be configured" @@ -80,6 +89,9 @@ user_key: 'user key' ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "at most one of token & token_file must be configured" @@ -100,6 +112,9 @@ monospace: true ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "at most one of monospace & html must be configured" diff --git a/notify/rocketchat/config.go b/notify/rocketchat/config.go index c0dd7580c8..e22184abaf 100644 --- a/notify/rocketchat/config.go +++ b/notify/rocketchat/config.go @@ -93,7 +93,9 @@ func (c *RocketchatConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the RocketchatConfig for correctness. diff --git a/notify/sns/config.go b/notify/sns/config.go index f7a0e25be6..202d0cfe95 100644 --- a/notify/sns/config.go +++ b/notify/sns/config.go @@ -58,7 +58,9 @@ func (c *SNSConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the SNSConfig for correctness. diff --git a/notify/sns/config_test.go b/notify/sns/config_test.go index 27e70fa43c..924362a129 100644 --- a/notify/sns/config_test.go +++ b/notify/sns/config_test.go @@ -90,6 +90,9 @@ sigv4: t.Run("", func(t *testing.T) { var cfg SNSConfig err := yaml.UnmarshalStrict([]byte(tc.in), &cfg) + if err == nil { + err = cfg.Validate() + } if err != nil { if !tc.err { t.Errorf("expecting no error, got %q", err) diff --git a/notify/telegram/config.go b/notify/telegram/config.go index 5f02739cd4..0d94365a29 100644 --- a/notify/telegram/config.go +++ b/notify/telegram/config.go @@ -59,7 +59,9 @@ func (c *TelegramConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the TelegramConfig for correctness. diff --git a/notify/telegram/config_test.go b/notify/telegram/config_test.go index adaa1576df..75cd061043 100644 --- a/notify/telegram/config_test.go +++ b/notify/telegram/config_test.go @@ -95,6 +95,9 @@ parse_mode: invalid t.Run(tt.name, func(t *testing.T) { var cfg TelegramConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) + if err == nil { + err = cfg.Validate() + } require.Equal(t, tt.expected, err) }) diff --git a/notify/webhook/config.go b/notify/webhook/config.go index 5ddabb86d0..7c3d277e2d 100644 --- a/notify/webhook/config.go +++ b/notify/webhook/config.go @@ -57,7 +57,9 @@ func (c *WebhookConfig) UnmarshalYAML(unmarshal func(any) error) error { if err := unmarshal((*plain)(c)); err != nil { return err } - return c.Validate() + // Validation happens in Config.UnmarshalYAML so that errors from all + // notifier configs can be reported together instead of one at a time. + return nil } // Validate checks the WebhookConfig for correctness. diff --git a/notify/webhook/config_test.go b/notify/webhook/config_test.go index a9db06e704..7b4bee2d93 100644 --- a/notify/webhook/config_test.go +++ b/notify/webhook/config_test.go @@ -24,6 +24,9 @@ func TestWebhookURLIsPresent(t *testing.T) { in := `{}` var cfg WebhookConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "one of url or url_file must be configured" @@ -42,6 +45,9 @@ url_file: 'http://example.com' ` var cfg WebhookConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) + if err == nil { + err = cfg.Validate() + } expected := "at most one of url & url_file must be configured" From e5eec1b3ed709a7ce0c6e5eff65c98536fe29d8c Mon Sep 17 00:00:00 2001 From: Bisman-Singh Date: Tue, 1 Sep 2026 12:41:12 +0530 Subject: [PATCH 2/3] config: also validate notifier configs of duplicate receivers A duplicate receiver name no longer short-circuits validation of that receiver's notifier configurations, so their errors are reported alongside the duplicate-name error. Signed-off-by: Bisman-Singh --- config/config.go | 3 ++- config/config_test.go | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/config/config.go b/config/config.go index ce2d49a4f0..01ce517c59 100644 --- a/config/config.go +++ b/config/config.go @@ -388,8 +388,9 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { for _, rcv := range c.Receivers { if _, ok := names[rcv.Name]; ok { + // Record the duplicate name but keep validating this receiver's + // notifier configurations so their errors are reported as well. errs = errors.Join(errs, fmt.Errorf("notification config name %q is not unique", rcv.Name)) - continue } for _, wh := range rcv.WebhookConfigs { if wh == nil { diff --git a/config/config_test.go b/config/config_test.go index 3bd41e0dbd..6af7eedf2d 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -135,6 +135,27 @@ receivers: "at most one of url & url_file must be configured", }, }, + { + // A duplicate receiver name must not hide the validation errors + // of the duplicate's own notifier configurations. + name: "duplicate receiver name with an invalid notifier config", + in: ` +route: + receiver: team-X + +receivers: +- name: 'team-X' + webhook_configs: + - url: 'http://example.com/' +- name: 'team-X' + webhook_configs: + - send_resolved: true +`, + expectedErrs: []string{ + `notification config name "team-X" is not unique`, + "one of url or url_file must be configured", + }, + }, { name: "invalid notifier configs of different types in a single receiver", in: ` From 37b1a488f1e047b9f1dacc356c28817d11b751af Mon Sep 17 00:00:00 2001 From: Bisman-Singh Date: Wed, 2 Sep 2026 22:35:05 +0530 Subject: [PATCH 3/3] config: accumulate validation errors through yaml.TypeError yaml.v2 keeps decoding when an UnmarshalYAML returns a *yaml.TypeError and reports all of them together, so notifiers now return their validation errors that way and Config.UnmarshalYAML flattens them back into plain errors. Validation stays inside each notifier's UnmarshalYAML and Config.UnmarshalYAML does not grow. Fixes #4990 Refs #4991 Signed-off-by: Bisman-Singh --- config/common/notifierconfig.go | 47 ++++++ config/config.go | 190 +++++------------------ config/config_test.go | 251 +++++++++++++------------------ config/notifiers.go | 46 +++--- config/notifiers_test.go | 47 +++--- notify/discord/config.go | 6 +- notify/incidentio/config.go | 6 +- notify/jira/config.go | 6 +- notify/mattermost/config.go | 10 +- notify/mattermost/config_test.go | 15 +- notify/msteams/config.go | 6 +- notify/msteamsv2/config.go | 6 +- notify/opsgenie/config.go | 6 +- notify/opsgenie/config_test.go | 3 - notify/pagerduty/config.go | 6 +- notify/pagerduty/config_test.go | 20 +-- notify/pushover/config.go | 6 +- notify/pushover/config_test.go | 25 +-- notify/rocketchat/config.go | 6 +- notify/sns/config.go | 6 +- notify/sns/config_test.go | 3 - notify/telegram/config.go | 6 +- notify/telegram/config_test.go | 9 +- notify/webhook/config.go | 6 +- notify/webhook/config_test.go | 12 +- 25 files changed, 277 insertions(+), 473 deletions(-) diff --git a/config/common/notifierconfig.go b/config/common/notifierconfig.go index 6689e6d9e2..ade56dedef 100644 --- a/config/common/notifierconfig.go +++ b/config/common/notifierconfig.go @@ -13,6 +13,13 @@ package common +import ( + "errors" + "strings" + + "gopkg.in/yaml.v2" +) + // NotifierConfig contains base options common across all notifier configurations. type NotifierConfig struct { VSendResolved bool `yaml:"send_resolved" json:"send_resolved"` @@ -28,3 +35,43 @@ func (nc *NotifierConfig) SendResolved() bool { type Validator interface { Validate() error } + +// AsValidationError returns err as a *yaml.TypeError so that the YAML decoder +// records it and continues decoding instead of aborting at the first invalid +// notifier config. The decoder collects every such error across the document, +// which is what allows a configuration to report all of its invalid notifier +// configs at once. Errors that already are a *yaml.TypeError, including the +// decoder's own type and unknown field errors, are returned unchanged, and so +// is nil. +func AsValidationError(err error) error { + if err == nil { + return nil + } + if te, ok := errors.AsType[*yaml.TypeError](err); ok { + return te + } + return &yaml.TypeError{Errors: []string{err.Error()}} +} + +// FlattenValidationErrors turns the errors collected through AsValidationError +// back into plain errors at the top level of the configuration, so that a single +// validation error reads exactly as it did before and several read one per +// line. Errors reported by the decoder itself, which carry a "line N:" prefix, +// keep their usual *yaml.TypeError form. +func FlattenValidationErrors(err error) error { + te, ok := errors.AsType[*yaml.TypeError](err) + if !ok { + return err + } + errs := make([]error, 0, len(te.Errors)) + for _, msg := range te.Errors { + if strings.HasPrefix(msg, "line ") { + return te + } + errs = append(errs, errors.New(msg)) + } + if len(errs) == 1 { + return errs[0] + } + return errors.Join(errs...) +} diff --git a/config/config.go b/config/config.go index 01ce517c59..ca112e88e4 100644 --- a/config/config.go +++ b/config/config.go @@ -317,7 +317,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { // again, we have to hide it using a type indirection. type plain Config if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.FlattenValidationErrors(err) } // If a global block was open but empty the default global config is overwritten. @@ -382,46 +382,28 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { names := map[string]struct{}{} - // Validation errors are accumulated across all receivers and notifier - // configs so that a single invalid entry does not hide the others. - var errs error - for _, rcv := range c.Receivers { if _, ok := names[rcv.Name]; ok { - // Record the duplicate name but keep validating this receiver's - // notifier configurations so their errors are reported as well. - errs = errors.Join(errs, fmt.Errorf("notification config name %q is not unique", rcv.Name)) + return fmt.Errorf("notification config name %q is not unique", rcv.Name) } for _, wh := range rcv.WebhookConfigs { if wh == nil { - errs = errors.Join(errs, errors.New("missing webhook config")) - continue - } - if err := wh.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing webhook config") } wh.HTTPConfig = cmp.Or(wh.HTTPConfig, c.Global.HTTPConfig) } for _, ec := range rcv.EmailConfigs { if ec == nil { - errs = errors.Join(errs, errors.New("missing email config")) - continue - } - if err := ec.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing email config") } ec.TLSConfig = cmp.Or(ec.TLSConfig, c.Global.SMTPTLSConfig) ec.Smarthost = cmp.Or(ec.Smarthost, c.Global.SMTPSmarthost) if ec.Smarthost.String() == "" { - errs = errors.Join(errs, errors.New("no global SMTP smarthost set")) - continue + return errors.New("no global SMTP smarthost set") } ec.From = cmp.Or(ec.From, c.Global.SMTPFrom) if ec.From == "" { - errs = errors.Join(errs, errors.New("no global SMTP from set")) - continue + return errors.New("no global SMTP from set") } ec.Hello = cmp.Or(ec.Hello, c.Global.SMTPHello) ec.AuthUsername = cmp.Or(ec.AuthUsername, c.Global.SMTPAuthUsername) @@ -445,14 +427,10 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { for _, sc := range rcv.SlackConfigs { if sc == nil { sc = &SlackConfig{} - } else if err := sc.Validate(); err != nil { - errs = errors.Join(errs, err) - continue } sc.AppURL = cmp.Or(sc.AppURL, c.Global.SlackAppURL) if sc.AppURL == nil { - errs = errors.Join(errs, errors.New("no global Slack App URL set")) - continue + return errors.New("no global Slack App URL set") } // we only want to set the app token from global if there's no local authorization or webhook url if sc.AppToken == "" && len(sc.AppTokenFile) == 0 && (sc.HTTPConfig == nil || sc.HTTPConfig.Authorization == nil) && sc.APIURL == nil { @@ -464,8 +442,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { sc.APIURLFile = c.Global.SlackAPIURLFile } if sc.APIURL == nil && len(sc.APIURLFile) == 0 && sc.AppToken == "" && len(sc.AppTokenFile) == 0 { - errs = errors.Join(errs, errors.New("no Slack API URL nor App token set either inline or in a file")) - continue + return errors.New("no Slack API URL nor App token set either inline or in a file") } if sc.HTTPConfig == nil { // we don't want to change the global http config when setting the receiver's http config, do we do a copy @@ -474,8 +451,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } if sc.AppToken != "" || len(sc.AppTokenFile) != 0 { if sc.HTTPConfig.Authorization != nil { - errs = errors.Join(errs, errors.New("http authorization can't be set when using Slack App tokens")) - continue + return errors.New("http authorization can't be set when using Slack App tokens") } sc.HTTPConfig.Authorization = &commoncfg.Authorization{ Type: "Bearer", @@ -487,62 +463,41 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, poc := range rcv.PushoverConfigs { if poc == nil { - errs = errors.Join(errs, errors.New("missing pushover config")) - continue - } - if err := poc.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing pushover config") } poc.HTTPConfig = cmp.Or(poc.HTTPConfig, c.Global.HTTPConfig) } for _, pdc := range rcv.PagerdutyConfigs { if pdc == nil { - errs = errors.Join(errs, errors.New("missing pagerduty config")) - continue - } - if err := pdc.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing pagerduty config") } pdc.HTTPConfig = cmp.Or(pdc.HTTPConfig, c.Global.HTTPConfig) pdc.URL = cmp.Or(pdc.URL, c.Global.PagerdutyURL) if pdc.URL == nil { - errs = errors.Join(errs, errors.New("no global PagerDuty URL set")) - continue + return errors.New("no global PagerDuty URL set") } } for _, iio := range rcv.IncidentioConfigs { if iio == nil { - errs = errors.Join(errs, errors.New("missing incidentio config")) - continue - } - if err := iio.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing incidentio config") } iio.HTTPConfig = cmp.Or(iio.HTTPConfig, c.Global.HTTPConfig) } for _, ogc := range rcv.OpsGenieConfigs { if ogc == nil { ogc = &opsgenie.OpsGenieConfig{} - } else if err := ogc.Validate(); err != nil { - errs = errors.Join(errs, err) - continue } ogc.HTTPConfig = cmp.Or(ogc.HTTPConfig, c.Global.HTTPConfig) ogc.APIURL = cmp.Or(ogc.APIURL, c.Global.OpsGenieAPIURL) if ogc.APIURL == nil { - errs = errors.Join(errs, errors.New("no global OpsGenie URL set")) - continue + return errors.New("no global OpsGenie URL set") } if !strings.HasSuffix(ogc.APIURL.Path, "/") { ogc.APIURL.Path += "/" } if ogc.APIKey == "" && len(ogc.APIKeyFile) == 0 { if c.Global.OpsGenieAPIKey == "" && len(c.Global.OpsGenieAPIKeyFile) == 0 { - errs = errors.Join(errs, errors.New("no global OpsGenie API Key set either inline or in a file")) - continue + return errors.New("no global OpsGenie API Key set either inline or in a file") } ogc.APIKey = c.Global.OpsGenieAPIKey ogc.APIKeyFile = c.Global.OpsGenieAPIKeyFile @@ -551,21 +506,16 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { for _, wcc := range rcv.WechatConfigs { if wcc == nil { wcc = &WechatConfig{} - } else if err := wcc.Validate(); err != nil { - errs = errors.Join(errs, err) - continue } wcc.HTTPConfig = cmp.Or(wcc.HTTPConfig, c.Global.HTTPConfig) wcc.APIURL = cmp.Or(wcc.APIURL, c.Global.WeChatAPIURL) if wcc.APIURL == nil { - errs = errors.Join(errs, errors.New("no global Wechat URL set")) - continue + return errors.New("no global Wechat URL set") } if wcc.APISecret == "" && len(wcc.APISecretFile) == 0 { if c.Global.WeChatAPISecret == "" && len(c.Global.WeChatAPISecretFile) == 0 { - errs = errors.Join(errs, errors.New("no global Wechat Api Secret set either inline or in a file")) - continue + return errors.New("no global Wechat Api Secret set either inline or in a file") } wcc.APISecret = c.Global.WeChatAPISecret wcc.APISecretFile = c.Global.WeChatAPISecretFile @@ -573,8 +523,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { wcc.CorpID = cmp.Or(wcc.CorpID, c.Global.WeChatAPICorpID) if wcc.CorpID == "" { - errs = errors.Join(errs, errors.New("no global Wechat CorpID set")) - continue + return errors.New("no global Wechat CorpID set") } if !strings.HasSuffix(wcc.APIURL.Path, "/") { @@ -583,26 +532,19 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, voc := range rcv.VictorOpsConfigs { if voc == nil { - errs = errors.Join(errs, errors.New("missing victorops config")) - continue - } - if err := voc.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing victorops config") } voc.HTTPConfig = cmp.Or(voc.HTTPConfig, c.Global.HTTPConfig) voc.APIURL = cmp.Or(voc.APIURL, c.Global.VictorOpsAPIURL) if voc.APIURL == nil { - errs = errors.Join(errs, errors.New("no global VictorOps URL set")) - continue + return errors.New("no global VictorOps URL set") } if !strings.HasSuffix(voc.APIURL.Path, "/") { voc.APIURL.Path += "/" } if voc.APIKey == "" && len(voc.APIKeyFile) == 0 { if c.Global.VictorOpsAPIKey == "" && len(c.Global.VictorOpsAPIKeyFile) == 0 { - errs = errors.Join(errs, errors.New("no global VictorOps API Key set")) - continue + return errors.New("no global VictorOps API Key set") } voc.APIKey = c.Global.VictorOpsAPIKey voc.APIKeyFile = c.Global.VictorOpsAPIKeyFile @@ -610,31 +552,20 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, sns := range rcv.SNSConfigs { if sns == nil { - errs = errors.Join(errs, errors.New("missing sns config")) - continue - } - if err := sns.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing sns config") } sns.HTTPConfig = cmp.Or(sns.HTTPConfig, c.Global.HTTPConfig) } for _, telegram := range rcv.TelegramConfigs { if telegram == nil { - errs = errors.Join(errs, errors.New("missing telegram config")) - continue - } - if err := telegram.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing telegram config") } telegram.HTTPConfig = cmp.Or(telegram.HTTPConfig, c.Global.HTTPConfig) telegram.APIUrl = cmp.Or(telegram.APIUrl, c.Global.TelegramAPIUrl) if telegram.BotToken == "" && len(telegram.BotTokenFile) == 0 { if c.Global.TelegramBotToken == "" && len(c.Global.TelegramBotTokenFile) == 0 { - errs = errors.Join(errs, errors.New("missing bot_token or bot_token_file on telegram_config")) - continue + return errors.New("missing bot_token or bot_token_file on telegram_config") } telegram.BotToken = c.Global.TelegramBotToken telegram.BotTokenFile = c.Global.TelegramBotTokenFile @@ -642,58 +573,35 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, discord := range rcv.DiscordConfigs { if discord == nil { - errs = errors.Join(errs, errors.New("missing discord config")) - continue - } - if err := discord.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing discord config") } discord.HTTPConfig = cmp.Or(discord.HTTPConfig, c.Global.HTTPConfig) if discord.WebhookURL == nil && len(discord.WebhookURLFile) == 0 { - errs = errors.Join(errs, errors.New("no discord webhook URL or URLFile provided")) - continue + return errors.New("no discord webhook URL or URLFile provided") } } for _, webex := range rcv.WebexConfigs { if webex == nil { - errs = errors.Join(errs, errors.New("missing webex config")) - continue - } - if err := webex.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing webex config") } webex.HTTPConfig = cmp.Or(webex.HTTPConfig, c.Global.HTTPConfig) webex.APIURL = cmp.Or(webex.APIURL, c.Global.WebexAPIURL) if webex.APIURL == nil { - errs = errors.Join(errs, errors.New("no global Webex URL set")) - continue + return errors.New("no global Webex URL set") } } for _, msteams := range rcv.MSTeamsConfigs { if msteams == nil { - errs = errors.Join(errs, errors.New("missing msteams config")) - continue - } - if err := msteams.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing msteams config") } msteams.HTTPConfig = cmp.Or(msteams.HTTPConfig, c.Global.HTTPConfig) if msteams.WebhookURL == nil && len(msteams.WebhookURLFile) == 0 { - errs = errors.Join(errs, errors.New("no msteams webhook URL or URLFile provided")) - continue + return errors.New("no msteams webhook URL or URLFile provided") } } for _, msteamsv2 := range rcv.MSTeamsV2Configs { if msteamsv2 == nil { - errs = errors.Join(errs, errors.New("missing msteamsv2 config")) - continue - } - if err := msteamsv2.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing msteamsv2 config") } if msteamsv2.HTTPConfig == nil { // copy the global config so receiver-level mutations don't affect it @@ -706,47 +614,35 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { msteamsv2.HTTPConfig.ProxyURL = c.Global.HTTPConfig.ProxyURL } if msteamsv2.WebhookURL == nil && len(msteamsv2.WebhookURLFile) == 0 { - errs = errors.Join(errs, errors.New("no msteamsv2 webhook URL or URLFile provided")) - continue + return errors.New("no msteamsv2 webhook URL or URLFile provided") } } for _, jira := range rcv.JiraConfigs { if jira == nil { - errs = errors.Join(errs, errors.New("missing jira config")) - continue - } - if err := jira.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing jira config") } jira.HTTPConfig = cmp.Or(jira.HTTPConfig, c.Global.HTTPConfig) jira.APIURL = cmp.Or(jira.APIURL, c.Global.JiraAPIURL) if jira.APIURL == nil { - errs = errors.Join(errs, errors.New("no global Jira Cloud URL set")) - continue + return errors.New("no global Jira Cloud URL set") } } for _, rocketchatcfg := range rcv.RocketchatConfigs { if rocketchatcfg == nil { rocketchatcfg = &rocketchat.RocketchatConfig{} - } else if err := rocketchatcfg.Validate(); err != nil { - errs = errors.Join(errs, err) - continue } rocketchatcfg.HTTPConfig = cmp.Or(rocketchatcfg.HTTPConfig, c.Global.HTTPConfig) rocketchatcfg.APIURL = cmp.Or(rocketchatcfg.APIURL, c.Global.RocketchatAPIURL) if rocketchatcfg.TokenID == nil && len(rocketchatcfg.TokenIDFile) == 0 { if c.Global.RocketchatTokenID == nil && len(c.Global.RocketchatTokenIDFile) == 0 { - errs = errors.Join(errs, errors.New("no global Rocketchat TokenID set either inline or in a file")) - continue + return errors.New("no global Rocketchat TokenID set either inline or in a file") } rocketchatcfg.TokenID = c.Global.RocketchatTokenID rocketchatcfg.TokenIDFile = c.Global.RocketchatTokenIDFile } if rocketchatcfg.Token == nil && len(rocketchatcfg.TokenFile) == 0 { if c.Global.RocketchatToken == nil && len(c.Global.RocketchatTokenFile) == 0 { - errs = errors.Join(errs, errors.New("no global Rocketchat Token set either inline or in a file")) - continue + return errors.New("no global Rocketchat Token set either inline or in a file") } rocketchatcfg.Token = c.Global.RocketchatToken rocketchatcfg.TokenFile = c.Global.RocketchatTokenFile @@ -754,18 +650,12 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { } for _, mattermost := range rcv.MattermostConfigs { if mattermost == nil { - errs = errors.Join(errs, errors.New("missing mattermost config")) - continue - } - if err := mattermost.Validate(); err != nil { - errs = errors.Join(errs, err) - continue + return errors.New("missing mattermost config") } mattermost.HTTPConfig = cmp.Or(mattermost.HTTPConfig, c.Global.HTTPConfig) if mattermost.WebhookURL == nil && len(mattermost.WebhookURLFile) == 0 { if c.Global.MattermostWebhookURL == nil && len(c.Global.MattermostWebhookURLFile) == 0 { - errs = errors.Join(errs, errors.New("missing webhook_url or webhook_url_file on mattermost_config")) - continue + return errors.New("missing webhook_url or webhook_url_file on mattermost_config") } mattermost.WebhookURL = c.Global.MattermostWebhookURL mattermost.WebhookURLFile = c.Global.MattermostWebhookURLFile @@ -775,10 +665,6 @@ func (c *Config) UnmarshalYAML(unmarshal func(any) error) error { names[rcv.Name] = struct{}{} } - if errs != nil { - return errs - } - // The root route must not have any matchers as it is the fallback node // for all alerts. if c.Route == nil { diff --git a/config/config_test.go b/config/config_test.go index 6af7eedf2d..bb90ff1fb4 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -109,151 +109,6 @@ receivers: } } -func TestReceiverValidationErrorsAccumulate(t *testing.T) { - tests := []struct { - name string - in string - - expectedErrs []string - }{ - { - // https://github.com/prometheus/alertmanager/issues/4990 - name: "multiple invalid notifier configs in a single receiver", - in: ` -route: - receiver: team-X - -receivers: -- name: 'team-X' - webhook_configs: - - send_resolved: true - - url: 'http://example.com/' - url_file: '/etc/secrets/webhook-url' -`, - expectedErrs: []string{ - "one of url or url_file must be configured", - "at most one of url & url_file must be configured", - }, - }, - { - // A duplicate receiver name must not hide the validation errors - // of the duplicate's own notifier configurations. - name: "duplicate receiver name with an invalid notifier config", - in: ` -route: - receiver: team-X - -receivers: -- name: 'team-X' - webhook_configs: - - url: 'http://example.com/' -- name: 'team-X' - webhook_configs: - - send_resolved: true -`, - expectedErrs: []string{ - `notification config name "team-X" is not unique`, - "one of url or url_file must be configured", - }, - }, - { - name: "invalid notifier configs of different types in a single receiver", - in: ` -route: - receiver: team-X - -receivers: -- name: 'team-X' - webhook_configs: - - send_resolved: true - pagerduty_configs: - - url: 'https://example.com/' -`, - expectedErrs: []string{ - "one of url or url_file must be configured", - "missing service or routing key in PagerDuty config", - }, - }, - { - // https://github.com/prometheus/alertmanager/issues/4991 - name: "invalid notifier configs across multiple receivers", - in: ` -route: - receiver: team-A - -receivers: -- name: 'team-A' - webhook_configs: - - send_resolved: true -- name: 'team-B' - email_configs: - - smarthost: 'smtp.example.com:587' - from: 'alertmanager@example.com' -- name: 'team-C' - pagerduty_configs: - - url: 'https://example.com/' -`, - expectedErrs: []string{ - "one of url or url_file must be configured", - "missing to address in email config", - "missing service or routing key in PagerDuty config", - }, - }, - { - name: "missing global fallbacks are accumulated across receivers", - in: ` -route: - receiver: team-A - -receivers: -- name: 'team-A' - email_configs: - - to: 'team-A@example.com' -- name: 'team-B' - webhook_configs: - - url: 'http://example.com/' - url_file: '/etc/secrets/webhook-url' -`, - expectedErrs: []string{ - "no global SMTP smarthost set", - "at most one of url & url_file must be configured", - }, - }, - { - name: "duplicate receiver name is accumulated with notifier config errors", - in: ` -route: - receiver: team-X - -receivers: -- name: 'team-X' - webhook_configs: - - send_resolved: true -- name: 'team-X' -`, - expectedErrs: []string{ - "one of url or url_file must be configured", - "notification config name \"team-X\" is not unique", - }, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - _, err := Load(tc.in) - - expected := strings.Join(tc.expectedErrs, "\n") - - if err == nil { - t.Fatalf("no error returned, expected:\n%v", expected) - } - if err.Error() != expected { - t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) - } - }) - } -} - func TestReceiverExists(t *testing.T) { in := ` route: @@ -1727,10 +1582,8 @@ func TestRocketchatNoToken(t *testing.T) { if err == nil { t.Fatalf("Expected an error parsing %s: %s", "testdata/conf.rocketchat-no-token.yml", err) } - // Both receivers in the fixture lack a token, so the error is reported once per receiver. - expected := "no global Rocketchat Token set either inline or in a file\nno global Rocketchat Token set either inline or in a file" - if err.Error() != expected { - t.Errorf("Expected: %s\nGot: %s", expected, err.Error()) + if err.Error() != "no global Rocketchat Token set either inline or in a file" { + t.Errorf("Expected: %s\nGot: %s", "no global Rocketchat Token set either inline or in a file", err.Error()) } } @@ -2164,3 +2017,103 @@ receivers: t.Errorf("expected local proxy_url %q, got %q", "http://local-proxy.example.com:8080", got) } } + +func TestReceiverValidationErrorsAccumulate(t *testing.T) { + tests := []struct { + name string + in string + expected string + }{ + { + // https://github.com/prometheus/alertmanager/issues/4990 + name: "multiple invalid notifier configs in a single receiver", + in: ` +route: + receiver: team-X + +receivers: +- name: 'team-X' + webhook_configs: + - send_resolved: true + - url: 'http://example.com/' + url_file: '/etc/secrets/webhook-url' +`, + expected: "one of url or url_file must be configured\n" + + "at most one of url & url_file must be configured", + }, + { + // The example from https://github.com/prometheus/alertmanager/issues/4990: + // the decoder's own type error and the validation error are reported + // together instead of the latter hiding the former. + name: "type errors and validation errors are reported together", + in: ` +receivers: + - name: 'webhook-critical' + webhook_configs: + - url: 'http://example.com/api' + send_resolved: "yes_please" + - url: '' + http_config: + basic_auth: + username: 'admin' +route: + group_by: ['alertname'] + receiver: 'webhook-critical' +`, + expected: "yaml: unmarshal errors:\n" + + " line 6: cannot unmarshal !!str `yes_please` into bool\n" + + " one of url or url_file must be configured", + }, + { + name: "invalid notifier configs of different types in a single receiver", + in: ` +route: + receiver: team-X + +receivers: +- name: 'team-X' + webhook_configs: + - send_resolved: true + pagerduty_configs: + - url: 'https://example.com/' +`, + expected: "one of url or url_file must be configured\n" + + "missing service or routing key in PagerDuty config", + }, + { + // https://github.com/prometheus/alertmanager/issues/4991 + name: "invalid notifier configs across multiple receivers", + in: ` +route: + receiver: team-A + +receivers: +- name: 'team-A' + webhook_configs: + - send_resolved: true +- name: 'team-B' + email_configs: + - smarthost: 'smtp.example.com:587' + from: 'alertmanager@example.com' +- name: 'team-C' + pagerduty_configs: + - url: 'https://example.com/' +`, + expected: "one of url or url_file must be configured\n" + + "missing to address in email config\n" + + "missing service or routing key in PagerDuty config", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := Load(tc.in) + if err == nil { + t.Fatalf("no error returned, expected:\n%v", tc.expected) + } + if err.Error() != tc.expected { + t.Errorf("\nexpected:\n%v\ngot:\n%v", tc.expected, err.Error()) + } + }) + } +} diff --git a/config/notifiers.go b/config/notifiers.go index f19ab51208..ec04cf245c 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -105,11 +105,9 @@ func (c *WebexConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultWebexConfig type plain WebexConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } func (c *WebexConfig) Validate() error { @@ -163,7 +161,7 @@ func (c *EmailConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultEmailConfig type plain EmailConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } // Header names are case insensitive. The normalization loop below // detects duplicates and builds a canonical header map in one pass. @@ -173,15 +171,13 @@ func (c *EmailConfig) UnmarshalYAML(unmarshal func(any) error) error { for h, v := range c.Headers { normalized := textproto.CanonicalMIMEHeaderKey(h) if _, ok := normalizedHeaders[normalized]; ok { - return fmt.Errorf("duplicate header %q in email config", normalized) + return amcommoncfg.AsValidationError(fmt.Errorf("duplicate header %q in email config", normalized)) } normalizedHeaders[normalized] = v } c.Headers = normalizedHeaders - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } func (c *EmailConfig) Validate() error { @@ -221,7 +217,7 @@ type SlackAction struct { func (c *SlackAction) UnmarshalYAML(unmarshal func(any) error) error { type plain SlackAction if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } if c.URL != "" { // Clear all message action fields. @@ -231,9 +227,9 @@ func (c *SlackAction) UnmarshalYAML(unmarshal func(any) error) error { } else if c.Name != "" { c.URL = "" } else { - return errors.New("missing name or url in Slack action configuration") + return amcommoncfg.AsValidationError(errors.New("missing name or url in Slack action configuration")) } - return c.Validate() + return amcommoncfg.AsValidationError(c.Validate()) } func (c *SlackAction) Validate() error { @@ -260,9 +256,9 @@ type SlackConfirmationField struct { func (c *SlackConfirmationField) UnmarshalYAML(unmarshal func(any) error) error { type plain SlackConfirmationField if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - return c.Validate() + return amcommoncfg.AsValidationError(c.Validate()) } func (c *SlackConfirmationField) Validate() error { @@ -286,9 +282,9 @@ type SlackField struct { func (c *SlackField) UnmarshalYAML(unmarshal func(any) error) error { type plain SlackField if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - return c.Validate() + return amcommoncfg.AsValidationError(c.Validate()) } func (c *SlackField) Validate() error { @@ -350,11 +346,9 @@ func (c *SlackConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultSlackConfig type plain SlackConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } func (c *SlackConfig) Validate() error { @@ -402,16 +396,14 @@ func (c *WechatConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultWechatConfig type plain WechatConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } if c.MessageType == "" { c.MessageType = "text" } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } func (c *WechatConfig) Validate() error { @@ -448,11 +440,9 @@ func (c *VictorOpsConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultVictorOpsConfig type plain VictorOpsConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } func (c *VictorOpsConfig) Validate() error { diff --git a/config/notifiers_test.go b/config/notifiers_test.go index 7a931b5c34..86be7a7421 100644 --- a/config/notifiers_test.go +++ b/config/notifiers_test.go @@ -29,16 +29,13 @@ to: '' ` var cfg EmailConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "missing to address in email config" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -58,7 +55,7 @@ headers: if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -123,16 +120,13 @@ routing_key: '' ` var cfg VictorOpsConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "missing Routing key in VictorOps config" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } }) @@ -145,16 +139,13 @@ api_key_file: /global_file ` var cfg VictorOpsConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "at most one of api_key & api_key_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } }) @@ -168,16 +159,13 @@ custom_fields: ` var cfg VictorOpsConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "victorOps config contains custom field entity_state which cannot be used as it conflicts with the fixed/static fields" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } @@ -306,9 +294,6 @@ api_url: 'https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXX for _, rt := range tests { var cfg SlackConfig err := yaml.UnmarshalStrict([]byte(rt.in), &cfg) - if err == nil { - err = cfg.Validate() - } // Check if an error occurred when it was NOT expected to. if rt.expectedErr == "" && err != nil { @@ -319,7 +304,7 @@ api_url: 'https://hooks.slack.com/services/T00000000/B00000000/XXXXXXXXXXXXXXXXX t.Fatalf("\nno error returned, expected:\n%v", rt.expectedErr) } // Check that the error that occurred was what was expected. - if err != nil && err.Error() != rt.expectedErr { + if err != nil && err.Error() != "yaml: unmarshal errors:\n "+rt.expectedErr { t.Errorf("\nexpected:\n%v\ngot:\n%v", rt.expectedErr, err.Error()) } } @@ -376,7 +361,7 @@ fields: t.Fatalf("\nno error returned, expected:\n%v", rt.expected) } // Check that the error that occurred was what was expected. - if err != nil && err.Error() != rt.expected { + if err != nil && err.Error() != "yaml: unmarshal errors:\n "+rt.expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", rt.expected, err.Error()) } } @@ -565,11 +550,12 @@ http_config: t.Run(tt.name, func(t *testing.T) { var cfg WebexConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) - if err == nil { - err = cfg.Validate() - } - require.Equal(t, tt.expected, err) + if tt.expected != nil { + require.EqualError(t, err, "yaml: unmarshal errors:\n "+tt.expected.Error()) + } else { + require.NoError(t, err) + } }) } } @@ -626,11 +612,12 @@ headers: {X-Custom-Header: CustomValue, X-CUSTOM-HEADER: AnotherValue} t.Run(tt.name, func(t *testing.T) { var cfg EmailConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) - if err == nil { - err = cfg.Validate() - } - require.Equal(t, tt.expected, err) + if tt.expected != nil { + require.EqualError(t, err, "yaml: unmarshal errors:\n "+tt.expected.Error()) + } else { + require.NoError(t, err) + } }) } } diff --git a/notify/discord/config.go b/notify/discord/config.go index b872fe1d5e..39237497a7 100644 --- a/notify/discord/config.go +++ b/notify/discord/config.go @@ -50,12 +50,10 @@ func (c *DiscordConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = defaultDiscordConfig type plain DiscordConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the DiscordConfig for correctness. diff --git a/notify/incidentio/config.go b/notify/incidentio/config.go index c6eef1fbe1..a6ad01142c 100644 --- a/notify/incidentio/config.go +++ b/notify/incidentio/config.go @@ -64,11 +64,9 @@ func (c *IncidentioConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = defaultIncidentioConfig type plain IncidentioConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the IncidentioConfig for correctness. diff --git a/notify/jira/config.go b/notify/jira/config.go index c18a7f2d38..07e235a29e 100644 --- a/notify/jira/config.go +++ b/notify/jira/config.go @@ -91,12 +91,10 @@ func (c *JiraConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultJiraConfig type plain JiraConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the JiraConfig for correctness. diff --git a/notify/mattermost/config.go b/notify/mattermost/config.go index 1c5b999e77..ac66396a76 100644 --- a/notify/mattermost/config.go +++ b/notify/mattermost/config.go @@ -58,9 +58,9 @@ type MattermostField struct { func (c *MattermostField) UnmarshalYAML(unmarshal func(any) error) error { type plain MattermostField if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - return c.Validate() + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the MattermostField for correctness. @@ -132,12 +132,10 @@ func (c *MattermostConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultMattermostConfig type plain MattermostConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the MattermostConfig for correctness. diff --git a/notify/mattermost/config_test.go b/notify/mattermost/config_test.go index bc05387e9a..1aac96f694 100644 --- a/notify/mattermost/config_test.go +++ b/notify/mattermost/config_test.go @@ -63,7 +63,11 @@ value: some value var cfg MattermostField err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) - require.Equal(t, tt.expected, err) + if tt.expected != nil { + require.EqualError(t, err, "yaml: unmarshal errors:\n "+tt.expected.Error()) + } else { + require.NoError(t, err) + } }) } } @@ -124,11 +128,12 @@ attachments: t.Run(tt.name, func(t *testing.T) { var cfg MattermostConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) - if err == nil { - err = cfg.Validate() - } - require.Equal(t, tt.expected, err) + if tt.expected != nil { + require.EqualError(t, err, "yaml: unmarshal errors:\n "+tt.expected.Error()) + } else { + require.NoError(t, err) + } }) } } diff --git a/notify/msteams/config.go b/notify/msteams/config.go index 85c5b0bddf..69d83216bb 100644 --- a/notify/msteams/config.go +++ b/notify/msteams/config.go @@ -45,12 +45,10 @@ func (c *MSTeamsConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultMSTeamsConfig type plain MSTeamsConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the MSTeamsConfig for correctness. diff --git a/notify/msteamsv2/config.go b/notify/msteamsv2/config.go index 182c503440..6af473bf94 100644 --- a/notify/msteamsv2/config.go +++ b/notify/msteamsv2/config.go @@ -43,12 +43,10 @@ func (c *MSTeamsV2Config) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultMSTeamsV2Config type plain MSTeamsV2Config if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the MSTeamsV2Config for correctness. diff --git a/notify/opsgenie/config.go b/notify/opsgenie/config.go index 70521b4af1..5ac2d05a0a 100644 --- a/notify/opsgenie/config.go +++ b/notify/opsgenie/config.go @@ -66,11 +66,9 @@ func (c *OpsGenieConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultOpsGenieConfig type plain OpsGenieConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the OpsGenieConfig for correctness. diff --git a/notify/opsgenie/config_test.go b/notify/opsgenie/config_test.go index 201435c5a7..87e0868766 100644 --- a/notify/opsgenie/config_test.go +++ b/notify/opsgenie/config_test.go @@ -90,9 +90,6 @@ api_url: http://example.com var cfg OpsGenieConfig err := yaml.UnmarshalStrict([]byte(tc.in), &cfg) - if err == nil { - err = cfg.Validate() - } if tc.err { if err == nil { t.Fatalf("expected error but got none") diff --git a/notify/pagerduty/config.go b/notify/pagerduty/config.go index 8cd2064ed8..e7622cd46d 100644 --- a/notify/pagerduty/config.go +++ b/notify/pagerduty/config.go @@ -87,7 +87,7 @@ func (c *PagerdutyConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultPagerdutyConfig type plain PagerdutyConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } if c.Details == nil { c.Details = make(map[string]any) @@ -100,9 +100,7 @@ func (c *PagerdutyConfig) UnmarshalYAML(unmarshal func(any) error) error { c.Details[k] = v } } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the PagerdutyConfig for correctness. diff --git a/notify/pagerduty/config_test.go b/notify/pagerduty/config_test.go index 9b2d2a0b24..696e0e6467 100644 --- a/notify/pagerduty/config_test.go +++ b/notify/pagerduty/config_test.go @@ -27,16 +27,13 @@ routing_key: '' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "missing service or routing key in PagerDuty config" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } }) @@ -48,16 +45,13 @@ routing_key_file: 'xyz' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "at most one of routing_key & routing_key_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } }) @@ -70,16 +64,13 @@ service_key: '' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "missing service or routing key in PagerDuty config" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } }) @@ -91,16 +82,13 @@ service_key_file: 'xyz' ` var cfg PagerdutyConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "at most one of service_key & service_key_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } }) diff --git a/notify/pushover/config.go b/notify/pushover/config.go index 41d35a5d82..f4f31fde2d 100644 --- a/notify/pushover/config.go +++ b/notify/pushover/config.go @@ -78,11 +78,9 @@ func (c *PushoverConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultPushoverConfig type plain PushoverConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the PushoverConfig for correctness. diff --git a/notify/pushover/config_test.go b/notify/pushover/config_test.go index 230bb6d052..d38e8053af 100644 --- a/notify/pushover/config_test.go +++ b/notify/pushover/config_test.go @@ -25,16 +25,13 @@ user_key: '' ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "one of user_key or user_key_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -46,16 +43,13 @@ user_key_file: /pushover/user_key ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "at most one of user_key & user_key_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -67,16 +61,13 @@ token: '' ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "one of token or token_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -89,16 +80,13 @@ user_key: 'user key' ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "at most one of token & token_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -112,16 +100,13 @@ monospace: true ` var cfg PushoverConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "at most one of monospace & html must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } diff --git a/notify/rocketchat/config.go b/notify/rocketchat/config.go index e22184abaf..a178ef64fa 100644 --- a/notify/rocketchat/config.go +++ b/notify/rocketchat/config.go @@ -91,11 +91,9 @@ func (c *RocketchatConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultRocketchatConfig type plain RocketchatConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the RocketchatConfig for correctness. diff --git a/notify/sns/config.go b/notify/sns/config.go index 202d0cfe95..4924439c36 100644 --- a/notify/sns/config.go +++ b/notify/sns/config.go @@ -56,11 +56,9 @@ func (c *SNSConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultSNSConfig type plain SNSConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the SNSConfig for correctness. diff --git a/notify/sns/config_test.go b/notify/sns/config_test.go index 924362a129..27e70fa43c 100644 --- a/notify/sns/config_test.go +++ b/notify/sns/config_test.go @@ -90,9 +90,6 @@ sigv4: t.Run("", func(t *testing.T) { var cfg SNSConfig err := yaml.UnmarshalStrict([]byte(tc.in), &cfg) - if err == nil { - err = cfg.Validate() - } if err != nil { if !tc.err { t.Errorf("expecting no error, got %q", err) diff --git a/notify/telegram/config.go b/notify/telegram/config.go index 0d94365a29..2f027a3055 100644 --- a/notify/telegram/config.go +++ b/notify/telegram/config.go @@ -57,11 +57,9 @@ func (c *TelegramConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = DefaultTelegramConfig type plain TelegramConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the TelegramConfig for correctness. diff --git a/notify/telegram/config_test.go b/notify/telegram/config_test.go index 75cd061043..7daef24fde 100644 --- a/notify/telegram/config_test.go +++ b/notify/telegram/config_test.go @@ -95,11 +95,12 @@ parse_mode: invalid t.Run(tt.name, func(t *testing.T) { var cfg TelegramConfig err := yaml.UnmarshalStrict([]byte(tt.in), &cfg) - if err == nil { - err = cfg.Validate() - } - require.Equal(t, tt.expected, err) + if tt.expected != nil { + require.EqualError(t, err, "yaml: unmarshal errors:\n "+tt.expected.Error()) + } else { + require.NoError(t, err) + } }) } } diff --git a/notify/webhook/config.go b/notify/webhook/config.go index 7c3d277e2d..2fea05914c 100644 --- a/notify/webhook/config.go +++ b/notify/webhook/config.go @@ -55,11 +55,9 @@ func (c *WebhookConfig) UnmarshalYAML(unmarshal func(any) error) error { *c = defaultWebhookConfig type plain WebhookConfig if err := unmarshal((*plain)(c)); err != nil { - return err + return amcommoncfg.AsValidationError(err) } - // Validation happens in Config.UnmarshalYAML so that errors from all - // notifier configs can be reported together instead of one at a time. - return nil + return amcommoncfg.AsValidationError(c.Validate()) } // Validate checks the WebhookConfig for correctness. diff --git a/notify/webhook/config_test.go b/notify/webhook/config_test.go index 7b4bee2d93..ea435e7442 100644 --- a/notify/webhook/config_test.go +++ b/notify/webhook/config_test.go @@ -24,16 +24,13 @@ func TestWebhookURLIsPresent(t *testing.T) { in := `{}` var cfg WebhookConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "one of url or url_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -45,16 +42,13 @@ url_file: 'http://example.com' ` var cfg WebhookConfig err := yaml.UnmarshalStrict([]byte(in), &cfg) - if err == nil { - err = cfg.Validate() - } expected := "at most one of url & url_file must be configured" if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } } @@ -74,7 +68,7 @@ http_config: if err == nil { t.Fatalf("no error returned, expected:\n%v", expected) } - if err.Error() != expected { + if err.Error() != "yaml: unmarshal errors:\n "+expected { t.Errorf("\nexpected:\n%v\ngot:\n%v", expected, err.Error()) } }