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

[core] Unable to set CfnIdentityPoolProps allowUnauthenticatedIdentities to false #10455

Closed
harry-stevenson-privitar opened this issue Sep 21, 2020 · 4 comments · Fixed by #10539
Assignees
Labels
@aws-cdk/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@harry-stevenson-privitar

When creating a Cognito Identity pool using the CfnIdentityPool class, setting the allowUnauthenticatedIdentities field to false on the CfnIdentityPoolProps builder results in no properties being included in the cloudformation template produced by running cdk synth. This property is required to be set, and so any attempt at a cdk deploy will fail to create the resource. When the value is set to true, the property is correctly included in the synthesised cloudformation.

Reproduction Steps

Main app:

public class ProductionApp {

    public static void main(final String[] args) {
        App app = new App();

        Environment environment = Environment.builder().account(Constants.AWS_ACCOUNT_ID).region(Constants.DEPLOYMENT_REGION).build();
        StackProps stackProperties = StackProps.builder().env(environment).build();

        new ReproductionStack(app, "test", stackProperties);

        app.synth();
    }
}

ReproductionStack:

public class ReproductionStack extends Stack {
    public ReproductionStack(final Construct scope, final String id, final StackProps props) {
        super(scope, id, props);

        CfnIdentityPoolProps properties = CfnIdentityPoolProps.builder()
                .allowUnauthenticatedIdentities(false)
                .build();
        
        new CfnIdentityPool(this, "testMyPool", properties);
    }
}

Output of cdk synth:

Resources:
  testMyPool:
    Type: AWS::Cognito::IdentityPool
    Metadata:
      aws:cdk:path: test/testMyPool
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Modules: aws-cdk=1.63.0,@aws-cdk/assets=1.63.0,@aws-cdk/aws-applicationautoscaling=1.63.0,@aws-cdk/aws-autoscaling-common=1.63.0,@aws-cdk/aws-certificatemanager=1.63.0,@aws-cdk/aws-cloudformation=1.63.0,@aws-cdk/aws-cloudwatch=1.63.0,@aws-cdk/aws-codeguruprofiler=1.63.0,@aws-cdk/aws-cognito=1.63.0,@aws-cdk/aws-ec2=1.63.0,@aws-cdk/aws-efs=1.63.0,@aws-cdk/aws-events=1.63.0,@aws-cdk/aws-iam=1.63.0,@aws-cdk/aws-kms=1.63.0,@aws-cdk/aws-lambda=1.63.0,@aws-cdk/aws-logs=1.63.0,@aws-cdk/aws-route53=1.63.0,@aws-cdk/aws-s3=1.63.0,@aws-cdk/aws-s3-assets=1.63.0,@aws-cdk/aws-sns=1.63.0,@aws-cdk/aws-sqs=1.63.0,@aws-cdk/aws-ssm=1.63.0,@aws-cdk/cloud-assembly-schema=1.63.0,@aws-cdk/core=1.63.0,@aws-cdk/custom-resources=1.63.0,@aws-cdk/cx-api=1.63.0,@aws-cdk/region-info=1.63.0,jsii-runtime=Java/1.8.0_261

Output of cdk synth when .allowUnauthenticatedIdentities(true) is used:

Resources:
  testMyPool:
    Type: AWS::Cognito::IdentityPool
    Properties:
      AllowUnauthenticatedIdentities: true
    Metadata:
      aws:cdk:path: test/testMyPool
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Modules: aws-cdk=1.63.0,@aws-cdk/assets=1.63.0,@aws-cdk/aws-applicationautoscaling=1.63.0,@aws-cdk/aws-autoscaling-common=1.63.0,@aws-cdk/aws-certificatemanager=1.63.0,@aws-cdk/aws-cloudformation=1.63.0,@aws-cdk/aws-cloudwatch=1.63.0,@aws-cdk/aws-codeguruprofiler=1.63.0,@aws-cdk/aws-cognito=1.63.0,@aws-cdk/aws-ec2=1.63.0,@aws-cdk/aws-efs=1.63.0,@aws-cdk/aws-events=1.63.0,@aws-cdk/aws-iam=1.63.0,@aws-cdk/aws-kms=1.63.0,@aws-cdk/aws-lambda=1.63.0,@aws-cdk/aws-logs=1.63.0,@aws-cdk/aws-route53=1.63.0,@aws-cdk/aws-s3=1.63.0,@aws-cdk/aws-s3-assets=1.63.0,@aws-cdk/aws-sns=1.63.0,@aws-cdk/aws-sqs=1.63.0,@aws-cdk/aws-ssm=1.63.0,@aws-cdk/cloud-assembly-schema=1.63.0,@aws-cdk/core=1.63.0,@aws-cdk/custom-resources=1.63.0,@aws-cdk/cx-api=1.63.0,@aws-cdk/region-info=1.63.0,jsii-runtime=Java/1.8.0_261

