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

Better feedback on python SDK for list requirements #2310

Open
jaxxstorm opened this issue Jan 10, 2023 · 5 comments
Open

Better feedback on python SDK for list requirements #2310

jaxxstorm opened this issue Jan 10, 2023 · 5 comments
Labels
kind/enhancement Improvements or new features

Comments

@jaxxstorm
Copy link
Contributor

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 jaxxstorm added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Jan 10, 2023
@jaxxstorm
Copy link
Contributor Author

Please feel free to modify the issue title!

@guineveresaenger
Copy link
Contributor

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?

@guineveresaenger guineveresaenger removed the needs-triage Needs attention from the triage team label Jan 13, 2023
@kamatsuoka
Copy link

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
Copy link

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

@ar-qun
Copy link

ar-qun commented Nov 29, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

5 participants