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

Deprecate Hipchat Watcher actions #39160

Merged
merged 7 commits into from Feb 20, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/reference/settings/notification-settings.asciidoc
Expand Up @@ -200,6 +200,8 @@ You can configure the following HipChat notification settings in
`elasticsearch.yml`. For more information about sending notifications
via HipChat, see {xpack-ref}/actions-hipchat.html#configuring-hipchat-actions[Configuring HipChat].

deprecated[6.7.0,Hipchat notification has been deprecated as Hipchat has ceased operation]

`xpack.notification.hipchat` ::
Specifies account information for sending notifications
via HipChat. You can specify the following HipChat account attributes:
Expand Down
2 changes: 2 additions & 0 deletions x-pack/docs/en/watcher/actions/hipchat.asciidoc
@@ -1,6 +1,8 @@
[[actions-hipchat]]
=== HipChat Action

deprecated[6.7.0,Hipchat actions have been deprecated as Hipchat has ceased operation]

Use the `hipchat` action to send messages to https://www.hipchat.com[HipChat]
rooms or users. To send HipChat messages, you must
<<configuring-hipchat, configure at least one HipChat account>> in `elasticsearch.yml`.
Expand Down
Expand Up @@ -57,6 +57,7 @@ private DeprecationChecks() {
NodeDeprecationChecks::tlsv1ProtocolDisabled,
NodeDeprecationChecks::transportSslEnabledWithoutSecurityEnabled,
NodeDeprecationChecks::watcherNotificationsSecureSettingsCheck,
NodeDeprecationChecks::watcherHipchatNotificationSettingsCheck,
NodeDeprecationChecks::auditIndexSettingsCheck
));

Expand Down
Expand Up @@ -154,7 +154,6 @@ static DeprecationIssue discoveryConfigurationCheck(Settings nodeSettings, Plugi

static DeprecationIssue watcherNotificationsSecureSettingsCheck(Settings nodeSettings, PluginsAndModules plugins) {
if (false == nodeSettings.getByPrefix("xpack.notification.email.account.").filter(s -> s.endsWith(".smtp.password")).isEmpty()
|| false == nodeSettings.getByPrefix("xpack.notification.hipchat.account.").filter(s -> s.endsWith(".auth_token")).isEmpty()
|| false == nodeSettings.getByPrefix("xpack.notification.jira.account.")
.filter(s -> s.endsWith(".url") || s.endsWith(".user") || s.endsWith(".password")).isEmpty()
|| false == nodeSettings.getByPrefix("xpack.notification.pagerduty.account.")
Expand All @@ -169,6 +168,17 @@ static DeprecationIssue watcherNotificationsSecureSettingsCheck(Settings nodeSet
return null;
}

static DeprecationIssue watcherHipchatNotificationSettingsCheck(Settings nodeSettings, PluginsAndModules plugins) {
dakrone marked this conversation as resolved.
Show resolved Hide resolved
if (nodeSettings.getByPrefix("xpack.notification.hipchat.").size() > 0) {
return new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be a warning since it only possibly an issue. @gwbrown - thoughts on critical vs. warning ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Depends on how 7.0 behaves if you leave this setting in place. Most removed node settings are CRITICAL because Elasticsearch will refuse to start if you have unrecognized settings in elasticsearch.yml. I think that's the case here, so CRITICAL is the correct level, although I could be wrong if Watcher is doing something funny with prefix settings.

"Watcher Hipchat notifications will be removed in the next major release",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html" +
"#watcher-notifications-account-settings",
"[hipchat] actions are deprecated and should be removed from watch definitions");
}
return null;
}

static DeprecationIssue azureRepositoryChanges(Settings nodeSettings, PluginsAndModules plugins) {
if (plugins.getPluginInfos().stream().anyMatch(pluginInfo -> "repository-azure".equals(pluginInfo.getName()))) {
return new DeprecationIssue(DeprecationIssue.Level.WARNING,
Expand Down
Expand Up @@ -162,13 +162,6 @@ public void testWatcherNotificationsSecureSettings() {
"https://www.elastic.co/guide/en/elasticsearch/reference/master/breaking-changes-7.0.html"
+ "#watcher-notifications-account-settings",
"account authentication settings must use the keystore");
assertSettingsAndIssue("xpack.notification.hipchat.account." + randomAlphaOfLength(4) + ".auth_token", randomAlphaOfLength(4),
expected);
expected = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
"Watcher notification accounts' authentication settings must be defined securely",
"https://www.elastic.co/guide/en/elasticsearch/reference/master/breaking-changes-7.0.html"
+ "#watcher-notifications-account-settings",
"account authentication settings must use the keystore");
assertSettingsAndIssue("xpack.notification.jira.account." + randomAlphaOfLength(4) + ".url", randomAlphaOfLength(4), expected);
assertSettingsAndIssue("xpack.notification.jira.account." + randomAlphaOfLength(4) + ".user", randomAlphaOfLength(4), expected);
assertSettingsAndIssue("xpack.notification.jira.account." + randomAlphaOfLength(4) + ".password",
Expand All @@ -188,6 +181,16 @@ public void testWatcherNotificationsSecureSettings() {
assertSettingsAndIssue("xpack.notification.slack.account." + randomAlphaOfLength(4) + ".url", randomAlphaOfLength(4), expected);
}

public void testWatcherHipchatSettings() {
DeprecationIssue expected = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
"Watcher Hipchat notifications will be removed in the next major release",
"https://www.elastic.co/guide/en/elasticsearch/reference/7.0/breaking-changes-7.0.html" +
"#watcher-notifications-account-settings",
"[hipchat] actions are deprecated and should be removed from watch definitions");
assertSettingsAndIssues(Settings.builder().put("xpack.notification.hipchat.account.profile", randomAlphaOfLength(4)).build(),
expected);
}

public void testTribeNodeCheck() {
String tribeSetting = "tribe." + randomAlphaOfLengthBetween(1, 20) + ".cluster.name";
DeprecationIssue expected = new DeprecationIssue(DeprecationIssue.Level.CRITICAL,
Expand Down
Expand Up @@ -20,6 +20,10 @@
import java.io.IOException;
import java.util.Objects;

/**
* @deprecated Hipchat actions will be removed in Elasticsearch 7.0 since Hipchat is defunct
*/
@Deprecated
public class HipChatAction implements Action {

public static final String TYPE = "hipchat";
Expand Down
Expand Up @@ -6,6 +6,7 @@
package org.elasticsearch.xpack.watcher.actions.hipchat;

import org.apache.logging.log4j.LogManager;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.xpack.core.watcher.actions.ActionFactory;
import org.elasticsearch.xpack.watcher.common.text.TextTemplateEngine;
Expand All @@ -18,6 +19,7 @@ public class HipChatActionFactory extends ActionFactory {

private final TextTemplateEngine templateEngine;
private final HipChatService hipchatService;
private final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(HipChatActionFactory.class));

public HipChatActionFactory(TextTemplateEngine templateEngine, HipChatService hipchatService) {
super(LogManager.getLogger(ExecutableHipChatAction.class));
Expand All @@ -27,6 +29,7 @@ public HipChatActionFactory(TextTemplateEngine templateEngine, HipChatService hi

@Override
public ExecutableHipChatAction parseExecutable(String watchId, String actionId, XContentParser parser) throws IOException {
deprecationLogger.deprecatedAndMaybeLog("hipchat_action", "hipchat actions are deprecated and will be removed in 7.0");
HipChatAction action = HipChatAction.parse(watchId, actionId, parser);
HipChatAccount account = hipchatService.getAccount(action.account);
account.validateParsedTemplate(watchId, actionId, action.message);
Expand Down
Expand Up @@ -48,6 +48,7 @@ public void testParseAction() throws Exception {
assertThat(parsedAction.action(), is(action));

verify(account, times(1)).validateParsedTemplate("_w1", "_a1", action.message);
assertWarnings("hipchat actions are deprecated and will be removed in 7.0");
}

public void testParseActionUnknownAccount() throws Exception {
Expand All @@ -59,5 +60,6 @@ public void testParseActionUnknownAccount() throws Exception {
XContentParser parser = createParser(jsonBuilder);
parser.nextToken();
expectThrows(IllegalArgumentException.class, () -> factory.parseExecutable("_w1", "_a1", parser));
assertWarnings("hipchat actions are deprecated and will be removed in 7.0");
}
}