Skip to content

Commit

Permalink
Remove usage of allowedConfigFeatures (#387)
Browse files Browse the repository at this point in the history
* Remove allowConfigFeatures from GetPluginFeaturesResponse and NotificationCore interface

Signed-off-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>

* Remove allowedConfigFeatures plugin setting

Signed-off-by: Mohammad Qureshi <47198598+qreshi@users.noreply.github.com>
  • Loading branch information
qreshi authored Mar 28, 2022
1 parent ef43aad commit bc4369a
Show file tree
Hide file tree
Showing 9 changed files with 2 additions and 92 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ interface NotificationCore {
*/
fun getAllowedConfigTypes(): List<String>

/**
* Get list of allowed config features
*/
fun getAllowedConfigFeatures(): List<String>

/**
* Get map of plugin features
*/
Expand Down
1 change: 0 additions & 1 deletion notifications/core/src/main/config/notifications-core.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,4 @@ opensearch.notifications.core:
socketTimeout: 50000
hostDenyList: []
allowedConfigTypes: ["slack","chime","webhook","email","sns","ses_account","smtp_account","email_group"]
allowedConfigFeatures: ["alerting", "index_management", "reports"]
tooltipSupport: true
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,6 @@ object NotificationCoreImpl : NotificationCore {
)
}

/**
* Get list of allowed config features
*/
override fun getAllowedConfigFeatures(): List<String> {
return AccessController.doPrivileged(
PrivilegedAction {
PluginSettings.allowedConfigFeatures
} as PrivilegedAction<List<String>>?
)
}

/**
* Get map of plugin features
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,6 @@ internal object PluginSettings {
*/
private const val ALLOWED_CONFIG_TYPE_KEY = "$KEY_PREFIX.allowedConfigTypes"

/**
* Setting to choose allowed config features.
*/
private const val ALLOWED_CONFIG_FEATURE_KEY = "$KEY_PREFIX.allowedConfigFeatures"

/**
* Setting to enable tooltip in UI
*/
Expand Down Expand Up @@ -141,15 +136,6 @@ internal object PluginSettings {
"email_group"
)

/**
* Default config feature list
*/
private val DEFAULT_ALLOWED_CONFIG_FEATURES = listOf(
"alerting",
"index_management",
"reports"
)

/**
* Default email host deny list
*/
Expand All @@ -171,12 +157,6 @@ internal object PluginSettings {
@Volatile
var allowedConfigTypes: List<String>

/**
* list of allowed config features.
*/
@Volatile
var allowedConfigFeatures: List<String>

/**
* Email size limit setting
*/
Expand Down Expand Up @@ -258,7 +238,6 @@ internal object PluginSettings {
?: DEFAULT_CONNECTION_TIMEOUT_MILLISECONDS
socketTimeout = (settings?.get(SOCKET_TIMEOUT_MILLISECONDS_KEY)?.toInt()) ?: DEFAULT_SOCKET_TIMEOUT_MILLISECONDS
allowedConfigTypes = settings?.getAsList(ALLOWED_CONFIG_TYPE_KEY, null) ?: DEFAULT_ALLOWED_CONFIG_TYPES
allowedConfigFeatures = settings?.getAsList(ALLOWED_CONFIG_FEATURE_KEY, null) ?: DEFAULT_ALLOWED_CONFIG_FEATURES
tooltipSupport = settings?.getAsBoolean(TOOLTIP_SUPPORT_KEY, true) ?: DEFAULT_TOOLTIP_SUPPORT
hostDenyList = settings?.getAsList(HOST_DENY_LIST_KEY, null) ?: DEFAULT_HOST_DENY_LIST
destinationSettings = if (settings != null) loadDestinationSettings(settings) else DEFAULT_DESTINATION_SETTINGS
Expand Down Expand Up @@ -318,13 +297,6 @@ internal object PluginSettings {
NodeScope, Dynamic
)

val ALLOWED_CONFIG_FEATURES: Setting<List<String>> = Setting.listSetting(
ALLOWED_CONFIG_FEATURE_KEY,
DEFAULT_ALLOWED_CONFIG_FEATURES,
{ it },
NodeScope, Dynamic
)

val TOOLTIP_SUPPORT: Setting<Boolean> = Setting.boolSetting(
TOOLTIP_SUPPORT_KEY,
defaultSettings[TOOLTIP_SUPPORT_KEY]!!.toBoolean(),
Expand Down Expand Up @@ -364,7 +336,6 @@ internal object PluginSettings {
CONNECTION_TIMEOUT_MILLISECONDS,
SOCKET_TIMEOUT_MILLISECONDS,
ALLOWED_CONFIG_TYPES,
ALLOWED_CONFIG_FEATURES,
TOOLTIP_SUPPORT,
HOST_DENY_LIST,
EMAIL_USERNAME,
Expand All @@ -377,7 +348,6 @@ internal object PluginSettings {
*/
private fun updateSettingValuesFromLocal(clusterService: ClusterService) {
allowedConfigTypes = ALLOWED_CONFIG_TYPES.get(clusterService.settings)
allowedConfigFeatures = ALLOWED_CONFIG_FEATURES.get(clusterService.settings)
emailSizeLimit = EMAIL_SIZE_LIMIT.get(clusterService.settings)
emailMinimumHeaderLength = EMAIL_MINIMUM_HEADER_LENGTH.get(clusterService.settings)
maxConnections = MAX_CONNECTIONS.get(clusterService.settings)
Expand Down Expand Up @@ -430,11 +400,6 @@ internal object PluginSettings {
log.debug("$LOG_PREFIX:$ALLOWED_CONFIG_TYPE_KEY -autoUpdatedTo-> $clusterAllowedConfigTypes")
allowedConfigTypes = clusterAllowedConfigTypes
}
val clusterAllowedConfigFeatures = clusterService.clusterSettings.get(ALLOWED_CONFIG_FEATURES)
if (clusterAllowedConfigFeatures != null) {
log.debug("$LOG_PREFIX:$ALLOWED_CONFIG_FEATURE_KEY -autoUpdatedTo-> $clusterAllowedConfigFeatures")
allowedConfigFeatures = clusterAllowedConfigFeatures
}
val clusterTooltipSupport = clusterService.clusterSettings.get(TOOLTIP_SUPPORT)
if (clusterTooltipSupport != null) {
log.debug("$LOG_PREFIX:$TOOLTIP_SUPPORT_KEY -autoUpdatedTo-> $clusterTooltipSupport")
Expand All @@ -461,10 +426,6 @@ internal object PluginSettings {
allowedConfigTypes = it
log.info("$LOG_PREFIX:$ALLOWED_CONFIG_TYPE_KEY -updatedTo-> $it")
}
clusterService.clusterSettings.addSettingsUpdateConsumer(ALLOWED_CONFIG_FEATURES) {
allowedConfigFeatures = it
log.info("$LOG_PREFIX:$ALLOWED_CONFIG_FEATURE_KEY -updatedTo-> $it")
}
clusterService.clusterSettings.addSettingsUpdateConsumer(EMAIL_SIZE_LIMIT) {
emailSizeLimit = it
log.info("$LOG_PREFIX:$EMAIL_SIZE_LIMIT_KEY -updatedTo-> $it")
Expand Down Expand Up @@ -542,7 +503,6 @@ internal object PluginSettings {
connectionTimeout = DEFAULT_CONNECTION_TIMEOUT_MILLISECONDS
socketTimeout = DEFAULT_SOCKET_TIMEOUT_MILLISECONDS
allowedConfigTypes = DEFAULT_ALLOWED_CONFIG_TYPES
allowedConfigFeatures = DEFAULT_ALLOWED_CONFIG_FEATURES
tooltipSupport = DEFAULT_TOOLTIP_SUPPORT
hostDenyList = DEFAULT_HOST_DENY_LIST
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class NotificationCoreImplTests {
@Test
fun `test all get configs APIs return the default value`() {
assertEquals(defaultConfigTypes, NotificationCoreImpl.getAllowedConfigTypes())
assertEquals(defaultConfigFeatures, NotificationCoreImpl.getAllowedConfigFeatures())
assertEquals(defaultPluginFeatures, NotificationCoreImpl.getPluginFeatures())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ internal class PluginSettingsTests {
private val httpSocketTimeoutKey = "$httpKeyPrefix.socketTimeout"
private val httpHostDenyListKey = "$httpKeyPrefix.hostDenyList"
private val allowedConfigTypeKey = "$keyPrefix.allowedConfigTypes"
private val allowedConfigFeatureKey = "$keyPrefix.allowedConfigFeatures"
private val tooltipSupportKey = "$keyPrefix.tooltipSupport"

private val defaultSettings = Settings.builder()
Expand All @@ -57,14 +56,6 @@ internal class PluginSettingsTests {
"email_group"
)
)
.putList(
allowedConfigFeatureKey,
listOf(
"alerting",
"index_management",
"reports"
)
)
.put(tooltipSupportKey, true)
.build()

Expand Down Expand Up @@ -93,7 +84,6 @@ internal class PluginSettingsTests {
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.ALLOWED_CONFIG_FEATURES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.HOST_DENY_LIST
)
Expand Down Expand Up @@ -124,10 +114,6 @@ internal class PluginSettingsTests {
defaultSettings[allowedConfigTypeKey],
PluginSettings.allowedConfigTypes.toString()
)
Assertions.assertEquals(
defaultSettings[allowedConfigFeatureKey],
PluginSettings.allowedConfigFeatures.toString()
)
Assertions.assertEquals(
defaultSettings[tooltipSupportKey],
PluginSettings.tooltipSupport.toString()
Expand All @@ -149,7 +135,6 @@ internal class PluginSettingsTests {
.put(httpSocketTimeoutKey, 100)
.putList(httpHostDenyListKey, listOf("sample"))
.putList(allowedConfigTypeKey, listOf("slack"))
.putList(allowedConfigFeatureKey, listOf("alerting"))
.put(tooltipSupportKey, false)
.build()

Expand All @@ -165,7 +150,6 @@ internal class PluginSettingsTests {
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.ALLOWED_CONFIG_FEATURES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.HOST_DENY_LIST
)
Expand Down Expand Up @@ -200,10 +184,6 @@ internal class PluginSettingsTests {
listOf("slack"),
clusterService.clusterSettings.get(PluginSettings.ALLOWED_CONFIG_TYPES)
)
Assertions.assertEquals(
listOf("alerting"),
clusterService.clusterSettings.get(PluginSettings.ALLOWED_CONFIG_FEATURES)
)
Assertions.assertEquals(
false,
clusterService.clusterSettings.get(PluginSettings.TOOLTIP_SUPPORT)
Expand All @@ -225,7 +205,6 @@ internal class PluginSettingsTests {
PluginSettings.CONNECTION_TIMEOUT_MILLISECONDS,
PluginSettings.SOCKET_TIMEOUT_MILLISECONDS,
PluginSettings.ALLOWED_CONFIG_TYPES,
PluginSettings.ALLOWED_CONFIG_FEATURES,
PluginSettings.TOOLTIP_SUPPORT,
PluginSettings.HOST_DENY_LIST
)
Expand Down Expand Up @@ -260,10 +239,6 @@ internal class PluginSettingsTests {
defaultSettings[allowedConfigTypeKey],
clusterService.clusterSettings.get(PluginSettings.ALLOWED_CONFIG_TYPES).toString()
)
Assertions.assertEquals(
defaultSettings[allowedConfigFeatureKey],
clusterService.clusterSettings.get(PluginSettings.ALLOWED_CONFIG_FEATURES).toString()
)
Assertions.assertEquals(
defaultSettings[tooltipSupportKey],
clusterService.clusterSettings.get(PluginSettings.TOOLTIP_SUPPORT).toString()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ internal class GetPluginFeaturesAction @Inject constructor(
user: User?
): GetPluginFeaturesResponse {
val allowedConfigTypes = CoreProvider.core.getAllowedConfigTypes()
val allowedConfigFeatures = CoreProvider.core.getAllowedConfigFeatures()
val pluginFeatures = CoreProvider.core.getPluginFeatures()
return GetPluginFeaturesResponse(
allowedConfigTypes,
allowedConfigFeatures,
pluginFeatures
)
return GetPluginFeaturesResponse(allowedConfigTypes, pluginFeatures)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class GetPluginFeaturesIT : PluginRestTestCase() {
RestStatus.OK.status
)
Assert.assertFalse(getResponse.get("allowed_config_type_list").asJsonArray.isEmpty)
Assert.assertFalse(getResponse.get("allowed_config_feature_list").asJsonArray.isEmpty)
val pluginFeatures = getResponse.get("plugin_features").asJsonObject
Assert.assertFalse(pluginFeatures.keySet().isEmpty())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,9 @@ internal class PluginActionTests {
@Test
fun `Get plugin features action should call back action listener`() {
val allowedConfigTypes = listOf("type1")
val allowedConfigFeatures = listOf("feature1")
val pluginFeatures = mapOf(Pair("FeatureKey1", "Feature1"))
val request = mock(GetPluginFeaturesRequest::class.java)
val response = GetPluginFeaturesResponse(allowedConfigTypes, allowedConfigFeatures, pluginFeatures)
val response = GetPluginFeaturesResponse(allowedConfigTypes, pluginFeatures)

val getPluginFeaturesAction = GetPluginFeaturesAction(
transportService, client, actionFilters, xContentRegistry
Expand Down

0 comments on commit bc4369a

Please sign in to comment.