Skip to content

Commit

Permalink
refactor: name "toCloudFormation" internal (renamed to "_toCloudForma…
Browse files Browse the repository at this point in the history
…tion") (#2047)

The method `toCloudFormation` is not supposed to be directly called by
users. Mark it as `@internal` and rename to `_toCloudFormation`.

Fixes #2044
Related #2016

BREAKING CHANGE: “toCloudFormation” is now internal and should not be called directly. Instead use “app.synthesizeStack”
  • Loading branch information
Elad Ben-Israel authored Mar 19, 2019
1 parent 6a671f2 commit 515868b
Show file tree
Hide file tree
Showing 36 changed files with 128 additions and 124 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assert/lib/expect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function expect(stack: api.SynthesizedStack | cdk.Stack, skipValidation =

sstack = {
name: stack.name,
template: stack.toCloudFormation(),
template: stack._toCloudFormation(),
metadata: collectStackMetadata(stack.node),
environment: {
name: 'test',
Expand All @@ -36,7 +36,7 @@ export function expect(stack: api.SynthesizedStack | cdk.Stack, skipValidation =
}

function isStackClassInstance(x: api.SynthesizedStack | cdk.Stack): x is cdk.Stack {
return 'toCloudFormation' in x;
return '_toCloudFormation' in x;
}

function collectStackMetadata(root: cdk.ConstructNode): api.StackMetadata {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/assets-docker/test/test.image-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export = {
});

// THEN
const template = stack.toCloudFormation();
const template = stack._toCloudFormation();

test.deepEqual(template.Parameters.ImageImageName5E684353, {
Type: 'String',
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assets/test/test.asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export = {
});

// verify that now the template contains parameters for this asset
const template = stack.toCloudFormation();
const template = stack._toCloudFormation();
test.equal(template.Parameters.MyAssetS3Bucket68C9B344.Type, 'String');
test.equal(template.Parameters.MyAssetS3VersionKey68E1A45D.Type, 'String');

Expand Down Expand Up @@ -74,7 +74,7 @@ export = {
});

// verify that now the template contains parameters for this asset
const template = stack.toCloudFormation();
const template = stack._toCloudFormation();
test.equal(template.Parameters.MyAssetS3Bucket68C9B344.Type, 'String');
test.equal(template.Parameters.MyAssetS3VersionKey68E1A45D.Type, 'String');

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/test/test.deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export = {

function synthesize() {
stack.node.prepareTree();
return stack.toCloudFormation();
return stack._toCloudFormation();
}
},

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-apigateway/test/test.restapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ export = {

// THEN
stack.node.prepareTree();
test.deepEqual(stack.toCloudFormation().Outputs.MyRestApiRestApiIdB93C5C2D, {
test.deepEqual(stack._toCloudFormation().Outputs.MyRestApiRestApiIdB93C5C2D, {
Value: { Ref: 'MyRestApi2D1F47A9' },
Export: { Name: 'MyRestApiRestApiIdB93C5C2D' }
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ function setupStepScaling(intervals: appscaling.ScalingInterval[]) {
scalingSteps: intervals
});

return new ScalingStackTemplate(stack.toCloudFormation());
return new ScalingStackTemplate(stack._toCloudFormation());
}

class ScalingStackTemplate {
Expand Down
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export = {
expect(stack).to(haveResource("AWS::S3::Bucket"));
expect(stack).to(haveResource("AWS::S3::BucketPolicy", ExpectedBucketPolicyProperties));
expect(stack).to(not(haveResource("AWS::Logs::LogGroup")));
const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
const trail: any = stack._toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.deepEqual(trail.DependsOn, ['MyAmazingCloudTrailS3Policy39C120B0']);
test.done();
},
Expand Down Expand Up @@ -97,7 +97,7 @@ export = {
PolicyName: logsRolePolicyName,
Roles: [{ Ref: 'MyAmazingCloudTrailLogsRoleF2CCF977' }],
}));
const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
const trail: any = stack._toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.deepEqual(trail.DependsOn, [logsRolePolicyName, logsRoleName, 'MyAmazingCloudTrailS3Policy39C120B0']);
test.done();
},
Expand All @@ -116,7 +116,7 @@ export = {
expect(stack).to(haveResource("AWS::Logs::LogGroup", {
RetentionInDays: 7
}));
const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
const trail: any = stack._toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.deepEqual(trail.DependsOn, [logsRolePolicyName, logsRoleName, 'MyAmazingCloudTrailS3Policy39C120B0']);
test.done();
},
Expand All @@ -134,7 +134,7 @@ export = {
expect(stack).to(not(haveResource("AWS::Logs::LogGroup")));
expect(stack).to(not(haveResource("AWS::IAM::Role")));

const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
const trail: any = stack._toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.equals(trail.Properties.EventSelectors.length, 1);
const selector = trail.Properties.EventSelectors[0];
test.equals(selector.ReadWriteType, null, "Expected selector read write type to be undefined");
Expand All @@ -160,7 +160,7 @@ export = {
expect(stack).to(not(haveResource("AWS::Logs::LogGroup")));
expect(stack).to(not(haveResource("AWS::IAM::Role")));

const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
const trail: any = stack._toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.equals(trail.Properties.EventSelectors.length, 1);
const selector = trail.Properties.EventSelectors[0];
test.equals(selector.ReadWriteType, "ReadOnly", "Expected selector read write type to be Read");
Expand All @@ -179,7 +179,7 @@ export = {

new CloudTrail(stack, 'MyAmazingCloudTrail', { managementEvents: ReadWriteType.WriteOnly });

const trail: any = stack.toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
const trail: any = stack._toCloudFormation().Resources.MyAmazingCloudTrail54516E8D;
test.equals(trail.Properties.EventSelectors.length, 1);
const selector = trail.Properties.EventSelectors[0];
test.equals(selector.ReadWriteType, "WriteOnly", "Expected selector read write type to be All");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ export = {
});

test.throws(() => {
stack.toCloudFormation();
stack._toCloudFormation();
}, /deploymentInAlarm/);

test.done();
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export = {
],
});

test.notDeepEqual(stack.toCloudFormation(), {});
test.notDeepEqual(stack._toCloudFormation(), {});
test.deepEqual([], pipeline.node.validateTree());
test.done();
},
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-iam/test/test.role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ export = {
assumedBy: new ServicePrincipal('sns.amazonaws.com')
});

test.ok(!('MyRoleDefaultPolicyA36BE1DD' in stack.toCloudFormation().Resources), 'initially created without a policy');
test.ok(!('MyRoleDefaultPolicyA36BE1DD' in stack._toCloudFormation().Resources), 'initially created without a policy');

role.addToPolicy(new PolicyStatement().addResource('myresource').addAction('myaction'));
test.ok(stack.toCloudFormation().Resources.MyRoleDefaultPolicyA36BE1DD, 'policy resource created');
test.ok(stack._toCloudFormation().Resources.MyRoleDefaultPolicyA36BE1DD, 'policy resource created');

expect(stack).toMatch({ Resources:
{ MyRoleF48FFE04:
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-s3/test/test.bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ export = {
test.deepEqual(bucket.bucketArn, bucketArn);
test.deepEqual(bucket.node.resolve(bucket.bucketName), 'my-bucket');

test.deepEqual(stack.toCloudFormation(), {}, 'the ref is not a real resource');
test.deepEqual(stack._toCloudFormation(), {}, 'the ref is not a real resource');
test.done();
},

Expand Down Expand Up @@ -925,7 +925,7 @@ export = {
bucket.grantWrite(writer);
bucket.grantDelete(deleter);

const resources = stack.toCloudFormation().Resources;
const resources = stack._toCloudFormation().Resources;
const actions = (id: string) => resources[id].Properties.PolicyDocument.Statement[0].Action;

test.deepEqual(actions('WriterDefaultPolicyDC585BCE'), [ 's3:DeleteObject*', 's3:PutObject*', 's3:Abort*' ]);
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-s3/test/test.notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ export = {
bucket.onObjectCreated(dest);

stack.node.prepareTree();
test.deepEqual(stack.toCloudFormation().Resources.BucketNotifications8F2E257D, {
test.deepEqual(stack._toCloudFormation().Resources.BucketNotifications8F2E257D, {
Type: 'Custom::S3BucketNotifications',
Properties: {
ServiceToken: { 'Fn::GetAtt': [ 'BucketNotificationsHandler050a0587b7544547bf325f094a3db8347ECC3691', 'Arn' ] },
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-sqs/test/test.sqs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export = {
test.deepEqual(stack.node.resolve(imports.queueUrl), { 'Fn::ImportValue': 'QueueQueueUrlC30FF916' });

// the exporting stack has Outputs for QueueARN and QueueURL
const outputs = stack.toCloudFormation().Outputs;
const outputs = stack._toCloudFormation().Outputs;
test.deepEqual(outputs.QueueQueueArn8CF496D5, { Value: { 'Fn::GetAtt': [ 'Queue4A7E3555', 'Arn' ] }, Export: { Name: 'QueueQueueArn8CF496D5' } });
test.deepEqual(outputs.QueueQueueUrlC30FF916, { Value: { Ref: 'Queue4A7E3555' }, Export: { Name: 'QueueQueueUrlC30FF916' } });

Expand Down Expand Up @@ -252,7 +252,7 @@ export = {
keyArn: { 'Fn::ImportValue': 'QueueWithCustomKeyKeyArn537F6E42' }
});

test.deepEqual(stack.toCloudFormation().Outputs, {
test.deepEqual(stack._toCloudFormation().Outputs, {
"QueueWithCustomKeyQueueArnD326BB9B": {
"Value": {
"Fn::GetAtt": [
Expand Down Expand Up @@ -300,7 +300,7 @@ export = {
keyArn: { 'Fn::ImportValue': 'QueueWithManagedKeyKeyArn9C42A85D' }
});

test.deepEqual(stack.toCloudFormation().Outputs, {
test.deepEqual(stack._toCloudFormation().Outputs, {
"QueueWithManagedKeyQueueArn8798A14E": {
"Value": {
"Fn::GetAtt": [
Expand Down Expand Up @@ -406,7 +406,7 @@ export = {

// make sure the queue policy is added as a dependency to the bucket
// notifications resource so it will be created first.
test.deepEqual(stack.toCloudFormation().Resources.BucketNotifications8F2E257D.DependsOn, [ 'QueuePolicy25439813' ]);
test.deepEqual(stack._toCloudFormation().Resources.BucketNotifications8F2E257D.DependsOn, [ 'QueuePolicy25439813' ]);

test.done();
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/cfn-condition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class CfnCondition extends CfnRefElement implements ICfnConditionExpressi
this.expression = props && props.expression;
}

public toCloudFormation(): object {
public _toCloudFormation(): object {
if (!this.expression) {
return { };
}
Expand Down
8 changes: 5 additions & 3 deletions packages/@aws-cdk/cdk/lib/cfn-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export abstract class CfnElement extends Construct {
* @returns The construct as a stack element or undefined if it is not a stack element.
*/
public static isCfnElement(construct: IConstruct): construct is CfnElement {
return ('logicalId' in construct && 'toCloudFormation' in construct);
return ('logicalId' in construct && '_toCloudFormation' in construct);
}

/**
Expand Down Expand Up @@ -100,8 +100,10 @@ export abstract class CfnElement extends Construct {
* }
* }
* }
*
* @internal
*/
public abstract toCloudFormation(): object;
public abstract _toCloudFormation(): object;

/**
* Automatically detect references in this CfnElement
Expand All @@ -117,7 +119,7 @@ export abstract class CfnElement extends Construct {
// This does make the assumption that the error will not be rectified,
// but the error will be thrown later on anyway. If the error doesn't
// get thrown down the line, we may miss references.
this.node.recordReference(...findTokens(this, () => this.toCloudFormation()));
this.node.recordReference(...findTokens(this, () => this._toCloudFormation()));
} catch (e) {
if (e.type !== 'CfnSynthesisError') { throw e; }
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/cfn-mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class CfnMapping extends CfnRefElement {
return Fn.findInMap(this.logicalId, key1, key2);
}

public toCloudFormation(): object {
public _toCloudFormation(): object {
return {
Mappings: {
[this.logicalId]: this.mapping
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/cfn-output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class CfnOutput extends CfnElement {
return fn().importValue(this.obtainExportName());
}

public toCloudFormation(): object {
public _toCloudFormation(): object {
return {
Outputs: {
[this.logicalId]: {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/cfn-parameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class CfnParameter extends CfnRefElement {
this.stringListValue = this.value.toList();
}

public toCloudFormation(): object {
public _toCloudFormation(): object {
return {
Parameters: {
[this.logicalId]: {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ export class CfnResource extends CfnRefElement {
/**
* Emits CloudFormation for this resource.
*/
public toCloudFormation(): object {
public _toCloudFormation(): object {
try {
// merge property overrides onto properties and then render (and validate).
const tags = CfnResource.isTaggable(this) ? this.tags.renderTags() : undefined;
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/cfn-rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class CfnRule extends CfnRefElement {
});
}

public toCloudFormation(): object {
public _toCloudFormation(): object {
return {
Rules: {
[this.logicalId]: {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/lib/include.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class Include extends CfnElement {
this.template = props.template;
}

public toCloudFormation() {
public _toCloudFormation() {
return this.template;
}
}
10 changes: 6 additions & 4 deletions packages/@aws-cdk/cdk/lib/stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,11 @@ export class Stack extends Construct {

/**
* Returns the CloudFormation template for this stack by traversing
* the tree and invoking toCloudFormation() on all Entity objects.
* the tree and invoking _toCloudFormation() on all Entity objects.
*
* @internal
*/
public toCloudFormation() {
public _toCloudFormation() {
// before we begin synthesis, we shall lock this stack, so children cannot be added
this.node.lock();

Expand All @@ -158,7 +160,7 @@ export class Stack extends Construct {
};

const elements = cfnElements(this);
const fragments = elements.map(e => this.node.resolve(e.toCloudFormation()));
const fragments = elements.map(e => this.node.resolve(e._toCloudFormation()));

// merge in all CloudFormation fragments collected from the tree
for (const fragment of fragments) {
Expand Down Expand Up @@ -446,7 +448,7 @@ export class Stack extends Construct {
const template = `${this.node.id}.template.json`;

// write the CloudFormation template as a JSON file
session.store.writeJson(template, this.toCloudFormation());
session.store.writeJson(template, this._toCloudFormation());

const artifact: cxapi.Artifact = {
type: cxapi.ArtifactType.AwsCloudFormationStack,
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/test/test.arn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export = {
new CfnOutput(stack2, 'SomeValue', { value: arn });

// THEN
test.deepEqual(stack2.node.resolve(stack2.toCloudFormation()), {
test.deepEqual(stack2.node.resolve(stack2._toCloudFormation()), {
Outputs: {
SomeValue: {
Value: {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cdk/test/test.condition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export = {
});

// THEN
test.deepEqual(stack.toCloudFormation(), {
test.deepEqual(stack._toCloudFormation(), {
Parameters: { Param1: { Type: 'String' } },
Conditions: {
Condition1: { 'Fn::Equals': [ 'a', 'b' ] },
Expand Down Expand Up @@ -45,7 +45,7 @@ export = {

// THEN
test.ok(cdk.unresolved(propValue));
test.deepEqual(stack.toCloudFormation(), {
test.deepEqual(stack._toCloudFormation(), {
Resources: {
MyResource: {
Type: 'AWS::Foo::Bar',
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/cdk/test/test.include.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export = {

new Include(stack, 'T1', { template: clone(template) });

test.deepEqual(stack.toCloudFormation(), {
test.deepEqual(stack._toCloudFormation(), {
Parameters: { MyParam: { Type: 'String', Default: 'Hello' } },
Resources: {
MyResource1: { Type: 'ResourceType1', Properties: { P1: 1, P2: 2 } },
Expand All @@ -24,7 +24,7 @@ export = {
new CfnOutput(stack, 'MyOutput', { description: 'Out!', disableExport: true });
new CfnParameter(stack, 'MyParam2', { type: 'Integer' });

test.deepEqual(stack.toCloudFormation(), {
test.deepEqual(stack._toCloudFormation(), {
Parameters: {
MyParam: { Type: 'String', Default: 'Hello' },
MyParam2: { Type: 'Integer' } },
Expand All @@ -46,7 +46,7 @@ export = {
new CfnOutput(stack, 'MyOutput', { description: 'Out!' });
new CfnParameter(stack, 'MyParam', { type: 'Integer' }); // duplicate!

test.throws(() => stack.toCloudFormation());
test.throws(() => stack._toCloudFormation());
test.done();
},
};
Expand Down
Loading

0 comments on commit 515868b

Please sign in to comment.