Skip to content

Commit

Permalink
feat(cloudformation): Treat template as string instead of yaml (#3849)
Browse files Browse the repository at this point in the history
To allow for templates containing instinsic functions (e.g. !Ref)
this changes the cloudformation operation so the parameter is processed
as a string, instead of a map.

For backwards compatibility, if the received value is a map, it will
be dumped as a string before continuing.
  • Loading branch information
gregjones authored and ethanfrogers committed Jul 30, 2019
1 parent 50573da commit dd4d701
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.netflix.spinnaker.clouddriver.aws.deploy.converters;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.netflix.spinnaker.clouddriver.aws.AmazonOperation;
import com.netflix.spinnaker.clouddriver.aws.deploy.description.DeployCloudFormationDescription;
import com.netflix.spinnaker.clouddriver.aws.deploy.ops.DeployCloudFormationAtomicOperation;
Expand All @@ -35,9 +36,26 @@ public AtomicOperation convertOperation(Map input) {

@Override
public DeployCloudFormationDescription convertDescription(Map input) {
input = fixTemplateBody(input);

DeployCloudFormationDescription converted =
getObjectMapper().convertValue(input, DeployCloudFormationDescription.class);
converted.setCredentials(getCredentialsObject((String) input.get("credentials")));
return converted;
}

/** Previous implementation processed templateBody as a Map, now it is a string */
private Map fixTemplateBody(Map input) {
if (input.get("templateBody") != null && !(input.get("templateBody") instanceof String)) {
String template;
try {
template = getObjectMapper().writeValueAsString(input.get("templateBody"));
} catch (JsonProcessingException e) {
throw new IllegalArgumentException(
"Could not serialize CloudFormation Stack template body", e);
}
input.put("templateBody", template);
}
return input;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
public class DeployCloudFormationDescription extends AbstractAmazonCredentialsDescription {

private String stackName;
private Map<String, Object> templateBody = new HashMap<>();
private String templateBody;
private Map<String, String> parameters = new HashMap<>();
private Map<String, String> tags = new HashMap<>();
private String region;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import com.amazonaws.services.cloudformation.AmazonCloudFormation;
import com.amazonaws.services.cloudformation.model.*;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.clouddriver.aws.deploy.description.DeployCloudFormationDescription;
import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider;
Expand Down Expand Up @@ -57,13 +56,7 @@ public Map operate(List priorOutputs) {
AmazonCloudFormation amazonCloudFormation =
amazonClientProvider.getAmazonCloudFormation(
description.getCredentials(), description.getRegion());
String template;
try {
template = objectMapper.writeValueAsString(description.getTemplateBody());
} catch (JsonProcessingException e) {
throw new IllegalArgumentException(
"Could not serialize CloudFormation Stack template body", e);
}
String template = description.getTemplateBody();
List<Parameter> parameters =
description.getParameters().entrySet().stream()
.map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@
package com.netflix.spinnaker.clouddriver.aws.deploy.converters

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.clouddriver.aws.deploy.converters.CopyLastAsgAtomicOperationConverter
import com.netflix.spinnaker.clouddriver.aws.deploy.description.BasicAmazonDeployDescription
import com.netflix.spinnaker.clouddriver.aws.deploy.description.DeployCloudFormationDescription
import com.netflix.spinnaker.clouddriver.aws.deploy.ops.CopyLastAsgAtomicOperation
import com.netflix.spinnaker.clouddriver.aws.deploy.ops.DeployCloudFormationAtomicOperation
import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials
import com.netflix.spinnaker.clouddriver.security.AccountCredentialsProvider
Expand All @@ -43,7 +40,7 @@ class DeployCloudFormationAtomicOperationConverterSpec extends Specification {
converter.accountCredentialsProvider = accountCredentialsProvider
}

void "DeployCloudFormationConverter returns DeployCloudFormationDescription"() {
void "DeployCloudFormationConverter returns DeployCloudFormationDescription with Map templateBody"() {
setup:
def input = [stackName : "asgard",
templateBody : [ field1: "field1" ],
Expand All @@ -60,6 +57,31 @@ class DeployCloudFormationAtomicOperationConverterSpec extends Specification {

then:
description instanceof DeployCloudFormationDescription
((DeployCloudFormationDescription) description).templateBody == '{"field1":"field1"}'

when:
def operation = converter.convertOperation(input)

then:
operation instanceof DeployCloudFormationAtomicOperation
}

void "DeployCloudFormationConverter returns DeployCloudFormationDescription with string templateBody"() {
setup:
def input = [stackName : "asgard",
templateBody : 'field1: "field1"',
parameters : [ param1: "param1" ],
tags : [ tag1: "tag1" ],
capabilities : [ "cap1", "cap2" ],
region : "eu-west_1",
credentials : "credentials"]

when:
def description = converter.convertDescription(input)

then:
description instanceof DeployCloudFormationDescription
((DeployCloudFormationDescription) description).templateBody == 'field1: "field1"'

when:
def operation = converter.convertOperation(input)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class DeployCloudFormationAtomicOperationSpec extends Specification {
[
stackName: "stackTest",
region: "eu-west-1",
templateBody: [ key: "value" ],
templateBody: '{"key":"value"}',
parameters: [ key: "value"],
tags: [ key: "value" ],
capabilities: ["cap1", "cap2"],
Expand Down Expand Up @@ -92,7 +92,7 @@ class DeployCloudFormationAtomicOperationSpec extends Specification {
[
stackName: "stackTest",
region: "eu-west-1",
templateBody: [ key: "value" ],
templateBody: '{"key":"value"}',
parameters: [ key: "value" ],
tags: [ key: "value" ],
capabilities: ["cap1", "cap2"],
Expand Down Expand Up @@ -134,7 +134,7 @@ class DeployCloudFormationAtomicOperationSpec extends Specification {
[
stackName: "stackTest",
region: "eu-west-1",
templateBody: [ key: "value" ],
templateBody: 'key: "value"',
parameters: [ key: "value" ],
tags: [ key: "value" ],
capabilities: ["cap1", "cap2"],
Expand All @@ -161,7 +161,7 @@ class DeployCloudFormationAtomicOperationSpec extends Specification {
}
1* amazonCloudFormation.createChangeSet(_) >> { CreateChangeSetRequest request ->
assert request.getStackName() == "stackTest"
assert request.getTemplateBody() == '{"key":"value"}'
assert request.getTemplateBody() == 'key: "value"'
assert request.getParameters() == [ new Parameter().withParameterKey("key").withParameterValue("value") ]
assert request.getTags() == [ new Tag().withKey("key").withValue("value") ]
assert request.getCapabilities() == ["cap1", "cap2"]
Expand Down

0 comments on commit dd4d701

Please sign in to comment.