aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.35k stars 3.76k forks source link

sagemaker: Some CfnFeatureGroup properties generate incorrect capitalization #29897

Open adrianjhunter opened 2 months ago

adrianjhunter commented 2 months ago

Describe the bug

I attempted to create a new CfnFeatureGroup with the OnlineStoreConfig and OfflineStoreConfig properties configured. When the template is synthesized, the resulting CFN template uses lower camel case for these specific properties while the CFN specification requires upper camel case.

Expected Behavior

Expecting output in upper camel case as per spec.

TestFeatureGroup:
    Type: AWS::SageMaker::FeatureGroup
    Properties:
      FeatureGroupName: test
      OfflineStoreConfig:
        S3StorageConfig:
          S3Uri:
            Fn::Join:
              - ""
              - - s3://
                - Ref: offlineStore4208FAB6
          KmsKeyId:
            Ref: kmsKey3BB021CC
        DataCatalogConfig:
          Catalog: AwsDataCatalog
          Database: testdb
          TableName: test
      OnlineStoreConfig:
        EnableOnlineStore: true
        SecurityConfig:
          KmsKeyId:
            Ref: kmsKey3BB021CC
      ...

Current Behavior

As show below, the CFN template that is outputted is in lower camel case for the OfflineStorageConfig, OnlineStoreConfig, OnlineStoreSecurityConfig, DataCatalogConfig and S3StorageConfig properties.

TestFeatureGroup:
    Type: AWS::SageMaker::FeatureGroup
    Properties:
      FeatureGroupName: test
      OfflineStoreConfig:
        s3StorageConfig:
          s3Uri:
            Fn::Join:
              - ""
              - - s3://
                - Ref: offlineStore4208FAB6
          kmsKeyId:
            Ref: kmsKey3BB021CC
        dataCatalogConfig:
          catalog: AwsDataCatalog
          database: testdb
          tableName: test
      OnlineStoreConfig:
        enableOnlineStore: true
        securityConfig:
          kmsKeyId:
            Ref: kmsKey3BB021CC
      ...

Reproduction Steps

Below is a snippet of some CDK resources which creates an S3 bucket, KMS key and a FeatureGroup referencing said bucket and key.

    const bucket = new s3.Bucket(this, 'offlineStore');
    const kms_key = new kms.Key(this, 'kmsKey');

    new sagemaker.CfnFeatureGroup(this, 'TestFeatureGroup', {
      featureGroupName: "test",
      eventTimeFeatureName: 'eventTime',
      recordIdentifierFeatureName: 'recordIdentifier',
      featureDefinitions: [
        {
          featureName: 'recordIdentifier',
          featureType: 'String'
        },
        {
          featureName: 'eventTime',
          featureType: 'String'
        }
      ],
      onlineStoreConfig: {
        enableOnlineStore: true,
        securityConfig: {
          kmsKeyId: kms_key.keyId,
        }
      },
      offlineStoreConfig: {
        s3StorageConfig: {
          s3Uri: bucket.s3UrlForObject(),
          kmsKeyId: kms_key.keyId,
        },
        dataCatalogConfig: {
          catalog: 'AwsDataCatalog',
          database: 'testdb',
          tableName: 'test'
        }
      },
    })

Possible Solution

Not sure where this error occurs, but originally discovered this using CDK Python before replicating in Typescript. So its pre the JSII step and might be related to the raw CFN import into L1 Constructs.

Additional Information/Context

No response

CDK CLI Version

2.138.0 (build 6b41c8b)

Framework Version

2.138.0

Node.js Version

v18.16.0

OS

MacOS 13.6.4

Language

TypeScript, Python

Language Version

No response

Other information

CFN doc of impacted properties found:

https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-offlinestoreconfig.html https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-s3storageconfig.html https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-datacatalogconfig.html https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-onlinestoreconfig.html https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-sagemaker-featuregroup-onlinestoresecurityconfig.html

pahud commented 2 months ago

They are actually untyped because of this patch to avoid breaking changes and you should see that from the IDE as below.

image

or from the API reference doc:

image

With the untyped properties, you will need to define JSON-like property and specify correct capitalization.

Check this out for more details about the patches https://github.com/aws/aws-cdk/issues/21767

Let me know if it helps for you.

github-actions[bot] commented 2 months ago

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

adrianjhunter commented 2 months ago

True, but if I use something like the OfflineStoreConfigProperty for instance (https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_sagemaker.CfnFeatureGroup.OfflineStoreConfigProperty.html), it is typed but has the incorrect case.

For instance, this snippet of guidance from https://docs.aws.amazon.com/cdk/api/v2/python/aws_cdk.aws_sagemaker/CfnFeatureGroup.html#offlinestoreconfigproperty

image

Doesn't actually work as it generates incorrect case. The any type helps, e.g. if I ignore most of the properties under CfnFeatureGroup, I would be able to use the resource, but I feel like the properties should be corrected.