aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.52k stars 3.86k forks source link

(aws_iam.PolicyDocument): Method 'fromJson(obj)' only takes parent 'Statement' JSON element #29975

Open frfavoreto opened 4 months ago

frfavoreto commented 4 months ago

Describe the bug

Method fromJson() from aws_iam.PolicyDocument ignores Id and Version parent elements of a JSON Policy text, taking only the Statement and its nested elements.

newPolicyDocument.addStatements(...obj.Statement.map((s: any) => PolicyStatement.fromJson(s))); return newPolicyDocument; }

https://github.com/aws/aws-cdk/blob/v2.137.0/packages/aws-cdk-lib/aws-iam/lib/policy-document.ts#L61

Expected Behavior

Method fromJson() to accept all the JSON elements passed as an input.

Current Behavior

When using the method, it only considers Statement element (and its sub elements) and auto generate Version one. During this process, Id is ignored and the synth'ed template doesn't have any Id element in the Policy Document.

Reproduction Steps

Create any IAM Policy from a JSON text (with Version and Id elements included) like below:

const myJsonText ={
    "Version": "2012-10-17",
    "Id": "KMS-Key-Policy-Example",
    "Statement": [
        {
          "Sid": "Example SID",
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::123456789012:root"
            },
            "Action": "kms:*",
            "Resource": "*"
        }
    ]
}

export class testStack extends Stack {
    constructor(scope: Construct, id: string, props?: StackProps) {
      super(scope, id, props);

      const myPolicyTest = iam.PolicyDocument.fromJson(myJsonText);

      const key = new kms.Key(this, "myKMSkey",
        {
            policy: myPolicyTest,
        });

    }
  }

The generated policy does not have Id element. "Version": "2012-10-17" is automatically generated.

"myKMSkey6B023671": {
   "Type": "AWS::KMS::Key",
   "Properties": {
    "KeyPolicy": {
     "Statement": [
      {
       "Action": "kms:*",
       "Effect": "Allow",
       "Principal": {
        "AWS": "arn:aws:iam::123456789012:root"
       },
       "Resource": "*",
       "Sid": "Example SID"
      }
     ],
     "Version": "2012-10-17"
    }
   },
   "UpdateReplacePolicy": "Retain",
   "DeletionPolicy": "Retain",
   "Metadata": {
    "aws:cdk:path": "KmsKeyMissingIdTsStack/myKMSkey/Resource"
   }
  },

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

All supported versions, including latest 2.139.0

Framework Version

No response

Node.js Version

20

OS

Mac

Language

TypeScript

Language Version

No response

Other information

No response

ashishdhingra commented 4 months ago

Reproducible using code below:

const myJsonText ={
      "Version": "testversion",
      "Id": "KMS-Key-Policy-Example",
      "Statement": [
          {
            "Sid": "Example SID",
              "Effect": "Allow",
              "Principal": {
                  "AWS": "arn:aws:iam::123456789012:root"
              },
              "Action": "kms:*",
              "Resource": "*"
          }
      ]
    };

    const myPolicyTest = iam.PolicyDocument.fromJson(myJsonText);
    console.debug(myPolicyTest.toJSON());

    const key = new kms.Key(this, "myKMSkey", {
      policy: myPolicyTest,
    });

It generates the following debug output during cdk synth. Id property appears to be ignored, whereas Version property appears to be auto-generated:

{
  Statement: [
    {
      Action: 'kms:*',
      Effect: 'Allow',
      Principal: [Object],
      Resource: '*',
      Sid: 'Example SID'
    }
  ],
  Version: '2012-10-17'
}
Resources:
  myKMSkey6B023671:
    Type: AWS::KMS::Key
    Properties:
      KeyPolicy:
        Statement:
          - Action: kms:*
            Effect: Allow
            Principal:
              AWS: arn:aws:iam::123456789012:root
            Resource: "*"
            Sid: Example SID
        Version: "2012-10-17"
    UpdateReplacePolicy: Retain
    DeletionPolicy: Retain
    Metadata:
      aws:cdk:path: TypescriptStack/myKMSkey/Resource
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Analytics: v2:deflate64:H4sIAAAAAAAA/yXGsQ5AMBAA0G+xt0dZ7EYbHyDVnuSUa+KKiPh3EdN7JZiqhiKzp2jng15ohLtP1gVlTxnCKnC3eKlm4hav57NDifvm8HsT2VOiyI/i6BFmyY+yAGPAZLMQ6W3nRCtC9/sC8s+Yhm8AAAA=
    Metadata:
      aws:cdk:path: TypescriptStack/CDKMetadata/Default
    Condition: CDKMetadataAvailable
...
...
ashishdhingra commented 4 months ago

@frfavoreto I was able to reproduce the issue at my end. According to this, setting Version property here appears to be by design. Can you share your use cases why you need them to be included in the JSON rather than auto generated or assigned?

frfavoreto commented 4 months ago

@ashishdhingra Version is not too important here, even though someone might want to use the older 2008-10-17 value for whatever retro-compatibility reasons.

However, Id is something useful, specially in audit compliance scenarios. If we are creating a policy from a custom JSON content using iam.PolicyDocument.fromJson() this is perfectly reasonable to expect the policy to be created entirely from the crafted JSON as an input. Whether the policy is going to be valid or not (depending on the service) this is something to be evaluated at deployment time.

Additionally, some services like SQS and SNS might require this element and have specific requirements for it.