diff --git a/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AmiIdResolver.groovy b/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AmiIdResolver.groovy index bbdd752e496..37d33622892 100644 --- a/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AmiIdResolver.groovy +++ b/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/AmiIdResolver.groovy @@ -41,7 +41,7 @@ class AmiIdResolver { } Image resolvedImage = amazonEC2.describeImages(req)?.images?.getAt(0) if (resolvedImage) { - return new ResolvedAmiResult(nameOrId, region, resolvedImage.imageId, resolvedImage.virtualizationType, resolvedImage.ownerId, resolvedImage.blockDeviceMappings) + return new ResolvedAmiResult(nameOrId, region, resolvedImage.imageId, resolvedImage.virtualizationType, resolvedImage.ownerId, resolvedImage.blockDeviceMappings, resolvedImage.public) } return null diff --git a/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ResolvedAmiResult.groovy b/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ResolvedAmiResult.groovy index 1b765909930..fb730124828 100644 --- a/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ResolvedAmiResult.groovy +++ b/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ResolvedAmiResult.groovy @@ -30,4 +30,5 @@ class ResolvedAmiResult { String virtualizationType String ownerId List blockDeviceMappings + Boolean isPublic } diff --git a/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperation.groovy b/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperation.groovy index 28a9c49349a..b6420b87a58 100644 --- a/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperation.groovy +++ b/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperation.groovy @@ -36,8 +36,6 @@ import org.springframework.beans.factory.annotation.Autowired class AllowLaunchAtomicOperation implements AtomicOperation { private static final String BASE_PHASE = "ALLOW_LAUNCH" - private static final int MAX_TARGET_DESCRIBE_ATTEMPTS = 2 - private static Task getTask() { TaskRepository.threadLocalTask.get() } @@ -68,8 +66,15 @@ class AllowLaunchAtomicOperation implements AtomicOperation { throw new IllegalArgumentException("unable to resolve AMI imageId from $description.amiName") } + // If the AMI is public, this is a no-op + if (resolvedAmi.isPublic) { + task.updateStatus BASE_PHASE, "AMI is public, no need to allow launch" + return resolvedAmi + } + // If the AMI was created/owned by a different account, switch to using that for modifying the image if (resolvedAmi.ownerId != sourceCredentials.accountId) { + if (resolvedAmi.getRegion()) sourceCredentials = accountCredentialsProvider.all.find { accountCredentials -> accountCredentials instanceof NetflixAmazonCredentials && ((AmazonCredentials) accountCredentials).accountId == resolvedAmi.ownerId @@ -85,7 +90,7 @@ class AllowLaunchAtomicOperation implements AtomicOperation { OperationPoller.retryWithBackoff({o -> sourceAmazonEC2.modifyImageAttribute(new ModifyImageAttributeRequest().withImageId(resolvedAmi.amiId).withLaunchPermission( new LaunchPermissionModifications().withAdd(new LaunchPermission().withUserId(targetCredentials.accountId)))) - }, 500, 3); + }, 500, 3) if (sourceCredentials == targetCredentials) { task.updateStatus BASE_PHASE, "Tag replication not required" diff --git a/clouddriver-aws/src/test/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperationUnitSpec.groovy b/clouddriver-aws/src/test/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperationUnitSpec.groovy index 5a478c48462..f9cd63a57a3 100644 --- a/clouddriver-aws/src/test/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperationUnitSpec.groovy +++ b/clouddriver-aws/src/test/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/AllowLaunchAtomicOperationUnitSpec.groovy @@ -216,6 +216,44 @@ class AllowLaunchAtomicOperationUnitSpec extends Specification { } + void "should return resolved public AMI without performing additional operations"() { + setup: + def ownerCredentials = TestCredential.named('owner') + def targetCredentials = TestCredential.named('target') + def ownerAmazonEc2 = Mock(AmazonEC2) + def targetAmazonEc2 = Mock(AmazonEC2) + + def description = new AllowLaunchDescription(account: 'target', amiName: 'ami-123456', region: 'us-west-1', credentials: ownerCredentials) + def op = new AllowLaunchAtomicOperation(description) + op.amazonClientProvider = Mock(AmazonClientProvider) + op.accountCredentialsProvider = Mock(AccountCredentialsProvider) + + when: + op.operate([]) + + then: + + with(op.accountCredentialsProvider) { + 1 * getCredentials('target') >> targetCredentials + } + + with(op.amazonClientProvider) { + 1 * getAmazonEC2(targetCredentials, _, true) >> targetAmazonEc2 + 1 * getAmazonEC2(ownerCredentials, _, true) >> ownerAmazonEc2 + } + + with(ownerAmazonEc2) { + 1 * describeImages(_) >> new DescribeImagesResult().withImages( + new Image() + .withImageId("ami-123456") + .withOwnerId(ownerCredentials.accountId) + .withPublic(true)) + } + + 0 * _ + + } + Closure constructDescribeTagsResult = { Map tags -> new DescribeTagsResult(tags: tags.collect {new TagDescription(key: it.key, value: it.value) }) }