pulumi / pulumi-aws

An Amazon Web Services (AWS) Pulumi resource package, providing multi-language access to AWS
Apache License 2.0
460 stars 155 forks source link

Better feedback on python SDK for list requirements #2310

Open jaxxstorm opened 1 year ago

jaxxstorm commented 1 year ago

Part of this was a mistake on my end, but I'm intrigued to determine if we can throw a better error or provide better feedback.

I defined an AWS KMS key with a policy, like so:

key = aws.kms.Key(
    f"key-{stack_name}",
    description="Used to encrypt ECS cluster logs",
    deletion_window_in_days=28,
    enable_key_rotation=True,
    policy=aws.iam.get_policy_document(
        statements=[
            aws.iam.GetPolicyDocumentStatementArgs(
                sid="Allow IAM user access",
                actions=["kms:*"],
                effect="Allow",
                principals=aws.iam.GetPolicyDocumentStatementPrincipalArgs(
                    type="AWS", identifiers=[f"arn:aws:iam::{account.account_id}:root"
                )],
            ),
            aws.iam.GetPolicyDocumentStatementArgs(
                sid="Allow Cloudwatch Logs access",
                actions=["kms:Encrypt*", "kms:Decrypt*", "kms:ReEncrypt*", "kms:GenerateDataKey*", "kms:DescribeKey"],
                effect="Allow",
                principals=aws.iam.GetPolicyDocumentStatementPrincipalArgs(
                    type="Service", identifiers=[f"log.{region}.amazonaws.com"
                )]
            )
        ]
    ).json,
)

This passes the MyPy type checker and compiles successfully.

However, there is a mistake here that I only get feedback on when I try to run a preview

  pulumi:pulumi:Stack (ecs-sandbox):
    error: Program failed with an unhandled exception:
    Traceback (most recent call last):
      File "/Users/lbriggs/src/github/jaxxstorm/pulumi-examples/ecs/./__main__.py", line 26, in <module>
        policy=aws.iam.get_policy_document(
      File "/Users/lbriggs/src/github/jaxxstorm/pulumi-examples/ecs/venv/lib/python3.10/site-packages/pulumi_aws/iam/get_policy_document.py", line 459, in get_policy_document
        __ret__ = pulumi.runtime.invoke('aws:iam/getPolicyDocument:getPolicyDocument', __args__, opts=opts, typ=GetPolicyDocumentResult).value
      File "/Users/lbriggs/src/github/jaxxstorm/pulumi-examples/ecs/venv/lib/python3.10/site-packages/pulumi/runtime/invoke.py", line 144, in invoke
        raise invoke_error
    Exception: invoke of aws:iam/getPolicyDocument:getPolicyDocument failed: Attribute must be a list ()

The issue itself is actually that I didn't make principals= a list, so it should be:

key = aws.kms.Key(
    f"key-{stack_name}",
    description="Used to encrypt ECS cluster logs",
    deletion_window_in_days=28,
    enable_key_rotation=True,
    policy=aws.iam.get_policy_document(
        statements=[
            aws.iam.GetPolicyDocumentStatementArgs(
                sid="Allow IAM user access",
                actions=["kms:*"],
                effect="Allow",
                principals=[aws.iam.GetPolicyDocumentStatementPrincipalArgs(
                    type="AWS", identifiers=[f"arn:aws:iam::{account.account_id}:root"]
                )],
            ),
            aws.iam.GetPolicyDocumentStatementArgs(
                sid="Allow Cloudwatch Logs access",
                actions=["kms:Encrypt*", "kms:Decrypt*", "kms:ReEncrypt*", "kms:GenerateDataKey*", "kms:DescribeKey"],
                effect="Allow",
                principals=[aws.iam.GetPolicyDocumentStatementPrincipalArgs(
                    type="Service", identifiers=[f"log.{region}.amazonaws.com"]
                )]
            )
        ]
    ).json,
)

This took me ages to figure out.

jaxxstorm commented 1 year ago

Please feel free to modify the issue title!

guineveresaenger commented 1 year ago

Reading this - it sounds like we should do a better job of telling the user which field on a resource or data source function is misconfigured, am I reading that right?

kamatsuoka commented 1 year ago

Just chiming in to say that the same vague "Attribute must be a list" error happens if you set aws:allowedAccountIds to a single value instead of a list. Lost an afternoon to that one.

jamesianberry commented 1 year ago

Thanks @jaxxstorm, this helped me work out that identifiers within principals also need to be a list - from the same obscure error

ar-qun commented 11 months ago

Just chiming in to say that the same vague "Attribute must be a list" error happens if you set aws:allowedAccountIds to a single value instead of a list. Lost an afternoon to that one.

I think this issue should be for the entire SDK, I had the same issue with GCP.