diff --git a/server/channels/app/health_check.go b/server/channels/app/health_check.go index 1e74bc7a510..e7ec3322e2c 100644 --- a/server/channels/app/health_check.go +++ b/server/channels/app/health_check.go @@ -14,6 +14,7 @@ import ( "github.com/mattermost/mattermost/server/public/shared/request" "github.com/mattermost/mattermost/server/v8/channels/store" "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck" + _ "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck/rules" ) const healthCheckInterval = time.Hour diff --git a/server/i18n/en.json b/server/i18n/en.json index 0401eb7537d..83b345fe0d1 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -14241,6 +14241,96 @@ "translation": "Unsupported syncable type '{{.Value}}'.", "description": "API error for LDAP group sync when the target type is neither team nor channel; {{.Value}} is the invalid type string and 'syncable' is the internal term for a team/channel that groups can sync to." }, + { + "id": "health.reason.config_unavailable", + "translation": "Config section unavailable.", + "description": "Health dashboard reason shown when a health check could not run because the server configuration it inspects could not be read, so the check's status is unknown." + }, + { + "id": "health.reason.rule_panicked", + "translation": "The rule failed unexpectedly and could not be evaluated.", + "description": "Health dashboard reason shown when a health check crashed while running, so its status is unknown." + }, + { + "id": "health.rule.push_bad_scheme.message.http", + "translation": "PushNotificationServer {{.url}} uses http://, so push notifications are sent in cleartext.", + "description": "Health dashboard finding detail when the push notification server URL starts with http:// instead of https://; {{.url}} is the configured URL. 'PushNotificationServer' is a config setting name and should not be translated." + }, + { + "id": "health.rule.push_bad_scheme.message.missing", + "translation": "PushNotificationServer {{.url}} has an invalid scheme. Mobile users will not receive notifications.", + "description": "Health dashboard finding detail when the push notification server URL starts with neither https:// nor http://; {{.url}} is the configured URL. 'PushNotificationServer' is a config setting name and should not be translated." + }, + { + "id": "health.rule.push_bad_scheme.remediation", + "translation": "Prefix PushNotificationServer with https:// and verify reachability from the app node.", + "description": "Health dashboard fix-it step for a push notification server URL that does not use https://; 'PushNotificationServer' is a config setting name and 'app node' is the Mattermost application server." + }, + { + "id": "health.rule.push_bad_scheme.title", + "translation": "PushNotificationServer does not use https://", + "description": "Health dashboard finding title when the push notification server URL does not start with https://; 'PushNotificationServer' is a config setting name and should not be translated." + }, + { + "id": "health.rule.push_empty_url.message", + "translation": "Every push attempt will fail with 'unsupported protocol scheme'.", + "description": "Health dashboard finding detail when push notifications are enabled but no push server URL is set; 'unsupported protocol scheme' is the literal error the server reports and should not be translated." + }, + { + "id": "health.rule.push_empty_url.remediation", + "translation": "Point PushNotificationServer to https://push.mattermost.com or a self-hosted push proxy.", + "description": "Health dashboard fix-it step for a blank push notification server URL; https://push.mattermost.com is Mattermost's hosted push service. 'PushNotificationServer' is a config setting name and should not be translated." + }, + { + "id": "health.rule.push_empty_url.title", + "translation": "Push notifications enabled but PushNotificationServer is blank", + "description": "Health dashboard finding title when push notifications are turned on but the push notification server URL is empty; 'PushNotificationServer' is a config setting name and should not be translated." + }, + { + "id": "health.rule.push_test_proxy.message", + "translation": "push-test.mattermost.com is the Mattermost test push proxy. It has no SLA, limited throughput, and is not intended for production use. Mobile push delivery will be unreliable for anything beyond small-scale testing.", + "description": "Health dashboard finding detail when the push notification server is push-test.mattermost.com, Mattermost's test push proxy that is not meant for production; 'SLA' means service level agreement." + }, + { + "id": "health.rule.push_test_proxy.remediation", + "translation": "Switch to https://push.mattermost.com (the production Hosted Push Notification Service included with Enterprise) or to a self-hosted push proxy.", + "description": "Health dashboard fix-it step for a server using the test push proxy; 'Hosted Push Notification Service' is a Mattermost product name." + }, + { + "id": "health.rule.push_test_proxy.title", + "translation": "PushNotificationServer points at push-test.mattermost.com", + "description": "Health dashboard finding title when the push notification server is Mattermost's test push proxy; 'PushNotificationServer' is a config setting name and should not be translated." + }, + { + "id": "health.rule.site_url_empty.message", + "translation": "SiteURL is required for webhook URLs, OAuth callbacks, mobile push payloads, and email links. Integrations are broken while it is empty.", + "description": "Health dashboard finding detail explaining what breaks while the Site URL setting is empty; 'SiteURL' is a config setting name and should not be translated." + }, + { + "id": "health.rule.site_url_empty.remediation", + "translation": "Set SiteURL to the public HTTPS URL of the server.", + "description": "Health dashboard fix-it step for an empty Site URL setting; 'SiteURL' is a config setting name and should not be translated." + }, + { + "id": "health.rule.site_url_empty.title", + "translation": "ServiceSettings.SiteURL is empty", + "description": "Health dashboard finding title when the Site URL setting is empty; 'ServiceSettings.SiteURL' is the config setting path and should not be translated." + }, + { + "id": "health.rule.site_url_http.message", + "translation": "SiteURL is {{.url}}. Session cookies and auth tokens traverse cleartext.", + "description": "Health dashboard finding detail when the Site URL uses http://; {{.url}} is the configured Site URL, and 'traverse cleartext' means they travel over the network unencrypted." + }, + { + "id": "health.rule.site_url_http.remediation", + "translation": "Serve via HTTPS and update SiteURL accordingly.", + "description": "Health dashboard fix-it step for a Site URL that uses http://; 'SiteURL' is a config setting name and should not be translated." + }, + { + "id": "health.rule.site_url_http.title", + "translation": "SiteURL uses http:// (not https)", + "description": "Health dashboard finding title when the Site URL starts with http:// instead of https://; 'SiteURL' is a config setting name and should not be translated." + }, { "id": "humanize.list_join", "translation": "{{.OtherItems}} and {{.LastItem}}", diff --git a/server/platform/shared/healthcheck/engine.go b/server/platform/shared/healthcheck/engine.go index 4fde52bfa7e..ebb3a2273b4 100644 --- a/server/platform/shared/healthcheck/engine.go +++ b/server/platform/shared/healthcheck/engine.go @@ -10,7 +10,10 @@ import ( "github.com/mattermost/mattermost/server/public/shared/mlog" ) -const ReasonRulePanicked = "health.reason.rule_panicked" +var ( + ReasonRulePanicked = TranslationId("health.reason.rule_panicked") + ReasonConfigUnavailable = TranslationId("health.reason.config_unavailable") +) // Evaluation names its rule by Code rather than copying the rule's metadata, which would be a second, staleable source of severity/area/surface. type Evaluation struct { diff --git a/server/platform/shared/healthcheck/engine_test.go b/server/platform/shared/healthcheck/engine_test.go index 24b639f61ef..702388df803 100644 --- a/server/platform/shared/healthcheck/engine_test.go +++ b/server/platform/shared/healthcheck/engine_test.go @@ -21,7 +21,7 @@ func TestEngineEvaluateContainsPanicsAndContinues(t *testing.T) { Code: "ALPHA", Area: model.AreaPlatform, Severity: SeverityWarning, - RuleText: model.RuleText{SummaryID: "health.rule.alpha.summary", RemediationID: "health.rule.alpha.remediation"}, + RuleText: model.RuleText{TitleID: "health.rule.alpha.title", RemediationID: "health.rule.alpha.remediation"}, Surface: SurfaceProduct, Volatility: VolatilityStable, Subject: "ServiceSettings.SiteURL", @@ -31,7 +31,7 @@ func TestEngineEvaluateContainsPanicsAndContinues(t *testing.T) { Code: "PANIC_RULE", Area: model.AreaPlatform, Severity: SeverityWarning, - RuleText: model.RuleText{SummaryID: "health.rule.panic_rule.summary", RemediationID: "health.rule.panic_rule.remediation"}, + RuleText: model.RuleText{TitleID: "health.rule.panic_rule.title", RemediationID: "health.rule.panic_rule.remediation"}, Surface: SurfaceProduct, Volatility: VolatilityStable, Subject: "ServiceSettings.SiteURL", diff --git a/server/platform/shared/healthcheck/i18n.go b/server/platform/shared/healthcheck/i18n.go new file mode 100644 index 00000000000..8e817b01d96 --- /dev/null +++ b/server/platform/shared/healthcheck/i18n.go @@ -0,0 +1,8 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package healthcheck + +// TranslationId returns id unchanged. `mmgotool i18n extract` harvests literal arguments to +// calls named TranslationId; public/shared/i18n is not imported to keep this package a pure leaf. +func TranslationId(id string) string { return id } diff --git a/server/platform/shared/healthcheck/registry.go b/server/platform/shared/healthcheck/registry.go index a8179e584be..057b7df2567 100644 --- a/server/platform/shared/healthcheck/registry.go +++ b/server/platform/shared/healthcheck/registry.go @@ -74,8 +74,8 @@ func (r *Registry) Validate() error { validationErrs = append(validationErrs, fmt.Errorf("rule %q must define Eval and/or EvalNode", rule.Code)) } - if rule.SummaryID == "" { - validationErrs = append(validationErrs, fmt.Errorf("rule %q summary id cannot be empty", rule.Code)) + if rule.TitleID == "" { + validationErrs = append(validationErrs, fmt.Errorf("rule %q title id cannot be empty", rule.Code)) } if rule.RemediationID == "" { validationErrs = append(validationErrs, fmt.Errorf("rule %q remediation id cannot be empty", rule.Code)) @@ -95,9 +95,9 @@ func (r *Registry) Validate() error { validationErrs = append(validationErrs, fmt.Errorf("rule %q has unknown volatility %q", rule.Code, rule.Volatility)) } - expectedSummaryID := "health.rule." + strings.ToLower(rule.Code) + ".summary" - if rule.SummaryID != "" && rule.SummaryID != expectedSummaryID { - validationErrs = append(validationErrs, fmt.Errorf("rule %q summary id must be %q", rule.Code, expectedSummaryID)) + expectedTitleID := "health.rule." + strings.ToLower(rule.Code) + ".title" + if rule.TitleID != "" && rule.TitleID != expectedTitleID { + validationErrs = append(validationErrs, fmt.Errorf("rule %q title id must be %q", rule.Code, expectedTitleID)) } expectedRemediationID := "health.rule." + strings.ToLower(rule.Code) + ".remediation" if rule.RemediationID != "" && rule.RemediationID != expectedRemediationID { diff --git a/server/platform/shared/healthcheck/registry_test.go b/server/platform/shared/healthcheck/registry_test.go index ce48f8a41e4..5715b5979c9 100644 --- a/server/platform/shared/healthcheck/registry_test.go +++ b/server/platform/shared/healthcheck/registry_test.go @@ -70,15 +70,15 @@ func TestRegistryValidate(t *testing.T) { expectedSubtext: "code cannot be empty", }, { - name: "empty summary id", + name: "empty title id", rules: []Rule{ func() Rule { - rule := validRule("EMPTY_SUMMARY") - rule.SummaryID = "" + rule := validRule("EMPTY_TITLE") + rule.TitleID = "" return rule }(), }, - expectedSubtext: "summary id cannot be empty", + expectedSubtext: "title id cannot be empty", }, { name: "empty remediation id", @@ -136,15 +136,15 @@ func TestRegistryValidate(t *testing.T) { expectedSubtext: "unknown volatility", }, { - name: "summary convention", + name: "title convention", rules: []Rule{ func() Rule { - rule := validRule("SUMMARY_CONVENTION") - rule.SummaryID = "health.rule.other_rule.summary" + rule := validRule("TITLE_CONVENTION") + rule.TitleID = "health.rule.other_rule.title" return rule }(), }, - expectedSubtext: "summary id must be", + expectedSubtext: "title id must be", }, { name: "remediation convention", diff --git a/server/platform/shared/healthcheck/rule_test.go b/server/platform/shared/healthcheck/rule_test.go index a926af9b33a..c39d9e779a1 100644 --- a/server/platform/shared/healthcheck/rule_test.go +++ b/server/platform/shared/healthcheck/rule_test.go @@ -31,7 +31,7 @@ func validRule(code string) Rule { Code: code, Area: model.AreaPlatform, Severity: SeverityWarning, - RuleText: model.RuleText{SummaryID: "health.rule." + strings.ToLower(code) + ".summary", RemediationID: "health.rule." + strings.ToLower(code) + ".remediation"}, + RuleText: model.RuleText{TitleID: "health.rule." + strings.ToLower(code) + ".title", RemediationID: "health.rule." + strings.ToLower(code) + ".remediation"}, Surface: SurfaceProduct, Volatility: VolatilityStable, Subject: "ServiceSettings.SiteURL", diff --git a/server/platform/shared/healthcheck/rules/cluster/site_url.go b/server/platform/shared/healthcheck/rules/cluster/site_url.go new file mode 100644 index 00000000000..3995af00fb8 --- /dev/null +++ b/server/platform/shared/healthcheck/rules/cluster/site_url.go @@ -0,0 +1,81 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package cluster + +import ( + "strings" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck" +) + +func init() { + healthcheck.Register(siteURLEmpty, siteURLHTTP) +} + +const ( + siteURLSubject = "ServiceSettings.SiteURL" + siteURLConsolePath = "/admin_console/environment/web_server" + siteURLDocsURL = "https://mattermost.com/pl/configure-site-url" +) + +var siteURLEmpty = healthcheck.Rule{ + Code: "SITE_URL_EMPTY", + Area: model.AreaCluster, + Severity: healthcheck.SeverityCritical, + RuleText: model.RuleText{ + TitleID: healthcheck.TranslationId("health.rule.site_url_empty.title"), + RemediationID: healthcheck.TranslationId("health.rule.site_url_empty.remediation"), + DocsURL: siteURLDocsURL, + ConsolePath: siteURLConsolePath, + }, + Surface: healthcheck.SurfaceProduct, + Volatility: healthcheck.VolatilityStable, + Subject: siteURLSubject, + Eval: evalSiteURLEmpty, +} + +var siteURLHTTP = healthcheck.Rule{ + Code: "SITE_URL_HTTP", + Area: model.AreaCluster, + Severity: healthcheck.SeverityWarning, + RuleText: model.RuleText{ + TitleID: healthcheck.TranslationId("health.rule.site_url_http.title"), + RemediationID: healthcheck.TranslationId("health.rule.site_url_http.remediation"), + DocsURL: siteURLDocsURL, + ConsolePath: siteURLConsolePath, + }, + Surface: healthcheck.SurfaceProduct, + Volatility: healthcheck.VolatilityStable, + Subject: siteURLSubject, + Eval: evalSiteURLHTTP, +} + +func siteURL(s *healthcheck.Snapshot) (string, bool) { + return s.ConfigString(func(cfg *model.Config) *string { return cfg.ServiceSettings.SiteURL }) +} + +func evalSiteURLEmpty(s *healthcheck.Snapshot) []healthcheck.Result { + url, ok := siteURL(s) + switch { + case !ok: + return []healthcheck.Result{healthcheck.Unknown(healthcheck.ReasonConfigUnavailable)} + case url == "": + return []healthcheck.Result{healthcheck.Firing(healthcheck.TranslationId("health.rule.site_url_empty.message"))} + default: + return []healthcheck.Result{healthcheck.Resolved()} + } +} + +func evalSiteURLHTTP(s *healthcheck.Snapshot) []healthcheck.Result { + url, ok := siteURL(s) + switch { + case !ok: + return []healthcheck.Result{healthcheck.Unknown(healthcheck.ReasonConfigUnavailable)} + case strings.HasPrefix(strings.ToLower(url), "http://"): + return []healthcheck.Result{healthcheck.Firing(healthcheck.TranslationId("health.rule.site_url_http.message")).WithDetail("url", url)} + default: + return []healthcheck.Result{healthcheck.Resolved()} + } +} diff --git a/server/platform/shared/healthcheck/rules/cluster/site_url_test.go b/server/platform/shared/healthcheck/rules/cluster/site_url_test.go new file mode 100644 index 00000000000..765b43fba5f --- /dev/null +++ b/server/platform/shared/healthcheck/rules/cluster/site_url_test.go @@ -0,0 +1,86 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package cluster + +import ( + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func siteURLSnapshot(url string) *healthcheck.Snapshot { + cfg := &model.Config{} + cfg.ServiceSettings.SiteURL = new(url) + + return &healthcheck.Snapshot{ + Config: &model.SupportPacketConfig{Config: cfg}, + Sections: map[model.WorkspaceSection]error{model.SectionConfig: nil}, + } +} + +type siteURLWant struct { + state healthcheck.State + messageID string + url string +} + +func TestSiteURLRules(t *testing.T) { + t.Parallel() + + resolved := siteURLWant{state: healthcheck.StateResolved} + unknown := siteURLWant{state: healthcheck.StateUnknown, messageID: healthcheck.ReasonConfigUnavailable} + rules := []healthcheck.Rule{siteURLEmpty, siteURLHTTP} + + testCases := []struct { + name string + snapshot *healthcheck.Snapshot + want map[string]siteURLWant + }{ + { + name: "config section absent", + snapshot: &healthcheck.Snapshot{}, + want: map[string]siteURLWant{"SITE_URL_EMPTY": unknown, "SITE_URL_HTTP": unknown}, + }, + { + name: "empty", + snapshot: siteURLSnapshot(""), + want: map[string]siteURLWant{ + "SITE_URL_EMPTY": {state: healthcheck.StateFiring, messageID: "health.rule.site_url_empty.message"}, + "SITE_URL_HTTP": resolved, + }, + }, + { + name: "http", + snapshot: siteURLSnapshot("http://mattermost.example.com"), + want: map[string]siteURLWant{ + "SITE_URL_EMPTY": resolved, + "SITE_URL_HTTP": {state: healthcheck.StateFiring, messageID: "health.rule.site_url_http.message", url: "http://mattermost.example.com"}, + }, + }, + { + name: "https", + snapshot: siteURLSnapshot("https://mattermost.example.com"), + want: map[string]siteURLWant{"SITE_URL_EMPTY": resolved, "SITE_URL_HTTP": resolved}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + for _, rule := range rules { + results := rule.Eval(tc.snapshot) + require.Len(t, results, 1, rule.Code) + + want := tc.want[rule.Code] + assert.Equal(t, want.state, results[0].State, rule.Code) + assert.Equal(t, want.messageID, results[0].MessageID, rule.Code) + assert.Equal(t, want.url, results[0].Details["url"], rule.Code) + } + }) + } +} diff --git a/server/platform/shared/healthcheck/rules/notifications/push.go b/server/platform/shared/healthcheck/rules/notifications/push.go new file mode 100644 index 00000000000..0254cf691b7 --- /dev/null +++ b/server/platform/shared/healthcheck/rules/notifications/push.go @@ -0,0 +1,129 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package notifications + +import ( + "net/url" + "strings" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck" +) + +func init() { + healthcheck.Register(pushEmptyURL, pushBadScheme, pushTestProxy) +} + +// The codes share a Subject so a mute on one condition never hides another. +const ( + pushSubject = "EmailSettings.PushNotificationServer" + pushConsolePath = "/admin_console/environment/push_notification_server" + pushDocsURL = "https://mattermost.com/pl/setup-push-notifications" +) + +var pushEmptyURL = healthcheck.Rule{ + Code: "PUSH_EMPTY_URL", + Area: model.AreaNotifications, + Severity: healthcheck.SeverityCritical, + RuleText: model.RuleText{ + TitleID: healthcheck.TranslationId("health.rule.push_empty_url.title"), + RemediationID: healthcheck.TranslationId("health.rule.push_empty_url.remediation"), + DocsURL: pushDocsURL, + ConsolePath: pushConsolePath, + }, + Surface: healthcheck.SurfaceProduct, + Volatility: healthcheck.VolatilityStable, + Subject: pushSubject, + Eval: evalPushEmptyURL, +} + +var pushBadScheme = healthcheck.Rule{ + Code: "PUSH_BAD_SCHEME", + Area: model.AreaNotifications, + Severity: healthcheck.SeverityCritical, + RuleText: model.RuleText{ + TitleID: healthcheck.TranslationId("health.rule.push_bad_scheme.title"), + RemediationID: healthcheck.TranslationId("health.rule.push_bad_scheme.remediation"), + DocsURL: pushDocsURL, + ConsolePath: pushConsolePath, + }, + Surface: healthcheck.SurfaceProduct, + Volatility: healthcheck.VolatilityStable, + Subject: pushSubject, + Eval: evalPushBadScheme, +} + +var pushTestProxy = healthcheck.Rule{ + Code: "PUSH_TEST_PROXY", + Area: model.AreaNotifications, + Severity: healthcheck.SeverityWarning, + RuleText: model.RuleText{ + TitleID: healthcheck.TranslationId("health.rule.push_test_proxy.title"), + RemediationID: healthcheck.TranslationId("health.rule.push_test_proxy.remediation"), + DocsURL: pushDocsURL, + ConsolePath: pushConsolePath, + }, + Surface: healthcheck.SurfaceProduct, + Volatility: healthcheck.VolatilityStable, + Subject: pushSubject, + Eval: evalPushTestProxy, +} + +// pushServer returns the configured push server, with enabled=false when push is off. +func pushServer(s *healthcheck.Snapshot) (server string, enabled, ok bool) { + enabled, ok = s.ConfigBool(func(cfg *model.Config) *bool { return cfg.EmailSettings.SendPushNotifications }) + if !ok || !enabled { + return "", enabled, ok + } + + server, ok = s.ConfigString(func(cfg *model.Config) *string { return cfg.EmailSettings.PushNotificationServer }) + return server, true, ok +} + +func hasScheme(server, scheme string) bool { + return strings.HasPrefix(strings.ToLower(server), scheme) +} + +func isTestProxy(server string) bool { + u, err := url.Parse(server) + return err == nil && u.Scheme == "https" && strings.EqualFold(u.Hostname(), "push-test.mattermost.com") +} + +func evalPushEmptyURL(s *healthcheck.Snapshot) []healthcheck.Result { + server, enabled, ok := pushServer(s) + switch { + case !ok: + return []healthcheck.Result{healthcheck.Unknown(healthcheck.ReasonConfigUnavailable)} + case enabled && server == "": + return []healthcheck.Result{healthcheck.Firing(healthcheck.TranslationId("health.rule.push_empty_url.message"))} + default: + return []healthcheck.Result{healthcheck.Resolved()} + } +} + +func evalPushBadScheme(s *healthcheck.Snapshot) []healthcheck.Result { + server, enabled, ok := pushServer(s) + switch { + case !ok: + return []healthcheck.Result{healthcheck.Unknown(healthcheck.ReasonConfigUnavailable)} + case !enabled || server == "" || hasScheme(server, "https://"): + return []healthcheck.Result{healthcheck.Resolved()} + case hasScheme(server, "http://"): + return []healthcheck.Result{healthcheck.Firing(healthcheck.TranslationId("health.rule.push_bad_scheme.message.http")).WithDetail("url", server)} + default: + return []healthcheck.Result{healthcheck.Firing(healthcheck.TranslationId("health.rule.push_bad_scheme.message.missing")).WithDetail("url", server)} + } +} + +func evalPushTestProxy(s *healthcheck.Snapshot) []healthcheck.Result { + server, enabled, ok := pushServer(s) + switch { + case !ok: + return []healthcheck.Result{healthcheck.Unknown(healthcheck.ReasonConfigUnavailable)} + case enabled && isTestProxy(server): + return []healthcheck.Result{healthcheck.Firing(healthcheck.TranslationId("health.rule.push_test_proxy.message")).WithDetail("url", server)} + default: + return []healthcheck.Result{healthcheck.Resolved()} + } +} diff --git a/server/platform/shared/healthcheck/rules/notifications/push_test.go b/server/platform/shared/healthcheck/rules/notifications/push_test.go new file mode 100644 index 00000000000..b9bdeab0af9 --- /dev/null +++ b/server/platform/shared/healthcheck/rules/notifications/push_test.go @@ -0,0 +1,167 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package notifications + +import ( + "errors" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func pushSnapshot(enabled bool, url string) *healthcheck.Snapshot { + cfg := &model.Config{} + cfg.EmailSettings.SendPushNotifications = new(enabled) + cfg.EmailSettings.PushNotificationServer = new(url) + + return &healthcheck.Snapshot{ + Config: &model.SupportPacketConfig{Config: cfg}, + Sections: map[model.WorkspaceSection]error{model.SectionConfig: nil}, + } +} + +type pushWant struct { + state healthcheck.State + messageID string + url string +} + +func TestPushRules(t *testing.T) { + t.Parallel() + + resolved := pushWant{state: healthcheck.StateResolved} + unknown := pushWant{state: healthcheck.StateUnknown, messageID: healthcheck.ReasonConfigUnavailable} + rules := []healthcheck.Rule{pushEmptyURL, pushBadScheme, pushTestProxy} + + testCases := []struct { + name string + snapshot *healthcheck.Snapshot + want map[string]pushWant + }{ + { + name: "config section absent", + snapshot: &healthcheck.Snapshot{}, + want: map[string]pushWant{"PUSH_EMPTY_URL": unknown, "PUSH_BAD_SCHEME": unknown, "PUSH_TEST_PROXY": unknown}, + }, + { + name: "config section failed", + snapshot: &healthcheck.Snapshot{ + Config: pushSnapshot(true, "").Config, + Sections: map[model.WorkspaceSection]error{model.SectionConfig: errors.New("boom")}, + }, + want: map[string]pushWant{"PUSH_EMPTY_URL": unknown, "PUSH_BAD_SCHEME": unknown, "PUSH_TEST_PROXY": unknown}, + }, + { + name: "push disabled with blank url", + snapshot: pushSnapshot(false, ""), + want: map[string]pushWant{"PUSH_EMPTY_URL": resolved, "PUSH_BAD_SCHEME": resolved, "PUSH_TEST_PROXY": resolved}, + }, + { + name: "blank url", + snapshot: pushSnapshot(true, ""), + want: map[string]pushWant{ + "PUSH_EMPTY_URL": {state: healthcheck.StateFiring, messageID: "health.rule.push_empty_url.message"}, + "PUSH_BAD_SCHEME": resolved, + "PUSH_TEST_PROXY": resolved, + }, + }, + { + name: "http scheme", + snapshot: pushSnapshot(true, "http://push.example.com"), + want: map[string]pushWant{ + "PUSH_EMPTY_URL": resolved, + "PUSH_BAD_SCHEME": {state: healthcheck.StateFiring, messageID: "health.rule.push_bad_scheme.message.http", url: "http://push.example.com"}, + "PUSH_TEST_PROXY": resolved, + }, + }, + { + name: "missing scheme", + snapshot: pushSnapshot(true, "push.example.com"), + want: map[string]pushWant{ + "PUSH_EMPTY_URL": resolved, + "PUSH_BAD_SCHEME": {state: healthcheck.StateFiring, messageID: "health.rule.push_bad_scheme.message.missing", url: "push.example.com"}, + "PUSH_TEST_PROXY": resolved, + }, + }, + { + name: "https production proxy", + snapshot: pushSnapshot(true, "https://push.mattermost.com"), + want: map[string]pushWant{"PUSH_EMPTY_URL": resolved, "PUSH_BAD_SCHEME": resolved, "PUSH_TEST_PROXY": resolved}, + }, + { + name: "https test proxy", + snapshot: pushSnapshot(true, "https://push-test.mattermost.com"), + want: map[string]pushWant{ + "PUSH_EMPTY_URL": resolved, + "PUSH_BAD_SCHEME": resolved, + "PUSH_TEST_PROXY": {state: healthcheck.StateFiring, messageID: "health.rule.push_test_proxy.message", url: "https://push-test.mattermost.com"}, + }, + }, + { + name: "https test proxy with port and path", + snapshot: pushSnapshot(true, "HTTPS://Push-Test.mattermost.com:443/"), + want: map[string]pushWant{ + "PUSH_EMPTY_URL": resolved, + "PUSH_BAD_SCHEME": resolved, + "PUSH_TEST_PROXY": {state: healthcheck.StateFiring, messageID: "health.rule.push_test_proxy.message", url: "HTTPS://Push-Test.mattermost.com:443/"}, + }, + }, + { + name: "test proxy host only in query", + snapshot: pushSnapshot(true, "https://push.example.com/?ref=push-test.mattermost.com"), + want: map[string]pushWant{"PUSH_EMPTY_URL": resolved, "PUSH_BAD_SCHEME": resolved, "PUSH_TEST_PROXY": resolved}, + }, + { + name: "lookalike test proxy host", + snapshot: pushSnapshot(true, "https://push-test.mattermost.com.example.com"), + want: map[string]pushWant{"PUSH_EMPTY_URL": resolved, "PUSH_BAD_SCHEME": resolved, "PUSH_TEST_PROXY": resolved}, + }, + { + name: "http test proxy reports only the scheme", + snapshot: pushSnapshot(true, "http://push-test.mattermost.com"), + want: map[string]pushWant{ + "PUSH_EMPTY_URL": resolved, + "PUSH_BAD_SCHEME": {state: healthcheck.StateFiring, messageID: "health.rule.push_bad_scheme.message.http", url: "http://push-test.mattermost.com"}, + "PUSH_TEST_PROXY": resolved, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + for _, rule := range rules { + results := rule.Eval(tc.snapshot) + require.Len(t, results, 1, rule.Code) + + want := tc.want[rule.Code] + assert.Equal(t, want.state, results[0].State, rule.Code) + assert.Equal(t, want.messageID, results[0].MessageID, rule.Code) + assert.Equal(t, want.url, results[0].Details["url"], rule.Code) + } + }) + } +} + +func TestPushFingerprintsDifferByCode(t *testing.T) { + t.Parallel() + + blank := pushSnapshot(true, "") + require.Equal(t, healthcheck.StateFiring, evalPushEmptyURL(blank)[0].State) + require.Equal(t, healthcheck.StateResolved, evalPushBadScheme(blank)[0].State) + + httpURL := pushSnapshot(true, "http://push.example.com") + require.Equal(t, healthcheck.StateResolved, evalPushEmptyURL(httpURL)[0].State) + require.Equal(t, healthcheck.StateFiring, evalPushBadScheme(httpURL)[0].State) + + require.Equal(t, pushEmptyURL.Subject, pushBadScheme.Subject) + assert.NotEqual(t, + healthcheck.Fingerprint(pushEmptyURL.Code, pushEmptyURL.Subject, ""), + healthcheck.Fingerprint(pushBadScheme.Code, pushBadScheme.Subject, ""), + ) +} diff --git a/server/platform/shared/healthcheck/rules/rules.go b/server/platform/shared/healthcheck/rules/rules.go new file mode 100644 index 00000000000..dc769a00b0a --- /dev/null +++ b/server/platform/shared/healthcheck/rules/rules.go @@ -0,0 +1,11 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +// Package rules registers the built-in health rule catalog; importing it for side effect +// populates healthcheck.Builtin(). Each area package gets one blank import here. +package rules + +import ( + _ "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck/rules/cluster" + _ "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck/rules/notifications" +) diff --git a/server/platform/shared/healthcheck/rules/rules_test.go b/server/platform/shared/healthcheck/rules/rules_test.go new file mode 100644 index 00000000000..7d1a834e69e --- /dev/null +++ b/server/platform/shared/healthcheck/rules/rules_test.go @@ -0,0 +1,22 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package rules + +import ( + "testing" + + "github.com/mattermost/mattermost/server/v8/platform/shared/healthcheck" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuiltinRegistry(t *testing.T) { + registry := healthcheck.Builtin() + require.NoError(t, registry.Validate()) + + for _, code := range []string{"PUSH_EMPTY_URL", "PUSH_BAD_SCHEME", "PUSH_TEST_PROXY", "SITE_URL_EMPTY", "SITE_URL_HTTP"} { + _, ok := registry.Get(code) + assert.True(t, ok, code) + } +} diff --git a/server/platform/shared/healthcheck/store_memory.go b/server/platform/shared/healthcheck/store_memory.go index 57f7939fc1c..96a87bfc298 100644 --- a/server/platform/shared/healthcheck/store_memory.go +++ b/server/platform/shared/healthcheck/store_memory.go @@ -89,8 +89,8 @@ func (s *memoryStore) Upsert(findings []*model.HealthFinding) error { for fp, finding := range deduped { stored := cloneFinding(finding) stored.PreSave() - // Summary/Remediation/Message are rendered at the read boundary; the store never holds them. - stored.Summary = "" + // Title/Remediation/Message are rendered at the read boundary; the store never holds them. + stored.Title = "" stored.Remediation = "" stored.Message = "" diff --git a/server/platform/shared/healthcheck/storetest.go b/server/platform/shared/healthcheck/storetest.go index ff959a1ad52..2cdbb837b5b 100644 --- a/server/platform/shared/healthcheck/storetest.go +++ b/server/platform/shared/healthcheck/storetest.go @@ -226,7 +226,7 @@ func TestFindingStore(t *testing.T, newStore func(t *testing.T) FindingStore) { store := newStore(t) fp := model.NewId() finding := testFinding(fp, "check_cluster_status", 100) - finding.Summary = "rendered summary" + finding.Title = "rendered title" finding.Remediation = "rendered remediation" finding.Message = "rendered message" require.NoError(t, store.Upsert([]*model.HealthFinding{finding})) @@ -234,7 +234,7 @@ func TestFindingStore(t *testing.T, newStore func(t *testing.T) FindingStore) { got, err := store.GetByFingerprints([]string{fp}) require.NoError(t, err) require.Len(t, got, 1) - require.Empty(t, got[0].Summary) + require.Empty(t, got[0].Title) require.Empty(t, got[0].Remediation) require.Empty(t, got[0].Message) }) diff --git a/server/public/model/health_finding.go b/server/public/model/health_finding.go index eead0ff28ee..789b64198b7 100644 --- a/server/public/model/health_finding.go +++ b/server/public/model/health_finding.go @@ -30,9 +30,9 @@ type HealthFinding struct { // Details is render context and doubles as the interpolation params for MessageID. Details map[string]string `json:"details,omitempty"` - // Summary, Remediation and Message are filled only at the API/mmctl boundary from the + // Title, Remediation and Message are filled only at the API/mmctl boundary from the // request locale; they are always empty in a store result. - Summary string `json:"summary,omitempty"` + Title string `json:"title,omitempty"` Remediation string `json:"remediation,omitempty"` Message string `json:"message,omitempty"` @@ -73,7 +73,7 @@ func (f *HealthFinding) Render(t i18n.TranslateFunc, text RuleText) *HealthFindi } rendered := *f - rendered.Summary = translateFindingText(t, text.SummaryID, args) + rendered.Title = translateFindingText(t, text.TitleID, args) rendered.Remediation = translateFindingText(t, text.RemediationID, args) rendered.Message = translateFindingText(t, f.MessageID, args) diff --git a/server/public/model/health_finding_test.go b/server/public/model/health_finding_test.go index d49ae440d9e..7a4644b78e1 100644 --- a/server/public/model/health_finding_test.go +++ b/server/public/model/health_finding_test.go @@ -12,7 +12,7 @@ import ( func TestHealthFindingRender(t *testing.T) { text := RuleText{ - SummaryID: "health.rule.summary", + TitleID: "health.rule.title", RemediationID: "health.rule.remediation", } @@ -36,7 +36,7 @@ func TestHealthFindingRender(t *testing.T) { rendered := f.Render(translate, text) require.NotNil(t, rendered) - assert.Equal(t, "health.rule.summary:node-1", rendered.Summary) + assert.Equal(t, "health.rule.title:node-1", rendered.Title) assert.Equal(t, "health.rule.remediation:node-1", rendered.Remediation) assert.Equal(t, "health.rule.message:node-1", rendered.Message) }) @@ -63,7 +63,7 @@ func TestHealthFindingRender(t *testing.T) { rendered := f.Render(nil, text) - assert.Equal(t, "health.rule.summary", rendered.Summary) + assert.Equal(t, "health.rule.title", rendered.Title) assert.Equal(t, "health.rule.remediation", rendered.Remediation) assert.Equal(t, "health.rule.message", rendered.Message) }) @@ -75,7 +75,7 @@ func TestHealthFindingRender(t *testing.T) { return "translated" }, RuleText{}) - assert.Empty(t, rendered.Summary) + assert.Empty(t, rendered.Title) assert.Empty(t, rendered.Remediation) assert.Empty(t, rendered.Message) }) @@ -87,7 +87,7 @@ func TestHealthFindingRender(t *testing.T) { return "translated" }, text) - assert.Empty(t, f.Summary) + assert.Empty(t, f.Title) assert.Empty(t, f.Remediation) assert.Empty(t, f.Message) }) diff --git a/server/public/model/health_rule.go b/server/public/model/health_rule.go index c8fcdc78d3a..bf4a7794916 100644 --- a/server/public/model/health_rule.go +++ b/server/public/model/health_rule.go @@ -34,7 +34,7 @@ func AllHealthAreas() []HealthArea { } type RuleText struct { - SummaryID string `json:"summary_id"` + TitleID string `json:"title_id"` RemediationID string `json:"remediation_id"` DocsURL string `json:"docs_url,omitempty"` ConsolePath string `json:"console_path,omitempty"`