From a5ae8cf377f208bab8326a526c3f9cf1549456b6 Mon Sep 17 00:00:00 2001 From: Owen Diehl Date: Wed, 5 May 2021 16:21:53 -0400 Subject: [PATCH] Unredact/secret (#33723) * no longer redacts GETing proxied AM configs * removes unused testfile * testware fix * consistently roundtrips yaml<>json and doesnt redact secrets * lint --- pkg/services/ngalert/api/tooling/Makefile | 10 ++- .../api/tooling/definitions/alertmanager.go | 47 +++++++++++- .../tooling/definitions/alertmanager_test.go | 29 ++++++++ .../alertmanager_test_artifact.json | 36 +++++++++ .../alertmanager_test_artifact.yaml | 21 ++++++ pkg/services/ngalert/api/tooling/post.json | 74 ++++++++++++++----- pkg/services/ngalert/api/tooling/spec.json | 68 ++++++++++++++--- 7 files changed, 251 insertions(+), 34 deletions(-) create mode 100644 pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.json create mode 100644 pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.yaml diff --git a/pkg/services/ngalert/api/tooling/Makefile b/pkg/services/ngalert/api/tooling/Makefile index 98fc79f29d8..1d154ff4187 100644 --- a/pkg/services/ngalert/api/tooling/Makefile +++ b/pkg/services/ngalert/api/tooling/Makefile @@ -2,11 +2,19 @@ API_DIR = definitions GO_PKG_FILES = $(shell find $(API_DIR) -name *.go -print) +SWAGGER_TAG ?= latest + +PATH_DOWN = pkg/services/ngalert/api/tooling +PATH_UP = ../../../../.. spec.json: $(GO_PKG_FILES) # this is slow because this image does not use the cache # https://github.com/go-swagger/go-swagger/blob/v0.27.0/Dockerfile#L5 - docker run --rm -it -e GOPATH=${GOPATH} -v ${GOPATH}:${GOPATH} -w $$(pwd) go-swagger generate spec -m -o $@ + docker run --rm -it \ + -w /src/$(PATH_DOWN) \ + -v $$(pwd)/$(PATH_UP):/src \ + quay.io/goswagger/swagger:$(SWAGGER_TAG) \ + generate spec -m -o $@ post.json: spec.json go run cmd/clean-swagger/main.go -if $(<) -of $@ diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go index 02a68efc9ed..af371e642d2 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager.go @@ -6,9 +6,10 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/models" - amv2 "github.com/prometheus/alertmanager/api/v2/models" "github.com/prometheus/alertmanager/config" "gopkg.in/yaml.v3" + + amv2 "github.com/prometheus/alertmanager/api/v2/models" ) // swagger:route POST /api/alertmanager/{Recipient}/config/api/v1/alerts alertmanager RoutePostAlertingConfig @@ -196,6 +197,7 @@ type DatasourceReference struct { type PostableUserConfig struct { TemplateFiles map[string]string `yaml:"template_files" json:"template_files"` AlertmanagerConfig PostableApiAlertingConfig `yaml:"alertmanager_config" json:"alertmanager_config"` + amSimple map[string]interface{} `yaml:"-" json:"-"` } func (c *PostableUserConfig) UnmarshalJSON(b []byte) error { @@ -204,7 +206,23 @@ func (c *PostableUserConfig) UnmarshalJSON(b []byte) error { return err } - return c.validate() + // validate first + if err := c.validate(); err != nil { + return err + } + + type intermediate struct { + AlertmanagerConfig map[string]interface{} `yaml:"alertmanager_config" json:"alertmanager_config"` + } + + var tmp intermediate + if err := json.Unmarshal(b, &tmp); err != nil { + return err + } + // store the map[string]interface{} variant for re-encoding later without redaction + c.amSimple = tmp.AlertmanagerConfig + + return nil } func (c *PostableUserConfig) validate() error { @@ -226,7 +244,7 @@ func (c *PostableUserConfig) validate() error { // MarshalYAML implements yaml.Marshaller. func (c *PostableUserConfig) MarshalYAML() (interface{}, error) { - yml, err := yaml.Marshal(c.AlertmanagerConfig) + yml, err := yaml.Marshal(c.amSimple) if err != nil { return nil, err } @@ -266,6 +284,11 @@ func (c *PostableUserConfig) UnmarshalYAML(value *yaml.Node) error { type GettableUserConfig struct { TemplateFiles map[string]string `yaml:"template_files" json:"template_files"` AlertmanagerConfig GettableApiAlertingConfig `yaml:"alertmanager_config" json:"alertmanager_config"` + + // amSimple stores a map[string]interface of the decoded alertmanager config. + // This enables circumventing the underlying alertmanager secret type + // which redacts itself during encoding. + amSimple map[string]interface{} `yaml:"-" json:"-"` } func (c *GettableUserConfig) UnmarshalYAML(value *yaml.Node) error { @@ -285,10 +308,28 @@ func (c *GettableUserConfig) UnmarshalYAML(value *yaml.Node) error { return err } + if err := yaml.Unmarshal([]byte(tmp.AlertmanagerConfig), &c.amSimple); err != nil { + return err + } + c.TemplateFiles = tmp.TemplateFiles return nil } +func (c *GettableUserConfig) MarshalJSON() ([]byte, error) { + type plain struct { + TemplateFiles map[string]string `yaml:"template_files" json:"template_files"` + AlertmanagerConfig map[string]interface{} `yaml:"alertmanager_config" json:"alertmanager_config"` + } + + tmp := plain{ + TemplateFiles: c.TemplateFiles, + AlertmanagerConfig: c.amSimple, + } + + return json.Marshal(tmp) +} + type GettableApiAlertingConfig struct { Config `yaml:",inline"` diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go index 7cc02979eee..0e0bca32008 100644 --- a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test.go @@ -2,6 +2,8 @@ package definitions import ( "encoding/json" + "io/ioutil" + "strings" "testing" "github.com/prometheus/alertmanager/config" @@ -372,7 +374,34 @@ alertmanager_config: | return } require.Nil(t, err) + // Override the map[string]interface{} field for test simplicity. + // It's tested in Test_GettableUserConfigRoundtrip. + out.amSimple = nil require.Equal(t, tc.output, out) }) } } + +func Test_GettableUserConfigRoundtrip(t *testing.T) { + // raw contains secret fields. We'll unmarshal, re-marshal, and ensure + // the fields are not redacted. + yamlEncoded, err := ioutil.ReadFile("alertmanager_test_artifact.yaml") + require.Nil(t, err) + + jsonEncoded, err := ioutil.ReadFile("alertmanager_test_artifact.json") + require.Nil(t, err) + + // test GettableUserConfig (yamlDecode -> jsonEncode) + var tmp GettableUserConfig + require.Nil(t, yaml.Unmarshal(yamlEncoded, &tmp)) + out, err := json.MarshalIndent(&tmp, "", " ") + require.Nil(t, err) + require.Equal(t, strings.TrimSpace(string(jsonEncoded)), string(out)) + + // test PostableUserConfig (jsonDecode -> yamlEncode) + var tmp2 PostableUserConfig + require.Nil(t, json.Unmarshal(jsonEncoded, &tmp2)) + out, err = yaml.Marshal(&tmp2) + require.Nil(t, err) + require.Equal(t, string(yamlEncoded), string(out)) +} diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.json b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.json new file mode 100644 index 00000000000..42b62fd9f75 --- /dev/null +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.json @@ -0,0 +1,36 @@ +{ + "template_files": { + "foo": "bar" + }, + "alertmanager_config": { + "receivers": [ + { + "email_configs": [ + { + "auth_password": "shh", + "auth_username": "admin", + "from": "bar", + "headers": { + "Bazz": "buzz" + }, + "html": "there", + "text": "hi", + "to": "foo" + } + ], + "name": "am" + } + ], + "route": { + "continue": false, + "receiver": "am", + "routes": [ + { + "continue": false, + "receiver": "am" + } + ] + }, + "templates": [] + } +} diff --git a/pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.yaml b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.yaml new file mode 100644 index 00000000000..3b8a30ba18a --- /dev/null +++ b/pkg/services/ngalert/api/tooling/definitions/alertmanager_test_artifact.yaml @@ -0,0 +1,21 @@ +template_files: + foo: bar +alertmanager_config: | + receivers: + - email_configs: + - auth_password: shh + auth_username: admin + from: bar + headers: + Bazz: buzz + html: there + text: hi + to: foo + name: am + route: + continue: false + receiver: am + routes: + - continue: false + receiver: am + templates: [] diff --git a/pkg/services/ngalert/api/tooling/post.json b/pkg/services/ngalert/api/tooling/post.json index c6ca6e9256d..ec43435dbea 100644 --- a/pkg/services/ngalert/api/tooling/post.json +++ b/pkg/services/ngalert/api/tooling/post.json @@ -141,7 +141,7 @@ "AlertQuery": { "properties": { "datasourceUid": { - "description": "Grafana data source unique identifer; it should be '-100' for a Server Side Expression operation.", + "description": "Grafana data source unique identifier; it should be '-100' for a Server Side Expression operation.", "type": "string", "x-go-name": "DatasourceUID" }, @@ -584,12 +584,8 @@ "Failure": { "$ref": "#/definitions/ResponseDetails" }, - "GettableAlert": { - "$ref": "#/definitions/gettableAlert" - }, - "GettableAlerts": { - "$ref": "#/definitions/gettableAlerts" - }, + "GettableAlert": {}, + "GettableAlerts": {}, "GettableApiAlertingConfig": { "properties": { "global": { @@ -856,9 +852,7 @@ "GettableSilence": { "$ref": "#/definitions/gettableSilence" }, - "GettableSilences": { - "$ref": "#/definitions/gettableSilences" - }, + "GettableSilences": {}, "GettableUserConfig": { "properties": { "alertmanager_config": { @@ -896,6 +890,10 @@ "slack_api_url": { "$ref": "#/definitions/SecretURL" }, + "slack_api_url_file": { + "type": "string", + "x-go-name": "SlackAPIURLFile" + }, "smtp_auth_identity": { "type": "string", "x-go-name": "SMTPAuthIdentity" @@ -964,6 +962,9 @@ "description": "FollowRedirects specifies whether the client should follow HTTP 3xx redirects.\nThe omitempty flag is not set, because it would be hidden from the\nmarshalled configuration when set to false.", "type": "boolean" }, + "OAuth2": { + "$ref": "#/definitions/OAuth2" + }, "ProxyURL": { "$ref": "#/definitions/URL" }, @@ -1120,6 +1121,37 @@ "type": "object", "x-go-package": "github.com/prometheus/alertmanager/config" }, + "OAuth2": { + "properties": { + "ClientID": { + "type": "string" + }, + "ClientSecret": { + "$ref": "#/definitions/Secret" + }, + "ClientSecretFile": { + "type": "string" + }, + "EndpointParams": { + "additionalProperties": { + "type": "string" + }, + "type": "object" + }, + "Scopes": { + "items": { + "type": "string" + }, + "type": "array" + }, + "TokenURL": { + "type": "string" + } + }, + "title": "OAuth2 is the oauth2 client configuration.", + "type": "object", + "x-go-package": "github.com/prometheus/common/config" + }, "OpsGenieConfig": { "properties": { "api_key": { @@ -2012,6 +2044,10 @@ "api_url": { "$ref": "#/definitions/SecretURL" }, + "api_url_file": { + "type": "string", + "x-go-name": "APIURLFile" + }, "callback_id": { "type": "string", "x-go-name": "CallbackID" @@ -2206,7 +2242,6 @@ "x-go-package": "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" }, "URL": { - "description": "The general form represented is:\n\n[scheme:][//[userinfo@]host][/]path[?query][#fragment]\n\nURLs that do not start with a slash after the scheme are interpreted as:\n\nscheme:opaque[?query][#fragment]\n\nNote that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.\nA consequence is that it is impossible to tell which slashes in the Path were\nslashes in the raw URL and which were %2f. This distinction is rarely important,\nbut when it is, the code should use RawPath, an optional field which only gets\nset if the default encoding is different from Path.\n\nURL's String method uses the EscapedPath method to obtain the path. See the\nEscapedPath method for more details.", "properties": { "ForceQuery": { "type": "boolean" @@ -2239,9 +2274,9 @@ "$ref": "#/definitions/Userinfo" } }, - "title": "A URL represents a parsed URL (technically, a URI reference).", + "title": "URL is a custom URL type that allows validation at configuration load time.", "type": "object", - "x-go-package": "net/url" + "x-go-package": "github.com/prometheus/common/config" }, "Userinfo": { "description": "The Userinfo type is an immutable encapsulation of username and\npassword details for a URL. An existing Userinfo value is guaranteed\nto have a username set (potentially empty, as allowed by RFC 2396),\nand optionally a password.", @@ -2271,6 +2306,9 @@ "api_key": { "$ref": "#/definitions/Secret" }, + "api_key_file": { + "$ref": "#/definitions/Secret" + }, "api_url": { "$ref": "#/definitions/URL" }, @@ -2410,7 +2448,7 @@ "alerts": { "description": "alerts", "items": { - "$ref": "#/definitions/gettableAlert" + "$ref": "#/definitions/GettableAlert" }, "type": "array", "x-go-name": "Alerts" @@ -2419,7 +2457,7 @@ "$ref": "#/definitions/labelSet" }, "receiver": { - "$ref": "#/definitions/Receiver" + "$ref": "#/definitions/receiver" } }, "required": [ @@ -3166,7 +3204,7 @@ "200": { "description": "AlertGroups", "schema": { - "$ref": "#/definitions/alertGroups" + "$ref": "#/definitions/AlertGroups" } }, "400": { @@ -3281,7 +3319,7 @@ "200": { "description": "GettableSilences", "schema": { - "$ref": "#/definitions/gettableSilences" + "$ref": "#/definitions/GettableSilences" } }, "400": { @@ -3303,7 +3341,7 @@ "in": "body", "name": "Silence", "schema": { - "$ref": "#/definitions/postableSilence" + "$ref": "#/definitions/PostableSilence" } }, { diff --git a/pkg/services/ngalert/api/tooling/spec.json b/pkg/services/ngalert/api/tooling/spec.json index 881081a4e83..d2e511461cd 100644 --- a/pkg/services/ngalert/api/tooling/spec.json +++ b/pkg/services/ngalert/api/tooling/spec.json @@ -190,7 +190,7 @@ "200": { "description": "AlertGroups", "schema": { - "$ref": "#/definitions/alertGroups" + "$ref": "#/definitions/AlertGroups" } }, "400": { @@ -305,7 +305,7 @@ "200": { "description": "GettableSilences", "schema": { - "$ref": "#/definitions/gettableSilences" + "$ref": "#/definitions/GettableSilences" } }, "400": { @@ -327,7 +327,7 @@ "name": "Silence", "in": "body", "schema": { - "$ref": "#/definitions/postableSilence" + "$ref": "#/definitions/PostableSilence" } }, { @@ -982,7 +982,7 @@ "title": "AlertQuery represents a single query associated with an alert definition.", "properties": { "datasourceUid": { - "description": "Grafana data source unique identifer; it should be '-100' for a Server Side Expression operation.", + "description": "Grafana data source unique identifier; it should be '-100' for a Server Side Expression operation.", "type": "string", "x-go-name": "DatasourceUID" }, @@ -1427,10 +1427,10 @@ "$ref": "#/definitions/ResponseDetails" }, "GettableAlert": { - "$ref": "#/definitions/gettableAlert" + "$ref": "#/definitions/GettableAlert" }, "GettableAlerts": { - "$ref": "#/definitions/gettableAlerts" + "$ref": "#/definitions/GettableAlerts" }, "GettableApiAlertingConfig": { "type": "object", @@ -1699,7 +1699,7 @@ "$ref": "#/definitions/gettableSilence" }, "GettableSilences": { - "$ref": "#/definitions/gettableSilences" + "$ref": "#/definitions/GettableSilences" }, "GettableUserConfig": { "type": "object", @@ -1739,6 +1739,10 @@ "slack_api_url": { "$ref": "#/definitions/SecretURL" }, + "slack_api_url_file": { + "type": "string", + "x-go-name": "SlackAPIURLFile" + }, "smtp_auth_identity": { "type": "string", "x-go-name": "SMTPAuthIdentity" @@ -1808,6 +1812,9 @@ "description": "FollowRedirects specifies whether the client should follow HTTP 3xx redirects.\nThe omitempty flag is not set, because it would be hidden from the\nmarshalled configuration when set to false.", "type": "boolean" }, + "OAuth2": { + "$ref": "#/definitions/OAuth2" + }, "ProxyURL": { "$ref": "#/definitions/URL" }, @@ -1963,6 +1970,37 @@ }, "x-go-package": "github.com/prometheus/alertmanager/config" }, + "OAuth2": { + "type": "object", + "title": "OAuth2 is the oauth2 client configuration.", + "properties": { + "ClientID": { + "type": "string" + }, + "ClientSecret": { + "$ref": "#/definitions/Secret" + }, + "ClientSecretFile": { + "type": "string" + }, + "EndpointParams": { + "type": "object", + "additionalProperties": { + "type": "string" + } + }, + "Scopes": { + "type": "array", + "items": { + "type": "string" + } + }, + "TokenURL": { + "type": "string" + } + }, + "x-go-package": "github.com/prometheus/common/config" + }, "OpsGenieConfig": { "type": "object", "title": "OpsGenieConfig configures notifications via OpsGenie.", @@ -2859,6 +2897,10 @@ "api_url": { "$ref": "#/definitions/SecretURL" }, + "api_url_file": { + "type": "string", + "x-go-name": "APIURLFile" + }, "callback_id": { "type": "string", "x-go-name": "CallbackID" @@ -3051,9 +3093,8 @@ "x-go-package": "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" }, "URL": { - "description": "The general form represented is:\n\n[scheme:][//[userinfo@]host][/]path[?query][#fragment]\n\nURLs that do not start with a slash after the scheme are interpreted as:\n\nscheme:opaque[?query][#fragment]\n\nNote that the Path field is stored in decoded form: /%47%6f%2f becomes /Go/.\nA consequence is that it is impossible to tell which slashes in the Path were\nslashes in the raw URL and which were %2f. This distinction is rarely important,\nbut when it is, the code should use RawPath, an optional field which only gets\nset if the default encoding is different from Path.\n\nURL's String method uses the EscapedPath method to obtain the path. See the\nEscapedPath method for more details.", "type": "object", - "title": "A URL represents a parsed URL (technically, a URI reference).", + "title": "URL is a custom URL type that allows validation at configuration load time.", "properties": { "ForceQuery": { "type": "boolean" @@ -3086,7 +3127,7 @@ "$ref": "#/definitions/Userinfo" } }, - "x-go-package": "net/url" + "x-go-package": "github.com/prometheus/common/config" }, "Userinfo": { "description": "The Userinfo type is an immutable encapsulation of username and\npassword details for a URL. An existing Userinfo value is guaranteed\nto have a username set (potentially empty, as allowed by RFC 2396),\nand optionally a password.", @@ -3118,6 +3159,9 @@ "api_key": { "$ref": "#/definitions/Secret" }, + "api_key_file": { + "$ref": "#/definitions/Secret" + }, "api_url": { "$ref": "#/definitions/URL" }, @@ -3262,7 +3306,7 @@ "description": "alerts", "type": "array", "items": { - "$ref": "#/definitions/gettableAlert" + "$ref": "#/definitions/GettableAlert" }, "x-go-name": "Alerts" }, @@ -3270,7 +3314,7 @@ "$ref": "#/definitions/labelSet" }, "receiver": { - "$ref": "#/definitions/Receiver" + "$ref": "#/definitions/receiver" } }, "x-go-name": "AlertGroup",