From 1346237777f26296195e348b31402c98c2a5cd6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Thu, 10 Oct 2019 23:27:14 +0100 Subject: [PATCH] fix(project): refactor proxy & external_uri handling Fixes #1024 --- cmd/karma/alerts.go | 4 +- docs/CONFIGURATION.md | 91 +++++++++++++-- internal/alertmanager/model_test.go | 104 ++++++++++-------- internal/alertmanager/models.go | 20 ++-- internal/alertmanager/upstream.go | 4 - internal/models/alertmanager.go | 5 +- .../AlertGroup/Silence/DeleteSilence.js | 4 +- .../AlertGroup/Silence/DeleteSilence.test.js | 10 +- .../Silence/__snapshots__/index.test.js.snap | 2 +- .../AlertGrid/AlertGroup/Silence/index.js | 2 +- .../AlertGroup/Silence/index.test.js | 4 +- .../SilenceSubmit/SilenceSubmitProgress.js | 8 +- .../SilenceSubmitProgress.test.js | 16 +-- 13 files changed, 184 insertions(+), 90 deletions(-) diff --git a/cmd/karma/alerts.go b/cmd/karma/alerts.go index 7efec171f..2d08528e8 100644 --- a/cmd/karma/alerts.go +++ b/cmd/karma/alerts.go @@ -110,7 +110,7 @@ func getUpstreams() models.AlertmanagerAPISummary { u := models.AlertmanagerAPIStatus{ Name: upstream.Name, - URI: upstream.SanitizedURI(), + URI: upstream.InternalURI(), PublicURI: upstream.PublicURI(), Headers: map[string]string{}, Error: upstream.Error(), @@ -119,7 +119,7 @@ func getUpstreams() models.AlertmanagerAPISummary { ClusterMembers: members, } if !upstream.ProxyRequests { - for k, v := range uri.HeadersForBasicAuth(u.PublicURI) { + for k, v := range uri.HeadersForBasicAuth(upstream.URI) { u.Headers[k] = v } for k, v := range upstream.HTTPHeaders { diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 248b69378..f2155049b 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -73,13 +73,9 @@ alertmanager: (see below), then the username & password information will be stripped from the URI and `Authorization` header using Basic Auth will be set for all in browser requests. - To set a different URI for all browser requests (can be any valid URI) see - `external_uri` option below. -- `external_uri` - base URI of this Alertmanager server used for all browser - requests, which currently means requests sent to alertmanager when creating, - editing or deleting silences from karma web UI (unless proxy mode is set, see - above). - This option cannot be used when `proxy` is enabled. +- `external_uri` - this option allows to override base URI of this Alertmanager + used for browser links and also silence requests (but only when proxy mode is + not enabled). - `timeout` - timeout for requests send to this Alertmanager server, a string in [time.Duration](https://golang.org/pkg/time/#ParseDuration) format. - `proxy` - if enabled requests from user browsers to this Alertmanager will be @@ -104,6 +100,87 @@ alertmanager: These custom headers will be sent with every request to the alert manager instance. +Note: there are multiple supported combination of URI settings which result in +a slightly different behavior. Settings that control it are: + +- `uri` - this option tells karma backend the URI that should be used to collect + all alerts and silence data from given Alertmanager instance. This setting is + required. +- `proxy` - this option when set to true enables karma backend to proxy all + silence management requests (creating, editing or deleting silences via karma + UI), so when the user creates a silence via karma UI the browser makes a + request to karma backend, the backend then forwards this request to the + Alertmanager using the value of `uri` option as the URI. + When this option is set to `false` all browser requests will use `uri` value. + This setting is optional, default value for it is `false`. +- `external_uri` - this option tells karma how the browser should connect to + given Alertmanager instance, it can be used for silence management requests + (creating, editing or deleting silences via karma UI) and how to generate + links to silences in Alertmanager web UI. Behavior of this option depends on + the value of `proxy` setting. + When proxy mode is enabled: + - silence management requests will use karma backend URI + - silence links to Alertmanager web UI will use `external_uri` value as base + URI + When proxy mode is disabled: + - silence management requests will use `external_uri` value as base URI + - silence links to Alertmanager web UI will use `external_uri` value as base + URI + +Breakdown of all combination of settings: + +1. Only `uri` is set: + + ```YAML + uri: http://localhost:123 + ``` + + Karma would use those URIs for: + + | Backend | Silence management | Silence links | + |-|-|-| + | `http://localhost:123` | `http://localhost:123` | `http://localhost:123` | + +1. Proxy mode is enabled: + + ```YAML + uri: http://localhost:123 + proxy: true + ``` + + Karma would use those URIs for: + + | Backend | Silence management | Silence links | + |-|-|-| + | `http://localhost:123` | Karma internal URI | `http://localhost:123` | + +1. `external_uri` is set, but proxy mode is disabled: + + ```YAML + uri: http://localhost:123 + external_uri: http://example.com + ``` + + Karma would use those URIs for: + + | Backend | Silence management | Silence links | + |-|-|-| + | `http://localhost:123` | `http://example.com` | `http://example.com` | + +1. Proxy mode is enabled and `external_uri` is set: + + ```YAML + uri: http://localhost:123 + proxy: true + external_uri: http://example.com + ``` + + Karma would use those URIs for: + + | Backend | Silence management | Silence links | + |-|-|-| + | `http://localhost:123` | Karma internal URI | `http://example.com` | + Example with two production Alertmanager instances running in HA mode and a staging instance that is also proxied and requires a custom auth header: diff --git a/internal/alertmanager/model_test.go b/internal/alertmanager/model_test.go index 89a6e879b..c0872402d 100644 --- a/internal/alertmanager/model_test.go +++ b/internal/alertmanager/model_test.go @@ -5,83 +5,99 @@ import ( ) type uriTest struct { - rawURI string - extURI string - proxy bool - publicURI string + rawURI string + extURI string + proxy bool + internalURI string + publicURI string } var uriTests = []uriTest{ { - rawURI: "http://alertmanager.example.com", - proxy: false, - publicURI: "http://alertmanager.example.com", + rawURI: "http://alertmanager.example.com", + proxy: false, + internalURI: "http://alertmanager.example.com", + publicURI: "http://alertmanager.example.com", }, { - rawURI: "http://alertmanager.example.com/foo", - proxy: false, - publicURI: "http://alertmanager.example.com/foo", + rawURI: "http://alertmanager.example.com/foo", + proxy: false, + internalURI: "http://alertmanager.example.com/foo", + publicURI: "http://alertmanager.example.com/foo", }, { - rawURI: "http://alertmanager.example.com", - proxy: true, - publicURI: "/proxy/alertmanager/test", + rawURI: "http://alertmanager.example.com", + proxy: true, + internalURI: "/proxy/alertmanager/test", + publicURI: "http://alertmanager.example.com", }, { - rawURI: "http://alertmanager.example.com/foo", - proxy: true, - publicURI: "/proxy/alertmanager/test", + rawURI: "http://alertmanager.example.com/foo", + proxy: true, + internalURI: "/proxy/alertmanager/test", + publicURI: "http://alertmanager.example.com/foo", }, { - rawURI: "http://user:pass@alertmanager.example.com", - proxy: false, - publicURI: "http://alertmanager.example.com", + rawURI: "http://user:pass@alertmanager.example.com", + proxy: false, + internalURI: "http://alertmanager.example.com", + publicURI: "http://user:pass@alertmanager.example.com", }, { - rawURI: "https://user:pass@alertmanager.example.com/foo", - proxy: false, - publicURI: "https://alertmanager.example.com/foo", + rawURI: "https://user:pass@alertmanager.example.com/foo", + proxy: false, + internalURI: "https://alertmanager.example.com/foo", + publicURI: "https://user:pass@alertmanager.example.com/foo", }, { - rawURI: "http://user:pass@alertmanager.example.com", - proxy: true, - publicURI: "/proxy/alertmanager/test", + rawURI: "http://user:pass@alertmanager.example.com", + proxy: true, + internalURI: "/proxy/alertmanager/test", + publicURI: "http://user:pass@alertmanager.example.com", }, { - rawURI: "http://user:pass@alertmanager.example.com", - extURI: "http://am.example.com", - proxy: true, - publicURI: "/proxy/alertmanager/test", + rawURI: "http://user:pass@alertmanager.example.com", + extURI: "http://am.example.com", + proxy: true, + internalURI: "/proxy/alertmanager/test", + publicURI: "http://am.example.com", }, { - rawURI: "http://alertmanager.example.com", - extURI: "http://am.example.com", - proxy: true, - publicURI: "/proxy/alertmanager/test", + rawURI: "http://alertmanager.example.com", + extURI: "http://am.example.com", + proxy: true, + internalURI: "/proxy/alertmanager/test", + publicURI: "http://am.example.com", }, { - rawURI: "http://user:pass@alertmanager.example.com", - extURI: "http://am.example.com", - proxy: false, - publicURI: "http://am.example.com", + rawURI: "http://user:pass@alertmanager.example.com", + extURI: "http://am.example.com", + proxy: false, + internalURI: "http://am.example.com", + publicURI: "http://am.example.com", }, { - rawURI: "http://alertmanager.example.com", - extURI: "http://am.example.com", - proxy: false, - publicURI: "http://am.example.com", + rawURI: "http://alertmanager.example.com", + extURI: "http://am.example.com", + proxy: false, + internalURI: "http://am.example.com", + publicURI: "http://am.example.com", }, } func TestAlertmanagerURI(t *testing.T) { - for _, test := range uriTests { + for i, test := range uriTests { am, err := NewAlertmanager("test", test.rawURI, WithExternalURI(test.extURI), WithProxy(test.proxy)) if err != nil { t.Error(err) } if am.PublicURI() != test.publicURI { - t.Errorf("Public URI mismatch, expected '%s' => '%s', got '%s' (proxy: %v)", - test.rawURI, test.publicURI, am.PublicURI(), test.proxy) + t.Errorf("[%d] Public URI mismatch, expected '%s' => '%s', got '%s' (proxy: %v)", + i, test.rawURI, test.publicURI, am.PublicURI(), test.proxy) + } + if am.InternalURI() != test.internalURI { + t.Errorf("[%d] Internal URI mismatch, expected '%s' => '%s', got '%s' (proxy: %v)", + i, test.rawURI, test.internalURI, am.InternalURI(), test.proxy) } } } diff --git a/internal/alertmanager/models.go b/internal/alertmanager/models.go index 9a4dcd6fb..076aba691 100644 --- a/internal/alertmanager/models.go +++ b/internal/alertmanager/models.go @@ -195,9 +195,8 @@ func (am *Alertmanager) pullSilences(version string) error { return nil } -// PublicURI is the URI of this Alertmanager we put in JSON response -// it's either real full URI or a proxy relative URI -func (am *Alertmanager) PublicURI() string { +// InternalURI is the URI of this Alertmanager that will be used for all request made by the UI +func (am *Alertmanager) InternalURI() string { if am.ProxyRequests { sub := fmt.Sprintf("/proxy/alertmanager/%s", am.Name) uri := path.Join(config.Config.Listen.Prefix, sub) @@ -206,12 +205,22 @@ func (am *Alertmanager) PublicURI() string { // skip it return uri + "/" } + return uri } + + // strip all user/pass information, fetch() doesn't support it anyway + return uri.WithoutUserinfo(am.PublicURI()) +} + +// PublicURI is the URI of this Alertmanager that will be used for browser links +func (am *Alertmanager) PublicURI() string { + // external_uri is always the first setting to check for browser links if am.ExternalURI != "" { return am.ExternalURI } - return uri.WithoutUserinfo(am.URI) + + return am.URI } func (am *Alertmanager) pullAlerts(version string) error { @@ -488,9 +497,6 @@ func (am *Alertmanager) Error() string { // SanitizedURI returns a copy of Alertmanager.URI with password replaced by // "xxx" func (am *Alertmanager) SanitizedURI() string { - am.lock.RLock() - defer am.lock.RUnlock() - return uri.SanitizeURI(am.URI) } diff --git a/internal/alertmanager/upstream.go b/internal/alertmanager/upstream.go index f325bceb7..9b05582e6 100644 --- a/internal/alertmanager/upstream.go +++ b/internal/alertmanager/upstream.go @@ -66,10 +66,6 @@ func RegisterAlertmanager(am *Alertmanager) error { return fmt.Errorf("alertmanager upstream '%s' already exist", am.Name) } - if am.ExternalURI != "" && am.ProxyRequests { - return fmt.Errorf("alertmanager upstream '%s' is configured with both proxy and external_uri, only one of those options can be enabled", am.Name) - } - for _, existingAM := range upstreams { if existingAM.URI == am.URI { return fmt.Errorf("alertmanager upstream '%s' already collects from '%s'", existingAM.Name, existingAM.URI) diff --git a/internal/models/alertmanager.go b/internal/models/alertmanager.go index 2eb08980d..e25d40e2b 100644 --- a/internal/models/alertmanager.go +++ b/internal/models/alertmanager.go @@ -25,10 +25,9 @@ type AlertmanagerInstance struct { // AlertmanagerAPIStatus describes the Alertmanager instance overall health type AlertmanagerAPIStatus struct { Name string `json:"name"` - // this is real Alertmanager instance URI + // this is the Alertmanager URI used for all requests made by the UI URI string `json:"uri"` - // this is URI client should use to talk to this Alertmanager, it might be - // same as real or proxied URI + // this is the Alertmanager URI used for links in the browser PublicURI string `json:"publicURI"` Headers map[string]string `json:"headers"` Error string `json:"error"` diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.js index d22aa16e8..42b8eaf29 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.js @@ -163,8 +163,8 @@ const DeleteSilenceModalContent = observer( const isOpenAPI = semver.satisfies(alertmanager.version, ">=0.16.0"); const uri = isOpenAPI - ? `${alertmanager.publicURI}/api/v2/silence/${silenceID}` - : `${alertmanager.publicURI}/api/v1/silence/${silenceID}`; + ? `${alertmanager.uri}/api/v2/silence/${silenceID}` + : `${alertmanager.uri}/api/v1/silence/${silenceID}`; this.deleteState.fetch = FetchWithCredentials(uri, { method: "DELETE", diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js index f74f47a93..d5b677009 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/DeleteSilence.test.js @@ -125,7 +125,7 @@ describe("", () => { it("[v1] sends a DELETE request after clicking 'Confirm' button", async () => { await VerifyResponse({ status: "success" }); expect(fetch.mock.calls[1][0]).toBe( - "http://am.example.com/api/v1/silence/123456789" + "http://localhost/api/v1/silence/123456789" ); expect(fetch.mock.calls[1][1]).toMatchObject({ method: "DELETE" }); }); @@ -134,7 +134,7 @@ describe("", () => { alertmanager.version = "0.16.2"; await VerifyResponse({ status: "success" }); expect(fetch.mock.calls[1][0]).toBe( - "http://am.example.com/api/v2/silence/123456789" + "http://localhost/api/v2/silence/123456789" ); expect(fetch.mock.calls[1][1]).toMatchObject({ method: "DELETE" }); }); @@ -143,7 +143,7 @@ describe("", () => { alertmanager.headers = { Authorization: "Basic ***" }; await VerifyResponse({ status: "success" }); expect(fetch.mock.calls[1][0]).toBe( - "http://am.example.com/api/v1/silence/123456789" + "http://localhost/api/v1/silence/123456789" ); expect(fetch.mock.calls[1][1]).toMatchObject({ credentials: "include", @@ -157,7 +157,7 @@ describe("", () => { alertmanager.version = "0.16.2"; await VerifyResponse({ status: "success" }); expect(fetch.mock.calls[1][0]).toBe( - "http://am.example.com/api/v2/silence/123456789" + "http://localhost/api/v2/silence/123456789" ); expect(fetch.mock.calls[1][1]).toMatchObject({ credentials: "include", @@ -169,7 +169,7 @@ describe("", () => { it("'Confirm' button is no-op after successful DELETE", async () => { const tree = await VerifyResponse({ status: "success" }); expect(fetch.mock.calls[1][0]).toBe( - "http://am.example.com/api/v1/silence/123456789" + "http://localhost/api/v1/silence/123456789" ); expect(fetch.mock.calls[1][1]).toMatchObject({ method: "DELETE" }); diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/__snapshots__/index.test.js.snap b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/__snapshots__/index.test.js.snap index cfe8b3f6a..0c1639837 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/__snapshots__/index.test.js.snap +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/__snapshots__/index.test.js.snap @@ -137,7 +137,7 @@ exports[` matches snapshot with expaned details 1`] = ` -
diff --git a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js index 51630994e..d877deeb2 100644 --- a/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js +++ b/ui/src/Components/Grid/AlertGrid/AlertGroup/Silence/index.test.js @@ -261,11 +261,11 @@ describe("", () => { expect(endsAt.html()).toMatch(/text-danger/); }); - it("id links to Alertmanager silence view via alertmanager.uri", () => { + it("id links to Alertmanager silence view via alertmanager.publicURI", () => { const tree = MountedSilenceDetails(jest.fn()); const link = tree.find("a"); expect(link.props().href).toBe( - "file:///mock/#/silences/4cf5fd82-1edd-4169-99d1-ff8415e72179" + "http://example.com/#/silences/4cf5fd82-1edd-4169-99d1-ff8415e72179" ); }); }); diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js index d16ed4a6e..4068727fe 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js +++ b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.js @@ -106,8 +106,8 @@ const SilenceSubmitProgress = observer( const isOpenAPI = semver.satisfies(am.version, ">=0.16.0"); const uri = isOpenAPI - ? `${am.publicURI}/api/v2/silences` - : `${am.publicURI}/api/v1/silences`; + ? `${am.uri}/api/v2/silences` + : `${am.uri}/api/v1/silences`; this.submitState.fetch = FetchWithCredentials(uri, { method: "POST", @@ -122,7 +122,7 @@ const SilenceSubmitProgress = observer( if (result.ok) { return result .json() - .then(r => this.parseOpenAPIResponse(am.uri, r)); + .then(r => this.parseOpenAPIResponse(am.publicURI, r)); } else { return result.text().then(text => { this.submitState.markFailed(text); @@ -132,7 +132,7 @@ const SilenceSubmitProgress = observer( } else { return result .json() - .then(r => this.parseAlertmanagerResponse(am.uri, r)); + .then(r => this.parseAlertmanagerResponse(am.publicURI, r)); } }) .catch(err => { diff --git a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js index 73d7852fb..4fe66a414 100644 --- a/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js +++ b/ui/src/Components/SilenceModal/SilenceSubmit/SilenceSubmitProgress.test.js @@ -13,7 +13,7 @@ beforeEach(() => { instances: [ { name: "mockAlertmanager", - uri: "file:///mock", + uri: "http://localhost", publicURI: "http://example.com", headers: { foo: "bar" }, error: "", @@ -53,7 +53,7 @@ describe("", () => { const tree = MountedSilenceSubmitProgress(); await expect(tree.instance().submitState.fetch).resolves.toBeUndefined(); const uri = fetch.mock.calls[0][0]; - expect(uri).toBe("http://example.com/api/v1/silences"); + expect(uri).toBe("http://localhost/api/v1/silences"); }); it("[v2] appends /api/v2/silences to the passed URI", async () => { @@ -61,7 +61,7 @@ describe("", () => { const tree = MountedSilenceSubmitProgress(); await expect(tree.instance().submitState.fetch).resolves.toBeUndefined(); const uri = fetch.mock.calls[0][0]; - expect(uri).toBe("http://example.com/api/v1/silences"); + expect(uri).toBe("http://localhost/api/v1/silences"); }); it("sends correct JSON payload", () => { @@ -92,7 +92,7 @@ describe("", () => { instances: [ { name: "am1", - uri: "file:///mock", + uri: "http://am1.example.com", publicURI: "http://am1.example.com", headers: {}, error: "", @@ -102,7 +102,7 @@ describe("", () => { }, { name: "am2", - uri: "file:///mock", + uri: "http://am2.example.com", publicURI: "http://am2.example.com", headers: {}, error: "", @@ -148,7 +148,7 @@ describe("", () => { instances: [ { name: "am1", - uri: "file:///mock", + uri: "http://am1.example.com", publicURI: "http://am1.example.com", headers: {}, error: "", @@ -223,7 +223,7 @@ describe("", () => { .find("a") .getDOMNode() .getAttribute("href") - ).toBe("file:///mock/#/silences/123"); + ).toBe("http://example.com/#/silences/123"); }); it("[v2] renders success icon on successful fetch", async () => { @@ -247,7 +247,7 @@ describe("", () => { .find("a") .getDOMNode() .getAttribute("href") - ).toBe("file:///mock/#/silences/123"); + ).toBe("http://example.com/#/silences/123"); }); it("[v1] renders error icon on failed fetch", async () => {