From 4eee01e00687e41db8c82deb002903d40c6586be Mon Sep 17 00:00:00 2001
From: Jiahua Chen <joe@sourcegraph.com>
Date: Thu, 10 Oct 2019 19:38:56 -0700
Subject: [PATCH] site_alerts: add config warning for critical.externalURL
 (#5959)

* site_alerts: add runtime validation for critical.externalURL

* Apply suggestions from code review

Co-Authored-By: Stephen Gutekanst <stephen.gutekanst@gmail.com>

* Apply suggestions from code review

Co-Authored-By: Stephen Gutekanst <stephen.gutekanst@gmail.com>

* Fix e2e test CSS selector

* Update e2e waitForSelector
---
 cmd/frontend/graphqlbackend/site_alerts.go | 30 ++++++++++++++++++++--
 web/src/e2e/e2e.test.ts                    |  6 ++---
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/cmd/frontend/graphqlbackend/site_alerts.go b/cmd/frontend/graphqlbackend/site_alerts.go
index 859331d8342..33448de5fec 100644
--- a/cmd/frontend/graphqlbackend/site_alerts.go
+++ b/cmd/frontend/graphqlbackend/site_alerts.go
@@ -8,6 +8,7 @@ import (
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/globals"
 	"github.com/sourcegraph/sourcegraph/internal/actor"
 	"github.com/sourcegraph/sourcegraph/internal/conf"
+	"github.com/sourcegraph/sourcegraph/internal/jsonc"
 )
 
 // Alert implements the GraphQL type Alert.
@@ -60,6 +61,19 @@ func (r *siteResolver) Alerts(ctx context.Context) ([]*Alert, error) {
 	return alerts, nil
 }
 
+// getConfigWarnings identifies problems with the configuration that a site
+// admin should address, but do not prevent Sourcegraph from running.
+func getConfigWarnings() (problems conf.Problems, err error) {
+	var c conf.Unified
+	if err := jsonc.Unmarshal(globals.ConfigurationServerFrontendOnly.Raw().Critical, &c.Critical); err != nil {
+		return nil, err
+	}
+	if c.Critical.ExternalURL == "" {
+		problems = append(problems, conf.NewCriticalProblem("`externalURL` was empty and it is required to be configured for Sourcegraph to work correctly."))
+	}
+	return problems, nil
+}
+
 func init() {
 	// Warn about invalid site configuration.
 	AlertFuncs = append(AlertFuncs, func(args AlertFuncArgs) []*Alert {
@@ -79,6 +93,18 @@ func init() {
 				},
 			}
 		}
+
+		configWarnings, err := getConfigWarnings()
+		if err != nil {
+			return []*Alert{
+				{
+					TypeValue:    AlertTypeError,
+					MessageValue: `Update [**critical configuration**](/help/admin/management_console) to resolve problems: ` + err.Error(),
+				},
+			}
+		}
+		problems = append(problems, configWarnings...)
+
 		if len(problems) == 0 {
 			return nil
 		}
@@ -89,7 +115,7 @@ func init() {
 		if len(criticalProblems) > 0 {
 			alerts = append(alerts, &Alert{
 				TypeValue: AlertTypeWarning,
-				MessageValue: `[**Update critical configuration**](/help/admin/management_console) to resolve problems.` +
+				MessageValue: `[**Update critical configuration**](/help/admin/management_console) to resolve problems:` +
 					"\n* " + strings.Join(criticalProblems.Messages(), "\n* "),
 			})
 		}
@@ -98,7 +124,7 @@ func init() {
 		if len(siteProblems) > 0 {
 			alerts = append(alerts, &Alert{
 				TypeValue: AlertTypeWarning,
-				MessageValue: `[**Update site configuration**](/site-admin/configuration) to resolve problems.` +
+				MessageValue: `[**Update site configuration**](/site-admin/configuration) to resolve problems:` +
 					"\n* " + strings.Join(siteProblems.Messages(), "\n* "),
 			})
 		}
diff --git a/web/src/e2e/e2e.test.ts b/web/src/e2e/e2e.test.ts
index 807e78ad5e7..c10262f70a4 100644
--- a/web/src/e2e/e2e.test.ts
+++ b/web/src/e2e/e2e.test.ts
@@ -121,11 +121,11 @@ describe('e2e test suite', () => {
                 selectMethod: 'keyboard',
             })
             await driver.page.click('.e2e-settings-file .e2e-save-toolbar-save')
-            await driver.page.waitForSelector('.e2e-global-alert .global-alerts__alert', { visible: true })
+            await driver.page.waitForSelector('.e2e-global-alert .notices .global-alerts__alert', { visible: true })
             await driver.page.evaluate(message => {
-                const elem = document.querySelector('.e2e-global-alert .global-alerts__alert')
+                const elem = document.querySelector('.e2e-global-alert .notices .global-alerts__alert')
                 if (!elem) {
-                    throw new Error('No .e2e-global-alert .global-alerts__alert element found')
+                    throw new Error('No .e2e-global-alert .notices .global-alerts__alert element found')
                 }
                 if (!(elem as HTMLElement).innerText.includes(message)) {
                     throw new Error('Expected "' + message + '" message, but didn\'t find it')