fix(preconfigured): Allow PreconfiguredJobStage parameters to map to …

…one-dimensional array property values (spinnaker#3830) * fix(preconfigured): Update setNestedValue to handle one-dimensional array objects * fix(preconfigured): Handle case where array property is missing from root object * refactor(preconfigured): Use Groovy regex syntax * refactor(preconfigured): Update exception message to be more meaningful Co-authored-by: Mark Vulfson <markvu@live.com> Co-authored-by: Mark Vulfson <markvu@live.com>
Kaixiang-AWS · Aug 6, 2020 · 3fa29ca · 3fa29ca
1 parent 837207a
commit 3fa29ca
Show file tree

Hide file tree

Showing 2 changed files with 155 additions and 3 deletions.
diff --git a/...n/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStage.groovy b/...n/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStage.groovy
@@ -28,6 +28,8 @@ import org.springframework.stereotype.Component
 
 import javax.annotation.Nonnull
 
+import java.util.List
+
 @Component
 class PreconfiguredJobStage extends RunJobStage {
 
@@ -95,13 +97,38 @@ class PreconfiguredJobStage extends RunJobStage {
     String[] props = mapping.split(/\./)
     Object current = root
     for (int i = 0; i < props.length - 1; i++) {
-      Object next = current[props[i]]
-      if (next == null) {
-        throw new IllegalArgumentException("no property ${props[i]} on $current")
+      Object next
+      if(props[i] ==~ /.*\[\d+\]$/) {
+        next = getValueFromArrayExpression(current, props[i])
+      } else {
+        next = current[props[i]]
+        if (next == null) {
+          throw new IllegalArgumentException("no property ${props[i]} on $current")
+        }
       }
       current = next
     }
     current[props.last()] = value
   }
 
+  private static Object getValueFromArrayExpression(Object root, String expression) {
+    // TODO: Do we need to handle arrays nested in other arrays?
+    String[] parts = expression.split(/[\[\]]/)
+    String propName = parts[0]
+    int index = -1
+    try {
+      index = Integer.parseInt(parts[1])
+    } catch (NumberFormatException ex) {
+      throw new IllegalArgumentException("Unable to parse index from expresion ${expression}", ex)
+    }
+    List<Object> nextList = root[propName]
+    if (nextList == null) {
+      throw new IllegalArgumentException("no property ${propName} on $root")
+    }
+    if (nextList.size() <= index || index < 0) {
+      throw new IllegalArgumentException("Invalid index $index for list $propName")
+    }
+    return nextList.get(index)
+  }
+
 }
diff --git a/...oovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStageSpec.groovy b/...oovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStageSpec.groovy
@@ -20,6 +20,8 @@ import com.netflix.spinnaker.orca.api.preconfigured.jobs.PreconfiguredJobStagePa
 import com.netflix.spinnaker.orca.clouddriver.service.JobService
 import com.netflix.spinnaker.orca.clouddriver.config.KubernetesPreconfiguredJobProperties
 import com.netflix.spinnaker.orca.clouddriver.tasks.job.DestroyJobTask
+import io.kubernetes.client.models.V1Container
+import io.kubernetes.client.models.V1EnvVar
 import io.kubernetes.client.models.V1Job
 import spock.lang.Specification
 
@@ -99,4 +101,127 @@ class PreconfiguredJobStageSpec extends Specification {
     // verify that stage manifest has the correctly overridden name
     stage.getContext().get("manifest").metadata.name == overriddenName
   }
+
+  def "setNestedValue can handle array property mappings"() {
+    given:
+    def manifestEnvValue = "defaultValue"
+    def overriddenValue = "newValue"
+    def stage = stage {
+      type = "test"
+      context = [account: "test"]
+    }
+    def property = new KubernetesPreconfiguredJobProperties(
+      enabled: true,
+      label: "test",
+      type: "test",
+      cloudProvider: "kubernetes",
+      parameters: [
+        new PreconfiguredJobStageParameter(
+          mapping: "manifest.spec.template.spec.containers[0].env[0].value",
+          defaultValue: overriddenValue,
+          name: "envVariable"
+        )
+      ],
+      manifest: new V1Job(spec: [template: [spec: [containers: [new V1Container(env: [new V1EnvVar(name: "foo", value: manifestEnvValue)])]]]])
+    )
+
+    def jobService = Mock(JobService) {
+      2 * getPreconfiguredStages() >> {
+        return [
+          property
+        ]
+      }
+    }
+
+    when:
+    PreconfiguredJobStage preconfiguredJobStage = new PreconfiguredJobStage(Mock(DestroyJobTask), [], Optional.of(jobService))
+    preconfiguredJobStage.buildTaskGraph(stage)
+
+    then:
+    // verify that underlying job configuration hasn't been modified
+    def preconfiguredJob = (KubernetesPreconfiguredJobProperties) jobService.getPreconfiguredStages().get(0)
+    preconfiguredJob.getManifest().getSpec().getTemplate().getSpec().getContainers()[0].getEnv()[0].getValue() == manifestEnvValue
+    // verify that stage manifest has the correctly overridden name
+    stage.getContext().get("manifest").spec.template.spec.containers[0].env[0].value == overriddenValue
+  }
+
+  def "setNestedValue throws an error if the index is invalid"() {
+    given:
+    def manifestEnvValue = "defaultValue"
+    def overriddenValue = "newValue"
+    def stage = stage {
+      type = "test"
+      context = [account: "test"]
+    }
+    def property = new KubernetesPreconfiguredJobProperties(
+      enabled: true,
+      label: "test",
+      type: "test",
+      cloudProvider: "kubernetes",
+      parameters: [
+        new PreconfiguredJobStageParameter(
+          mapping: "manifest.spec.template.spec.containers[0].env[1].value",
+          defaultValue: overriddenValue,
+          name: "envVariable"
+        )
+      ],
+      manifest: new V1Job(spec: [template: [spec: [containers: [new V1Container(env: [new V1EnvVar(name: "foo", value: manifestEnvValue)])]]]])
+    )
+
+    def jobService = Mock(JobService) {
+      1 * getPreconfiguredStages() >> {
+        return [
+          property
+        ]
+      }
+    }
+
+    when:
+    PreconfiguredJobStage preconfiguredJobStage = new PreconfiguredJobStage(Mock(DestroyJobTask), [], Optional.of(jobService))
+    preconfiguredJobStage.buildTaskGraph(stage)
+
+    then:
+    def ex = thrown(IllegalArgumentException)
+    assert ex.getMessage().startsWith("Invalid index 1 for list")
+  }
+
+  def "setNestedValue throws an error if an array property name is invalid"() {
+    given:
+    def manifestEnvValue = "defaultValue"
+    def overriddenValue = "newValue"
+    def stage = stage {
+      type = "test"
+      context = [account: "test"]
+    }
+    def property = new KubernetesPreconfiguredJobProperties(
+      enabled: true,
+      label: "test",
+      type: "test",
+      cloudProvider: "kubernetes",
+      parameters: [
+        new PreconfiguredJobStageParameter(
+          mapping: "manifest.spec.template.spec.containers[0].missingProperty[0].value",
+          defaultValue: overriddenValue,
+          name: "envVariable"
+        )
+      ],
+      manifest: new V1Job(spec: [template: [spec: [containers: [new V1Container(env: [new V1EnvVar(name: "foo", value: manifestEnvValue)])]]]])
+    )
+
+    def jobService = Mock(JobService) {
+      1 * getPreconfiguredStages() >> {
+        return [
+          property
+        ]
+      }
+    }
+
+    when:
+    PreconfiguredJobStage preconfiguredJobStage = new PreconfiguredJobStage(Mock(DestroyJobTask), [], Optional.of(jobService))
+    preconfiguredJobStage.buildTaskGraph(stage)
+
+    then:
+    def ex = thrown(IllegalArgumentException)
+    assert ex.getMessage().startsWith("no property missingProperty on")
+  }
 }