SecureSettings in NotificationService are buggy

[albertzaharovits]

NotificationService implementations (eg SlackService, PagerDutyService, ..) are buggy.

They fail the communication service if the secure_* variant of the settings are put in the cluster state as opposed to inside the keystore, as documented https://www.elastic.co/guide/en/elastic-stack-overview/6.3/actions-pagerduty.html#configuring-pagerduty (for eg).

The normal behavior should be to simply ignore the setting when in the cluster state. In this case the service would fail to start and complain that the setting is not found in the keystore.

[elasticmachine]

Pinging @elastic/es-core-infra

[jakommo]

The most easy repro I found for this is:
On a vanilla 6.4.3, run bin/elasticsearch-keystore add xpack.notification.pagerduty.account.my_pagerduty_account.secure_service_api_key and add a random value, start Elasticsearch and try to update a cluster setting i.e. curl -H "Content-Type: application/json" -u elastic:XXX -XPUT 'http://localhost:9200/_cluster/settings' -d'{"transient": {"cluster.routing.allocation.enable": "none"}}', this results in:

[2018-11-14T11:41:47,819][DEBUG][o.e.a.a.c.s.TransportClusterUpdateSettingsAction] [oh63Ywq] failed to perform [cluster_update_settings]
java.lang.IllegalStateException: setting [xpack.notification.pagerduty.account.my_pagerduty_account.secure_service_api_key] is not dynamic
	at org.elasticsearch.common.settings.Setting.newUpdater(Setting.java:506) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.settings.Setting$AffixSetting$1.lambda$getValue$2(Setting.java:636) ~[elasticsearch-6.4.3.jar:6.4.3]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) ~[?:1.8.0_192]
	at java.util.stream.DistinctOps$1$2.accept(DistinctOps.java:175) ~[?:1.8.0_192]
	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) ~[?:1.8.0_192]
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175) ~[?:1.8.0_192]
	at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1556) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) ~[?:1.8.0_192]
	at java.util.stream.StreamSpliterators$WrappingSpliterator.forEachRemaining(StreamSpliterators.java:312) ~[?:1.8.0_192]
	at java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:742) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) ~[?:1.8.0_192]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) ~[?:1.8.0_192]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:1.8.0_192]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418) ~[?:1.8.0_192]
	at org.elasticsearch.common.settings.Setting$AffixSetting$1.getValue(Setting.java:632) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.settings.Setting$AffixSetting$1.getValue(Setting.java:621) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.settings.AbstractScopedSettings.validateUpdate(AbstractScopedSettings.java:134) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.action.admin.cluster.settings.SettingsUpdater.updateSettings(SettingsUpdater.java:126) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.action.admin.cluster.settings.TransportClusterUpdateSettingsAction$1.execute(TransportClusterUpdateSettingsAction.java:183) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.ClusterStateUpdateTask.execute(ClusterStateUpdateTask.java:45) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService.executeTasks(MasterService.java:639) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:268) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService.runTasks(MasterService.java:198) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService$Batcher.run(MasterService.java:133) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:150) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:188) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:624) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedEsThreadPoolExecutor.java:244) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:207) [elasticsearch-6.4.3.jar:6.4.3]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_192]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_192]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_192]
[2018-11-14T11:41:47,830][WARN ][r.suppressed             ] path: /_cluster/settings, params: {}
java.lang.IllegalStateException: setting [xpack.notification.pagerduty.account.my_pagerduty_account.secure_service_api_key] is not dynamic
	at org.elasticsearch.common.settings.Setting.newUpdater(Setting.java:506) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.settings.Setting$AffixSetting$1.lambda$getValue$2(Setting.java:636) ~[elasticsearch-6.4.3.jar:6.4.3]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184) ~[?:1.8.0_192]
	at java.util.stream.DistinctOps$1$2.accept(DistinctOps.java:175) ~[?:1.8.0_192]
	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193) ~[?:1.8.0_192]
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:175) ~[?:1.8.0_192]
	at java.util.HashMap$KeySpliterator.forEachRemaining(HashMap.java:1556) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) ~[?:1.8.0_192]
	at java.util.stream.StreamSpliterators$WrappingSpliterator.forEachRemaining(StreamSpliterators.java:312) ~[?:1.8.0_192]
	at java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:742) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) ~[?:1.8.0_192]
	at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151) ~[?:1.8.0_192]
	at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174) ~[?:1.8.0_192]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:1.8.0_192]
	at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418) ~[?:1.8.0_192]
	at org.elasticsearch.common.settings.Setting$AffixSetting$1.getValue(Setting.java:632) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.settings.Setting$AffixSetting$1.getValue(Setting.java:621) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.settings.AbstractScopedSettings.validateUpdate(AbstractScopedSettings.java:134) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.action.admin.cluster.settings.SettingsUpdater.updateSettings(SettingsUpdater.java:126) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.action.admin.cluster.settings.TransportClusterUpdateSettingsAction$1.execute(TransportClusterUpdateSettingsAction.java:183) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.ClusterStateUpdateTask.execute(ClusterStateUpdateTask.java:45) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService.executeTasks(MasterService.java:639) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService.calculateTaskOutputs(MasterService.java:268) ~[elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService.runTasks(MasterService.java:198) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.MasterService$Batcher.run(MasterService.java:133) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.TaskBatcher.runIfNotProcessed(TaskBatcher.java:150) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.cluster.service.TaskBatcher$BatchedTask.run(TaskBatcher.java:188) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:624) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.runAndClean(PrioritizedEsThreadPoolExecutor.java:244) [elasticsearch-6.4.3.jar:6.4.3]
	at org.elasticsearch.common.util.concurrent.PrioritizedEsThreadPoolExecutor$TieBreakingPrioritizedRunnable.run(PrioritizedEsThreadPoolExecutor.java:207) [elasticsearch-6.4.3.jar:6.4.3]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_192]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_192]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_192]

For pager duty, the only workaround I found was to remove the secure setting from the keystore and use the "old" way of setting it via elasticsearch.yml.