Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(artifacts): Add stage artifact resolver #2702

Merged
merged 1 commit into from
Mar 6, 2019
Merged

feat(artifacts): Add stage artifact resolver #2702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/ArtifactResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,7 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -60,11 +53,14 @@ public class ArtifactResolver {

private final ObjectMapper objectMapper;
private final ExecutionRepository executionRepository;
private final ContextParameterProcessor contextParameterProcessor;

@Autowired
public ArtifactResolver(ObjectMapper objectMapper, ExecutionRepository executionRepository) {
public ArtifactResolver(ObjectMapper objectMapper, ExecutionRepository executionRepository,
ContextParameterProcessor contextParameterProcessor) {
this.objectMapper = objectMapper;
this.executionRepository = executionRepository;
this.contextParameterProcessor = contextParameterProcessor;
}

public @Nonnull
Expand Down Expand Up @@ -106,6 +102,26 @@ List<Artifact> getAllArtifacts(@Nonnull Execution execution) {
return emittedArtifacts;
}

/**
* Used to fully resolve a bound artifact on a stage that can either select
* an expected artifact ID for an expected artifact defined in a prior stage
* or as a trigger constraint OR define an inline expression-evaluable default artifact.
* @param stage The stage containing context to evaluate expressions on the bound artifact.
* @param id An expected artifact id. Either id or artifact must be specified.
* @param artifact An inline default artifact. Either id or artifact must be specified.
* @return A bound artifact with expressions evaluated.
*/
public @Nullable Artifact getBoundArtifactForStage(Stage stage, @Nullable String id, @Nullable Artifact artifact) {
Artifact boundArtifact = id != null ? getBoundArtifactForId(stage, id) : artifact;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it necessary to evaluate the passed in Artifact too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the passed in Artifact will come from an inline default artifact which may have expressions:

image

Map<String, Object> boundArtifactMap = objectMapper.convertValue(boundArtifact, new TypeReference<Map<String, Object>>() {
});

Map<String, Object> evaluatedBoundArtifactMap = contextParameterProcessor.process(boundArtifactMap,
contextParameterProcessor.buildExecutionContext(stage, true), true);

return objectMapper.convertValue(evaluatedBoundArtifactMap, Artifact.class);
}

public @Nullable
Artifact getBoundArtifactForId(
@Nonnull Stage stage, @Nullable String id) {
Expand Down
82 changes: 54 additions & 28 deletions ...core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/ArtifactResolverSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,38 @@ import com.netflix.spinnaker.orca.pipeline.model.DefaultTrigger
import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository
import spock.lang.Specification
import spock.lang.Unroll

import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage

class ArtifactResolverSpec extends Specification {
def makeArtifactResolver() {
return new ArtifactResolver(new ObjectMapper(), Mock(ExecutionRepository))
return new ArtifactResolver(new ObjectMapper(), Mock(ExecutionRepository),
new ContextParameterProcessor())
}

def "should resolve expressions in stage-inlined artifacts"() {
setup:
def execution = pipeline {
stage {
name = "upstream stage"
type = "stage1"
refId = "1"
}
}

execution.trigger = new DefaultTrigger('manual')
execution.trigger.other['buildNumber'] = 100
execution.trigger.artifacts.add(Artifact.builder().type('http/file').name('build/libs/my-jar-100.jar').build())

when:
def artifact = makeArtifactResolver().getBoundArtifactForStage(execution.stages[0], null, Artifact.builder()
.type('http/file')
.name('build/libs/my-jar-${trigger[\'buildNumber\']}.jar')
.build())

then:
artifact.name == 'build/libs/my-jar-100.jar'
}

def "should find upstream artifacts in small pipeline"() {
Expand All @@ -45,28 +71,28 @@ class ArtifactResolverSpec extends Specification {
artifacts.find { it.type == "extra" } != null

where:
execution = pipeline {
stage {
name = "upstream stage"
type = "stage1"
refId = "1"
outputs.artifacts = [new Artifact(type: "1")]
}
stage {
name = "upstream stage"
type = "stage2"
refId = "2"
requisiteStageRefIds = ["1"]
outputs.artifacts = [new Artifact(type: "2"), new Artifact(type: "extra")]
}
stage {
name = "desired"
requisiteStageRefIds = ["2"]
}
execution = pipeline {
stage {
name = "upstream stage"
type = "stage1"
refId = "1"
outputs.artifacts = [new Artifact(type: "1")]
}
stage {
name = "upstream stage"
type = "stage2"
refId = "2"
requisiteStageRefIds = ["1"]
outputs.artifacts = [new Artifact(type: "2"), new Artifact(type: "extra")]
}
stage {
name = "desired"
requisiteStageRefIds = ["2"]
}
}
}

def "should find upstream artifacts only" () {
def "should find upstream artifacts only"() {
when:
def desired = execution.getStages().find { it.name == "desired" }
def artifactResolver = makeArtifactResolver()
Expand Down Expand Up @@ -98,7 +124,7 @@ class ArtifactResolverSpec extends Specification {
}
}

def "should find artifacts from trigger and upstream stages" () {
def "should find artifacts from trigger and upstream stages"() {
when:
def execution = pipeline {
stage {
Expand All @@ -124,7 +150,7 @@ class ArtifactResolverSpec extends Specification {
artifacts.find { it.type == "trigger" } != null
}

def "should find no artifacts" () {
def "should find no artifacts"() {
when:
def execution = pipeline {
stage {
Expand All @@ -146,16 +172,16 @@ class ArtifactResolverSpec extends Specification {
artifacts.size == 0
}

def "should find a bound artifact from upstream stages" () {
def "should find a bound artifact from upstream stages"() {
when:
def execution = pipeline {
stage {
name = "upstream stage"
type = "stage1"
refId = "1"
outputs.resolvedExpectedArtifacts = [
new ExpectedArtifact(id: "1", boundArtifact: new Artifact(type: "correct")),
new ExpectedArtifact(id: "2", boundArtifact: new Artifact(type: "incorrect"))
new ExpectedArtifact(id: "1", boundArtifact: new Artifact(type: "correct")),
new ExpectedArtifact(id: "2", boundArtifact: new Artifact(type: "incorrect"))
]
}
stage {
Expand All @@ -175,15 +201,15 @@ class ArtifactResolverSpec extends Specification {
artifact.type == "correct"
}

def "should find a bound artifact from a trigger" () {
def "should find a bound artifact from a trigger"() {
when:
def execution = pipeline {
stage {
name = "upstream stage"
type = "stage1"
refId = "1"
outputs.resolvedExpectedArtifacts = [
new ExpectedArtifact(id: "2", boundArtifact: new Artifact(type: "incorrect"))
new ExpectedArtifact(id: "2", boundArtifact: new Artifact(type: "incorrect"))
]
}
stage {
Expand Down Expand Up @@ -237,7 +263,7 @@ class ArtifactResolverSpec extends Specification {
new ExpectedArtifact(matchArtifact: new Artifact(type: "docker/.*", name: "none")) | [new Artifact(type: "docker/image", name: "bad"), new Artifact(type: "docker/image", name: "image")]
}

def "should find all artifacts from an execution, in reverse order" () {
def "should find all artifacts from an execution, in reverse order"() {
when:
def execution = pipeline {
stage {
Expand Down
2 changes: 1 addition & 1 deletion ...50/src/test/groovy/com/netflix/spinnaker/orca/front50/DependentPipelineStarterSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class DependentPipelineStarterSpec extends Specification {

ObjectMapper mapper = OrcaObjectMapper.newInstance()
ExecutionRepository executionRepository = Mock(ExecutionRepository)
ArtifactResolver artifactResolver = Spy(ArtifactResolver, constructorArgs: [mapper, executionRepository])
ArtifactResolver artifactResolver = Spy(ArtifactResolver, constructorArgs: [mapper, executionRepository, new ContextParameterProcessor()])

def "should only propagate credentials when explicitly provided"() {
setup:
Expand Down
3 changes: 2 additions & 1 deletion ...or/src/test/groovy/com/netflix/spinnaker/orca/igor/tasks/MonitorJenkinsJobTaskSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import com.netflix.spinnaker.orca.pipeline.model.Stage
import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository
import com.netflix.spinnaker.orca.pipeline.tasks.artifacts.BindProducedArtifactsTask
import com.netflix.spinnaker.orca.pipeline.util.ArtifactResolver
import com.netflix.spinnaker.orca.pipeline.util.ContextParameterProcessor
import retrofit.RetrofitError
import retrofit.client.Response
import spock.lang.Shared
Expand All @@ -35,7 +36,7 @@ import spock.lang.Unroll

class MonitorJenkinsJobTaskSpec extends Specification {
def executionRepository = Mock(ExecutionRepository)
def artifactResolver = new ArtifactResolver(new ObjectMapper(), executionRepository)
def artifactResolver = new ArtifactResolver(new ObjectMapper(), executionRepository, new ContextParameterProcessor())

@Subject
MonitorJenkinsJobTask task = new MonitorJenkinsJobTask()
Expand Down
2 changes: 1 addition & 1 deletion ...eb/src/test/groovy/com/netflix/spinnaker/orca/controllers/OperationsControllerSpec.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ class OperationsControllerSpec extends Specification {
startedPipeline
}
executionRepository.retrievePipelinesForPipelineConfigId(*_) >> Observable.empty()
ArtifactResolver realArtifactResolver = new ArtifactResolver(mapper, executionRepository)
ArtifactResolver realArtifactResolver = new ArtifactResolver(mapper, executionRepository, new ContextParameterProcessor())

// can't use @subject, since we need to test the behavior of otherwise mocked-out 'artifactResolver'
def tempController = new OperationsController(
Expand Down