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

Remove usage of allowedConfigFeatures #387

Merged
merged 2 commits into from
Mar 28, 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 @@ -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
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