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-iot: CfnJobTemplate types mismatched cases #32258

Open
1 task done
jknotamzn opened this issue Nov 23, 2024 · 7 comments
Open
1 task done

aws-iot: CfnJobTemplate types mismatched cases #32258

jknotamzn opened this issue Nov 23, 2024 · 7 comments
Labels
@aws-cdk/aws-iot Related to AWS IoT bug This issue is a bug. effort/medium Medium work item – several days of effort p2

Comments

@jknotamzn
Copy link

Describe the bug

Several types are incorrectly cased in CfnJobTemplate under aws-iot version 2.166.0

const timeoutConfig: CfnJobTemplate.TimeoutConfigProperty = {
    inProgressTimeoutInMinutes: 100
};

...
Properties validation failed for resource ********* with message:
[#/TimeoutConfig: extraneous key [inProgressTimeoutInMinutes] is not permitted]

It seems like multiple properties are affected.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

Type should be:

(Note the capital I / Pascal case vs camel case)

    /**
     * Specifies the amount of time each device has to finish its execution of the job.
     *
     * A timer is started when the job execution status is set to `IN_PROGRESS` . If the job execution status is not set to another terminal state before the timer expires, it will be automatically set to `TIMED_OUT` .
     *
     * @struct
     * @stability external
     * @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iot-jobtemplate-timeoutconfig.html
     */
    interface TimeoutConfigProperty {
        /**
         * Specifies the amount of time, in minutes, this device has to finish execution of this job.
         *
         * The timeout interval can be anywhere between 1 minute and 7 days (1 to 10080 minutes). The in progress timer can't be updated and will apply to all job executions for the job. Whenever a job execution remains in the IN_PROGRESS status for longer than this interval, the job execution will fail and switch to the terminal `TIMED_OUT` status.
         *
         * @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iot-jobtemplate-timeoutconfig.html#cfn-iot-jobtemplate-timeoutconfig-inprogresstimeoutinminutes
         */
        readonly InProgressTimeoutInMinutes: number;
    }

Current Behavior

Several properties have this issue in CfnJobTemplate.

[#/TimeoutConfig: extraneous key [inProgressTimeoutInMinutes] is not permitted, #/JobExecutionsRolloutConfig: extraneous key [maximu
mPerMinute] is not permitted, #/JobExecutionsRolloutConfig: extraneous key [exponentialRate] is not permitted]

Reproduction Steps

const timeoutConfig: CfnJobTemplate.TimeoutConfigProperty = {
    inProgressTimeoutInMinutes: 100
};


const jobTemplate = new CfnJobTemplate(this, `jobTemplate1`, {
    jobTemplateId: 'SOME_ID',
    description: 'Description',
    document: '',
    timeoutConfig
}

Build & deploy

results in:

[#/TimeoutConfig: extraneous key [inProgressTimeoutInMinutes] is not permitted

Possible Solution

Types need to be corrected

Additional Information/Context

No response

CDK CLI Version

2.166.0

Framework Version

2.166.0

Node.js Version

v20.18.0

OS

MacOS

Language

TypeScript

Language Version

5.6.3

Other information

The documentation is conflicted on what it should be, but Pascal case does work if done if forced by using CfnJobTemplate without the property types because they're all 'any'.

Pascal
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iot-jobtemplate-timeoutconfig.html#cfn-iot-jobtemplate-timeoutconfig-inprogresstimeoutinminutes

Camel
https://awscli.amazonaws.com/v2/documentation/api/latest/reference/iot/create-job-template.html

Camel
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_iot.CfnJobTemplate.TimeoutConfigProperty.html

@jknotamzn jknotamzn added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 23, 2024
@github-actions github-actions bot added @aws-cdk/aws-iot Related to AWS IoT potential-regression Marking this issue as a potential regression to be checked by team member labels Nov 23, 2024
@pahud pahud self-assigned this Nov 25, 2024
@pahud
Copy link
Contributor

pahud commented Nov 25, 2024

Hi,

The timeoutConfig is actually untyped(type any)

image

see: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_iot.CfnJobTemplate.html#timeoutconfig

So you need to specify the raw JSON like

const jobTemplate = new iot.CfnJobTemplate(this, `jobTemplate1`, {
      jobTemplateId: 'SOME_ID',
      description: 'Description',
      document: '',
      timeoutConfig: {
        InProgressTimeoutInMinutes: 100
      }
  });

@pahud pahud added the p3 label Nov 25, 2024
@pahud pahud removed their assignment Nov 25, 2024
@pahud pahud added effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Nov 25, 2024
@jknotamzn
Copy link
Author

For the whole CfnJobTemplate that is correct, but the broken-out property types are incorrect:

import { CfnJobTemplate } from 'aws-cdk-lib/aws-iot';
 const timeoutConfig: CfnJobTemplate.TimeoutConfigProperty = {
    inProgressTimeoutInMinutes: 100
}
export declare namespace CfnJobTemplate {
    /**
     * Specifies the amount of time each device has to finish its execution of the job.
     *
     * A timer is started when the job execution status is set to `IN_PROGRESS` . If the job execution status is not set to another terminal state before the timer expires, it will be automatically set to `TIMED_OUT` .
     *
     * @struct
     * @stability external
     * @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iot-jobtemplate-timeoutconfig.html
     */
    interface TimeoutConfigProperty {
        /**
         * Specifies the amount of time, in minutes, this device has to finish execution of this job.
         *
         * The timeout interval can be anywhere between 1 minute and 7 days (1 to 10080 minutes). The in progress timer can't be updated and will apply to all job executions for the job. Whenever a job execution remains in the IN_PROGRESS status for longer than this interval, the job execution will fail and switch to the terminal `TIMED_OUT` status.
         *
         * @see http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-iot-jobtemplate-timeoutconfig.html#cfn-iot-jobtemplate-timeoutconfig-inprogresstimeoutinminutes
         */
        readonly inProgressTimeoutInMinutes: number;
    }

@pahud pahud added the jsii This issue originates in jsii, or this feature must be implemented in jsii. label Nov 25, 2024
@pahud
Copy link
Contributor

pahud commented Nov 25, 2024

@jknotamzn agree this is confusing.

I am reaching out to the team for further inputs here.

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 26, 2024

I see what's going on here. The timeoutConfig property was once typed as JSON/any, without the nice type definition. The TimeoutConfigProperty type got added later.

However, because of the capitalization behavior change you are noticing, this change is backwards incompatible. We are therefore not able pick up the new type: we have to forever keep on emitting this type as JSON/any in order to not break existing programs. Which means you need to take care of capitalization yourself.

We should have a backlog item for this, but I can't find one offhand. I'll search some more and make a new one if necessary.

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 26, 2024

Created this: #32285

@rix0rrr rix0rrr added p2 effort/medium Medium work item – several days of effort and removed p3 potential-regression Marking this issue as a potential regression to be checked by team member effort/small Small work item – less than a day of effort jsii This issue originates in jsii, or this feature must be implemented in jsii. labels Nov 26, 2024
@mrgrain
Copy link
Contributor

mrgrain commented Nov 26, 2024

Root cause is as @rix0rrr described. However for this specific case we know the property (here timeoutConfig) was once a JSON and now has an explicit type definition. Even though we have to keep the type of timeoutConfig as any, we know what the correct result should be: PascalCase'd property names. The conversion from camelCase to PascalCase is one we already do for strongly typed objects. We could opportunistically apply this conversion on behalf of the user. This is maybe not strictly correct as we change the user's input despite the type being any / JSON - but I'd wager that in 99% case it's what a user wants.

@ashishdhingra
Copy link
Contributor

We should handle #32459 as well in case we decide to apply conversion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iot Related to AWS IoT bug This issue is a bug. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests

5 participants