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_redshift: parameterGroupName is marked as optional in L1 constructs but cloudformation requires it now #28591

Open
venturaville opened this issue Jan 5, 2024 · 3 comments
Labels
@aws-cdk/aws-redshift Related to Amazon Redshift bug This issue is a bug. effort/medium Medium work item – several days of effort needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p3

Comments

@venturaville
Copy link

Describe the bug

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_redshift.CfnClusterParameterGroup.html#parametergroupname-1
The parameterGroupName is listed as optional on the L1 constructs doc page ^

The L2 constructor does not set it (but does have something it tracks called ‘clusterParameterGroupName’):
https://github.com/aws/aws-cdk/blob/v2.118.0/packages/%40aws-cdk/aws-redshift-alpha/lib/parameter-group.ts#L76-L86
It also has not changed in ~ 7 months

| CREATE_FAILED | AWS::Redshift::ClusterParameterGroup | MyParameterGroup...
Resource handler returned message: "1 validation error detected: Value null at 'parameterGroupName' failed to satisfy constraint: Member must not be null (Service: Redshift, Status Code: 400, Request ID: ...

The L2 constructor provides no way to set the parameterGroupName on the L1 construct manually, the L1 construct CfnClusterParameterGroup does not require it, but cloudformation itself does require it.

AFAIK, this behavior has changed semi recently as we have other CDK setting parameter groups which does not have parameterGroupName set in the final cloudformation generated.

Expected Behavior

Cloudformation should have accepted this if the parameterGroupName was optional as before.

Current Behavior

We get this error from cloudformation itself:

| CREATE_FAILED        | AWS::Redshift::ClusterParameterGroup        | MyParameterGroup...
Resource handler returned message: "1 validation error detected: Value null at 'parameterGroupName' failed to satisfy constraint: Member must not be null (Service: Redshift, Status Code: 400, Request ID: ...

This is different from previous behavior seen with the same CDK creating other clusters.

Reproduction Steps

    import * as redshift from '@aws-cdk/aws-redshift-alpha';

    const wlm = [
      {
        rules: [            ],
          },
      ];
    const loggingParamGroup = new redshift.ClusterParameterGroup(this, 'MyParameterGroup', {
      description: `Parameter Group for Cluster MyCluster`,
      parameters: {
        enable_user_activity_logging: 'true',
        wlm_json_configuration: JSON.stringify(wlm),
      },
    });

Possible Solution

  • Make the L2 construct in aws-redshift-alpha set the parameterGroupName from clusterParameterGroupName (see: parameter-group.ts

OR

  • Provide a way for the parameterGroupName to be manually set from the L2 construct onto the L1 construct

OR

  • Cloudformation should set the name to something reasonable when the name is not provided instead of requiring

If cloudformation is not updated, then the optional setting on parameterGroupName should be marked as required instead.

Additional Information/Context

No response

CDK CLI Version

2.117.0

Framework Version

No response

Node.js Version

v21.5.0

OS

MacOS

Language

TypeScript

Language Version

No response

Other information

No response

@venturaville venturaville added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jan 5, 2024
@github-actions github-actions bot added the @aws-cdk/aws-redshift Related to Amazon Redshift label Jan 5, 2024
@pahud
Copy link
Contributor

pahud commented Jan 5, 2024

I just checked the CNF spec for us-east-1
https://d1uauaxba7bl26.cloudfront.net/latest/gzip/CloudFormationResourceSpecification.json

    "AWS::Redshift::ClusterParameterGroup.Parameter": {
      "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-redshift-clusterparametergroup-parameter.html",
      "Properties": {
        "ParameterValue": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-redshift-clusterparametergroup-parameter.html#cfn-redshift-clusterparametergroup-parameter-parametervalue",
          "UpdateType": "Mutable",
          "Required": true,
          "PrimitiveType": "String"
        },
        "ParameterName": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-redshift-clusterparametergroup-parameter.html#cfn-redshift-clusterparametergroup-parameter-parametername",
          "UpdateType": "Mutable",
          "Required": true,
          "PrimitiveType": "String"
        }
      }

Yes it is required.

And I checked this from @aws-cdk/aws-service-spec@v0.0.40 which is required as well. I believe the next release of aws-cdk should include aws-service-spec@v0.0.40 and before that you will need escape hatches e.g. addPropertyOverride() to override this property to satisfy this requirement for cloudformation. And after that, we'll need another PR to improve the L2 construct.

I believe CFN doc might reflect this change in the next few days.

@pahud pahud added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jan 5, 2024
@mmarthab
Copy link

Hi, document not updated yet to reflect this. Experienced the same issue today creating the parameter group via CDK failing with the same error.

@pahud
Copy link
Contributor

pahud commented Jun 3, 2024

No I was wrong.

According to the CFN spec:
https://d1uauaxba7bl26.cloudfront.net/latest/gzip/CloudFormationResourceSpecification.json

ParameterGroupName is not required.

"AWS::Redshift::ClusterParameterGroup": {
      "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-redshift-clusterparametergroup.html",
      "Properties": {
        "Description": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-redshift-clusterparametergroup.html#cfn-redshift-clusterparametergroup-description",
          "UpdateType": "Immutable",
          "Required": true,
          "PrimitiveType": "String"
        },
        "Parameters": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-redshift-clusterparametergroup.html#cfn-redshift-clusterparametergroup-parameters",
          "UpdateType": "Mutable",
          "Required": false,
          "Type": "List",
          "ItemType": "Parameter",
          "DuplicatesAllowed": true
        },
        "ParameterGroupName": {
          "Documentation": "http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-redshift-clusterparametergroup.html#cfn-redshift-clusterparametergroup-parametergroupname",
          "UpdateType": "Immutable",
          "Required": false,
          "PrimitiveType": "String"
        },

Are you still having this issue?

We need to report this to the CFN team.

Can you share your synthesized template for this resource?

@pahud pahud added the needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. label Jun 3, 2024
@pahud pahud added p3 and removed p2 labels Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-redshift Related to Amazon Redshift bug This issue is a bug. effort/medium Medium work item – several days of effort needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. p3
Projects
None yet
Development

No branches or pull requests

3 participants