Tags: should error for duplicate tag keys

[ahammond]

Describe the bug

"Please note that Tag keys are case insensitive." We have developers who discovered the inconsistency around tag key case insensitivity the hard way.

Expected Behavior

Cdk should have error out and refused in cases where tag names are only differentiated by case. Cfn should probably do this, too, but... protecting the user from the stupidity of Cfn is basically why Cdk exists.

Current Behavior

CDK will happily put tags all over the place. Your Cfn run will do different things depending on the resources involved. So you'll go and remove the duplicate tags, and Cdk will happily build the Cfn and then blammo, you don't have a tag. Hope you weren't doing ABAC or anything...

Reproduction Steps

https://github.com/ahammond/repro-tag-collision-cdk

import { App, Stack, StackProps, Tags } from 'aws-cdk-lib';
import { AnyPrincipal, Role } from 'aws-cdk-lib/aws-iam';
import { Construct } from 'constructs';

import { name } from './helpers';

const app = new App();

class Repro extends Stack {
  constructor(scope: Construct, props?: StackProps) {
    super(scope, name.pascal, props);
    const role = new Role(this, 'Role', { assumedBy: new AnyPrincipal() });

    Tags.of(role).add('Foo', 'bar');
    // 1 - Deploy once with the following `foo` (note lower case) tag commented out.
    // 2 - Deploy 2nd time with the following line uncommented. Should have errored at CDK level.
    // Cfn should have errored (on some resources it gives a "Please note that Tag keys are case insensitive.")
    // INSTEAD what it does is change the `Foo` tag's name to `foo`.
    //Tags.of(role).add('foo', 'bar');
    // 3- Comment out and deploy a 3rd time. Note that your role now doesn't have either the `foo` or the `Foo` tag.
    // Cfn has happily cleaned up your mess and broken you.
    // As a bonus, you have also achieved an inconsistent stack state.
  }
}

new Repro(app);

app.synth();

Possible Solution

Error when name collisions are detected.

Additional Information/Context

No response

CDK CLI Version

2.86.0 (build 1130fab)

Framework Version

same

Node.js Version

v16.20.0

OS

MacOS

Language

Typescript

Language Version

4.9.5

Other information

No response

[jeisson-clickup]

Reporting the same issue here.
We duplicated some tags, and when we tried to fix them and delete those we didn't need, we realized that all our tags had gone.

[evgenyka]

Related to #15343

[peterwoodworth]

Official docs on tagging states that tagging is case sensitive

If a service is not accepting tags that only vary in casing, then that should be a bug with the service

[ahammond]

So... this is actually worse then. I thought it was just a newly exposed edge case around Cfn, but instead it looks like this sounds like it's actually a CDK specific bug. From the above linked doc:

Tag naming limits and requirements
The following basic naming and usage requirements apply to tags:

Each resource can have a maximum of 50 user created tags.

System created tags that begin with aws: are reserved for AWS use, and do not count against this limit. You can't edit or delete a tag that begins with the aws: prefix.

**For each resource, each tag key must be unique, and each tag key can have only one value.**

The tag key must be a minimum of 1 and a maximum of 128 Unicode characters in UTF-8.

The tag value must be a minimum of 0 and a maximum of 256 Unicode characters in UTF-8.

Allowed characters can vary by AWS service. For information about what characters you can use to tag resources in a particular AWS service, see its documentation. In general, the allowed characters are letters, numbers, spaces representable in UTF-8, and the following characters: _ . : / = + - @.

**Tag keys and values are case sensitive. As a best practice, decide on a strategy for capitalizing tags, and consistently implement that strategy across all resource types. For example, decide whether to use Costcenter, costcenter, or CostCenter, and use the same convention for all tags. Avoid using similar tags with inconsistent case treatment.**

[ahammond]

@pahud As far as this being a P2? AWS encourages enterprise customers to use ABAC based resource policies to manage access to resources. It is rare to see ABAC that doesn't focus around tags. This bug has caused 2 weeks of havoc just for us. I'm glad we contained it before it affected prod, but... the blast radius here is "every customer who uses ABAC", which is to say "every enterprise customer". @TheRealAmazonKendra FYI this is what we were talking about earlier today.

[peterwoodworth]

Please correct me if I'm not following properly, but I don't think CDK is doing anything wrong here - and instead you encountered a nasty bug with IAM / the CloudFormation implementation of Role.

If I perform the exact steps with instead lets say, a Lambda Function, then everything works and deploys exactly as expected. I can deploy both tags at once, or one at a time, and I can remove them in any order and the tags will always reflect what I defined in my CDK stack.

[rix0rrr]

IAM documentation also states that tags are case sensitive.

It would seem that the bug, if true, lives in CFN (assuming that IAM's service behavior is correctly documented).

[rix0rrr]

Whaddaya know, my assumption that the service would be correctly documented is wrong:

$ aws --region eu-west-1 iam tag-role --role-name MyRole --tags '{"Key": "Test", "Value": "1"}'
$ aws --region eu-west-1 iam tag-role --role-name MyRole --tags '{"Key": "test", "Value": "1"}'
$ aws --region eu-west-1 iam list-role-tags --role-name MyRole
{
    "Tags": [
        {
            "Key": "test",
            "Value": "1"
        }
    ],
    "IsTruncated": false
}

[rix0rrr]

Created a support case with IAM. Internal reference D88398323.

[ahammond]

Yup, it's IAM.

[ahammond]

So... where does the "Please note that Tag keys are case insensitive." message come from? Because it runs without erroring when I just tweak the role (this is what blew us up).

[peterwoodworth]

@ahammond it errors out if you deploy both tags at once. But it doesn't if you add them one at a time

[rix0rrr]

The behavior by IAM is documented. Not at the top of the page, but somewhere in the middle of the page:

Case sensitivity – Case sensitivity for tag keys differs depending on the type of IAM resource that is tagged. Tag key values for IAM users and roles are not case sensitive, but case is preserved.

(source)

That makes it a CloudFormation issue since the AWS::IAM::Role handler is not honoring its desired-state contract. Created an ticket to CloudFormation, internal reference D88663713

[ahammond]

@rix0rrr this is the only case I know of where tag keys aren't case sensitive. If there are others, it might be worth building some kind of safety net into CDK. IDK how hard that would be (have the Tags code detect and warn when someone publishes two tags which would collide if case was ignored. Possibly limit this to only AWS::IAM::Roles)

If there AREN'T others, I'd love to see pressure on the IAM team to conform their behaviour with the Rest of AWS. Because, OMG is this ever nasty.

[evgenyka]

https://docs.aws.amazon.com/IAM/latest/UserGuide/id_tags.html#id_tags_rules:
Tag key values are not case sensitive:
IAM roles
IAM users
So far, CDK is not validating tags on the client side. And specific use case of IAM roles/users looks like wrongly handled by CloudFormation. The proper solution is that the CDK team will work with CloudFormation to address that specific case.

[evgenyka]

@ahammond - assuming we are not changing the current behaviour of IAM (escalated that to the relevant service team at AWS) and CDK is not validating tags on the client side (see my previous comment), do you believe that surfacing that via CDK (warning message, for example) will be beneficial for AWS customers?

The following lists show the differences in case sensitivity for tag keys that are attached to IAM resources.
Tag key values are not case sensitive:
IAM roles
IAM users

Tag key values are case sensitive:
Customer managed policies
Instance profiles
OpenID Connect identity providers
SAML identity providers
Server certificates
Virtual MFA devices

[ahammond]

@evgenyka any kind of warning would really help. Cdk is in a great position to protect customers from an otherwise pretty rough experience.

[ahammond]

@evgenyka It looks like your previous comment only covers inconsistent behaviour for tag keys in IAM. What about other AWS services? For the rest of this conversation, I'm going to call "case sensitive key values" "normal" since that's what anyone reading the docs is likely to assume. For other services, are there any which aren't "normal"?

[peterwoodworth]

Honestly I don't think it's at all realistic to expect us to handle this. That requires somehow tracking and always keeping up to date exactly how each service implements tagging, and somehow applying that knowledge to all CfnResources with tagging.

This should be up to CloudFormation and the implementation of CloudFormation resources to get right if the tagging for a resource differs from what the standard should be