Merge pull request #22708 from vespa-engine/ean/add-html-mail-for-notifications

Include HTML content for notification mails

2 files changed, 110 insertions, 3 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java
index 36a254f5c4b..f896d3747af 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/notification/Notifier.java
@@ -1,6 +1,7 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.notification;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.text.Text;
 import com.yahoo.vespa.flags.FetchVector;
@@ -19,6 +20,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 /**
@@ -34,6 +36,9 @@ public class Notifier {
 
     private static final Logger log = Logger.getLogger(Notifier.class.getName());
 
+    // Minimal url pattern matcher to detect hardcoded URLs in Notification messages
+    private static final Pattern urlPattern = Pattern.compile("https://[\\w\\d./]+");
+
     public Notifier(CuratorDb curatorDb, ZoneRegistry zoneRegistry, Mailer mailer, FlagSource flagSource) {
         this.curatorDb = Objects.requireNonNull(curatorDb);
         this.mailer = Objects.requireNonNull(mailer);
@@ -103,16 +108,32 @@ public class Notifier {
         }
     }
 
-    private Mail mailOf(FormattedNotification content, Collection<String> recipients) {
+    public Mail mailOf(FormattedNotification content, Collection<String> recipients) {
         var notification = content.notification();
         var subject = Text.format("[%s] %s Vespa Notification for %s", notification.level().toString().toUpperCase(), content.prettyType(), applicationIdSource(notification.source()));
         var body = new StringBuilder();
-        body.append(content.messagePrefix()).append("\n\n")
+        body.append(content.messagePrefix()).append("\n")
                 .append(notification.messages().stream().map(m -> " * " + m).collect(Collectors.joining("\n"))).append("\n")
                 .append("\n")
                 .append("Vespa Console link:\n")
                 .append(content.uri().toString());
-        return new Mail(recipients, subject, body.toString());
+        var html = new StringBuilder();
+        html.append(content.messagePrefix()).append("<br>\n")
+                .append("<ul>\n")
+                .append(notification.messages().stream()
+                        .map(Notifier::linkify)
+                        .map(m -> "<li>" + m + "</li>")
+                        .collect(Collectors.joining("<br>\n")))
+                .append("</ul>\n")
+                .append("<br>\n")
+                .append("<a href=\"" + content.uri() + "\">Vespa Console</a>")
+                .append(content.uri().toString());
+        return new Mail(recipients, subject, body.toString(), html.toString());
+    }
+
+    @VisibleForTesting
+    static String linkify(String text) {
+        return urlPattern.matcher(text).replaceAll((res) -> String.format("<a href=\"%s\">%s</a>", res.group(), res.group()));
     }
 
     private String applicationIdSource(NotificationSource source) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java
new file mode 100644
index 00000000000..b241572bc6a
--- /dev/null
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/notification/NotifierTest.java
@@ -0,0 +1,86 @@
+package com.yahoo.vespa.hosted.controller.notification;
+
+import com.google.common.collect.ImmutableBiMap;
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.InstanceName;
+import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.TenantName;
+import com.yahoo.vespa.flags.Flags;
+import com.yahoo.vespa.flags.InMemoryFlagSource;
+import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockMailer;
+import com.yahoo.vespa.hosted.controller.integration.ZoneRegistryMock;
+import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
+import com.yahoo.vespa.hosted.controller.tenant.CloudTenant;
+import com.yahoo.vespa.hosted.controller.tenant.LastLoginInfo;
+import com.yahoo.vespa.hosted.controller.tenant.TenantContacts;
+import com.yahoo.vespa.hosted.controller.tenant.TenantInfo;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.time.Instant;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+public class NotifierTest {
+    private static final TenantName tenant = TenantName.from("tenant1");
+    private static final String email = "user1@example.com";
+
+    private static final CloudTenant cloudTenant = new CloudTenant(tenant,
+            Instant.now(),
+            LastLoginInfo.EMPTY,
+            Optional.empty(),
+            ImmutableBiMap.of(),
+            TenantInfo.empty()
+                    .withContacts(new TenantContacts(
+                            List.of(new TenantContacts.EmailContact(
+                                    List.of(TenantContacts.Audience.NOTIFICATIONS),
+                                    email)))),
+            List.of(),
+            Optional.empty());
+
+
+    MockCuratorDb curatorDb = new MockCuratorDb(SystemName.Public);
+
+    @Before
+    public void init() {
+        curatorDb.writeTenant(cloudTenant);
+    }
+
+    @Test
+    public void dispatch() {
+        var mailer = new MockMailer();
+        var flagSource = new InMemoryFlagSource().withBooleanFlag(Flags.NOTIFICATION_DISPATCH_FLAG.id(), true);
+        var notifier = new Notifier(curatorDb, new ZoneRegistryMock(SystemName.cd), mailer, flagSource);
+
+        var notification = new Notification(Instant.now(), Notification.Type.testPackage, Notification.Level.warning,
+                NotificationSource.from(ApplicationId.from(tenant, ApplicationName.defaultName(), InstanceName.defaultName())),
+                List.of("test package has production tests, but no production tests are declared in deployment.xml",
+                        "see https://docs.vespa.ai/en/testing.html for details on how to write system tests for Vespa"));
+        notifier.dispatch(notification);
+        assertEquals(1, mailer.inbox(email).size());
+        var mail = mailer.inbox(email).get(0);
+
+        assertEquals("[WARNING] Test package Vespa Notification for tenant1.default.default", mail.subject());
+        assertEquals("There are problems with tests for default.default<br>\n" +
+                "<ul>\n" +
+                "<li>test package has production tests, but no production tests are declared in deployment.xml</li><br>\n" +
+                "<li>see <a href=\"https://docs.vespa.ai/en/testing.html\">https://docs.vespa.ai/en/testing.html</a> for details on how to write system tests for Vespa</li></ul>\n" +
+                "<br>\n" +
+                "<a href=\"https://dashboard.tld/tenant1/default\">Vespa Console</a>https://dashboard.tld/tenant1/default",
+                mail.htmlMessage().get());
+    }
+
+    @Test
+    public void linkify() {
+        var data = Map.of(
+                "Hello. https://example.com/foo/bar.html is a nice place.", "Hello. <a href=\"https://example.com/foo/bar.html\">https://example.com/foo/bar.html</a> is a nice place.",
+                "No url.", "No url.");
+        data.forEach((input, expected) -> assertEquals(expected, Notifier.linkify(input)));
+    }
+}
\ No newline at end of file