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

addPropertyOverride() does not seem to work #2677

Closed
spg opened this issue May 29, 2019 · 2 comments · Fixed by #2685 or MechanicalRock/tech-radar#14 · May be fixed by MechanicalRock/cdk-constructs#5, MechanicalRock/cdk-constructs#6 or MechanicalRock/cdk-constructs#7
Labels
bug This issue is a bug.

Comments

@spg
Copy link
Contributor

spg commented May 29, 2019

Describe the bug
addPropertyOverride does not seem to work

To Reproduce
Synthetize the following stack:

import cdk = require('@aws-cdk/cdk');
import s3 = require('@aws-cdk/aws-s3');


export class TestStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const bucket = new s3.Bucket(this, 'MyBucket');
    const bucketResource = bucket.node.findChild('Resource') as s3.CfnBucket;
    bucketResource.addPropertyOverride('AccessControl', 'bucket-owner-read');
  }
}

yields the following:

npx cdk synth test-stack
Resources:
  MyBucketF68F3FF0:
    Type: AWS::S3::Bucket
    DeletionPolicy: Retain
    Metadata:
      aws:cdk:path: test-stack/MyBucket/Resource
    Properties: {}
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Modules: aws-cdk=0.32.0,@aws-cdk/aws-events=0.32.0,@aws-cdk/aws-iam=0.32.0,@aws-cdk/aws-kms=0.32.0,@aws-cdk/aws-s3=0.32.0,@aws-cdk/aws-s3-notifications=0.32.0,@aws-cdk/cdk=0.32.0,@aws-cdk/cx-api=0.32.0,@aws-cdk/region-info=0.32.0,jsii-runtime=node.js/v11.6.0

Note that AccessControl is absent from the synth output.

Expected behavior
AccessControl: "bucket-owner-read" should be seen in the synth output

Version:

  • OS: OS X 10.14.5
  • Programming Language: Typescript
  • CDK Version: 0.32.0
@spg spg added the bug This issue is a bug. label May 29, 2019
@eladb
Copy link
Contributor

eladb commented May 30, 2019

Try to use camel case: addPropertyOverride('accessControl', 'bucket-owner-read').

This is actually not the intended behavior and we should fix, but in the meantime, I hope this will allow you to workaround the issue.

eladb pushed a commit that referenced this issue May 30, 2019
Resource overrides (`addOverride` and `addPropertyOverride`) should be
applied after rendering properties at the L1 level. Otherwise, validation
and capitalization changes would be applied to overrides and this
contradicts the idea of being able to specify arbitrary overrides as
"patches" to the synthesized resource.

The previous behavior had two adverse effects:
1. If a property was unknown, it would be omitted from the resource
2. Properties names would need to be capitalized in camel case instead of 1:1 with the CFN schema.

Fixes #2677

BREAKING CHANGE: Properties passed to `addPropertyOverride` should match in capitalization to the CloudFormation schema (normally pascal case). For example, `addPropertyOverride('accessControl', 'xxx')` should now be `addPropertyOverride('AccessControl', 'xxx')`.
eladb pushed a commit that referenced this issue May 30, 2019
Resource overrides (`addOverride` and `addPropertyOverride`) should be
applied after rendering properties at the L1 level. Otherwise, validation
and capitalization changes would be applied to overrides and this
contradicts the idea of being able to specify arbitrary overrides as
"patches" to the synthesized resource.

The previous behavior had two adverse effects:
1. If a property was unknown, it would be omitted from the resource
2. Properties names would need to be capitalized in camel case instead of 1:1 with the CFN schema.

Fixes #2677

BREAKING CHANGE: Properties passed to `addPropertyOverride` should match in capitalization to the CloudFormation schema (normally pascal case). For example, `addPropertyOverride('accessControl', 'xxx')` should now be `addPropertyOverride('AccessControl', 'xxx')`.
This was referenced Dec 12, 2019
@machender1
Copy link

I am facing a similar issue when trying to override Path property on an instance profile. I do not see path getting attached to my resource generated on cfn.

`if (node instanceof iam.CfnInstanceProfile ) {

      //node.addPropertyOverride('Path', this.pathArn);
      const instanceProfileResource = node.node.findChild('Resource') as iam.CfnInstanceProfile;
      instanceProfileResource.addPropertyOverride('path', this.pathArn);
  }`

Produces ...
ASGInstanceProfile0A2834D7: Type: AWS::IAM::InstanceProfile Properties: Roles: - Ref: ConfluanceTestRoleEB6CE1EB Metadata: aws:cdk:path: Ec2AppCdkStack/ASG/InstanceProfile

Could you please advise on what am i missing here to add Path to instance profile?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment