Skip to content

Commit

Permalink
fix(notifications): Fix regression from spinnaker#3280 (spinnaker#3284)
Browse files Browse the repository at this point in the history
spinnaker#3280 introduced a regression where if there are multiple notifications
and the exeuction has SCM info (e.g. jenkins trigger) the pipeline notifications won't get sent out.

Why you ask? Side-effects, my friends!

`contextParameterProcessor.process` actually MUTATES the context that is passed in (it extracts the scm info which starts out as a list and then replaces it with an actual SourceControl object :mindblown:)
see https://github.com/spinnaker/orca/blob/master/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ContextParameterProcessor.java#L170
I wanted to not jackson map the whole execution every single notification we eval so I cached and there-in lay my mistake...

This change, does not reuse the executionContext map avoiding this problem
  • Loading branch information
marchello2000 authored and mergify[bot] committed Nov 6, 2019
1 parent 7075c6b commit 3d04062
Showing 1 changed file with 1 addition and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,8 @@ class EchoNotifyingExecutionListener implements ExecutionListener {
}

private void processSpelInNotifications(Execution execution) {
Map executionMap = objectMapper.convertValue(execution, Map)

List<Map<String, Object>> spelProcessedNotifications = execution.notifications.collect({
Map executionMap = objectMapper.convertValue(execution, Map)
contextParameterProcessor.process(it, executionMap, true)
})
execution.notifications = spelProcessedNotifications
Expand Down

0 comments on commit 3d04062

Please sign in to comment.