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

Detect and correct "incorrect" use of new FnJoin #516

Merged
merged 3 commits into from
Aug 7, 2018
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
2 changes: 1 addition & 1 deletion examples/cdk-examples-typescript/advanced-usage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class CloudFormationExample extends cdk.Stack {
// outputs are constructs the synthesize into the template's "Outputs" section
new cdk.Output(this, 'Output', {
description: 'This is an output of the template',
value: new cdk.FnJoin(',', new cdk.AwsAccountId(), '/', param.ref)
value: new cdk.FnConcat(new cdk.AwsAccountId(), '/', param.ref)
});

// stack.templateOptions can be used to specify template-level options
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-events/test/test.rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export = {
// tokens can also used for JSON templates, but that means escaping is
// the responsibility of the user.
rule.addTarget(t4, {
jsonTemplate: new cdk.FnJoin(' ', '"', 'hello', '\"world\"', '"'),
jsonTemplate: new cdk.FnJoin(' ', ['"', 'hello', '\"world\"', '"']),
});

expect(stack).toMatch({
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-rds/lib/cluster-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,6 @@ export class Endpoint {
constructor(address: DBClusterEndpointAddress, port: Port) {
this.hostname = address;
this.port = port;
this.socketAddress = new cdk.FnJoin(":", address, port);
this.socketAddress = new cdk.FnJoin(":", [address, port]);
}
}
94 changes: 49 additions & 45 deletions packages/@aws-cdk/cdk/lib/cloudformation/fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ export class Fn extends Token {
}

/**
* The intrinsic function Fn::FindInMap returns the value corresponding to keys in a two-level
* The intrinsic function ``Fn::FindInMap`` returns the value corresponding to keys in a two-level
* map that is declared in the Mappings section.
*/
export class FnFindInMap extends Fn {
/**
* Creates an Fn::FindInMap function.
* Creates an ``Fn::FindInMap`` function.
* @param mapName The logical name of a mapping declared in the Mappings section that contains the keys and values.
* @param topLevelKey The top-level key name. Its value is a list of key-value pairs.
* @param secondLevelKey The second-level key name, which is set to one of the keys from the list assigned to TopLevelKey.
Expand All @@ -28,11 +28,11 @@ export class FnFindInMap extends Fn {
}

/**
* The Fn::GetAtt intrinsic function returns the value of an attribute from a resource in the template.
* The ``Fn::GetAtt`` intrinsic function returns the value of an attribute from a resource in the template.
*/
export class FnGetAtt extends Fn {
/**
* Creates a Fn::GetAtt function.
* Creates a ``Fn::GetAtt`` function.
* @param logicalNameOfResource The logical name (also called logical ID) of the resource that contains the attribute that you want.
* @param attributeName The name of the resource-specific attribute whose value you want. See the resource's reference page for details about the attributes available for that resource type.
*/
Expand All @@ -42,15 +42,15 @@ export class FnGetAtt extends Fn {
}

/**
* The intrinsic function Fn::GetAZs returns an array that lists Availability Zones for a
* The intrinsic function ``Fn::GetAZs`` returns an array that lists Availability Zones for a
* specified region. Because customers have access to different Availability Zones, the intrinsic
* function Fn::GetAZs enables template authors to write templates that adapt to the calling
* function ``Fn::GetAZs`` enables template authors to write templates that adapt to the calling
* user's access. That way you don't have to hard-code a full list of Availability Zones for a
* specified region.
*/
export class FnGetAZs extends Fn {
/**
* Creates an Fn::GetAZs function.
* Creates an ``Fn::GetAZs`` function.
* @param region The name of the region for which you want to get the Availability Zones.
* You can use the AWS::Region pseudo parameter to specify the region in
* which the stack is created. Specifying an empty string is equivalent to
Expand All @@ -62,13 +62,13 @@ export class FnGetAZs extends Fn {
}

/**
* The intrinsic function Fn::ImportValue returns the value of an output exported by another stack.
* The intrinsic function ``Fn::ImportValue`` returns the value of an output exported by another stack.
* You typically use this function to create cross-stack references. In the following example
* template snippets, Stack A exports VPC security group values and Stack B imports them.
*/
export class FnImportValue extends Fn {
/**
* Creates an Fn::ImportValue function.
* Creates an ``Fn::ImportValue`` function.
* @param sharedValueToImport The stack output value that you want to import.
*/
constructor(sharedValueToImport: string) {
Expand All @@ -77,40 +77,44 @@ export class FnImportValue extends Fn {
}

/**
* The intrinsic function Fn::Join appends a set of values into a single value, separated by
* The intrinsic function ``Fn::Join`` appends a set of values into a single value, separated by
* the specified delimiter. If a delimiter is the empty string, the set of values are concatenated
* with no delimiter.
*/
export class FnJoin extends Fn {
/**
* Creates an Fn::Join function.
* @param delimiter The value you want to occur between fragments. The delimiter will occur between fragments only. It will not terminate the final value.
* Creates an ``Fn::Join`` function.
* @param delimiter The value you want to occur between fragments. The delimiter will occur between fragments only.
* It will not terminate the final value.
* @param listOfValues The list of values you want combined.
*/
constructor(delimiter: string, ...listOfValues: any[]) {
constructor(delimiter: string, listOfValues: any[]) {
if (listOfValues.length === 0) {
throw new Error(`FnJoin requires at least one value to be provided`);
}
super('Fn::Join', [ delimiter, listOfValues ]);
}
}

/**
* Alias for Fn::Join('', [ values ]).
* Alias for ``FnJoin('', listOfValues)``.
*/
export class FnConcat extends FnJoin {
/**
* Creates an Fn::Join function with an empty delimiter.
* Creates an ``Fn::Join`` function with an empty delimiter.
* @param listOfValues The list of values to concatenate.
*/
constructor(...listOfValues: any[]) {
super('', ...listOfValues);
super('', listOfValues);
}
}

/**
* The intrinsic function Fn::Select returns a single object from a list of objects by index.
* The intrinsic function ``Fn::Select`` returns a single object from a list of objects by index.
*/
export class FnSelect extends Fn {
/**
* Creates an Fn::Select function.
* Creates an ``Fn::Select`` function.
* @param index The index of the object to retrieve. This must be a value from zero to N-1, where N represents the number of elements in the array.
* @param array The list of objects to select from. This list must not be null, nor can it have null entries.
*/
Expand All @@ -121,13 +125,13 @@ export class FnSelect extends Fn {

/**
* To split a string into a list of string values so that you can select an element from the
* resulting string list, use the Fn::Split intrinsic function. Specify the location of splits
* with a delimiter, such as , (a comma). After you split a string, use the Fn::Select function
* resulting string list, use the ``Fn::Split`` intrinsic function. Specify the location of splits
* with a delimiter, such as , (a comma). After you split a string, use the ``Fn::Select`` function
* to pick a specific element.
*/
export class FnSplit extends Fn {
/**
* Create an Fn::Split function.
* Create an ``Fn::Split`` function.
* @param delimiter A string value that determines where the source string is divided.
* @param source The string value that you want to split.
*/
Expand All @@ -137,13 +141,13 @@ export class FnSplit extends Fn {
}

/**
* The intrinsic function Fn::Sub substitutes variables in an input string with values that
* The intrinsic function ``Fn::Sub`` substitutes variables in an input string with values that
* you specify. In your templates, you can use this function to construct commands or outputs
* that include values that aren't available until you create or update a stack.
*/
export class FnSub extends Fn {
/**
* Creates an Fn::Sub function.
* Creates an ``Fn::Sub`` function.
* @param body A string with variables that AWS CloudFormation substitutes with their
* associated values at runtime. Write variables as ${MyVarName}. Variables
* can be template parameter names, resource logical IDs, resource attributes,
Expand All @@ -158,14 +162,14 @@ export class FnSub extends Fn {
}

/**
* The intrinsic function Fn::Base64 returns the Base64 representation of the input string.
* The intrinsic function ``Fn::Base64`` returns the Base64 representation of the input string.
* This function is typically used to pass encoded data to Amazon EC2 instances by way of
* the UserData property.
*/
export class FnBase64 extends Fn {

/**
* Creates an Fn::Base64 function.
* Creates an ``Fn::Base64`` function.
* @param data The string value you want to convert to Base64.
*/
constructor(data: any) {
Expand All @@ -174,11 +178,11 @@ export class FnBase64 extends Fn {
}

/**
* The intrinsic function Fn::Cidr returns the specified Cidr address block.
* The intrinsic function ``Fn::Cidr`` returns the specified Cidr address block.
*/
export class FnCidr extends Fn {
/**
* Creates an Fn::Cidr function.
* Creates an ``Fn::Cidr`` function.
* @param ipBlock The user-specified default Cidr address block.
* @param count The number of subnets' Cidr block wanted. Count can be 1 to 256.
* @param sizeMask The digit covered in the subnet.
Expand All @@ -192,14 +196,14 @@ export class FnCidr extends Fn {
}

/**
* You can use intrinsic functions, such as Fn::If, Fn::Equals, and Fn::Not, to conditionally
* You can use intrinsic functions, such as ``Fn::If``, ``Fn::Equals``, and ``Fn::Not``, to conditionally
* create stack resources. These conditions are evaluated based on input parameters that you
* declare when you create or update a stack. After you define all your conditions, you can
* associate them with resources or resource properties in the Resources and Outputs sections
* of a template.
*
* You define all conditions in the Conditions section of a template except for Fn::If conditions.
* You can use the Fn::If condition in the metadata attribute, update policy attribute, and property
* You define all conditions in the Conditions section of a template except for ``Fn::If`` conditions.
* You can use the ``Fn::If`` condition in the metadata attribute, update policy attribute, and property
* values in the Resources section and Outputs sections of a template.
*
* You might use conditions when you want to reuse a template that can create resources in different
Expand All @@ -216,7 +220,7 @@ export class FnCondition extends Fn {

/**
* Returns true if all the specified conditions evaluate to true, or returns false if any one
* of the conditions evaluates to false. Fn::And acts as an AND operator. The minimum number of
* of the conditions evaluates to false. ``Fn::And`` acts as an AND operator. The minimum number of
* conditions that you can include is 2, and the maximum is 10.
*/
export class FnAnd extends FnCondition {
Expand All @@ -231,7 +235,7 @@ export class FnAnd extends FnCondition {
*/
export class FnEquals extends FnCondition {
/**
* Creates an Fn::Equals condition function.
* Creates an ``Fn::Equals`` condition function.
* @param lhs A value of any type that you want to compare.
* @param rhs A value of any type that you want to compare.
*/
Expand All @@ -242,14 +246,14 @@ export class FnEquals extends FnCondition {

/**
* Returns one value if the specified condition evaluates to true and another value if the
* specified condition evaluates to false. Currently, AWS CloudFormation supports the Fn::If
* specified condition evaluates to false. Currently, AWS CloudFormation supports the ``Fn::If``
* intrinsic function in the metadata attribute, update policy attribute, and property values
* in the Resources section and Outputs sections of a template. You can use the AWS::NoValue
* pseudo parameter as a return value to remove the corresponding property.
*/
export class FnIf extends FnCondition {
/**
* Creates an Fn::If condition function.
* Creates an ``Fn::If`` condition function.
* @param condition A reference to a condition in the Conditions section. Use the condition's name to reference it.
* @param valueIfTrue A value to be returned if the specified condition evaluates to true.
* @param valueIfFalse A value to be returned if the specified condition evaluates to false.
Expand All @@ -261,12 +265,12 @@ export class FnIf extends FnCondition {

/**
* Returns true for a condition that evaluates to false or returns false for a condition that evaluates to true.
* Fn::Not acts as a NOT operator.
* ``Fn::Not`` acts as a NOT operator.
*/
export class FnNot extends FnCondition {
/**
* Creates an Fn::Not condition function.
* @param condition A condition such as Fn::Equals that evaluates to true or false.
* Creates an ``Fn::Not`` condition function.
* @param condition A condition such as ``Fn::Equals`` that evaluates to true or false.
*/
constructor(condition: FnCondition) {
super('Fn::Not', [ condition ]);
Expand All @@ -275,12 +279,12 @@ export class FnNot extends FnCondition {

/**
* Returns true if any one of the specified conditions evaluate to true, or returns false if
* all of the conditions evaluates to false. Fn::Or acts as an OR operator. The minimum number
* all of the conditions evaluates to false. ``Fn::Or`` acts as an OR operator. The minimum number
* of conditions that you can include is 2, and the maximum is 10.
*/
export class FnOr extends FnCondition {
/**
* Creates an Fn::Or condition function.
* Creates an ``Fn::Or`` condition function.
* @param condition A condition that evaluates to true or false.
*/
constructor(...condition: FnCondition[]) {
Expand All @@ -293,7 +297,7 @@ export class FnOr extends FnCondition {
*/
export class FnContains extends FnCondition {
/**
* Creates an Fn::Contains function.
* Creates an ``Fn::Contains`` function.
* @param listOfStrings A list of strings, such as "A", "B", "C".
* @param value A string, such as "A", that you want to compare against a list of strings.
*/
Expand All @@ -307,7 +311,7 @@ export class FnContains extends FnCondition {
*/
export class FnEachMemberEquals extends FnCondition {
/**
* Creates an Fn::EachMemberEquals function.
* Creates an ``Fn::EachMemberEquals`` function.
* @param listOfStrings A list of strings, such as "A", "B", "C".
* @param value A string, such as "A", that you want to compare against a list of strings.
*/
Expand All @@ -322,7 +326,7 @@ export class FnEachMemberEquals extends FnCondition {
*/
export class FnEachMemberIn extends FnCondition {
/**
* Creates an Fn::EachMemberIn function.
* Creates an ``Fn::EachMemberIn`` function.
* @param stringsToCheck A list of strings, such as "A", "B", "C". AWS CloudFormation checks whether each member in the strings_to_check parameter is in the strings_to_match parameter.
* @param stringsToMatch A list of strings, such as "A", "B", "C". Each member in the strings_to_match parameter is compared against the members of the strings_to_check parameter.
*/
Expand All @@ -336,7 +340,7 @@ export class FnEachMemberIn extends FnCondition {
*/
export class FnRefAll extends FnCondition {
/**
* Creates an Fn::RefAll function.
* Creates an ``Fn::RefAll`` function.
* @param parameterType An AWS-specific parameter type, such as AWS::EC2::SecurityGroup::Id or
* AWS::EC2::VPC::Id. For more information, see Parameters in the AWS
* CloudFormation User Guide.
Expand All @@ -351,7 +355,7 @@ export class FnRefAll extends FnCondition {
*/
export class FnValueOf extends FnCondition {
/**
* Creates an Fn::ValueOf function.
* Creates an ``Fn::ValueOf`` function.
* @param parameterOrLogicalId The name of a parameter for which you want to retrieve attribute values. The parameter must be declared in the Parameters section of the template.
* @param attribute The name of an attribute from which you want to retrieve a value.
*/
Expand All @@ -365,7 +369,7 @@ export class FnValueOf extends FnCondition {
*/
export class FnValueOfAll extends FnCondition {
/**
* Creates an Fn::ValueOfAll function.
* Creates an ``Fn::ValueOfAll`` function.
* @param parameterType An AWS-specific parameter type, such as AWS::EC2::SecurityGroup::Id or AWS::EC2::VPC::Id. For more information, see Parameters in the AWS CloudFormation User Guide.
* @param attribute The name of an attribute from which you want to retrieve a value. For more information about attributes, see Supported Attributes.
*/
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/cdk/lib/cloudformation/output.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export class StringListOutput extends Construct {
condition: props.condition,
disableExport: props.disableExport,
export: props.export,
value: new FnJoin(this.separator, ...props.values)
value: new FnJoin(this.separator, props.values)
});
}

Expand All @@ -224,4 +224,4 @@ export class StringListOutput extends Construct {

return ret;
}
}
}
11 changes: 11 additions & 0 deletions packages/@aws-cdk/cdk/test/cloudformation/test.fn.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import nodeunit = require('nodeunit');
import fn = require('../../lib/cloudformation/fn');

export = nodeunit.testCase({
'Fn::Join': {
'rejects empty list of arguments to join'(test: nodeunit.Test) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure there aren’t already tests for these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reasonably:

  1. Nothing showed up in /test/ when I grep'd for FnJoin(.
  2. There was no test.fn.ts and that's where I expect those unit tests to be

test.throws(() => new fn.FnJoin('.', []));
test.done();
}
}
});
2 changes: 1 addition & 1 deletion packages/aws-cdk-docs/src/cloudformation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ Intrinsic Functions
.. code-block:: js

import cdk = require('@aws-cdk/cdk');
new cdk.FnJoin(",", ...)
new cdk.FnJoin(",", [...])

.. _pseudo_parameters:

Expand Down