Skip to content

Commit

Permalink
fix(expressions): make sure trigger is a map
Browse files Browse the repository at this point in the history
Some places uses `ContextParameterProcessor.buildExecutionContext` which (correctly) converts the
`trigger` in the pipeline to an object (instead of keeping it as `Trigger` class).
However, when tasks run, they use `.withMergedContext` which opts to build it's own SpEL
executionContext (in `.augmentContext`). This is needed so that fancy look up in ancestor stages works.
Fix `.augmentContext` to convert the trigger to an object/map.

This addresses issues evaluating expressions like
`${trigger.buildNumber ?: #stage('s1').context.buildNumber}`` when no `buildNumber` is present on the `trigger`
  • Loading branch information
marchello2000 committed Apr 5, 2019
1 parent 6cc4aa7 commit db55dea
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ public class EvaluateVariablesTask implements Task {
public EvaluateVariablesTask() {
}

/**
* Copies previously evaluated expressions to the outputs map for consumption by subsequent stages.
* The variables aren't evaluated here because would've been evaluated already by a call to
* e.g. ExpressionAware.Stage#withMergedContext
*/
@Nonnull
@Override
public TaskResult execute(@Nonnull Stage stage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class EvaluateVariablesTaskSpec extends Specification {
@Subject
task = new EvaluateVariablesTask()

void "Should correctly evaulate variables"() {
void "Should correctly copy evaluated variables"() {
setup:
def stage = stage {
refId = "1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@

package com.netflix.spinnaker.orca.q.handler

import com.fasterxml.jackson.databind.ObjectMapper
import com.fasterxml.jackson.module.kotlin.convertValue
import com.netflix.spinnaker.orca.exceptions.ExceptionHandler
import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper
import com.netflix.spinnaker.orca.pipeline.expressions.ExpressionEvaluationSummary
import com.netflix.spinnaker.orca.pipeline.expressions.PipelineExpressionEvaluator
import com.netflix.spinnaker.orca.pipeline.model.Execution
Expand All @@ -35,6 +38,9 @@ interface ExpressionAware {
val contextParameterProcessor: ContextParameterProcessor
val log: Logger
get() = LoggerFactory.getLogger(javaClass)
private val mapper: ObjectMapper
get() = OrcaObjectMapper.newInstance()


fun Stage.withMergedContext(): Stage {
val evalSummary = ExpressionEvaluationSummary()
Expand Down Expand Up @@ -130,7 +136,9 @@ interface ExpressionAware {

private fun StageContext.augmentContext(execution: Execution): StageContext =
if (execution.type == PIPELINE) {
this + mapOf("trigger" to execution.trigger, "execution" to execution)
this + mapOf(
"trigger" to mapper.convertValue<Map<String, Any>>(execution.trigger),
"execution" to execution)
} else {
this
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import org.jetbrains.spek.api.dsl.on
import org.jetbrains.spek.api.lifecycle.CachingMode.GROUP
import org.jetbrains.spek.subject.SubjectSpek
import org.threeten.extra.Minutes
import java.lang.RuntimeException
import java.time.Duration
import kotlin.reflect.jvm.jvmName

Expand Down Expand Up @@ -1123,6 +1122,47 @@ object RunTaskHandlerTest : SubjectSpek<RunTaskHandler>({
}
}

describe("can reference non-existent trigger props") {
mapOf(
"\${trigger.type == 'manual'}" to true,
"\${trigger.buildNumber == null}" to true,
"\${trigger.quax ?: 'no quax'}" to "no quax"
).forEach { expression, expected ->
given("an expression $expression in the stage context") {
val pipeline = pipeline {
stage {
refId = "1"
type = "whatever"
context["expr"] = expression
trigger = DefaultTrigger ("manual")
task {
id = "1"
startTime = clock.instant().toEpochMilli()
}
}
}
val message = RunTask(pipeline.type, pipeline.id, "foo", pipeline.stageByRef("1").id, "1", DummyTask::class.java)

beforeGroup {
whenever(task.execute(any())) doReturn TaskResult.SUCCEEDED
whenever(repository.retrieve(PIPELINE, message.executionId)) doReturn pipeline
}

afterGroup(::resetMocks)

action("the handler receives a message") {
subject.handle(message)
}

it("evaluates the expression") {
verify(task).execute(check {
assertThat(it.context["expr"]).isEqualTo(expected)
})
}
}
}
}

given("a reference to deployedServerGroups in the stage context") {
val pipeline = pipeline {
stage {
Expand Down

0 comments on commit db55dea

Please sign in to comment.