fix(jenkins): Fix Jenkins trigger serialization (#2774)

spinnaker · Mar 19, 2019 · b4da197 · b4da197
1 parent 2ea9f23
commit b4da197
Show file tree

Hide file tree

Showing 12 changed files with 184 additions and 95 deletions.
diff --git a/...-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTaskSpec.groovy b/...-bakery/src/test/groovy/com/netflix/spinnaker/orca/bakery/tasks/CreateBakeTaskSpec.groovy
@@ -21,12 +21,7 @@ import com.netflix.spinnaker.orca.bakery.api.BakeRequest
 import com.netflix.spinnaker.orca.bakery.api.BakeStatus
 import com.netflix.spinnaker.orca.bakery.api.BakeryService
 import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper
-import com.netflix.spinnaker.orca.pipeline.model.BuildInfo
-import com.netflix.spinnaker.orca.pipeline.model.Execution
-import com.netflix.spinnaker.orca.pipeline.model.JenkinsTrigger
-import com.netflix.spinnaker.orca.pipeline.model.SourceControl
-import com.netflix.spinnaker.orca.pipeline.model.Stage
-import com.netflix.spinnaker.orca.pipeline.model.Trigger
+import com.netflix.spinnaker.orca.pipeline.model.*
 import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver
 import retrofit.RetrofitError
 import retrofit.client.Response
@@ -36,9 +31,9 @@ import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Subject
 import spock.lang.Unroll
+
 import static com.netflix.spinnaker.orca.bakery.api.BakeStatus.State.COMPLETED
 import static com.netflix.spinnaker.orca.bakery.api.BakeStatus.State.RUNNING
-import static com.netflix.spinnaker.orca.pipeline.model.JenkinsTrigger.JenkinsArtifact
 import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
 import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage
 import static java.net.HttpURLConnection.HTTP_NOT_FOUND
@@ -118,79 +113,82 @@ class CreateBakeTaskSpec extends Specification {
   ]
 
   @Shared
-  def buildInfo = new BuildInfo(
-    "name", 0, "http://jenkins", [
-    new JenkinsArtifact("hodor_1.1_all.deb", "."),
-    new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
-    new JenkinsArtifact("hodor.1.1.nupkg", ".")
-  ], [], false, "SUCCESS", 'name#0'
+  def buildInfo = new JenkinsBuildInfo(
+    "name", 0, "http://jenkins", "SUCCESS",
+    [
+      new JenkinsArtifact("hodor_1.1_all.deb", "."),
+      new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
+      new JenkinsArtifact("hodor.1.1.nupkg", ".")
+    ]
   )
 
   @Shared
-  def buildInfoWithUrl = new BuildInfo(
-    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/",
+  def buildInfoWithUrl = new JenkinsBuildInfo(
+    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
     [
       new JenkinsArtifact("hodor_1.1_all.deb", "."),
       new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
       new JenkinsArtifact("hodor.1.1.nupkg", ".")
-    ], [], false, "SUCCESS", 'name#0'
+    ]
   )
 
   @Shared
-  def buildInfoWithFoldersUrl = new BuildInfo(
-    "name", 0, "http://spinnaker.builds.test.netflix.net/job/folder/job/SPINNAKER-package-echo/69/",
+  def buildInfoWithFoldersUrl = new JenkinsBuildInfo(
+    "name", 0, "http://spinnaker.builds.test.netflix.net/job/folder/job/SPINNAKER-package-echo/69/", "SUCCESS",
     [
       new JenkinsArtifact("hodor_1.1_all.deb", "."),
       new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
       new JenkinsArtifact("hodor.1.1.nupkg", ".")
-    ], [], false, "SUCCESS", 'name#0'
+    ]
   )
 
   @Shared
-  def buildInfoWithUrlAndSCM = new BuildInfo(
-    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/",
+  def buildInfoWithUrlAndSCM = new JenkinsBuildInfo(
+    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
     [
       new JenkinsArtifact("hodor_1.1_all.deb", "."),
       new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
       new JenkinsArtifact("hodor.1.1.nupkg", ".")
-    ], [
-    new SourceControl("refs/remotes/origin/master", "master", "f83a447f8d02a40fa84ec9d4d0dccd263d51782d")
-  ], false, "SUCCESS", 'name#0'
+    ],
+    [new SourceControl("refs/remotes/origin/master", "master", "f83a447f8d02a40fa84ec9d4d0dccd263d51782d")]
   )
 
   @Shared
-  def buildInfoWithUrlAndTwoSCMs = new BuildInfo(
-    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/",
+  def buildInfoWithUrlAndTwoSCMs = new JenkinsBuildInfo(
+    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
     [
       new JenkinsArtifact("hodor_1.1_all.deb", "."),
       new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
       new JenkinsArtifact("hodor.1.1.nupkg", ".")
-    ], [
-    new SourceControl("refs/remotes/origin/master", "master", "f83a447f8d02a40fa84ec9d4d0dccd263d51782d"),
-    new SourceControl("refs/remotes/origin/some-feature", "some-feature", "1234567f8d02a40fa84ec9d4d0dccd263d51782d")
-  ], false, "SUCCESS", 'name#0'
+    ],
+    [
+      new SourceControl("refs/remotes/origin/master", "master", "f83a447f8d02a40fa84ec9d4d0dccd263d51782d"),
+      new SourceControl("refs/remotes/origin/some-feature", "some-feature", "1234567f8d02a40fa84ec9d4d0dccd263d51782d")
+    ]
   )
 
   @Shared
-  def buildInfoWithUrlAndMasterAndDevelopSCMs = new BuildInfo(
-    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/",
+  def buildInfoWithUrlAndMasterAndDevelopSCMs = new JenkinsBuildInfo(
+    "name", 0, "http://spinnaker.builds.test.netflix.net/job/SPINNAKER-package-echo/69/", "SUCCESS",
     [
       new JenkinsArtifact("hodor_1.1_all.deb", "."),
       new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
       new JenkinsArtifact("hodor.1.1.nupkg", ".")
-    ], [
-    new SourceControl("refs/remotes/origin/master", "master", "f83a447f8d02a40fa84ec9d4d0dccd263d51782d"),
-    new SourceControl("refs/remotes/origin/develop", "develop", "1234567f8d02a40fa84ec9d4d0dccd263d51782d")
-  ], false, "SUCCESS", 'name#0'
+    ],
+    [
+      new SourceControl("refs/remotes/origin/master", "master", "f83a447f8d02a40fa84ec9d4d0dccd263d51782d"),
+      new SourceControl("refs/remotes/origin/develop", "develop", "1234567f8d02a40fa84ec9d4d0dccd263d51782d")
+    ]
   )
 
   @Shared
-  def buildInfoNoMatch = new BuildInfo(
-    "name", 0, "http://jenkins", [
-    new JenkinsArtifact("hodornodor_1.1_all.deb", "."),
-    new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
-    new JenkinsArtifact("hodor.1.1.nupkg", ".")
-  ], [], false, "SUCCESS", 'name#0'
+  def buildInfoNoMatch = new JenkinsBuildInfo(
+    "name", 0, "http://jenkins", "SUCCESS",
+    [
+      new JenkinsArtifact("hodornodor_1.1_all.deb", "."),
+      new JenkinsArtifact("hodor-1.1.noarch.rpm", "."),
+      new JenkinsArtifact("hodor.1.1.nupkg", ".")
+    ]
   )
 
   @Shared

diff --git a/...lix/spinnaker/orca/clouddriver/tasks/providers/appengine/AppEngineBranchFinderSpec.groovy b/...lix/spinnaker/orca/clouddriver/tasks/providers/appengine/AppEngineBranchFinderSpec.groovy
@@ -16,8 +16,9 @@
 
 package com.netflix.spinnaker.orca.clouddriver.tasks.providers.appengine
 
-import com.netflix.spinnaker.orca.pipeline.model.BuildInfo
+
 import com.netflix.spinnaker.orca.pipeline.model.GitTrigger
+import com.netflix.spinnaker.orca.pipeline.model.JenkinsBuildInfo
 import com.netflix.spinnaker.orca.pipeline.model.JenkinsTrigger
 import spock.lang.Specification
 import spock.lang.Unroll
@@ -74,7 +75,7 @@ class AppEngineBranchFinderSpec extends Specification {
   def "(jenkins trigger) should resolve branch, using regex (if provided) to narrow down options"() {
     given:
     def trigger = new JenkinsTrigger("Jenkins", "poll_git_repo", 1, null)
-    trigger.buildInfo = new BuildInfo("poll_git_repo", 1, "http://jenkins", [], scm, false, "SUCCESS", "poll_git_repo#1")
+    trigger.buildInfo = new JenkinsBuildInfo("poll_git_repo", 1, "http://jenkins", "SUCCESS", [], scm)
 
     def operation = [
       trigger: [
@@ -97,7 +98,7 @@ class AppEngineBranchFinderSpec extends Specification {
   def "(jenkins trigger) should throw appropriate error if method cannot resolve exactly one branch"() {
     given:
     def trigger = new JenkinsTrigger("Jenkins", "poll_git_repo", 1, null)
-    trigger.buildInfo = new BuildInfo("poll_git_repo", 1, "http://jenkins", [], scm, false, "SUCCESS", "poll_git_repo#1")
+    trigger.buildInfo = new JenkinsBuildInfo("poll_git_repo", 1, "http://jenkins", "SUCCESS", [], scm)
 
     def operation = [
       trigger      : [

diff --git a/orca-core/orca-core.gradle b/orca-core/orca-core.gradle
@@ -17,6 +17,12 @@
 apply from: "$rootDir/gradle/kotlin.gradle"
 apply from: "$rootDir/gradle/spock.gradle"
 
+test {
+  useJUnitPlatform {
+    includeEngines "junit-vintage", "junit-jupiter"
+  }
+}
+
 dependencies {
   compile project(":orca-extensionpoint")
   compile spinnaker.dependency('guava')
@@ -48,4 +54,9 @@ dependencies {
 
   testCompile project(":orca-test")
   testCompile project(":orca-test-groovy")
+  testCompile spinnaker.dependency("junitJupiterApi")
+  testCompile spinnaker.dependency("assertj")
+
+  testRuntime spinnaker.dependency("junitJupiterEngine")
+  testRuntime "org.junit.vintage:junit-vintage-engine:${spinnaker.version('jupiter')}"
 }
diff --git a/...va/com/netflix/spinnaker/orca/pipeline/expressions/whitelisting/ReturnTypeRestrictor.java b/...va/com/netflix/spinnaker/orca/pipeline/expressions/whitelisting/ReturnTypeRestrictor.java
@@ -58,7 +58,7 @@ public interface ReturnTypeRestrictor extends InstantiationTypeRestrictor {
         Stage.class,
         Trigger.class,
         BuildInfo.class,
-        JenkinsTrigger.JenkinsArtifact.class,
+        JenkinsArtifact.class,
         SourceControl.class,
         ExecutionStatus.class,
         Execution.AuthenticationDetails.class,

diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/BuildInfo.kt b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/BuildInfo.kt
@@ -1,16 +1,12 @@
 package com.netflix.spinnaker.orca.pipeline.model
 
-import com.fasterxml.jackson.annotation.JsonCreator
-import com.fasterxml.jackson.annotation.JsonProperty
-
-data class BuildInfo<A>
-@JsonCreator constructor(
-        @param:JsonProperty("name") val name: String,
-        @param:JsonProperty("number") val number: Int,
-        @param:JsonProperty("url") val url: String,
-        @JsonProperty("artifacts") val artifacts: List<A>? = emptyList(),
-        @JsonProperty("scm") val scm: List<SourceControl>? = emptyList(),
-        @param:JsonProperty("building") val isBuilding: Boolean,
-        @param:JsonProperty("result") val result: String?,
-        @param:JsonProperty("fullDisplayName") val fullDisplayName: String = "$name#$number"
-)
+abstract class BuildInfo<A>(open val name: String?,
+                        open val number: Int,
+                        open val url: String?,
+                        open val result: String?,
+                        open val artifacts: List<A>? = emptyList(),
+                        open val scm: List<SourceControl>? = emptyList(),
+                        open val building: Boolean = false) {
+    var fullDisplayName: String? = null
+        get() = field ?: "$name#$number"
+}
diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/JenkinsTrigger.kt b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/JenkinsTrigger.kt
@@ -17,7 +17,6 @@
 package com.netflix.spinnaker.orca.pipeline.model
 
 import com.fasterxml.jackson.annotation.JsonCreator
-import com.fasterxml.jackson.annotation.JsonIgnore
 import com.fasterxml.jackson.annotation.JsonProperty
 import com.netflix.spinnaker.kork.artifacts.model.Artifact
 import com.netflix.spinnaker.kork.artifacts.model.ExpectedArtifact
@@ -41,12 +40,31 @@ data class JenkinsTrigger
 
   override var other: Map<String, Any> = mutableMapOf()
   override var resolvedExpectedArtifacts: List<ExpectedArtifact> = mutableListOf()
-  var buildInfo: BuildInfo<JenkinsArtifact>? = null
+  var buildInfo: JenkinsBuildInfo? = null
   var properties: Map<String, Any> = mutableMapOf()
+}
+
+class JenkinsArtifact
+@JsonCreator
+constructor(@param:JsonProperty("fileName") val fileName: String,
+            @param:JsonProperty("relativePath") val relativePath: String)
+
+class JenkinsBuildInfo
+@JsonCreator
+constructor(@param:JsonProperty("name") override val name: String?,
+            @param:JsonProperty("number") override val number: Int,
+            @param:JsonProperty("url") override val url: String?,
+            @param:JsonProperty("result") override val result: String?,
+            @param:JsonProperty("artifacts") override val artifacts: List<JenkinsArtifact>?,
+            @param:JsonProperty("scm") override val scm: List<SourceControl>?,
+            @param:JsonProperty("building") override var building: Boolean = false)
+    : BuildInfo<JenkinsArtifact>(name, number, url, result, artifacts, scm, building) {
 
-  data class JenkinsArtifact
-  @JsonCreator constructor(
-    @param:JsonProperty("fileName") val fileName: String,
-    @param:JsonProperty("relativePath") val relativePath: String
-  )
+    @JvmOverloads
+    constructor(name: String,
+                number: Int,
+                url: String,
+                result: String,
+                artifacts: List<JenkinsArtifact> = emptyList(),
+                scm: List<SourceControl> = emptyList()): this(name, number, url, result, artifacts, scm, false)
 }
diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/BuildDetailExtractor.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/BuildDetailExtractor.java
@@ -19,6 +19,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.netflix.spinnaker.orca.jackson.OrcaObjectMapper;
 import com.netflix.spinnaker.orca.pipeline.model.BuildInfo;
+import com.netflix.spinnaker.orca.pipeline.model.JenkinsBuildInfo;
 import com.netflix.spinnaker.orca.pipeline.model.SourceControl;
 import org.apache.commons.lang3.StringUtils;
 
@@ -37,7 +38,7 @@ public BuildDetailExtractor() {
     this.detailExtractors = Arrays.asList(new DefaultDetailExtractor(), new LegacyJenkinsUrlDetailExtractor());
   }
 
-  public boolean tryToExtractBuildDetails(BuildInfo buildInfo, Map<String, Object> request) {
+  public boolean tryToExtractBuildDetails(BuildInfo<?> buildInfo, Map<String, Object> request) {
     // The first strategy to succeed ends the loop. That is: the DefaultDetailExtractor is trying first
     // if it can not succeed the Legacy parser will be applied
     return detailExtractors.stream().anyMatch(it ->
@@ -46,9 +47,9 @@ public boolean tryToExtractBuildDetails(BuildInfo buildInfo, Map<String, Object>
   }
 
   @Deprecated
-  public boolean tryToExtractBuildDetails(Map<String, Object> buildInfo, Map<String, Object> request) {
+  public boolean tryToExtractJenkinsBuildDetails(Map<String, Object> buildInfo, Map<String, Object> request) {
     try {
-      return tryToExtractBuildDetails(mapper.convertValue(buildInfo, BuildInfo.class), request);
+      return tryToExtractBuildDetails(mapper.convertValue(buildInfo, JenkinsBuildInfo.class), request);
     } catch (IllegalArgumentException e) {
       return false;
     }

diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/PackageInfo.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/PackageInfo.java
@@ -22,8 +22,6 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.netflix.spinnaker.kork.artifacts.model.Artifact;
 import com.netflix.spinnaker.orca.pipeline.model.Stage;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import static com.netflix.spinnaker.orca.pipeline.model.Execution.ExecutionType.PIPELINE;
 import static java.lang.String.format;
 import static java.util.Collections.emptyList;
@@ -42,8 +40,6 @@
  */
 public class PackageInfo {
 
-  private final Logger log = LoggerFactory.getLogger(getClass());
-
   private final ObjectMapper mapper;
   private final Stage stage;
   private final List<Artifact> artifacts;
@@ -231,7 +227,7 @@ private Map<String, Object> createAugmentedRequest(Map<String, Object> trigger,
       if (packageIdentifier != null) {
         if (extractBuildDetails) {
           Map<String, Object> buildInfoForDetails = !buildArtifact.isEmpty() ? buildInfoCurrentExecution : triggerBuildInfo;
-          buildDetailExtractor.tryToExtractBuildDetails(buildInfoForDetails, stageContext);
+          buildDetailExtractor.tryToExtractJenkinsBuildDetails(buildInfoForDetails, stageContext);
         }
       }
     }

diff --git a/.../src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/BuildDetailExtractorSpec.groovy b/.../src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/BuildDetailExtractorSpec.groovy
@@ -15,25 +15,21 @@
  */
 package com.netflix.spinnaker.orca.pipeline.util
 
-import com.fasterxml.jackson.databind.ObjectMapper
-import org.springframework.beans.factory.annotation.Autowired
+
 import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
 
 class BuildDetailExtractorSpec extends Specification {
 
-  @Autowired
-  ObjectMapper mapper
-
   @Shared
   BuildDetailExtractor buildDetailExtractor = new BuildDetailExtractor()
 
   @Unroll
   def "Default detail from buildInfo"() {
 
     when:
-    buildDetailExtractor.tryToExtractBuildDetails(buildInfo, result)
+    buildDetailExtractor.tryToExtractJenkinsBuildDetails(buildInfo, result)
 
     then:
     result == expectedResult
@@ -48,7 +44,7 @@ class BuildDetailExtractorSpec extends Specification {
   def "Legacy Jenkins detail from the url"() {
 
     when:
-    buildDetailExtractor.tryToExtractBuildDetails(buildInfo, result)
+    buildDetailExtractor.tryToExtractJenkinsBuildDetails(buildInfo, result)
 
     then:
     result == expectedResult
@@ -64,7 +60,7 @@ class BuildDetailExtractorSpec extends Specification {
   def "Extract detail, missing fields and edge cases"() {
 
     when:
-    buildDetailExtractor.tryToExtractBuildDetails(buildInfo, result)
+    buildDetailExtractor.tryToExtractJenkinsBuildDetails(buildInfo, result)
 
     then:
     result == expectedResult

diff --git a/...test/groovy/com/netflix/spinnaker/orca/pipeline/util/ContextParameterProcessorSpec.groovy b/...test/groovy/com/netflix/spinnaker/orca/pipeline/util/ContextParameterProcessorSpec.groovy
@@ -302,7 +302,7 @@ class ContextParameterProcessorSpec extends Specification {
   @Unroll
   def "correctly compute scmInfo attribute"() {
     given:
-    context.trigger.buildInfo = new BuildInfo("name", 1, "http://jenkins", [], scm, false, "SUCCESS", 'name#1')
+    context.trigger.buildInfo = new JenkinsBuildInfo("name", 1, "http://jenkins", "SUCCESS", [], scm)
 
     def source = ['branch': '${scmInfo.branch}']