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

[aws-events] concatenating event fields should produce an error #9191

Closed
rirze opened this issue Jul 21, 2020 · 4 comments · Fixed by #13487
Closed

[aws-events] concatenating event fields should produce an error #9191

rirze opened this issue Jul 21, 2020 · 4 comments · Fixed by #13487
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Comments

@rirze
Copy link

rirze commented Jul 21, 2020

Creating an input transformer for a Cloudwatch Rule results in questionable syntax in the Cloudformation output causing the stack to unsuccessfully deploy.

Reproduction Steps

from aws_cdk import core
from aws_cdk import aws_events
from aws_cdk import aws_events_targets
from aws_cdk import aws_sns


class Scheduling(core.Construct):
    def __init__(self, scope: core.Construct, id: str):
        super().__init__(scope, id)

        topic = aws_sns.Topic(self, 'dummy_sns_topic')

        aws_events.Rule(self, 'test_rule',
                        description='test_rule',
                        schedule=aws_events.Schedule.rate(core.Duration.minutes(5)),
                        targets=[
                            aws_events_targets.SnsTopic(topic, message=aws_events.RuleTargetInput.from_object(
                                {
                                    'name': 'testing_at_' + aws_events.EventField.from_path('$.time')
                                }
                            ))
                        ])


class CdkTestRuleStack(core.Stack):

    def __init__(self, scope: core.Construct, id: str, **kwargs) -> None:
        super().__init__(scope, id, **kwargs)

        Scheduling(self, 'test_rule')

This results in CFN output where I've trimmed some of the irrelevant parts:

Resources:
  testruleB258B0E0:
    Type: AWS::Events::Rule
    Properties:
      Description: test_rule
      ScheduleExpression: rate(5 minutes)
      State: ENABLED
      Targets:
        - Arn:
            Ref: testruledummysnstopic5D840FE8
          Id: Target0
          InputTransformer:
            InputPathsMap:
              time: $.time
            InputTemplate: '{"name":"testing_at_<<${time>}'
    Metadata:
      aws:cdk:path: cdk-test-rule/test_rule/test_rule/Resource

