Merge branch 'main' into main

Hailong-am · Feb 19, 2024 · 1b348d0 · 1b348d0
2 parents 9882d74 + 39bd3bf
commit 1b348d0
Show file tree

Hide file tree

Showing 22 changed files with 110 additions and 39 deletions.
diff --git a/notifications/build.gradle b/notifications/build.gradle
@@ -68,11 +68,16 @@ allprojects {
 }
 
 configurations {
-    ktlint
+    ktlint {
+        resolutionStrategy {
+            force "ch.qos.logback:logback-classic:1.3.14"
+            force "ch.qos.logback:logback-core:1.3.14"
+        }
+    }
 }
 
 dependencies {
-    add("ktlint", "com.pinterest:ktlint:0.44.0") {
+    add("ktlint", "com.pinterest:ktlint:0.47.1") {
         attributes {
             attribute(Bundling.BUNDLING_ATTRIBUTE, objects.named(Bundling, Bundling.EXTERNAL))
         }

diff --git a/...pi/src/main/kotlin/org/opensearch/notifications/spi/model/destination/ChimeDestination.kt b/...pi/src/main/kotlin/org/opensearch/notifications/spi/model/destination/ChimeDestination.kt
@@ -9,5 +9,5 @@ package org.opensearch.notifications.spi.model.destination
  * This class holds the contents of a Chime destination
  */
 class ChimeDestination(
-    url: String,
+    url: String
 ) : WebhookDestination(url, DestinationType.CHIME)
diff --git a/...in/kotlin/org/opensearch/notifications/spi/model/destination/MicrosoftTeamsDestination.kt b/...in/kotlin/org/opensearch/notifications/spi/model/destination/MicrosoftTeamsDestination.kt
@@ -9,5 +9,5 @@ package org.opensearch.notifications.spi.model.destination
  * This class holds the contents of a Microsoft Teams destination
  */
 class MicrosoftTeamsDestination(
-    url: String,
+    url: String
 ) : WebhookDestination(url, DestinationType.MICROSOFT_TEAMS)
diff --git a/...pi/src/main/kotlin/org/opensearch/notifications/spi/model/destination/SlackDestination.kt b/...pi/src/main/kotlin/org/opensearch/notifications/spi/model/destination/SlackDestination.kt
@@ -9,5 +9,5 @@ package org.opensearch.notifications.spi.model.destination
  * This class holds the contents of a Slack destination
  */
 class SlackDestination(
-    url: String,
+    url: String
 ) : WebhookDestination(url, DestinationType.SLACK)
diff --git a/...-spi/src/main/kotlin/org/opensearch/notifications/spi/model/destination/SnsDestination.kt b/...-spi/src/main/kotlin/org/opensearch/notifications/spi/model/destination/SnsDestination.kt
@@ -9,7 +9,7 @@ package org.opensearch.notifications.spi.model.destination
  */
 data class SnsDestination(
     val topicArn: String,
-    val roleArn: String? = null,
+    val roleArn: String? = null
 ) : BaseDestination(DestinationType.SNS) {
     // sample topic arn -> arn:aws:sns:us-west-2:075315751589:test-notification
     val region: String = topicArn.split(":".toRegex()).toTypedArray()[3]

diff --git a/...core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt b/...core-spi/src/test/kotlin/org/opensearch/notifications/spi/utils/ValidationHelpersTests.kt
@@ -100,6 +100,7 @@ internal class ValidationHelpersTests {
     fun `validator identifies chime url as valid`() {
         assert(isValidUrl(CHIME_URL))
     }
+
     @Test
     fun `validator identifies microsoft teams url as valid`() {
         assert(isValidUrl(MICROSOFT_TEAMS_WEBHOOK_URL))

diff --git a/...ications/core/src/main/kotlin/org/opensearch/notifications/core/NotificationCorePlugin.kt b/...ications/core/src/main/kotlin/org/opensearch/notifications/core/NotificationCorePlugin.kt
@@ -41,6 +41,7 @@ class NotificationCorePlugin : ReloadablePlugin, Plugin(), ExtensiblePlugin {
         const val PLUGIN_NAME = "opensearch-notifications-core"
         const val LOG_PREFIX = "notifications-core"
     }
+
     /**
      * {@inheritDoc}
      */

diff --git a/...ns/core/src/main/kotlin/org/opensearch/notifications/core/client/DestinationHttpClient.kt b/...ns/core/src/main/kotlin/org/opensearch/notifications/core/client/DestinationHttpClient.kt
@@ -57,6 +57,7 @@ class DestinationHttpClient {
         this.httpClient = createHttpClient(connectionManager, false)
         this.httpClientDisableCookie = createHttpClient(connectionManager, true)
     }
+
     @OpenForTesting
     constructor(httpClient: CloseableHttpClient) {
         this.httpClient = httpClient
@@ -65,15 +66,20 @@ class DestinationHttpClient {
 
     companion object {
         private val log by logger(DestinationHttpClient::class.java)
+
         /**
          * all valid response status
          */
         private val VALID_RESPONSE_STATUS = Collections.unmodifiableSet(
             HashSet(
                 listOf(
-                    RestStatus.OK.status, RestStatus.CREATED.status, RestStatus.ACCEPTED.status,
-                    RestStatus.NON_AUTHORITATIVE_INFORMATION.status, RestStatus.NO_CONTENT.status,
-                    RestStatus.RESET_CONTENT.status, RestStatus.PARTIAL_CONTENT.status,
+                    RestStatus.OK.status,
+                    RestStatus.CREATED.status,
+                    RestStatus.ACCEPTED.status,
+                    RestStatus.NON_AUTHORITATIVE_INFORMATION.status,
+                    RestStatus.NO_CONTENT.status,
+                    RestStatus.RESET_CONTENT.status,
+                    RestStatus.PARTIAL_CONTENT.status,
                     RestStatus.MULTI_STATUS.status
                 )
             )

diff --git a/...ications/core/src/main/kotlin/org/opensearch/notifications/core/setting/PluginSettings.kt b/...ications/core/src/main/kotlin/org/opensearch/notifications/core/setting/PluginSettings.kt
@@ -308,19 +308,22 @@ internal object PluginSettings {
         EMAIL_SIZE_LIMIT_KEY,
         defaultSettings[EMAIL_SIZE_LIMIT_KEY]!!.toInt(),
         MINIMUM_EMAIL_SIZE_LIMIT,
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val EMAIL_MINIMUM_HEADER_LENGTH: Setting<Int> = Setting.intSetting(
         EMAIL_MINIMUM_HEADER_LENGTH_KEY,
         defaultSettings[EMAIL_MINIMUM_HEADER_LENGTH_KEY]!!.toInt(),
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val MAX_CONNECTIONS: Setting<Int> = Setting.intSetting(
         MAX_CONNECTIONS_KEY,
         defaultSettings[MAX_CONNECTIONS_KEY]!!.toInt(),
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val DISABLE_HTTP_COOKIE: Setting<Boolean> = Setting.boolSetting(
@@ -332,53 +335,62 @@ internal object PluginSettings {
     val MAX_CONNECTIONS_PER_ROUTE: Setting<Int> = Setting.intSetting(
         MAX_CONNECTIONS_PER_ROUTE_KEY,
         defaultSettings[MAX_CONNECTIONS_PER_ROUTE_KEY]!!.toInt(),
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val CONNECTION_TIMEOUT_MILLISECONDS: Setting<Int> = Setting.intSetting(
         CONNECTION_TIMEOUT_MILLISECONDS_KEY,
         defaultSettings[CONNECTION_TIMEOUT_MILLISECONDS_KEY]!!.toInt(),
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val SOCKET_TIMEOUT_MILLISECONDS: Setting<Int> = Setting.intSetting(
         SOCKET_TIMEOUT_MILLISECONDS_KEY,
         defaultSettings[SOCKET_TIMEOUT_MILLISECONDS_KEY]!!.toInt(),
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val ALLOWED_CONFIG_TYPES: Setting<List<String>> = Setting.listSetting(
         ALLOWED_CONFIG_TYPE_KEY,
         DEFAULT_ALLOWED_CONFIG_TYPES,
         { it },
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val TOOLTIP_SUPPORT: Setting<Boolean> = Setting.boolSetting(
         TOOLTIP_SUPPORT_KEY,
         defaultSettings[TOOLTIP_SUPPORT_KEY]!!.toBoolean(),
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     val LEGACY_ALERTING_HOST_DENY_LIST: Setting<List<String>> = Setting.listSetting(
         LEGACY_ALERTING_HOST_DENY_LIST_KEY,
         DEFAULT_HOST_DENY_LIST,
         { it },
-        NodeScope, Final, Deprecated
+        NodeScope,
+        Final,
+        Deprecated
     )
 
     val ALERTING_HOST_DENY_LIST: Setting<List<String>> = Setting.listSetting(
         ALERTING_HOST_DENY_LIST_KEY,
         LEGACY_ALERTING_HOST_DENY_LIST,
         { it },
-        NodeScope, Final
+        NodeScope,
+        Final
     )
 
     val HOST_DENY_LIST: Setting<List<String>> = Setting.listSetting(
         HOST_DENY_LIST_KEY,
         ALERTING_HOST_DENY_LIST,
         { it },
-        NodeScope, Dynamic
+        NodeScope,
+        Dynamic
     )
 
     private val LEGACY_EMAIL_USERNAME: Setting.AffixSetting<SecureString> = Setting.affixKeySetting(
@@ -436,6 +448,7 @@ internal object PluginSettings {
             DISABLE_HTTP_COOKIE,
         )
     }
+
     /**
      * Update the setting variables to setting values from local settings
      * @param clusterService cluster service instance

diff --git a/...cations/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt b/...cations/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt
@@ -5,10 +5,12 @@
 
 package org.opensearch.notifications.core.utils
 
+import inet.ipaddr.HostName
 import inet.ipaddr.IPAddressString
 import org.apache.hc.client5.http.classic.methods.HttpPatch
 import org.apache.hc.client5.http.classic.methods.HttpPost
 import org.apache.hc.client5.http.classic.methods.HttpPut
+import org.apache.logging.log4j.LogManager
 import org.opensearch.core.common.Strings
 import java.net.URL
 
@@ -37,9 +39,12 @@ fun isHostInDenylist(urlString: String, hostDenyList: List<String>): Boolean {
     val url = URL(urlString)
     if (url.host != null) {
         val ipStr = IPAddressString(url.host)
+        val hostStr = HostName(url.host)
         for (network in hostDenyList) {
-            val netStr = IPAddressString(network)
-            if (netStr.contains(ipStr)) {
+            val denyIpStr = IPAddressString(network)
+            val denyHostStr = HostName(network)
+            if (denyIpStr.contains(ipStr) || denyHostStr.equals(hostStr)) {
+                LogManager.getLogger().error("${url.host} is denied")
                 return true
             }
         }

diff --git a/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt b/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/ChimeDestinationTests.kt
@@ -41,7 +41,7 @@ internal class ChimeDestinationTests {
                 Arguments.of("\t", """\t"""),
                 Arguments.of("\b", """\b"""),
                 Arguments.of("\r", """\r"""),
-                Arguments.of("\"", """\""""),
+                Arguments.of("\"", """\"""")
             )
     }
 

diff --git a/...st/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt b/...st/kotlin/org/opensearch/notifications/core/destinations/CustomWebhookDestinationTests.kt
@@ -45,14 +45,15 @@ internal class CustomWebhookDestinationTests {
                 Arguments.of("PUT", HttpPut::class.java),
                 Arguments.of("PATCH", HttpPatch::class.java)
             )
+
         @JvmStatic
         fun escapeSequenceToRaw(): Stream<Arguments> =
             Stream.of(
                 Arguments.of("\n", """\n"""),
                 Arguments.of("\t", """\t"""),
                 Arguments.of("\b", """\b"""),
                 Arguments.of("\r", """\r"""),
-                Arguments.of("\"", """\""""),
+                Arguments.of("\"", """\"""")
             )
     }
 

diff --git a/...t/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt b/...t/kotlin/org/opensearch/notifications/core/destinations/MicrosoftTeamsDestinationTests.kt
@@ -43,7 +43,7 @@ internal class MicrosoftTeamsDestinationTests {
                 Arguments.of("\t", """\t"""),
                 Arguments.of("\b", """\b"""),
                 Arguments.of("\r", """\r"""),
-                Arguments.of("\"", """\""""),
+                Arguments.of("\"", """\"""")
             )
     }
 

diff --git a/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt b/...e/src/test/kotlin/org/opensearch/notifications/core/destinations/SlackDestinationTests.kt
@@ -43,7 +43,7 @@ internal class SlackDestinationTests {
                 Arguments.of("\t", """\t"""),
                 Arguments.of("\b", """\b"""),
                 Arguments.of("\r", """\r"""),
-                Arguments.of("\"", """\""""),
+                Arguments.of("\"", """\"""")
             )
     }
 

diff --git a/notifications/core/src/test/kotlin/org/opensearch/notifications/core/smtp/SmtpEmailTests.kt b/notifications/core/src/test/kotlin/org/opensearch/notifications/core/smtp/SmtpEmailTests.kt
@@ -51,7 +51,7 @@ class SmtpEmailTests {
             "opensearch.data",
             "base64",
             "VGVzdCBtZXNzYWdlCgo=",
-            "application/octet-stream",
+            "application/octet-stream"
         )
         DestinationTransportProvider.destinationTransportMap = mapOf(DestinationType.SMTP to SmtpDestinationTransport())
         val response = NotificationCoreImpl.sendMessage(smtpDestination, message, "ref")
@@ -77,7 +77,7 @@ class SmtpEmailTests {
             "opensearch.data",
             "base64",
             "VGVzdCBtZXNzYWdlCgo=",
-            "application/octet-stream",
+            "application/octet-stream"
         )
         DestinationTransportProvider.destinationTransportMap = mapOf(DestinationType.SMTP to SmtpDestinationTransport())
         val response = NotificationCoreImpl.sendMessage(smtpDestination, message, "ref")

diff --git a/...ns/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt b/...ns/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt
@@ -10,7 +10,8 @@ import org.junit.jupiter.api.Test
 
 internal class ValidationHelpersTests {
 
-    private val hostDentyList = listOf(
+    private val hostDenyList = listOf(
+        "www.amazon.com",
         "127.0.0.0/8",
         "10.0.0.0/8",
         "172.16.0.0/12",
@@ -20,8 +21,9 @@ internal class ValidationHelpersTests {
     )
 
     @Test
-    fun `test ips in denylist`() {
+    fun `test hosts in denylist`() {
         val ips = listOf(
+            "www.amazon.com",
             "127.0.0.1", // 127.0.0.0/8
             "10.0.0.1", // 10.0.0.0/8
             "10.11.12.13", // 10.0.0.0/8
@@ -31,15 +33,15 @@ internal class ValidationHelpersTests {
             "9.9.9.9"
         )
         for (ip in ips) {
-            assertEquals(true, isHostInDenylist("https://$ip", hostDentyList))
+            assertEquals(true, isHostInDenylist("https://$ip", hostDenyList), "address $ip was supposed to be identified as in the deny list, but was not")
         }
     }
 
     @Test
-    fun `test url in denylist`() {
-        val urls = listOf("https://www.amazon.com", "https://mytest.com", "https://mytest.com")
+    fun `test hosts not in denylist`() {
+        val urls = listOf("156.4.77.1", "www.something.com")
         for (url in urls) {
-            assertEquals(false, isHostInDenylist(url, hostDentyList))
+            assertEquals(false, isHostInDenylist("https://$url", hostDenyList), "address $url was not supposed to be identified as in the deny list, but was")
         }
     }
 }
diff --git a/...otifications/src/main/kotlin/org/opensearch/notifications/send/SendMessageActionHelper.kt b/...otifications/src/main/kotlin/org/opensearch/notifications/send/SendMessageActionHelper.kt
@@ -603,6 +603,7 @@ object SendMessageActionHelper {
             DestinationMessageResponse(RestStatus.FAILED_DEPENDENCY.status, "Failed to send notification")
         }
     }
+
     /**
      * Collects all child configs of the channel configurations (like email)
      * @param channels list of NotificationConfigDocInfo

diff --git a/...tifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt b/...tifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt
@@ -46,6 +46,7 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() {
         val error = sendResponse.get("error").asJsonObject
         Assert.assertNotNull(error.get("reason").asString)
     }
+
     @Suppress("EmptyFunctionBlock")
     fun `test send test slack message`() {
         // Create webhook notification config
@@ -78,6 +79,7 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() {
         val error = sendResponse.get("error").asJsonObject
         Assert.assertNotNull(error.get("reason").asString)
     }
+
     @Suppress("EmptyFunctionBlock")
     fun `test send test microsoft teams message`() {
         // Create webhook notification config

diff --git a/...ications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageWithMockServerIT.kt b/...ications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageWithMockServerIT.kt
@@ -46,7 +46,10 @@ internal class SendTestMessageWithMockServerIT : PluginRestTestCase() {
 
         // send test message
         val sendResponse = executeRequest(
-            RestRequest.Method.POST.name, "$PLUGIN_BASE_URI/feature/test/$configId", "", RestStatus.OK.status
+            RestRequest.Method.POST.name,
+            "$PLUGIN_BASE_URI/feature/test/$configId",
+            "",
+            RestStatus.OK.status
         )
 
         logger.info(sendResponse)

diff --git a/...ications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt b/...ications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt
@@ -92,13 +92,19 @@ class ConfigIndexingActionsTests {
         fun initialize() {
             /* use reflection to get private method */
             validateMicrosoftTeamsConfig = ConfigIndexingActions::class.java.getDeclaredMethod(
-                "validateMicrosoftTeamsConfig", MicrosoftTeams::class.java, User::class.java
+                "validateMicrosoftTeamsConfig",
+                MicrosoftTeams::class.java,
+                User::class.java
             )
             validateSlackConfig = ConfigIndexingActions::class.java.getDeclaredMethod(
-                "validateSlackConfig", Slack::class.java, User::class.java
+                "validateSlackConfig",
+                Slack::class.java,
+                User::class.java
             )
             validateChimeConfig = ConfigIndexingActions::class.java.getDeclaredMethod(
-                "validateChimeConfig", Chime::class.java, User::class.java
+                "validateChimeConfig",
+                Chime::class.java,
+                User::class.java
             )
 
             validateMicrosoftTeamsConfig.isAccessible = true
-Original file line number
+Diff line change
@@ Expand Up @@
             const val PLUGIN_NAME = "opensearch-notifications-core"
             const val LOG_PREFIX = "notifications-core"
         }
         /**
          * {@inheritDoc}
          */
@@ Expand Down @@