What did you expect to happen?

A Cloudformation template including the correct properties for the defined identity pool would be created.

What actually happened?

The incorrect Cloudformation template is generated.

Environment

  • CLI Version : 1.63.0 (build 7a68125)
  • Framework Version: 1.63.0 (build 7a68125)
  • Node.js Version: v12.14.1
  • OS : macOS Catalina Version 10.15.4
  • Language (Version): Java 8

This is 🐛 Bug Report

@harry-stevenson-privitar harry-stevenson-privitar added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 21, 2020
@github-actions github-actions bot added the @aws-cdk/aws-cognito Related to Amazon Cognito label Sep 21, 2020
@harry-stevenson-privitar
Copy link
Author

harry-stevenson-privitar commented Sep 21, 2020

Also of note, the same behaviour occurs when trying to use the CfnResource class.
When the pool is created with the code below, the same Cloudformation template is output as when created with CfnIdentityPool (correct when true, missing the property when false):

CfnResource.Builder.create(this, poolId)
                .type("AWS::Cognito::IdentityPool")
                .properties(new HashMap<String, Object>() {{
                            put("AllowUnauthenticatedIdentities", false);
                }}).build();

@harry-stevenson-privitar
Copy link
Author

Currently working around the issue with:

CfnIdentityPool pool = new CfnIdentityPool(this, "testMyPool", properties);
pool.addOverride("Properties.AllowUnauthenticatedIdentities", false);

@nija-at
Copy link
Contributor

nija-at commented Sep 25, 2020

Definitely something weird going on here.

It seems that when there all properties on a Cfn construct is false, they don't render into the CloudFormation template. There needs to be at least one property that is truthy for the properties section to be rendered.

Simply typescript app that reproduces this problem -

import { App, Duration, Stack } from '@aws-cdk/core';
import { CfnIdentityPool } from '@aws-cdk/aws-cognito';

const app = new App();
const stack = new Stack(app, 'mystack');

new CfnIdentityPool(stack, 'pool', {
  allowUnauthenticatedIdentities: false,
});

nija-at pushed a commit that referenced this issue Sep 25, 2020
Any CloudFormation resource that defines a single boolean property set
to false is not rendered to the CloudFormation template.

The bug is in implementation of `_toCloudFormation()` API in
`CfnResource`. It treated `false` and `undefined` the same way.

fixes #10455
@nija-at
Copy link
Contributor

nija-at commented Sep 25, 2020

The bug is in the core module and a fix has been prepared - #10539

@nija-at nija-at changed the title [cognito] Unable to set CfnIdentityPoolProps allowUnauthenticatedIdentities to false [core] Unable to set CfnIdentityPoolProps allowUnauthenticatedIdentities to false Sep 25, 2020
@nija-at nija-at added @aws-cdk/core Related to core CDK functionality and removed @aws-cdk/aws-cognito Related to Amazon Cognito labels Sep 25, 2020
@nija-at nija-at added effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Sep 25, 2020
@mergify mergify bot closed this as completed in #10539 Sep 30, 2020
mergify bot pushed a commit that referenced this issue Sep 30, 2020
…10539)

Any CloudFormation resource that defines a single boolean property set
to false is not rendered to the CloudFormation template.

The bug is in implementation of `_toCloudFormation()` API in
`CfnResource`. It treated `false` and `undefined` the same way.

fixes #10455


----

*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/core Related to core CDK functionality bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants