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.

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

refactor: make "toCloudFormation" internal #2047

Merged
merged 1 commit into from
Mar 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we renaming to _toCloudFormation given that we have aws/jsii#390 in the pipes? Can we not just @internal the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in the CDK I'd like to standardize (awslint) that internal members are also prefixed with an underscore to make it clear to maintainers that they are internal.

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