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

Proposal: Token.toString() #24

Closed
eladb opened this issue Jun 5, 2018 · 5 comments · Fixed by #518
Closed

Proposal: Token.toString() #24

eladb opened this issue Jun 5, 2018 · 5 comments · Fixed by #518
Assignees

Comments

@eladb
Copy link
Contributor

eladb commented Jun 5, 2018

References to runtime attributes of resources (the name of the bucket, the ARN of a topic, etc). At the moment, these are represented a Token objects. Tokens are objects that have a resolve method. When a stack is synthesized, every toke node in the JSON document is resolved and the result is plugged into the document.

One of the usability issues we are experiencing with tokens is that it is very common to need to format strings that reference them. For example:

const bucket = new s3.Bucket(this, 'MyBucket');
const rule = new cloudwatch.EventRule(this, 'Schedule', { scheduleExpression: 'rate(1 minute)' });
rule.addTarget(topic, {
  textTemplate: `Hello, bucket ${bucket.bucketName}`
});

Sadly, this code will not behave as expected. bucket.bucketName is an object of type BucketName, which extends Token, and not a string. The resulting string would be "Hello, bucket [Object]".

If we had a toString method for tokens, which would have been meaningful and substitutable during synthesis, the above code would behave as expected.

@eladb eladb changed the title Idea: Token.toString() Proposal: Token.toString() Jun 5, 2018
@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 5, 2018

At least it would help debuggability.

However, to make it do "the right thing" we'd need to change the context as well. We could make the Token stringify to ${Bucket}, but that would still require the containing string to be wrapped in a Fn::Sub.

As in, it would need to become this:

rule.addTarget(topic, {
  textTemplate: new FnSub(`Hello, bucket ${bucket.bucketName}`)
});

Still a pretty good addition because right now it's very inconvenient to use FnSub(), and this would go a long way towards making it "do the right thing" with zero extra effort. As a user, you wouldn't even know you were doing it wrong but we were fixing it for you :)

@eladb
Copy link
Contributor Author

eladb commented Jun 5, 2018

I would actually not use FnSub for the substitution but rather implement our own substitution scheme in resolve. It will give us way more flexibility. Also, we need to control when resolve is called on the token (which is during synthesis).

Here's some way we can implement this:

The first time a token is toStringed, we allocate a global key which maps to this object and return the key as a magic substitution. So, the above example "Hello, bucket ${bucket.bucketName}" will render to something like: Hello, bucket <<TOKEN#776#BucketName>>. Then during synthesis when we resolve(template), every time we see a string that contains /<<TOKEN#\d+#.+>>/ we go to the global map, find the object, call resolve on it and substitute the value.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 5, 2018

My point is, if it's a Token it most likely needs to resolve to a {Ref} or {Fn::GetAtt}. So do you plan to do wrapping of arbitrary strings in FnSub during resolve() to make those work?

Not saying it couldn't be done, but it might introduce some nasty edge cases.

@eladb
Copy link
Contributor Author

eladb commented Jun 5, 2018

Good point, but I would just use FnConcat(preString, resolvedToken, postString). This way we are not bound to substitutions supported by CloudFormation. resolvedToken can be anything.

@rix0rrr
Copy link
Contributor

rix0rrr commented Jun 5, 2018

It would definitely be an interesting experiment.

Do we expose toString() through jsii? I guess the answer would have to be yes, right? And rename then appropriately for every jsii client language as well?

@rix0rrr rix0rrr self-assigned this Aug 7, 2018
rix0rrr pushed a commit that referenced this issue Aug 7, 2018
Tokens (such as resource attributes) can now be implicitly converted
into strings. Parameters and attributes can now be combined into larger
strings using the native string facilities of the host language.

The process will be reversed by the resolve() function during synthesis,
which will split the string up again into string literals and
CloudFormation intrinsics.

The same transformation is applied during JSON.stringify(), so that
Tokens in a complex JSON structure are preserved.

Fixes #24 and #168.
rix0rrr added a commit that referenced this issue Aug 15, 2018
Tokens (such as resource attributes) can now be implicitly converted
into strings. This allows using any late-bound CloudFormation value
where strings can be used in the host language, and the native string
facilities of the host language can be used as well to further process
strings.

The process is reversed by the resolve() function during synthesis,
which will split the string up again into string literals and
CloudFormation intrinsics (combined by `{Fn::Join}`).

Introduces `CloudFormationJSON.stringify` to produce a similar result,
but used for JSON-encoding of complex objects that may contain Tokens.
This is used in JSON-encoded policy statements, CloudFormation actions,
state machine definitions, and more.

Additional changes:
- Add a VSCode launch config (with a helper script) to directly debug a
  unit test from within the IDE. This allows setting breakpoints.
- SecretParameter no longer duckily implements Token; this will not work
  in other jsii languages anyway.
- Nested `FnConcat` structures will be flattened in the CloudFormation
  output.

Fixes #24 and #168.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants