From b6cdd412aa1be790ab65989c4ea2e89f5afedd53 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 5 Sep 2023 14:15:54 +0100 Subject: [PATCH 1/2] 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 --- config/notifiers.go | 5 ----- test/cli/acceptance.go | 11 +++++++++++ test/cli/acceptance/cli_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/config/notifiers.go b/config/notifiers.go index db86b1a2f1..2650db5f3b 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 a0bc09ac74..fd94617c3b 100644 --- a/test/cli/acceptance.go +++ b/test/cli/acceptance.go @@ -658,6 +658,17 @@ 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) 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 3c4c835641..59d4cc6134 100644 --- a/test/cli/acceptance/cli_test.go +++ b/test/cli/acceptance/cli_test.go @@ -168,3 +168,36 @@ 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) +} From 28fabf53476da3a52ee1d3e443814a45880cc007 Mon Sep 17 00:00:00 2001 From: George Robinson Date: Tue, 5 Sep 2023 16:18:49 +0100 Subject: [PATCH 2/2] Add test for config routes test Signed-off-by: George Robinson --- test/cli/acceptance.go | 11 +++++++++++ test/cli/acceptance/cli_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/test/cli/acceptance.go b/test/cli/acceptance.go index fd94617c3b..0229b6594f 100644 --- a/test/cli/acceptance.go +++ b/test/cli/acceptance.go @@ -669,6 +669,17 @@ func (am *Alertmanager) showRouteCommand() ([]byte, error) { 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 59d4cc6134..4d5f9bc1fd 100644 --- a/test/cli/acceptance/cli_test.go +++ b/test/cli/acceptance/cli_test.go @@ -201,3 +201,36 @@ receivers: _, 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) +}