I'm expecting InputTemplate: '{"name":"testing_at_<time>}'. My question is, am I building this correctly or is this a bug?

Error Log

11:46:48 AM | CREATE_FAILED        | AWS::Events::Rule     | testruleB258B0E0
Invalid InputTemplate for target Target0 : [Source: (String)"{"name":"testing_at_<<${time>}"; line: 1, column: 61]. (
Service: AmazonCloudWatchEvents; Status Code: 400; Error Code: ValidationException; Request ID: b09abd15-ff6b-4006-93
63-55b950ebf41d)

        new Rule (/tmp/jsii-kernel-R4yAYC/node_modules/@aws-cdk/aws-events/lib/rule.js:25:26)
        \_ ~/.local/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7836:49
        \_ Kernel._wrapSandboxCode (~/.local/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.
js:8296
:20)
        \_ Kernel._create (~/.local/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7836:2
6)
        \_ Kernel.create (~/.local/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7583:21
)
        \_ KernelHost.processRequest (~/.local/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtim
e.js:73
71:28)
        \_ KernelHost.run (~/.local/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.js:7311:1
4)
        \_ Immediate._onImmediate (~/.local/lib/python3.7/site-packages/jsii/_embedded/jsii/jsii-runtime.j
s:7314:
37)
        \_ processImmediate (internal/timers.js:456:21)


 ❌  cdk-test-rule failed: Error: The stack named cdk-test-rule failed creation, it may need to be manually deleted fr
om the AWS console: ROLLBACK_COMPLETE
    at Object.waitForStack (~/.npm-packages/lib/node_modules/aws-cdk/lib/api/util/cloudformation.ts:264:11
)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at Object.deployStack (~/.npm-packages/lib/node_modules/aws-cdk/lib/api/deploy-stack.ts:267:26)
    at CdkToolkit.deploy (~/.npm-packages/lib/node_modules/aws-cdk/lib/cdk-toolkit.ts:181:24)
    at main (~/.npm-packages/lib/node_modules/aws-cdk/bin/cdk.ts:264:16)
    at initCommandLine (~/.npm-packages/lib/node_modules/aws-cdk/bin/cdk.ts:185:9)
The stack named cdk-test-rule failed creation, it may need to be manually deleted from the AWS console: ROLLBACK_COMPLETE

Environment

  • CLI Version : 1.51.0 (build 8c2d53c)
  • Framework Version: 1.51.0
  • Node.js Version: v13.14.0
  • OS : Ubuntu 20.04 (WSL)
  • Language (Version): Python 3.7

Other


This is 🐛 Bug Report

@rirze rirze added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 21, 2020
@rirze rirze changed the title [module] aws_events [aws_events] Input Transformer for Cloudwatch Rule generates odd syntax wrapping Jul 21, 2020
@rirze rirze closed this as completed Jul 21, 2020
@rirze rirze reopened this Jul 21, 2020
@SomayaB SomayaB changed the title [aws_events] Input Transformer for Cloudwatch Rule generates odd syntax wrapping [aws-events] Input Transformer for Cloudwatch Rule generates odd syntax wrapping Jul 21, 2020
@github-actions github-actions bot added the @aws-cdk/aws-events Related to CloudWatch Events label Jul 21, 2020
@ericzbeard ericzbeard removed the needs-triage This issue or PR still needs to be triaged. label Jul 22, 2020
@michaelwiles
Copy link
Contributor

michaelwiles commented Jul 30, 2020

Yeah... this kind of thing you cannot do in step functions.

You're basically asking event bridge to do is to concatenate strings in json and this cannot be done.

see common issues section

It's got nothing to do with cdk - it's an event bridge issue.

Try this:

targets=[
    aws_events_targets.SnsTopic(topic, message=aws_events.RuleTargetInput.from_object(
    {
        'name': aws_events.EventField.from_path('$.time')
    }))
]

@rirze
Copy link
Author

rirze commented Jul 30, 2020

Sure, I've figured that much out in the meantime and agree that it cannot be done.

With that in mind, should there be a in-library measure against this behavior? Should the CDK allow this to compile when it obviously results in strange wrapping tokens?

I think the CDK ought to detect this and raise an appropriate warning.

@ericzbeard ericzbeard removed their assignment Aug 3, 2020
@rix0rrr rix0rrr changed the title [aws-events] Input Transformer for Cloudwatch Rule generates odd syntax wrapping [aws-events] concatenating event fields should produce an error Aug 4, 2020
@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 and removed bug This issue is a bug. labels Aug 4, 2020
@SomayaB SomayaB assigned shivlaks and unassigned rix0rrr Aug 20, 2020
@NGL321 NGL321 assigned rix0rrr and unassigned shivlaks Jan 25, 2021
@thantos
Copy link
Contributor

thantos commented Mar 9, 2021

Event Bridge has been updated to support string formatting in a json object.

https://docs.aws.amazon.com/eventbridge/latest/APIReference/API_InputTransformer.html

"InputTransformer":
{
   "InputPathsMap": {"instance": "$.detail.instance","status": "$.detail.status"},
   "InputTemplate": "<instance> is in state <status>"
}
"InputTransformer": 
{
   "InputPathsMap": {"instance": "$.detail.instance","status": "$.detail.status"},
   "InputTemplate": "<instance> is in state \"<status>\""
}

We would expect something like this to work now

RuleTargetInput.fromObject({
   data: `some text ${EventField.fromPath('$')}`
})

@mergify mergify bot closed this as completed in #13487 Jun 4, 2021
mergify bot pushed a commit that referenced this issue Jun 4, 2021
Event Bridge transformers have been updated to support embedded variable replacement within strings within objects. 

```
{
   data: "some string <myValue>"
}
```

Previously input transformers only supported string when they were the only value of an object, or just static strings.

```
// Before Event Bridges's change
{
   data: <myValue>, // OK
   data2: "some string", // OK
   data3: "some string <myValue>" // NOT OK 
}
```

The CDK solution was to assume that developers knew this restriction, wrap the string variable in special characters, and replace the double quotes plus special character set with nothing after token replacement.

This caused issues like #9191. Where string tokens (`EventField`) within a string would give a cryptic error during Cfn deployment due the resulting invalid object string generated (missing a closing double quote and leaving the special characters behind). 

### Solution:

Removed the special character sequence addition and stripping and instead only replace any instances of `"<myValue>"` that are added.

* Iterate over the known input transform keys to reduce possible unexpected impact and give developers a backdoor to change their keys in the worst case.
* Edge Case: `"<myValue>"` can appear with escaped quote sequences `"something \"quoted\"<myValue>"`. This is a valid string variable replacement case. Used a lookback regex (`(?<!\\\\)\"\<${key}\>\"`) to avoid the prefix escaped quote when replacing transform input keys with quote-less keys. 

### Tradeoffs

Removed the addition of special characters to find the keys in the final json string. Instead search for the specific pattern of transform input keys that should exist within the output and handle the edge case describe above.

This SHOULD cover all edge cases as it is not valid to have a trailing quote without an escape (`"<myValue>"" //not valid`) and it is not valid to have a prefix quote that is not escaped (`""<myValue>" // not valid`).

This was done to reduce the small change of overlapping with a developer's content, to be more targeted, and because the above should prove that the edge case is covered.


https://docs.aws.amazon.com/eventbridge/latest/APIReference/API_InputTransformer.html

fixes #9191



----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jun 4, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
Event Bridge transformers have been updated to support embedded variable replacement within strings within objects. 

```
{
   data: "some string <myValue>"
}
```

Previously input transformers only supported string when they were the only value of an object, or just static strings.

```
// Before Event Bridges's change
{
   data: <myValue>, // OK
   data2: "some string", // OK
   data3: "some string <myValue>" // NOT OK 
}
```

The CDK solution was to assume that developers knew this restriction, wrap the string variable in special characters, and replace the double quotes plus special character set with nothing after token replacement.

This caused issues like aws#9191. Where string tokens (`EventField`) within a string would give a cryptic error during Cfn deployment due the resulting invalid object string generated (missing a closing double quote and leaving the special characters behind). 

### Solution:

Removed the special character sequence addition and stripping and instead only replace any instances of `"<myValue>"` that are added.

* Iterate over the known input transform keys to reduce possible unexpected impact and give developers a backdoor to change their keys in the worst case.
* Edge Case: `"<myValue>"` can appear with escaped quote sequences `"something \"quoted\"<myValue>"`. This is a valid string variable replacement case. Used a lookback regex (`(?<!\\\\)\"\<${key}\>\"`) to avoid the prefix escaped quote when replacing transform input keys with quote-less keys. 

### Tradeoffs

Removed the addition of special characters to find the keys in the final json string. Instead search for the specific pattern of transform input keys that should exist within the output and handle the edge case describe above.

This SHOULD cover all edge cases as it is not valid to have a trailing quote without an escape (`"<myValue>"" //not valid`) and it is not valid to have a prefix quote that is not escaped (`""<myValue>" // not valid`).

This was done to reduce the small change of overlapping with a developer's content, to be more targeted, and because the above should prove that the edge case is covered.


https://docs.aws.amazon.com/eventbridge/latest/APIReference/API_InputTransformer.html

fixes aws#9191



----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-events Related to CloudWatch Events effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants