Skip to content

Commit

Permalink
feat(slack): Make slack base url configurable (#633)
Browse files Browse the repository at this point in the history
* feat(slack): Make slack base url configurable

Allows to use slack notifications with compatible implementations

* Update echo-notifications/src/main/groovy/com/netflix/spinnaker/echo/config/SlackConfig.groovy

Co-Authored-By: Mark Vulfson <markvu@live.com>

* Change slackCompatibleAPI to forceUseIncomingWebhook.

* Improve slack config logging

Always print the url we're hitting and if using chat api or incoming
webhooks
  • Loading branch information
diasjorge authored and marchello2000 committed Oct 7, 2019
1 parent a8f8909 commit d41d723
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,24 @@ class SlackConfig {
final static String SLACK_INCOMING_WEBHOOK = 'https://hooks.slack.com'
final static String SLACK_CHAT_API = 'https://slack.com'

@Value('${slack.base-url:}')
String slackBaseUrl

@Value('${slack.force-use-incoming-webhook:false}')
Boolean forceUseIncomingWebhook;

@Bean
Endpoint slackEndpoint(@Qualifier("useIncomingWebHook") boolean useIncomingWebHook) {
log.info("Using Slack incoming webhooks: {}.", useIncomingWebHook)
String endpoint = useIncomingWebHook ? SLACK_INCOMING_WEBHOOK : SLACK_CHAT_API;
String endpoint;

if (StringUtils.isNotBlank(slackBaseUrl)) {
endpoint = slackBaseUrl
} else {
endpoint = useIncomingWebHook ? SLACK_INCOMING_WEBHOOK : SLACK_CHAT_API;
}

log.info("Using Slack {}: {}.", useIncomingWebHook ? "incoming webhook" : "chat api", endpoint)

newFixedEndpoint(endpoint)
}

Expand All @@ -73,7 +87,11 @@ class SlackConfig {

@Bean(name="useIncomingWebHook")
boolean useIncomingWebHook(@Value('${slack.token:}') String token) {
return StringUtils.isNotBlank(token) && token.count("/") >= 2
return forceUseIncomingWebhook || isIncomingWebhookToken(token)
}

def boolean isIncomingWebhookToken(String token) {
return (StringUtils.isNotBlank(token) && token.count("/") >= 2)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import spock.lang.Specification
import spock.lang.Subject

class SlackConfigSpec extends Specification {
@Subject SlackConfig slackConfig = new SlackConfig()
@Subject
SlackConfig slackConfig = new SlackConfig()

def 'test slack incoming web hook is inferred correctly'() {
given:
Expand All @@ -25,4 +26,49 @@ class SlackConfigSpec extends Specification {
"" | "https://slack.com" | false
null | "https://slack.com" | false
}

def 'test slack incoming web hook base url is defined'() {
given:

when:
slackConfig.setSlackBaseUrl(baseUrl)
def useIncomingHook = slackConfig.useIncomingWebHook(token)
def endpoint = slackConfig.slackEndpoint(useIncomingHook)


then:
useIncomingHook == expectedUseIncomingWebHook
endpoint.url == expectedEndpoint

where:
token | baseUrl | expectedEndpoint | expectedUseIncomingWebHook
"myOldFashionToken" | "https://example.com" | "https://example.com" | false
"myOldFashionToken" | "" | "https://slack.com" | false
"myOldFashionToken" | null | "https://slack.com" | false
"T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX" | "https://example.com" | "https://example.com" | true
"T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX" | "" | "https://hooks.slack.com" | true
"T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX" | null | "https://hooks.slack.com" | true
}

def 'test slack incoming web hook when forceUseIncomingWebhook'() {
given:
slackConfig.forceUseIncomingWebhook = true
slackConfig.slackBaseUrl = 'https://example.com'

when:
def useIncomingHook = slackConfig.useIncomingWebHook(token)
def endpoint = slackConfig.slackEndpoint(useIncomingHook)

then:
useIncomingHook == expectedUseIncomingWebHook
endpoint.url == expectedEndpoint

where:
token | expectedEndpoint | expectedUseIncomingWebHook
"myOldFashionToken" | "https://example.com" | true
"T00000000/B00000000/XXXXXXXXXXXXXXXXXXXXXXXX" | "https://example.com" | true
"OLD/FASHION" | "https://example.com" | true
"" | "https://example.com" | true
null | "https://example.com" | true
}
}

0 comments on commit d41d723

Please sign in to comment.