Skip to content

Commit f35a5ff

Browse files
edgarulgjasonmcintosh
authored and
mergify[bot]
committed
fix(pipelineRef): add resolvedExpectedArtifacts from pipelineTrigger to PipelineRefTrigger (#4816)
* fix(pipelineRef): add resolvedExpectedArtifacts from pipelineTrigger to PipelineRefTrigger * fix(pipelineRef): add tests around PipelineRefTrigger --------- Co-authored-by: Jason <jason.mcintosh@harness.io> (cherry picked from commit 3c65fe0)
1 parent 6768b78 commit f35a5ff

File tree

5 files changed

+237
-12
lines changed

5 files changed

+237
-12
lines changed

‎orca-sql/src/main/kotlin/com/netflix/spinnaker/orca/sql/PipelineRefTriggerDeserializerSupplier.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,10 @@ class PipelineRefTriggerDeserializerSupplier(
6161
isStrategy = get("strategy")?.booleanValue() == true,
6262
parentExecutionId = parentExecutionId,
6363
parentPipelineStageId = get("parentPipelineStageId")?.textValue()
64-
)
64+
).apply {
65+
resolvedExpectedArtifacts = get("resolvedExpectedArtifacts")?.listValue(parser) ?: mutableListOf()
66+
other = get("other")?.mapValue(parser) ?: mutableMapOf()
67+
}
6568
}
6669
}
6770

‎orca-sql/src/main/kotlin/com/netflix/spinnaker/orca/sql/pipeline/persistence/ExecutionMapper.kt

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
2626
import java.sql.ResultSet
2727
import org.jooq.DSLContext
2828
import org.jooq.impl.DSL.field
29-
import org.jooq.impl.DSL.name
3029
import org.slf4j.LoggerFactory
3130
import java.nio.charset.StandardCharsets
3231

@@ -142,11 +141,7 @@ class ExecutionMapper(
142141
fun convertPipelineRefTrigger(execution: PipelineExecution, context: DSLContext) {
143142
val trigger = execution.trigger
144143
if (trigger is PipelineRefTrigger) {
145-
val parentExecution = context
146-
.selectExecution(execution.type, compressionProperties)
147-
.where(field("id").eq(trigger.parentExecutionId))
148-
.fetchExecutions(mapper, 200, compressionProperties, context, pipelineRefEnabled)
149-
.firstOrNull()
144+
val parentExecution = fetchParentExecution(execution.type, trigger, context)
150145

151146
if (parentExecution == null) {
152147
// If someone deletes the parent execution, we'll be unable to load the full, valid child pipeline. Rather than
@@ -160,4 +155,13 @@ class ExecutionMapper(
160155
execution.trigger = trigger.toPipelineTrigger(parentExecution)
161156
}
162157
}
158+
159+
@VisibleForTesting
160+
fun fetchParentExecution(type: ExecutionType, trigger: PipelineRefTrigger, context: DSLContext): PipelineExecution? {
161+
return context
162+
.selectExecution(type, compressionProperties)
163+
.where(field("id").eq(trigger.parentExecutionId))
164+
.fetchExecutions(mapper, 200, compressionProperties, context, pipelineRefEnabled)
165+
.firstOrNull()
166+
}
163167
}

‎orca-sql/src/main/kotlin/com/netflix/spinnaker/orca/sql/pipeline/persistence/PipelineRefTrigger.kt

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,5 +100,8 @@ data class PipelineRefTrigger(
100100
isStrategy = isStrategy,
101101
parentExecution = parentExecution,
102102
parentPipelineStageId = parentPipelineStageId
103-
)
103+
).apply {
104+
this.resolvedExpectedArtifacts = this@PipelineRefTrigger.resolvedExpectedArtifacts
105+
this.other = this@PipelineRefTrigger.other
106+
}
104107
}

