BREAKING CHANGE: remove resource attribute types for string attributes #712

eladb · 2018-09-13T22:51:10Z

Implementation of #695 for resource attributes that return string.

Removed the code generation of attribute types for all resource attributes which return a string primitive and use stringified tokens to represent these attribute as strings.

Instead of .ref, we now generate a property based on the resource's RefKind which returns a string with a token that resolves to Ref (so instead of ref: QueueArn it is now queueArn: string). This is a much better experience because this way people don't need to guess what "ref" means (or use an artificial type).

This change also removes the cdk.Arn type (the utility functions are now available under cdk.ArnUtils.parse(s) and cdk.ArnUtils.format(components): string. The parse utility will discover if the ARN contains tokens and will not attempt to actually parse the string, but will rather use CFN intrinsics to split the ARN to it's components.

Remaining work:

Fix all build breaks (effectively removing all tokens from the world).
Verify all tests pass

Further ideas:

When working on Intrinsic functions and pseudo parameters hard to find, confusing to use #202, make Aws.region and friends return a stringified token instead of a token. Same for all intrinsic functions (Fn.xxx) which surely represent strings.
Introduce a helper function that will make it easier and less verbose to implement "exports". At the moment, it looks like this: new Output(this, 'RuleArn', { value: this.ruleArn }).makeImportValue().toString(). It should look like this Utils.export(this, 'RuleArn', this.ruleArn) or something like that.
Change the usage of tokens for lazy evaluation #736

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

…ribute-types

* Converted all attribute types to strings * Remove a stale example from aws-cloudformation * Add `--no-bail` to `build.sh` so it won't stop on the first error * Reduce some verbosity from cdk-build and cfn2ts

…ribute-types

packages/@aws-cdk/aws-apigateway/lib/deployment.ts

rix0rrr · 2018-09-18T07:54:47Z

packages/@aws-cdk/aws-cloudformation/examples/index.ts

@@ -1,104 +0,0 @@
-import lambda = require('@aws-cdk/aws-lambda');


If you don't want these files here, please move them instead of deleting them.

They are not compiling and became stale and unusable. What do you recommend we do?

Move them to examples/typescript-examples ?

packages/@aws-cdk/aws-cloudformation/test/integ.trivial-lambda-resource.ts

packages/@aws-cdk/aws-iam/lib/role.ts

packages/@aws-cdk/aws-s3/test/notification-dests.ts

packages/@aws-cdk/cdk/lib/cloudformation/arn.ts

rix0rrr · 2018-09-18T08:53:22Z

packages/@aws-cdk/cdk/lib/core/tokens.ts

@@ -102,11 +102,16 @@ export class Token {
 }

 /**
- * Returns true if obj is a token (i.e. has the resolve() method)
+ * Returns true if obj is a token (i.e. has the resolve() method or is a string
+ * that includes token markers).
 * @param obj The object to test.
 */
 export function isToken(obj: any): obj is Token {


Wouldn't hasToken() or containsToken() be more appropriate?

Renamed to unresolved, what do you think? (should it be isUnresolved)?

rix0rrr · 2018-09-18T08:56:25Z

tools/cfn2ts/lib/genspec.ts


-    const refType = new ClassDeclaration(refClass, baseClass);
-    return new Attribute('ref', refType, constructorArguments);
+    const refType = new ClassDeclaration(CodeName.forPrimitive('string'));


Actually the name ClassDeclaration is now wrong again, because we're never going to declare that class anymore.

We still have non-primitive attributes that we are emitting as strong types. Renamed to AttributeTypeDeclaration for now.

tools/cfn2ts/lib/index.ts

* Fix typo "managedPolicieArns" => "managedPolicyArns" * Remove unneeded `.toString` * Rename `isToken(x)` to `unresolved(x)` * Rename cfn2ts `ClassDeclaration` to `AttributeTypeDeclaration` * Revert log "generated code up-to-date" from cfn2ts

…ribute-types

* Restore custom resource example * Change `resource.getAtt` to return a token

__NOTICE__: This release includes a framework-wide [__breaking change__](#712) which changes the type of all the string resource attributes across the framework. Instead of using strong-types that extend `cdk.Token` (such as `QueueArn`, `TopicName`, etc), we now represent all these attributes as normal `string`s, and codify the tokens into the string (using the feature introduced in [#168](#168)). Furthermore, the `cdk.Arn` type has been removed. In order to format/parse ARNs, use the static methods on `cdk.ArnUtils`. See motivation and discussion in [#695](#695). * **cfn2ts:** use stringified tokens for resource attributes instead of strong types ([#712](#712)) ([6508f78](6508f78)), closes [#518](#518) [#695](#695) [#744](#744) * **aws-dynamodb:** Attribute type for keys, changes the signature of the `addPartitionKey` and `addSortKey` methods to be consistent across the board. ([#720](#720)) ([e6cc189](e6cc189)) * **aws-codebuild:** fix typo "priviledged" -> "privileged * **assets:** cab't use multiple assets in the same stack ([#725](#725)) ([bba2e5b](bba2e5b)), closes [#706](#706) * **aws-codebuild:** typo in BuildEnvironment "priviledged" -> "privileged ([#734](#734)) ([72fec36](72fec36)) * **aws-ecr:** fix addToResourcePolicy ([#737](#737)) ([eadbda5](eadbda5)) * **aws-events:** ruleName can now be specified ([#726](#726)) ([a7bc5ee](a7bc5ee)), closes [#708](#708) * **aws-lambda:** jsii use no long requires 'sourceAccount' ([#728](#728)) ([9e7d311](9e7d311)), closes [#714](#714) * **aws-s3:** remove `policy` argument ([#730](#730)) ([a79190c](a79190c)), closes [#672](#672) * **cdk:** "cdk init" java template is broken ([#732](#732)) ([281c083](281c083)), closes [#711](#711) [aws/jsii#233](aws/jsii#233) * **aws-apigateway:** new API Gateway Construct Library ([#665](#665)) ([b0f3857](b0f3857)) * **aws-cdk:** detect presence of EC2 credentials ([#724](#724)) ([8e8c295](8e8c295)), closes [#702](#702) [#130](#130) * **aws-codepipeline:** make the Stage insertion API in CodePipeline more flexible ([#460](#460)) ([d182818](d182818)) * **aws-codepipeline:** new "Pipeline#addStage" convenience method ([#647](#647)) ([25c9fa0](25c9fa0)) * **aws-rds:** add support for parameter groups ([#729](#729)) ([2541508](2541508)), closes [#719](#719) * **docs:** add documentation for CDK toolkit plugings ([#733](#733)) ([965b918](965b918)) * **dependencies:** upgrade to [jsii 0.7.6](https://github.com/awslabs/jsii/releases/tag/v0.7.6)

* v0.9.2 __NOTICE__: This release includes a framework-wide [__breaking change__](#712) which changes the type of all the string resource attributes across the framework. Instead of using strong-types that extend `cdk.Token` (such as `QueueArn`, `TopicName`, etc), we now represent all these attributes as normal `string`s, and codify the tokens into the string (using the feature introduced in [#168](#168)). Furthermore, the `cdk.Arn` type has been removed. In order to format/parse ARNs, use the static methods on `cdk.ArnUtils`. See motivation and discussion in [#695](#695). * **cfn2ts:** use stringified tokens for resource attributes instead of strong types ([#712](#712)) ([6508f78](6508f78)), closes [#518](#518) [#695](#695) [#744](#744) * **aws-dynamodb:** Attribute type for keys, changes the signature of the `addPartitionKey` and `addSortKey` methods to be consistent across the board. ([#720](#720)) ([e6cc189](e6cc189)) * **aws-codebuild:** fix typo "priviledged" -> "privileged * **assets:** cab't use multiple assets in the same stack ([#725](#725)) ([bba2e5b](bba2e5b)), closes [#706](#706) * **aws-codebuild:** typo in BuildEnvironment "priviledged" -> "privileged ([#734](#734)) ([72fec36](72fec36)) * **aws-ecr:** fix addToResourcePolicy ([#737](#737)) ([eadbda5](eadbda5)) * **aws-events:** ruleName can now be specified ([#726](#726)) ([a7bc5ee](a7bc5ee)), closes [#708](#708) * **aws-lambda:** jsii use no long requires 'sourceAccount' ([#728](#728)) ([9e7d311](9e7d311)), closes [#714](#714) * **aws-s3:** remove `policy` argument ([#730](#730)) ([a79190c](a79190c)), closes [#672](#672) * **cdk:** "cdk init" java template is broken ([#732](#732)) ([281c083](281c083)), closes [#711](#711) [aws/jsii#233](aws/jsii#233) * **aws-apigateway:** new API Gateway Construct Library ([#665](#665)) ([b0f3857](b0f3857)) * **aws-cdk:** detect presence of EC2 credentials ([#724](#724)) ([8e8c295](8e8c295)), closes [#702](#702) [#130](#130) * **aws-codepipeline:** make the Stage insertion API in CodePipeline more flexible ([#460](#460)) ([d182818](d182818)) * **aws-codepipeline:** new "Pipeline#addStage" convenience method ([#647](#647)) ([25c9fa0](25c9fa0)) * **aws-rds:** add support for parameter groups ([#729](#729)) ([2541508](2541508)), closes [#719](#719) * **docs:** add documentation for CDK toolkit plugings ([#733](#733)) ([965b918](965b918)) * **dependencies:** upgrade to [jsii 0.7.6](https://github.com/awslabs/jsii/releases/tag/v0.7.6)

Now that we can represent attributes as native strings and string lists, this covers the majority of resource attributes in CloudFormation. This change: * String attributes are represented as `string` (like before). * String list attribute are now represented as `string[]`. * Any other attribute types are represented as `cdk.Token`. Attributes that are represented as Tokens as of this change: * amazonmq has a bunch of `Integer` attributes (will be solved by #1455) * iot1click has a bunch of `Boolean` attributes * cloudformation has a JSON attribute * That's all. A few improvements to cfn2ts: * Auto-detect cfn2ts scope from package.json so it is more self-contained and doesn't rely on cdk-build-tools to run. * Added a "cfn2ts" npm script to all modules so it is now possible to regenerate all L1 via "lerna run cfn2ts". * Removed the premature optimization for avoiding code regeneration (it saved about 0.5ms). Fixes #1406 BREAKING CHANGE: any `CfnXxx` resource attributes that represented a list of strings are now typed as `string[]`s (via #1144). Attributes that represent strings, are still typed as `string` (#712) and all other attribute types are represented as `cdk.Token`.

Initial commit

a456155

eladb requested review from RomainMuller and rix0rrr September 13, 2018 22:51

eladb mentioned this pull request Sep 17, 2018

Do we really need strong types for all attribute types? #695

Closed

Elad Ben-Israel added 2 commits September 17, 2018 13:26

Merge remote-tracking branch 'origin/master' into benisrae/remove-att…

999b1e9

…ribute-types

Updates

32da352

* Converted all attribute types to strings * Remove a stale example from aws-cloudformation * Add `--no-bail` to `build.sh` so it won't stop on the first error * Reduce some verbosity from cdk-build and cfn2ts

eladb changed the title ~~[WIP] BREAKING CHANGE: remove resource attribute types for string attributes~~ BREAKING CHANGE: remove resource attribute types for string attributes Sep 17, 2018

Elad Ben-Israel added 8 commits September 17, 2018 21:36

Merge remote-tracking branch 'origin/master' into benisrae/remove-att…

00f4672

…ribute-types

Fix breaks after merge

8ac4942

Use ref instead of getatt for subnet id

73b5a37

Update expecations

acb96b7

Fix codebuild tests

b9f52a7

Update codebuild test

3ad255b

Update codepipeline integ test

449e961

update test to no-bail and codepipeline

934e0a2

rix0rrr reviewed Sep 18, 2018

View reviewed changes

eladb mentioned this pull request Sep 18, 2018

Change the usage of tokens for lazy evaluation #736

Closed

rix0rrr approved these changes Sep 19, 2018

View reviewed changes

Elad Ben-Israel added 5 commits September 19, 2018 14:42

CR followups

a1b5755

* Fix typo "managedPolicieArns" => "managedPolicyArns" * Remove unneeded `.toString` * Rename `isToken(x)` to `unresolved(x)` * Rename cfn2ts `ClassDeclaration` to `AttributeTypeDeclaration` * Revert log "generated code up-to-date" from cfn2ts

Merge remote-tracking branch 'origin/master' into benisrae/remove-att…

8e3ea6c

…ribute-types

Fix build break

51b2924

More followups

d84b871

* Restore custom resource example * Change `resource.getAtt` to return a token

Fix build break and announce errors

3817d21

eladb mentioned this pull request Sep 19, 2018

Consider if we should stringify non-string tokens #744

Closed

eladb merged commit 6508f78 into master Sep 19, 2018

eladb mentioned this pull request Sep 19, 2018

Deployment Configuration Construct for CodeDeploy L2 #653

Merged

eladb deleted the benisrae/remove-attribute-types branch November 19, 2018 12:31

eladb mentioned this pull request Jan 7, 2019

feat: no more generated attribute types in CFN layer (L1) #1489

Merged

4 tasks

NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019

SNYKabbott mentioned this pull request Feb 28, 2024

[Snyk] Security upgrade archiver from 3.0.0 to 7.0.0 SNYKabbott/aws-cdk#39

Open

sumodgeorge mentioned this pull request Mar 1, 2024

[Snyk] Security upgrade archiver from 2.1.1 to 7.0.0 sumodgeorge/aws-cdk#27

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BREAKING CHANGE: remove resource attribute types for string attributes #712

BREAKING CHANGE: remove resource attribute types for string attributes #712

eladb commented Sep 13, 2018 •

edited

rix0rrr Sep 18, 2018

eladb Sep 18, 2018

rix0rrr Sep 18, 2018

rix0rrr Sep 18, 2018

eladb Sep 19, 2018

rix0rrr Sep 18, 2018

eladb Sep 19, 2018

		@@ -1,104 +0,0 @@
		import lambda = require('@aws-cdk/aws-lambda');

BREAKING CHANGE: remove resource attribute types for string attributes #712

BREAKING CHANGE: remove resource attribute types for string attributes #712

Conversation

eladb commented Sep 13, 2018 • edited

rix0rrr Sep 18, 2018

Choose a reason for hiding this comment

eladb Sep 18, 2018

Choose a reason for hiding this comment

rix0rrr Sep 18, 2018

Choose a reason for hiding this comment

rix0rrr Sep 18, 2018

Choose a reason for hiding this comment

eladb Sep 19, 2018

Choose a reason for hiding this comment

rix0rrr Sep 18, 2018

Choose a reason for hiding this comment

eladb Sep 19, 2018

Choose a reason for hiding this comment

eladb commented Sep 13, 2018 •

edited