From 6ce841ca22b4724b5ac40e501d4198a40a262ecc Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 5 Sep 2023 17:53:24 +0100 Subject: [PATCH] Fix scheme required for webhook url in amtool (#3509) * Fix scheme required for webhook url in amtool This commit fixes issue #3505 where amtool would fail with "error: scheme required for webhook url" when using amtool with --alertmanager.url. The issue here is that UnmarshalYaml for WebhookConfig checks if the scheme is present when c.URL is non-nil. However, UnmarshalYaml for SecretURL returns a non-nil, default value url.URL{} if the response from api/v2/status contains as the webhook URL. Signed-off-by: George Robinson * Add test for config routes test Signed-off-by: George Robinson --------- Signed-off-by: George Robinson --- config/notifiers.go | 5 --- test/cli/acceptance.go | 22 +++++++++++ test/cli/acceptance/cli_test.go | 66 +++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 5 deletions(-) diff --git a/config/notifiers.go b/config/notifiers.go index db86b1a2..2650db5f 100644 --- a/config/notifiers.go +++ b/config/notifiers.go @@ -503,11 +503,6 @@ func (c *WebhookConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { if c.URL != nil && c.URLFile != "" { return fmt.Errorf("at most one of url & url_file must be configured") } - if c.URL != nil { - if c.URL.Scheme != "https" && c.URL.Scheme != "http" { - return fmt.Errorf("scheme required for webhook url") - } - } return nil } diff --git a/test/cli/acceptance.go b/test/cli/acceptance.go index a0bc09ac..0229b659 100644 --- a/test/cli/acceptance.go +++ b/test/cli/acceptance.go @@ -658,6 +658,28 @@ func (am *Alertmanager) UpdateConfig(conf string) { } } +func (am *Alertmanager) ShowRoute() ([]byte, error) { + return am.showRouteCommand() +} + +func (am *Alertmanager) showRouteCommand() ([]byte, error) { + amURLFlag := "--alertmanager.url=" + am.getURL("/") + args := []string{amURLFlag, "config", "routes", "show"} + cmd := exec.Command(amtool, args...) + return cmd.CombinedOutput() +} + +func (am *Alertmanager) TestRoute() ([]byte, error) { + return am.testRouteCommand() +} + +func (am *Alertmanager) testRouteCommand() ([]byte, error) { + amURLFlag := "--alertmanager.url=" + am.getURL("/") + args := []string{amURLFlag, "config", "routes", "test"} + cmd := exec.Command(amtool, args...) + return cmd.CombinedOutput() +} + func (am *Alertmanager) getURL(path string) string { return fmt.Sprintf("http://%s%s%s", am.apiAddr, am.opts.RoutePrefix, path) } diff --git a/test/cli/acceptance/cli_test.go b/test/cli/acceptance/cli_test.go index 3c4c8356..4d5f9bc1 100644 --- a/test/cli/acceptance/cli_test.go +++ b/test/cli/acceptance/cli_test.go @@ -168,3 +168,69 @@ receivers: t.Errorf("Incorrect number of silences queried, expected: %v, actual: %v", expectedSils, len(sils)) } } + +func TestRoutesShow(t *testing.T) { + t.Parallel() + + conf := ` +route: + receiver: "default" + group_by: [alertname] + group_wait: 1s + group_interval: 1s + repeat_interval: 1ms + +receivers: +- name: "default" + webhook_configs: + - url: 'http://%s' + send_resolved: true +` + + at := NewAcceptanceTest(t, &AcceptanceOpts{ + Tolerance: 1 * time.Second, + }) + co := at.Collector("webhook") + wh := NewWebhook(co) + + amc := at.AlertmanagerCluster(fmt.Sprintf(conf, wh.Address()), 1) + require.NoError(t, amc.Start()) + defer amc.Terminate() + + am := amc.Members()[0] + _, err := am.ShowRoute() + require.NoError(t, err) +} + +func TestRoutesTest(t *testing.T) { + t.Parallel() + + conf := ` +route: + receiver: "default" + group_by: [alertname] + group_wait: 1s + group_interval: 1s + repeat_interval: 1ms + +receivers: +- name: "default" + webhook_configs: + - url: 'http://%s' + send_resolved: true +` + + at := NewAcceptanceTest(t, &AcceptanceOpts{ + Tolerance: 1 * time.Second, + }) + co := at.Collector("webhook") + wh := NewWebhook(co) + + amc := at.AlertmanagerCluster(fmt.Sprintf(conf, wh.Address()), 1) + require.NoError(t, amc.Start()) + defer amc.Terminate() + + am := amc.Members()[0] + _, err := am.TestRoute() + require.NoError(t, err) +}