feat(artifacts): add pinned info, only use reference (#1014)

keel-core-test/src/main/kotlin/com/netflix/spinnaker/keel/persistence/ArtifactRepositoryTests.kt

@@ -14,6 +14,8 @@ import com.netflix.spinnaker.keel.api.artifacts.VirtualMachineOptions
 import com.netflix.spinnaker.keel.core.api.ArtifactVersionStatus
 import com.netflix.spinnaker.keel.core.api.EnvironmentArtifactPin
 import com.netflix.spinnaker.keel.core.api.EnvironmentArtifactVetoes
+import com.netflix.spinnaker.keel.core.api.Pinned
+import com.netflix.spinnaker.keel.core.api.PinnedEnvironment
 import com.netflix.spinnaker.time.MutableClock
 import dev.minutest.junit.JUnit5Minutests
 import dev.minutest.rootContext
@@ -32,6 +34,7 @@ import strikt.assertions.isEmpty
 import strikt.assertions.isEqualTo
 import strikt.assertions.isFalse
 import strikt.assertions.isNotEqualTo
+import strikt.assertions.isNotNull
 import strikt.assertions.isNull
 import strikt.assertions.isTrue
 import strikt.assertions.succeeded
@@ -87,7 +90,6 @@ abstract class ArtifactRepositoryTests<T : ArtifactRepository> : JUnit5Minutests
  val pin1 = EnvironmentArtifactPin(
  targetEnvironment = environment2.name, // staging
  reference = artifact2.reference,
- type = artifact2.type.name,
  version = version4, // the older release build
  pinnedBy = "keel@spinnaker",
  comment = "fnord")
@@ -492,17 +494,63 @@ abstract class ArtifactRepositoryTests<T : ArtifactRepository> : JUnit5Minutests
  .isNotEqualTo(pin1.version)
  }
 
- test("latestVersionApprovedIn prefers a pinned version over the latest approved version") {
- subject.pinEnvironment(manifest, pin1)
- expectThat(subject.latestVersionApprovedIn(manifest, artifact2, environment2.name))
- .isEqualTo(version4)
- .isEqualTo(pin1.version)
+ test("get env artifact version shows that artifact is not pinned") {
+ val envArtifactSummary = subject.getArtifactSummaryInEnvironment(
+ deliveryConfig = manifest,
+ environmentName = pin1.targetEnvironment,
+ artifactReference = artifact2.reference,
+ version = version4
+ )
+ expectThat(envArtifactSummary)
+ .isNotNull()
+ .get { isPinned }
+ .isFalse()
  }
 
- test("pinned version cannot be vetoed") {
- subject.pinEnvironment(manifest, pin1)
- expectThat(subject.markAsVetoedIn(manifest, artifact2, pin1.version!!, pin1.targetEnvironment))
- .isFalse()
+ context("once pinned") {
+ before {
+ subject.pinEnvironment(manifest, pin1)
+ }
+
+ test("latestVersionApprovedIn prefers a pinned version over the latest approved version") {
+ expectThat(subject.latestVersionApprovedIn(manifest, artifact2, environment2.name))
+ .isEqualTo(version4)
+ .isEqualTo(pin1.version)
+ }
+
+ test("pinned version cannot be vetoed") {
+ expectThat(subject.markAsVetoedIn(manifest, artifact2, pin1.version, pin1.targetEnvironment))
+ .isFalse()
+ }
+
+ test("getting pinned environments shows the pin") {
+ val pins = subject.getPinnedEnvironments(manifest)
+ expectThat(pins)
+ .hasSize(1)
+ .isEqualTo(listOf(PinnedEnvironment(
+ deliveryConfigName = manifest.name,
+ targetEnvironment = pin1.targetEnvironment,
+ artifact = artifact2,
+ version = version4,
+ pinnedBy = pin1.pinnedBy,
+ pinnedAt = clock.instant(),
+ comment = pin1.comment
+ )))
+ }
+
+ test("get env artifact version shows that artifact is pinned") {
+ val envArtifactSummary = subject.getArtifactSummaryInEnvironment(
+ deliveryConfig = manifest,
+ environmentName = pin1.targetEnvironment,
+ artifactReference = artifact2.reference,
+ version = version4
+ )
+ expect {
+ that(envArtifactSummary).isNotNull()
+ that(envArtifactSummary?.isPinned).isTrue()
+ that(envArtifactSummary?.pinned).isEqualTo(Pinned(by = pin1.pinnedBy, at = clock.instant(), comment = pin1.comment))
+ }
+ }
  }
  }
  }

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/core/api/ArtifactSummary.kt

@@ -57,10 +57,18 @@ data class ArtifactSummaryInEnvironment(
  val deployedAt: Instant? = null,
  val replacedAt: Instant? = null,
  val replacedBy: String? = null,
+ val isPinned: Boolean = false,
+ val pinned: Pinned? = null,
  val statefulConstraints: List<StatefulConstraintSummary> = emptyList(),
  val statelessConstraints: List<StatelessConstraintSummary> = emptyList()
 )
 
+data class Pinned(
+ val at: Instant,
+ val by: String?,
+ val comment: String?
+)
+
 @JsonInclude(Include.NON_NULL)
 data class StatefulConstraintSummary(
  val type: String,

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/core/api/EnvironmentArtifactPin.kt

@@ -1,12 +1,12 @@
 package com.netflix.spinnaker.keel.core.api
 
 import com.netflix.spinnaker.keel.api.artifacts.DeliveryArtifact
+import java.time.Instant
 
 data class EnvironmentArtifactPin(
  val targetEnvironment: String,
  val reference: String,
- val type: String,
- val version: String?,
+ val version: String,
  val pinnedBy: String?,
  val comment: String?
 )
@@ -15,7 +15,10 @@ data class PinnedEnvironment(
  val deliveryConfigName: String,
  val targetEnvironment: String,
  val artifact: DeliveryArtifact,
- val version: String
+ val version: String,
+ val pinnedBy: String?,
+ val pinnedAt: Instant?,
+ val comment: String?
 )
 
 data class EnvironmentArtifactVeto(

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/core/api/EnvironmentSummary.kt

@@ -51,7 +51,8 @@ data class ArtifactVersions(
  val type: ArtifactType,
  val reference: String,
  val statuses: Set<ArtifactStatus>,
- val versions: ArtifactVersionStatus
+ val versions: ArtifactVersionStatus,
+ val pinnedVersion: String?
 )
 
 data class ArtifactVersionStatus(

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/persistence/ArtifactRepository.kt

@@ -24,7 +24,7 @@ interface ArtifactRepository : PeriodicallyCheckedRepository<DeliveryArtifact> {
 
  fun get(name: String, type: ArtifactType, reference: String, deliveryConfigName: String): DeliveryArtifact
 
- fun get(deliveryConfigName: String, reference: String, type: ArtifactType): DeliveryArtifact
+ fun get(deliveryConfigName: String, reference: String): DeliveryArtifact
 
  fun isRegistered(name: String, type: ArtifactType): Boolean
 
@@ -199,17 +199,17 @@ interface ArtifactRepository : PeriodicallyCheckedRepository<DeliveryArtifact> {
  * @return list of [PinnedEnvironment]'s if any of the environments in
  * [deliveryConfig] have been pinned to a specific artifact version.
  */
- fun pinnedEnvironments(deliveryConfig: DeliveryConfig): List<PinnedEnvironment>
+ fun getPinnedEnvironments(deliveryConfig: DeliveryConfig): List<PinnedEnvironment>
 
  /**
  * Removes all artifact pins from [targetEnvironment].
  */
  fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String)
 
  /**
- * Removes a specific pin from [targetEnvironment], by [reference] and [type].
+ * Removes a specific pin from [targetEnvironment], by [reference].
  */
- fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String, reference: String, type: ArtifactType)
+ fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String, reference: String)
 
  /**
  * Given information about a delivery config, environment, artifact and version, returns a summary that can be
@@ -218,8 +218,7 @@ interface ArtifactRepository : PeriodicallyCheckedRepository<DeliveryArtifact> {
  fun getArtifactSummaryInEnvironment(
  deliveryConfig: DeliveryConfig,
  environmentName: String,
- artifactName: String,
- artifactType: ArtifactType,
+ artifactReference: String,
  version: String
  ): ArtifactSummaryInEnvironment?
 
@@ -280,12 +279,9 @@ class NoSuchArtifactException(name: String, type: ArtifactType) :
  constructor(artifact: DeliveryArtifact) : this(artifact.name, artifact.type)
 }
 
-class ArtifactReferenceNotFoundException(deliveryConfig: String, reference: String) :
- NoSuchEntityException("No artifact with reference $reference in delivery config $deliveryConfig is registered")
-
-class ArtifactNotFoundException(name: String, type: ArtifactType, reference: String, deliveryConfig: String?) :
- NoSuchEntityException("No $type artifact named $name with reference $reference in delivery config $deliveryConfig is registered") {
- constructor(artifact: DeliveryArtifact) : this(artifact.name, artifact.type, artifact.reference, artifact.deliveryConfigName)
+class ArtifactNotFoundException(reference: String, deliveryConfig: String?) :
+ NoSuchEntityException("No artifact with reference $reference in delivery config $deliveryConfig is registered") {
+ constructor(artifact: DeliveryArtifact) : this(artifact.reference, artifact.deliveryConfigName)
 }
 
 class ArtifactAlreadyRegistered(name: String, type: ArtifactType) :

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/persistence/CombinedRepository.kt

@@ -358,8 +358,8 @@ class CombinedRepository(
  override fun getArtifact(name: String, type: ArtifactType, reference: String, deliveryConfigName: String): DeliveryArtifact =
  artifactRepository.get(name, type, reference, deliveryConfigName)
 
- override fun getArtifact(deliveryConfigName: String, reference: String, type: ArtifactType): DeliveryArtifact =
- artifactRepository.get(deliveryConfigName, reference, type)
+ override fun getArtifact(deliveryConfigName: String, reference: String): DeliveryArtifact =
+ artifactRepository.get(deliveryConfigName, reference)
 
  override fun isRegistered(name: String, type: ArtifactType): Boolean =
  artifactRepository.isRegistered(name, type)
@@ -410,13 +410,13 @@ class CombinedRepository(
  artifactRepository.pinEnvironment(deliveryConfig, environmentArtifactPin)
 
  override fun pinnedEnvironments(deliveryConfig: DeliveryConfig): List<PinnedEnvironment> =
- artifactRepository.pinnedEnvironments(deliveryConfig)
+ artifactRepository.getPinnedEnvironments(deliveryConfig)
 
  override fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String) =
  artifactRepository.deletePin(deliveryConfig, targetEnvironment)
 
- override fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String, reference: String, type: ArtifactType) =
- artifactRepository.deletePin(deliveryConfig, targetEnvironment, reference, type)
+ override fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String, reference: String) =
+ artifactRepository.deletePin(deliveryConfig, targetEnvironment, reference)
 
  override fun vetoedEnvironmentVersions(deliveryConfig: DeliveryConfig): List<EnvironmentArtifactVetoes> =
  artifactRepository.vetoedEnvironmentVersions(deliveryConfig)
@@ -440,11 +440,10 @@ class CombinedRepository(
  override fun getArtifactSummaryInEnvironment(
  deliveryConfig: DeliveryConfig,
  environmentName: String,
- artifactName: String,
- artifactType: ArtifactType,
+ artifactReference: String,
  version: String
  ) = artifactRepository.getArtifactSummaryInEnvironment(
- deliveryConfig, environmentName, artifactName, artifactType, version
+ deliveryConfig, environmentName, artifactReference, version
  )
 
  // END ArtifactRepository methods

keel-core/src/main/kotlin/com/netflix/spinnaker/keel/persistence/KeelRepository.kt

@@ -159,7 +159,7 @@ interface KeelRepository {
 
  fun getArtifact(name: String, type: ArtifactType, reference: String, deliveryConfigName: String): DeliveryArtifact
 
- fun getArtifact(deliveryConfigName: String, reference: String, type: ArtifactType): DeliveryArtifact
+ fun getArtifact(deliveryConfigName: String, reference: String): DeliveryArtifact
 
  fun isRegistered(name: String, type: ArtifactType): Boolean
 
@@ -197,7 +197,7 @@ interface KeelRepository {
 
  fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String)
 
- fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String, reference: String, type: ArtifactType)
+ fun deletePin(deliveryConfig: DeliveryConfig, targetEnvironment: String, reference: String)
 
  fun vetoedEnvironmentVersions(deliveryConfig: DeliveryConfig): List<EnvironmentArtifactVetoes>
 
@@ -226,8 +226,7 @@ interface KeelRepository {
  fun getArtifactSummaryInEnvironment(
  deliveryConfig: DeliveryConfig,
  environmentName: String,
- artifactName: String,
- artifactType: ArtifactType,
+ artifactReference: String,
  version: String
  ): ArtifactSummaryInEnvironment?