‎orca-sql/src/test/kotlin/com/netflix/spinnaker/orca/sql/PipelineRefTriggerDeserializerSupplierTest.kt

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright 2024 Harness Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.netflix.spinnaker.orca.sql
18+
19+
import com.fasterxml.jackson.databind.ObjectMapper
20+
import com.fasterxml.jackson.databind.node.JsonNodeFactory
21+
import com.fasterxml.jackson.databind.node.ObjectNode
22+
import com.netflix.spinnaker.orca.sql.pipeline.persistence.PipelineRefTrigger
23+
import dev.minutest.junit.JUnit5Minutests
24+
import dev.minutest.rootContext
25+
import org.junit.jupiter.api.Assertions.assertTrue
26+
import org.junit.jupiter.api.Assertions.assertFalse
27+
import org.junit.jupiter.api.Assertions.assertEquals
28+
29+
class PipelineRefTriggerDeserializerSupplierTest : JUnit5Minutests {
30+
31+
fun tests() = rootContext {
32+
context("pipelineRef feature is disabled") {
33+
val deserializerSupplier = PipelineRefTriggerDeserializerSupplier(pipelineRefEnabled = false)
34+
val jsonNodeFactory = JsonNodeFactory.instance
35+
36+
test("predicate is true when the trigger is a pipelineRef") {
37+
val node = jsonNodeFactory.objectNode().apply {
38+
put("type", "pipelineRef")
39+
}
40+
assertTrue(deserializerSupplier.predicate(node))
41+
}
42+
43+
test("predicate is false when the trigger is not a pipelineRef") {
44+
val node = jsonNodeFactory.objectNode().apply {
45+
put("type", "manual")
46+
}
47+
assertFalse(deserializerSupplier.predicate(node))
48+
}
49+
}
50+
51+
context("pipelineRef feature is enabled") {
52+
val deserializerSupplier = PipelineRefTriggerDeserializerSupplier(pipelineRefEnabled = true)
53+
val jsonNodeFactory = JsonNodeFactory.instance
54+
55+
test("predicate is true when the trigger is a pipelineRef") {
56+
val node = jsonNodeFactory.objectNode().apply {
57+
put("type", "pipelineRef")
58+
}
59+
assertTrue(deserializerSupplier.predicate(node))
60+
}
61+
62+
test("predicate is true when the trigger has parentExecution") {
63+
val node = jsonNodeFactory.objectNode().apply {
64+
set<ObjectNode>("parentExecution", jsonNodeFactory.objectNode().put("id", "execution-id"))
65+
}
66+
assertTrue(deserializerSupplier.predicate(node))
67+
}
68+
69+
test("predicate is false when the trigger is not a pipelineRef") {
70+
val node = jsonNodeFactory.objectNode().apply {
71+
put("type", "manual")
72+
}
73+
assertFalse(deserializerSupplier.predicate(node))
74+
}
75+
76+
}
77+
78+
context("deserializing pipelineRef") {
79+
val deserializerSupplier = PipelineRefTriggerDeserializerSupplier(pipelineRefEnabled = true)
80+
val jsonNodeFactory = JsonNodeFactory.instance
81+
val jsonParser = ObjectMapper().createParser("")
82+
83+
test("all fields in pipelineRef are added") {
84+
val node = jsonNodeFactory.objectNode().apply {
85+
put("correlationId", "correlation-id")
86+
put("user", "test-user")
87+
set<ObjectNode>("parameters", jsonNodeFactory.objectNode().put("key1", "value1"))
88+
set<ObjectNode>("artifacts", jsonNodeFactory.arrayNode().add(jsonNodeFactory.objectNode().put("type", "artifact-type")))
89+
put("rebake", true)
90+
put("dryRun", false)
91+
put("strategy", true)
92+
put("parentExecutionId", "parent-execution-id")
93+
set<ObjectNode>("resolvedExpectedArtifacts", jsonNodeFactory.arrayNode().add(jsonNodeFactory.objectNode().put("id", "resolved-artifact-id")))
94+
set<ObjectNode>("other", jsonNodeFactory.objectNode().put("extra1", "value1"))
95+
}
96+
97+
val trigger = deserializerSupplier.deserializer(node, jsonParser) as PipelineRefTrigger
98+
99+
assertEquals("correlation-id", trigger.correlationId)
100+
assertEquals("test-user", trigger.user)
101+
assertEquals(mapOf("key1" to "value1"), trigger.parameters)
102+
assertEquals(1, trigger.artifacts.size)
103+
assertTrue(trigger.notifications.isEmpty())
104+
assertTrue(trigger.isRebake)
105+
assertFalse(trigger.isDryRun)
106+
assertTrue(trigger.isStrategy)
107+
assertEquals("parent-execution-id", trigger.parentExecutionId)
108+
assertEquals(1, trigger.resolvedExpectedArtifacts.size)
109+
assertEquals(1, trigger.other.size)
110+
}
111+
112+
test("pipelineTrigger is deserialized into pipelineRef") {
113+
val node = jsonNodeFactory.objectNode().apply {
114+
put("correlationId", "correlation-id")
115+
put("user", "test-user")
116+
set<ObjectNode>("parameters", jsonNodeFactory.objectNode().put("key1", "value1"))
117+
set<ObjectNode>("artifacts", jsonNodeFactory.arrayNode().add(jsonNodeFactory.objectNode().put("type", "artifact-type")))
118+
put("rebake", true)
119+
put("dryRun", false)
120+
put("strategy", true)
121+
set<ObjectNode>("parentExecution", jsonNodeFactory.objectNode().put("id", "parent-execution-id-from-pipeline-trigger"))
122+
set<ObjectNode>("resolvedExpectedArtifacts", jsonNodeFactory.arrayNode().add(jsonNodeFactory.objectNode().put("id", "resolved-artifact-id")))
123+
set<ObjectNode>("other", jsonNodeFactory.objectNode().put("extra1", "value1"))
124+
}
125+
126+
val trigger = deserializerSupplier.deserializer(node, jsonParser) as PipelineRefTrigger
127+
128+
assertEquals("correlation-id", trigger.correlationId)
129+
assertEquals("test-user", trigger.user)
130+
assertEquals(mapOf("key1" to "value1"), trigger.parameters)
131+
assertEquals(1, trigger.artifacts.size)
132+
assertTrue(trigger.notifications.isEmpty())
133+
assertTrue(trigger.isRebake)
134+
assertFalse(trigger.isDryRun)
135+
assertTrue(trigger.isStrategy)
136+
assertEquals("parent-execution-id-from-pipeline-trigger", trigger.parentExecutionId)
137+
assertEquals(1, trigger.resolvedExpectedArtifacts.size)
138+
assertEquals(1, trigger.other.size)
139+
}
140+
}
141+
}
142+
143+
}

