-
Notifications
You must be signed in to change notification settings - Fork 4k
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-cdk/aws-iot): CfnLocationAction asks for Date type despite spec wanting Timestamp type #22732
Comments
Thanks for reporting this @dreamorosi, This has been recently added to our cfnspec and the cfnspec itself looks good, so I think the issue is that we are messing up the conversion from the spec to the type. I wonder if this is because we have Timestamp as a Primitive Type in the Cfnspec, this could be overriding something somewhere?
And like you mentioned, we can work around this by using an override 🙂 Until this gets fixed we can use this feature with overrides |
@peterwoodworth I'd be happy to take over or support in fixing this. I assume removing the Timestamp from the primitive types is not an option? 😅 Can you help me and tell me how the files in packages/aws-cdk-lib/aws-iot/lib get build? |
I don't think I know how we should fix this - but I can provide you with some direction to get the ball rolling on fixing this 🙂
Correct, this will not be the way to go. Our Cfn resources get built based off the CfnSpec. We ingest this specification file, and create types and classes from it which model the actual CloudFormation resources. We split up the spec by each service. What you see here is basically what you're going to get, but you can see in this folder we have a bunch of patches that override the specification before we start creating classes and types based on the spec. I anticipate that we could fix this by slightly renaming the Timestamp associated with Iot through a patch file - however I don't have a better solution. Maybe someone else from the team, or you might have one 🙂 |
The overall problem seems to be that here the complex and primitive types get mixed.
Therefore, we can no longer differentiate between a timestamp reflecting a date and a timestamp as a complex type, such as it is the case here. After some testing it seems that this would require a very big rewrite which I'm not able to do myself. Therefore, I'm following @peterwoodworth's approach and try to patch the resource. |
…imitive type (#25460) We currently do not account for cases where a property type can have a complex type that matches one of our primitive types. For example, [AWS::IoT::TopicRule.LocationAction](https://github.com/aws/aws-cdk/blob/a0ad49df7b2536d800b4890ae0116e6ce26e6c55/packages/@aws-cdk/cfnspec/spec-source/specification/000_cfn/000_official/000_AWS_IoT.json#L1437-L1442) has a property `Timestamp` with a Type of [Timestamp](https://github.com/aws/aws-cdk/blob/a0ad49df7b2536d800b4890ae0116e6ce26e6c55/packages/@aws-cdk/cfnspec/spec-source/specification/000_cfn/000_official/000_AWS_IoT.json#L1727-L1742) When cfn2ts generates the TypeScript code it thinks that this is referring to the `PrimitiveType` `Timestamp` and coverts it to a `Date`. After running `gen` with these changes the only differences in the cfnspecs were `iot.generated.ts` ```diff @@ -10134,7 +10134,7 @@ export namespace CfnTopicRule { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iot-topicrule-locationaction.html#cfn-iot-topicrule-locationaction-timestamp */ - readonly timestamp?: Date | cdk.IResolvable; + readonly timestamp?: CfnTopicRule.TimestampProperty | cdk.IResolvable; /** * The name of the tracker resource in Amazon Location in which the location is updated. * @@ -10165,7 +10165,7 @@ function CfnTopicRule_LocationActionPropertyValidator(properties: any): cdk.Vali errors.collect(cdk.propertyValidator('longitude', cdk.validateString)(properties.longitude)); errors.collect(cdk.propertyValidator('roleArn', cdk.requiredValidator)(properties.roleArn)); errors.collect(cdk.propertyValidator('roleArn', cdk.validateString)(properties.roleArn)); - errors.collect(cdk.propertyValidator('timestamp', cdk.validateDate)(properties.timestamp)); + errors.collect(cdk.propertyValidator('timestamp', CfnTopicRule_TimestampPropertyValidator)(properties.timestamp)); errors.collect(cdk.propertyValidator('trackerName', cdk.requiredValidator)(properties.trackerName)); errors.collect(cdk.propertyValidator('trackerName', cdk.validateString)(properties.trackerName)); return errors.wrap('supplied properties not correct for "LocationActionProperty"'); @@ -10187,7 +10187,7 @@ function cfnTopicRuleLocationActionPropertyToCloudFormation(properties: any): an Latitude: cdk.stringToCloudFormation(properties.latitude), Longitude: cdk.stringToCloudFormation(properties.longitude), RoleArn: cdk.stringToCloudFormation(properties.roleArn), - Timestamp: cdk.dateToCloudFormation(properties.timestamp), + Timestamp: cfnTopicRuleTimestampPropertyToCloudFormation(properties.timestamp), TrackerName: cdk.stringToCloudFormation(properties.trackerName), }; } @@ -10206,7 +10206,7 @@ function CfnTopicRuleLocationActionPropertyFromCloudFormation(properties: any): ret.addPropertyResult('latitude', 'Latitude', cfn_parse.FromCloudFormation.getString(properties.Latitude)); ret.addPropertyResult('longitude', 'Longitude', cfn_parse.FromCloudFormation.getString(properties.Longitude)); ret.addPropertyResult('roleArn', 'RoleArn', cfn_parse.FromCloudFormation.getString(properties.RoleArn)); - ret.addPropertyResult('timestamp', 'Timestamp', properties.Timestamp != null ? cfn_parse.FromCloudFormation.getDate(properties.Timestamp) : undefined); + ret.addPropertyResult('timestamp', 'Timestamp', properties.Timestamp != null ? CfnTopicRuleTimestampPropertyFromCloudFormation(properties.Timestamp) : undefined); ret.addPropertyResult('trackerName', 'TrackerName', cfn_parse.FromCloudFormation.getString(properties.TrackerName)); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; ``` `sagemaker.generated.ts` ```diff @@ -2053,7 +2053,7 @@ export namespace CfnDataQualityJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-dataqualityjobdefinition-datasetformat.html#cfn-sagemaker-dataqualityjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnDataQualityJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnDataQualityJobDefinition.DatasetFormatProperty.Parquet` * @@ -2077,7 +2077,7 @@ function CfnDataQualityJobDefinition_DatasetFormatPropertyValidator(properties: errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnDataQualityJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnDataQualityJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -2095,7 +2095,7 @@ function cfnDataQualityJobDefinitionDatasetFormatPropertyToCloudFormation(proper CfnDataQualityJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnDataQualityJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnDataQualityJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -2111,7 +2111,7 @@ function CfnDataQualityJobDefinitionDatasetFormatPropertyFromCloudFormation(prop } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnDataQualityJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnDataQualityJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnDataQualityJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -11470,7 +11470,7 @@ export namespace CfnModelBiasJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-modelbiasjobdefinition-datasetformat.html#cfn-sagemaker-modelbiasjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnModelBiasJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnModelBiasJobDefinition.DatasetFormatProperty.Parquet` * @@ -11494,7 +11494,7 @@ function CfnModelBiasJobDefinition_DatasetFormatPropertyValidator(properties: an errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnModelBiasJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnModelBiasJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -11512,7 +11512,7 @@ function cfnModelBiasJobDefinitionDatasetFormatPropertyToCloudFormation(properti CfnModelBiasJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnModelBiasJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnModelBiasJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -11528,7 +11528,7 @@ function CfnModelBiasJobDefinitionDatasetFormatPropertyFromCloudFormation(proper } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnModelBiasJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnModelBiasJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnModelBiasJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -15402,7 +15402,7 @@ export namespace CfnModelExplainabilityJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-modelexplainabilityjobdefinition-datasetformat.html#cfn-sagemaker-modelexplainabilityjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnModelExplainabilityJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnModelExplainabilityJobDefinition.DatasetFormatProperty.Parquet` * @@ -15426,7 +15426,7 @@ function CfnModelExplainabilityJobDefinition_DatasetFormatPropertyValidator(prop errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnModelExplainabilityJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnModelExplainabilityJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -15444,7 +15444,7 @@ function cfnModelExplainabilityJobDefinitionDatasetFormatPropertyToCloudFormatio CfnModelExplainabilityJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnModelExplainabilityJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnModelExplainabilityJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -15460,7 +15460,7 @@ function CfnModelExplainabilityJobDefinitionDatasetFormatPropertyFromCloudFormat } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnModelExplainabilityJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnModelExplainabilityJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnModelExplainabilityJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -20808,7 +20808,7 @@ export namespace CfnModelQualityJobDefinition { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-modelqualityjobdefinition-datasetformat.html#cfn-sagemaker-modelqualityjobdefinition-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnModelQualityJobDefinition.JsonProperty | cdk.IResolvable; /** * `CfnModelQualityJobDefinition.DatasetFormatProperty.Parquet` * @@ -20832,7 +20832,7 @@ function CfnModelQualityJobDefinition_DatasetFormatPropertyValidator(properties: errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnModelQualityJobDefinition_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnModelQualityJobDefinition_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -20850,7 +20850,7 @@ function cfnModelQualityJobDefinitionDatasetFormatPropertyToCloudFormation(prope CfnModelQualityJobDefinition_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnModelQualityJobDefinitionCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnModelQualityJobDefinitionJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -20866,7 +20866,7 @@ function CfnModelQualityJobDefinitionDatasetFormatPropertyFromCloudFormation(pro } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnModelQualityJobDefinition.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnModelQualityJobDefinitionCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnModelQualityJobDefinitionJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; @@ -22693,7 +22693,7 @@ export namespace CfnMonitoringSchedule { * * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-monitoringschedule-datasetformat.html#cfn-sagemaker-monitoringschedule-datasetformat-json */ - readonly json?: any | cdk.IResolvable; + readonly json?: CfnMonitoringSchedule.JsonProperty | cdk.IResolvable; /** * `CfnMonitoringSchedule.DatasetFormatProperty.Parquet` * @@ -22717,7 +22717,7 @@ function CfnMonitoringSchedule_DatasetFormatPropertyValidator(properties: any): errors.collect(new cdk.ValidationResult('Expected an object, but received: ' + JSON.stringify(properties))); } errors.collect(cdk.propertyValidator('csv', CfnMonitoringSchedule_CsvPropertyValidator)(properties.csv)); - errors.collect(cdk.propertyValidator('json', cdk.validateObject)(properties.json)); + errors.collect(cdk.propertyValidator('json', CfnMonitoringSchedule_JsonPropertyValidator)(properties.json)); errors.collect(cdk.propertyValidator('parquet', cdk.validateBoolean)(properties.parquet)); return errors.wrap('supplied properties not correct for "DatasetFormatProperty"'); } @@ -22735,7 +22735,7 @@ function cfnMonitoringScheduleDatasetFormatPropertyToCloudFormation(properties: CfnMonitoringSchedule_DatasetFormatPropertyValidator(properties).assertSuccess(); return { Csv: cfnMonitoringScheduleCsvPropertyToCloudFormation(properties.csv), - Json: cdk.objectToCloudFormation(properties.json), + Json: cfnMonitoringScheduleJsonPropertyToCloudFormation(properties.json), Parquet: cdk.booleanToCloudFormation(properties.parquet), }; } @@ -22751,7 +22751,7 @@ function CfnMonitoringScheduleDatasetFormatPropertyFromCloudFormation(properties } const ret = new cfn_parse.FromCloudFormationPropertyObject<CfnMonitoringSchedule.DatasetFormatProperty>(); ret.addPropertyResult('csv', 'Csv', properties.Csv != null ? CfnMonitoringScheduleCsvPropertyFromCloudFormation(properties.Csv) : undefined); - ret.addPropertyResult('json', 'Json', properties.Json != null ? cfn_parse.FromCloudFormation.getAny(properties.Json) : undefined); + ret.addPropertyResult('json', 'Json', properties.Json != null ? CfnMonitoringScheduleJsonPropertyFromCloudFormation(properties.Json) : undefined); ret.addPropertyResult('parquet', 'Parquet', properties.Parquet != null ? cfn_parse.FromCloudFormation.getBoolean(properties.Parquet) : undefined); ret.addUnrecognizedPropertiesAsExtra(properties); return ret; ``` Closes #22732 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Describe the bug
The
iot.CfnTopicRule
construct now supports an action calledLocationAction
(link to CloudFormation docs).The
LocationAction
entity has a property calledTimestamp
which according to the CloudFormation docs is supposed to be of typeAWS::IoT::TopicRule Timestamp
and have shape:At present, in version
1.179.0
the property has incorrect type ofDate
, which causes the synth step to fail.Expected Behavior
The resource should have the correct type and allow to deploy correctly.
Current Behavior
Error:
Reproduction Steps
Possible Solution
Change the type to the correct one.
Additional Information/Context
Adding this manual override to the resource allows to deploy correctly, proving that the resource is mistyped in CDK:
CDK CLI Version
2.41.0
Framework Version
No response
Node.js Version
16.15.0
OS
Linux / macOS
Language
Typescript
Language Version
No response
Other information
No response
The text was updated successfully, but these errors were encountered: