Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support sending email message via Notifications passthrough API #158

Merged
merged 2 commits into from
Apr 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,9 @@ public String getMessage() {
@Override
public void writeTo(StreamOutput streamOutput) throws IOException {
super.writeTo(streamOutput);
// Making LegacyCustomWebhookMessage streamable is purely to support the new pass through API from ISM -> Notification plugin
// and it only supports LegacyCustomWebhookMessage when the url is already constructed by ISM.
// Making LegacyCustomWebhookMessage streamable is purely to support the new pass through API from Alerting/ISM -> Notification
// plugin
// and it only supports LegacyCustomWebhookMessage when the url is already constructed by Alerting/ISM.
if (Strings.isNullOrEmpty(getUrl())) {
throw new IllegalStateException("Cannot use LegacyCustomWebhookMessage across transport wire without defining full url.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@
public enum LegacyDestinationType {
LEGACY_CHIME,
LEGACY_SLACK,
LEGACY_CUSTOM_WEBHOOK
LEGACY_CUSTOM_WEBHOOK,
LEGACY_EMAIL
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.commons.destination.message;

import java.io.IOException;
import java.net.URI;
import java.util.List;

import org.opensearch.common.Strings;
import org.opensearch.common.io.stream.StreamInput;
import org.opensearch.common.io.stream.StreamOutput;
import org.opensearch.commons.notifications.model.MethodType;

/**
* This class holds the content of an CustomWebhook message
*/
public class LegacyEmailMessage extends LegacyBaseMessage {

private final String accountName;
private final String host;
private final int port;
private final String method;
private final String from;
private final List<String> recipients;
private final String subject;
private final String message;

private LegacyEmailMessage(
final String destinationName,
final String accountName,
final String host,
final Integer port,
final String method,
final String from,
final List<String> recipients,
final String subject,
final String message
) {
super(LegacyDestinationType.LEGACY_EMAIL, destinationName, message);

if (Strings.isNullOrEmpty(message)) {
throw new IllegalArgumentException("Message content is missing");
}

if (Strings.isNullOrEmpty(accountName)) {
throw new IllegalArgumentException("Account name should be provided");
}

if (Strings.isNullOrEmpty(host)) {
throw new IllegalArgumentException("Host name should be provided");
}

if (Strings.isNullOrEmpty(from)) {
throw new IllegalArgumentException("From address should be provided");
}

if (recipients == null || recipients.isEmpty()) {
throw new IllegalArgumentException("List of recipients should be provided");
}

this.message = message;
this.accountName = accountName;
this.host = host;
this.port = port == null ? 25 : port;

if (Strings.isNullOrEmpty(method)) {
// Default to "none"
this.method = "none";
} else if (!MethodType.NONE.toString().equals(method)
&& !MethodType.SSL.toString().equals(method)
&& !MethodType.START_TLS.toString().equals(method)) {
throw new IllegalArgumentException("Invalid method supplied. Only none, ssl and start_tls are allowed");
} else {
this.method = method;
}

this.from = from;
this.recipients = recipients;
this.subject = Strings.isNullOrEmpty(subject) ? destinationName : subject;
}

public LegacyEmailMessage(StreamInput streamInput) throws IOException {
super(streamInput);
this.message = super.getMessageContent();
this.accountName = streamInput.readString();
this.host = streamInput.readString();
this.port = streamInput.readInt();
this.method = streamInput.readString();
this.from = streamInput.readString();
this.recipients = streamInput.readStringList();
this.subject = streamInput.readString();
}

@Override
public String toString() {
return "DestinationType: "
+ getChannelType()
+ ", DestinationName:"
+ destinationName
+ ", AccountName:"
+ accountName
+ ", From: "
+ from
+ ", Host: "
+ host
+ ", Port: "
+ port
+ ", Method: "
+ method
+ ", Subject: <...>"
+ ", Message: <...>";
}

public static class Builder {
private final String destinationName;
private String accountName;
private String host;
private Integer port;
private String method;
private String from;
private List<String> recipients;
private String subject;
private String message;

public Builder(String destinationName) {
this.destinationName = destinationName;
}

public LegacyEmailMessage.Builder withAccountName(String accountName) {
this.accountName = accountName;
return this;
}

public LegacyEmailMessage.Builder withHost(String host) {
this.host = host;
return this;
}

public LegacyEmailMessage.Builder withPort(Integer port) {
this.port = port;
return this;
}

public LegacyEmailMessage.Builder withMethod(String method) {
this.method = method;
return this;
}

public LegacyEmailMessage.Builder withFrom(String from) {
this.from = from;
return this;
}

public LegacyEmailMessage.Builder withRecipients(List<String> recipients) {
this.recipients = recipients;
return this;
}

public LegacyEmailMessage.Builder withSubject(String subject) {
this.subject = subject;
return this;
}

public LegacyEmailMessage.Builder withMessage(String message) {
this.message = message;
return this;
}

public LegacyEmailMessage build() {
return new LegacyEmailMessage(
this.destinationName,
this.accountName,
this.host,
this.port,
this.method,
this.from,
this.recipients,
this.subject,
this.message
);
}
}

public String getAccountName() {
return accountName;
}

public String getHost() {
return host;
}

public int getPort() {
return port;
}

public String getMethod() {
return method;
}

public String getFrom() {
return from;
}

public List<String> getRecipients() {
return recipients;
}

public String getSubject() {
return subject;
}

public String getMessage() {
return message;
}

public URI getUri() {
return buildUri(null, null, host, port, null, null);
}

@Override
public void writeTo(StreamOutput streamOutput) throws IOException {
super.writeTo(streamOutput);
streamOutput.writeString(accountName);
streamOutput.writeString(host);
streamOutput.writeInt(port);
streamOutput.writeString(method);
streamOutput.writeString(from);
streamOutput.writeStringCollection(recipients);
streamOutput.writeString(subject);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import org.opensearch.commons.destination.message.LegacyBaseMessage
import org.opensearch.commons.destination.message.LegacyChimeMessage
import org.opensearch.commons.destination.message.LegacyCustomWebhookMessage
import org.opensearch.commons.destination.message.LegacyDestinationType
import org.opensearch.commons.destination.message.LegacyEmailMessage
import org.opensearch.commons.destination.message.LegacySlackMessage
import java.io.IOException

Expand Down Expand Up @@ -50,6 +51,7 @@ class LegacyPublishNotificationRequest : ActionRequest {
LegacyDestinationType.LEGACY_CHIME -> LegacyChimeMessage(input)
LegacyDestinationType.LEGACY_CUSTOM_WEBHOOK -> LegacyCustomWebhookMessage(input)
LegacyDestinationType.LEGACY_SLACK -> LegacySlackMessage(input)
LegacyDestinationType.LEGACY_EMAIL -> LegacyEmailMessage(input)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ import org.opensearch.common.io.stream.StreamInput
import org.opensearch.common.io.stream.StreamOutput
import org.opensearch.common.io.stream.Writeable
import org.opensearch.common.xcontent.ToXContent
import org.opensearch.common.xcontent.ToXContent.EMPTY_PARAMS
import org.opensearch.common.xcontent.XContentBuilder
import org.opensearch.common.xcontent.XContentHelper
import org.opensearch.common.xcontent.XContentParser
import org.opensearch.common.xcontent.XContentParserUtils
import org.opensearch.common.xcontent.XContentType
import org.opensearch.commons.notifications.NotificationConstants.EVENT_SOURCE_TAG
import org.opensearch.commons.notifications.NotificationConstants.STATUS_LIST_TAG
import org.opensearch.commons.utils.logger
Expand Down Expand Up @@ -102,4 +105,13 @@ data class NotificationEvent(
.field(STATUS_LIST_TAG, statusList)
.endObject()
}

// Overriding toString so consuming plugins can log/output this from the sendNotification response if needed
override fun toString(): String {
return try {
XContentHelper.toXContent(this, XContentType.JSON, EMPTY_PARAMS, true).utf8ToString()
} catch (e: IOException) {
super.toString() + " threw " + e.toString()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we publishing this information outside of the system?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alerting is using the response after publishing a message and setting that as the actionResponseContent in runAction.

Since this is the actionResult and we store it in the Alert it can be published externally via ctx.alert in messages.

I had to catch the IOException since detekt was complaining that toString() should not throw exceptions. If there are concerns here, should I log super.toString() + " threw " + e.toString() instead and make the returned string something generic?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but this would suppress the exception? which might be misleading for its consumers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I've read, it seems it's not that unusual to suppress the exception here. Conventionally, toString() should not be throwing exceptions.

Here is an example of a thread that I found when looking into it: https://stackoverflow.com/a/42011744

This is probably why the linter also failed on it. I can add a log entry as a follow-up change so the exception is logged at least for debugging purposes. If you disagree with this though, feel free to create an issue for it to discuss further.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on debug logger here instead

}
}
}
Loading