‎orca-sql/src/test/kotlin/com/netflix/spinnaker/orca/sql/pipeline/persistence/ExecutionMapperTest.kt

Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,27 @@ package com.netflix.spinnaker.orca.sql.pipeline.persistence
1818
import com.fasterxml.jackson.databind.ObjectMapper
1919
import com.netflix.spinnaker.config.CompressionType
2020
import com.netflix.spinnaker.config.ExecutionCompressionProperties
21+
import com.netflix.spinnaker.kork.artifacts.model.Artifact
22+
import com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact
23+
import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType
2124
import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution
2225
import com.netflix.spinnaker.orca.pipeline.model.DefaultTrigger
23-
import com.nhaarman.mockito_kotlin.*
26+
import com.netflix.spinnaker.orca.pipeline.model.PipelineExecutionImpl
27+
import com.netflix.spinnaker.orca.pipeline.model.PipelineTrigger
28+
import com.nhaarman.mockito_kotlin.mock
29+
import com.nhaarman.mockito_kotlin.doReturn
30+
import com.nhaarman.mockito_kotlin.verify
31+
import com.nhaarman.mockito_kotlin.any
32+
import com.nhaarman.mockito_kotlin.times
2433
import dev.minutest.junit.JUnit5Minutests
2534
import dev.minutest.rootContext
2635
import org.assertj.core.api.Assertions.assertThat
2736
import org.jooq.DSLContext
2837
import org.mockito.Mockito
38+
import strikt.api.expectThat
39+
import strikt.assertions.isA
40+
import strikt.assertions.isEmpty
41+
import strikt.assertions.isEqualTo
2942
import java.io.ByteArrayOutputStream
3043
import java.nio.charset.StandardCharsets
3144
import java.sql.ResultSet
@@ -69,16 +82,75 @@ class ExecutionMapperTest : JUnit5Minutests {
6982
val compressionProperties = ExecutionCompressionProperties().apply {
7083
enabled = false
7184
}
72-
val mapper = ExecutionMapper(mapper = ObjectMapper(), stageBatchSize = 200, compressionProperties, true)
73-
val mockedExecution = mock<PipelineExecution>()
7485
val database: DSLContext = Mockito.mock(DSLContext::class.java, Mockito.RETURNS_DEEP_STUBS)
7586

7687
test("conversion ignored when trigger is not PipelineRef") {
88+
val mockedExecution = mock<PipelineExecution>()
89+
val mapper = ExecutionMapper(mapper = ObjectMapper(), stageBatchSize = 200, compressionProperties = compressionProperties, true)
90+
val spyMapper = Mockito.spy(mapper)
91+
7792
doReturn(DefaultTrigger(type = "default")).`when`(mockedExecution).trigger
78-
mapper.convertPipelineRefTrigger(mockedExecution, database)
93+
spyMapper.convertPipelineRefTrigger(mockedExecution, database)
94+
verify(mockedExecution, times(1)).trigger
95+
verify(spyMapper, times(0)).fetchParentExecution(any(), any(), any())
96+
}
97+
98+
test("conversion is aborted when trigger is PipelineRef but parentExecution not found") {
99+
val mockedExecution = mock<PipelineExecution>()
100+
val mapper = ExecutionMapper(mapper = ObjectMapper(), stageBatchSize = 200, compressionProperties = compressionProperties, true)
101+
val spyMapper = Mockito.spy(mapper)
102+
103+
doReturn(PipelineRefTrigger(parentExecutionId = "test-parent-id")).`when`(mockedExecution).trigger
104+
doReturn(ExecutionType.PIPELINE).`when`(mockedExecution).type
105+
doReturn(null).`when`(spyMapper).fetchParentExecution(any(), any(), any())
106+
spyMapper.convertPipelineRefTrigger(mockedExecution, database)
79107
verify(mockedExecution, times(1)).trigger
108+
verify(spyMapper, times(1)).fetchParentExecution(any(), any(), any())
80109
}
81110

111+
test("conversion is processed when trigger is PipelineRef") {
112+
val correlationId = "test-correlation"
113+
val parentExecutionId = "test-execution"
114+
val parameters = mutableMapOf<String, Any>("test-parameter" to "test-body")
115+
val artifacts = mutableListOf(Artifact.builder().build())
116+
val resolvedExpectedArtifact = mutableListOf(ExpectedArtifact.builder().boundArtifact(Artifact.builder().build()).build())
117+
val otherTest = mutableMapOf<String, Any>("test-other" to "other-body")
118+
119+
val execution = PipelineExecutionImpl(ExecutionType.PIPELINE, "test-app").apply {
120+
trigger = PipelineRefTrigger(
121+
correlationId = correlationId,
122+
parentExecutionId = parentExecutionId,
123+
parameters = parameters,
124+
artifacts = artifacts
125+
).apply {
126+
resolvedExpectedArtifacts = resolvedExpectedArtifact
127+
other = otherTest
128+
}
129+
}
130+
131+
val mockedParentExecution = mock<PipelineExecution>()
132+
val mapper = ExecutionMapper(mapper = ObjectMapper(), stageBatchSize = 200, compressionProperties = compressionProperties, true)
133+
val spyMapper = Mockito.spy(mapper)
134+
135+
doReturn(mockedParentExecution).`when`(spyMapper).fetchParentExecution(any(), any(), any())
136+
137+
spyMapper.convertPipelineRefTrigger(execution, database)
138+
139+
expectThat(execution.trigger) {
140+
isA<PipelineTrigger>()
141+
get { this.correlationId }.isEqualTo(correlationId)
142+
get { this.parameters }.isEqualTo(parameters)
143+
get { this.artifacts }.isEqualTo(artifacts)
144+
get { this.resolvedExpectedArtifacts }.isEqualTo(resolvedExpectedArtifact)
145+
get { this.other }.isEqualTo(otherTest)
146+
get { this.notifications }.isEmpty()
147+
}
148+
149+
expectThat(execution.trigger as PipelineTrigger)
150+
.get(PipelineTrigger::parentExecution).isEqualTo(mockedParentExecution)
151+
152+
verify(spyMapper, times(1)).fetchParentExecution(any(), any(), any())
153+
}
82154
}
83155
}
84156
}

0 commit comments

Comments
 (0)