diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStage.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStage.groovy index f7b9be0b29..56d10110dc 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStage.groovy +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStage.groovy @@ -16,7 +16,7 @@ package com.netflix.spinnaker.orca.clouddriver.pipeline.job - +import com.fasterxml.jackson.databind.ObjectMapper import com.netflix.spinnaker.orca.clouddriver.config.PreconfiguredJobStageProperties import com.netflix.spinnaker.orca.clouddriver.exception.PreconfiguredJobNotFoundException import com.netflix.spinnaker.orca.clouddriver.service.JobService @@ -30,11 +30,13 @@ import org.springframework.stereotype.Component class PreconfiguredJobStage extends RunJobStage { private JobService jobService + private ObjectMapper objectMapper @Autowired public PreconfiguredJobStage(DestroyJobTask destroyJobTask, Optional optionalJobService) { super(destroyJobTask) this.jobService = optionalJobService.orElse(null) + this.objectMapper = new ObjectMapper() } @Override @@ -50,9 +52,14 @@ class PreconfiguredJobStage extends RunJobStage { } private Map overrideIfNotSetInContextAndOverrideDefault(Map context, PreconfiguredJobStageProperties preconfiguredJob) { + // without converting this object, assignments to `context[it]` will result in + // references being assigned instead of values which causes the overrides in context + // to override the underlying job. this avoids that problem by giving us a fresh "copy" + // to work wit + Map preconfiguredMap = objectMapper.convertValue(preconfiguredJob, Map.class) preconfiguredJob.getOverridableFields().each { - if (context[it] == null || preconfiguredJob[it] != null) { - context[it] = preconfiguredJob[it] + if (context[it] == null || preconfiguredMap[it] != null) { + context[it] = preconfiguredMap[it] } } preconfiguredJob.parameters.each { defaults -> diff --git a/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStageSpec.groovy b/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStageSpec.groovy index 99a9c5461e..c01afcc803 100644 --- a/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStageSpec.groovy +++ b/orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/pipeline/job/PreconfiguredJobStageSpec.groovy @@ -20,6 +20,7 @@ import com.netflix.spinnaker.orca.clouddriver.config.PreconfiguredJobStageParame 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.V1Job import spock.lang.Specification import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage @@ -53,7 +54,48 @@ class PreconfiguredJobStageSpec extends Specification { "cloudProvider" | "kubernetes" | "testJob" | [account: "test-account"] | new KubernetesPreconfiguredJobProperties(enabled: true, label: "testJob", type: "testJob", parameters: [], cloudProvider: "kubernetes") "cloudProvider" | "titus" | "testJob" | [account: "test-account"] | new KubernetesPreconfiguredJobProperties(enabled: true, label: "testJob", type: "testJob", parameters: [new PreconfiguredJobStageParameter(mapping: "cloudProvider", defaultValue: "titus")], cloudProvider: "kubernetes") "cloudProvider" | "somethingElse" | "testJob" | [account: "test-account", parameters: ["cloudProvider": "somethingElse"]] | new KubernetesPreconfiguredJobProperties(enabled: true, label: "testJob", type: "testJob", parameters: [new PreconfiguredJobStageParameter(mapping: "cloudProvider", defaultValue: "titus", "name": "cloudProvider")], cloudProvider: "kubernetes") + } + + def "should use copy of preconfigured job to populate context"() { + + given: + def manifestMetadataName = "defaultName" + def overriddenName = "fromParameter" + def stage = stage { + type = "test" + context = [account: "test"] + } + def property = new KubernetesPreconfiguredJobProperties( + enabled: true, + label: "test", + type: "test", + parameters: [ + new PreconfiguredJobStageParameter( + mapping: "manifest.metadata.name", + defaultValue: "fromParameter", + name: "metadataName" + ) + ], + manifest: new V1Job(metadata: [name: "defaultName"]) + ) + + 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().getMetadata().getName() == manifestMetadataName + // verify that stage manifest has the correctly overridden name + stage.getContext().get("manifest").metadata.name == overriddenName } }