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

(cli): format of tags in cdk.json is not validated, easy to get wrong #20854

Closed
SydneyUni-Jim opened this issue Jun 24, 2022 · 3 comments · Fixed by #21050
Closed

(cli): format of tags in cdk.json is not validated, easy to get wrong #20854

SydneyUni-Jim opened this issue Jun 24, 2022 · 3 comments · Fixed by #21050
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1 package/tools Related to AWS CDK Tools or CLI

Comments

@SydneyUni-Jim
Copy link
Contributor

Describe the bug

After adding tags to the local cdk.json, cdk deploy fails with an InvalidParameterType: Expected params.Tags to be an Array from aws-sdk.

Expected Behavior

The synthesized template deploys with the tags in cdk.json.

Current Behavior

❯ cdk deploy

✨  Synthesis time: 3.04s

HelloCdkStack: deploying...
[0%] start: Publishing 774231a5b4dfea162ee0964f8335b364aee003aa142c95fb1c4d3e49537db261:current_account-current_region
[100%] success: Published 774231a5b4dfea162ee0964f8335b364aee003aa142c95fb1c4d3e49537db261:current_account-current_region
HelloCdkStack: creating CloudFormation changeset...

 ❌  HelloCdkStack failed: InvalidParameterType: Expected params.Tags to be an Array
    at ParamValidator.fail (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/param_validator.js:50:37)
    at ParamValidator.validateType (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/param_validator.js:233:10)
    at ParamValidator.validateList (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/param_validator.js:100:14)
    at ParamValidator.validateMember (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/param_validator.js:91:21)
    at ParamValidator.validateStructure (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/param_validator.js:76:14)
    at ParamValidator.validateMember (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/param_validator.js:89:21)
    at ParamValidator.validate2 [as validate] (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/param_validator.js:34:10)
    at Request.VALIDATE_PARAMETERS (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/event_listeners.js:132:42)
    at Request.callListeners (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at callNextListener (/opt/homebrew/Cellar/aws-cdk/2.29.0/libexec/lib/node_modules/aws-sdk/lib/sequential_executor.js:96:12) {
  code: 'InvalidParameterType',
  time: 2022-06-24T02:18:15.714Z
}

Expected params.Tags to be an Array

Reproduction Steps

  1. mkdir hello-cdk
  2. cd hello-cdk
  3. cdk init app --language typescript
  4. cdk deploy

So far this works.

  1. Replace cdk.json with this cdk.json.
  2. cdk deploy

This fails.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.29.0 (build 47d7ec4)

Framework Version

No response

Node.js Version

v16.15.1

OS

macOS 12.4 arm64

Language

Typescript

Language Version

4.7.4

Other information

Documentation reference: https://docs.aws.amazon.com/cdk/v2/guide/cli.html#cli-config

tags - JSON object containing tags (key-value pairs) for the stack.

@SydneyUni-Jim SydneyUni-Jim added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 24, 2022
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Jun 24, 2022
@SydneyUni-Jim
Copy link
Contributor Author

For reference the replacement cdk.json is.

{
  "app": "npx ts-node --prefer-ts-exts bin/hello-cdk.ts",
  "context": {
    "@aws-cdk-containers/ecs-service-extensions:enableDefaultLogDriver": true,
    "@aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId": true,
    "@aws-cdk/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021": true,
    "@aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeResourceName": true,
    "@aws-cdk/aws-ec2:uniqueImdsv2TemplateName": true,
    "@aws-cdk/aws-iam:minimizePolicies": true,
    "@aws-cdk/aws-lambda:recognizeLayerVersion": true,
    "@aws-cdk/aws-lambda:recognizeVersionProps": true,
    "@aws-cdk/aws-rds:lowercaseDbIdentifier": true,
    "@aws-cdk/core:checkSecretUsage": true,
    "@aws-cdk/core:stackRelativeExports": true,
    "@aws-cdk/core:target-partitions": ["aws"],
    "@aws-cdk/core:validateSnapshotRemovalPolicy": true
  },
  "tags": {
    "Tag1": "value1",
    "Tag2": "value2"
  },
  "watch": {
    "include": [
      "**"
    ],
    "exclude": [
      "README.md",
      "cdk*.json",
      "**/*.d.ts",
      "**/*.js",
      "tsconfig.json",
      "package*.json",
      "yarn.lock",
      "node_modules",
      "test"
    ]
  }
}

@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 7, 2022

This is sorely underdocumented, and if I were you I would put the tags in the CDK App rather than in the CLI config, but for reference the format of the tag field needs to be:

{
  "tags": [
    {
       "Key": "...",
       "Value": "..."
    }
  ]
}

@rix0rrr rix0rrr changed the title cdk deploy: fails with tags in cdk.json (cli): format of tags in cdk.json is not validated, easy to get wrong Jul 7, 2022
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 7, 2022
@rix0rrr rix0rrr removed their assignment Jul 7, 2022
@mergify mergify bot closed this as completed in #21050 Jul 8, 2022
mergify bot pushed a commit that referenced this issue Jul 8, 2022
If tags is present in cdk.json, validate that it is an array of objects, and each object has a Tag string and a Value string. If tags is not structurally valid `cdk bootstrap` and `cdk deploy` fail with an error.

`tags must be an array of { Tag: string, Value: string } objects`

There is no attempt to validate the strings of each Tag and Value beyond that they are strings.

closes #20854 

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jul 8, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

daschaa pushed a commit to daschaa/aws-cdk that referenced this issue Jul 9, 2022
If tags is present in cdk.json, validate that it is an array of objects, and each object has a Tag string and a Value string. If tags is not structurally valid `cdk bootstrap` and `cdk deploy` fail with an error.

`tags must be an array of { Tag: string, Value: string } objects`

There is no attempt to validate the strings of each Tag and Value beyond that they are strings.

closes aws#20854 

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

*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
bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants