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

IAM: Problems sythesizing a role that can read from an SQS queue #622

Closed
millems opened this issue Aug 23, 2018 · 5 comments · Fixed by #629
Closed

IAM: Problems sythesizing a role that can read from an SQS queue #622

millems opened this issue Aug 23, 2018 · 5 comments · Fixed by #629

Comments

@millems
Copy link

millems commented Aug 23, 2018

Java code:

Queue queue = new Queue(this, "source-import-queue",
                        QueueProps.builder()
                                  .withQueueName("source-import")
                                  .build());

Bucket bucket = new Bucket(this, "source-import-bucket",
                           BucketProps.builder()
                                      .withBucketName("source-import")
                                      .build());
bucket.onObjectCreated(queue);
bucket.addLifecycleRule(LifecycleRule.builder()
                                     .withExpirationInDays(30)
                                     .build());

PolicyStatement policyStatement = new PolicyStatement(PolicyStatementEffect.Allow);
policyStatement.addActions("ReceiveMessage", "DeleteMessage", "DeleteMessageBatch");
policyStatement.addResource(queue);

Role role = new Role(this, "source-import-queue-reader",
                     RoleProps.builder()
                              .withAssumedBy(new AccountPrincipal("12345"))
                              .withRoleName("source-import-queue-reader")
                              .build());
role.addToPolicy(policyStatement);

Expected Behavior: Create a role with a policy that allows reading from the queue.

Actual Behavior:

Exception in thread "main" software.amazon.jsii.JsiiException: While synthesizing hello-cdk/source-import-queue-reader/DefaultPolicy/Resource: Trying to resolve() a Construct at /policyDocument/Statement/0/Resource
While synthesizing hello-cdk/source-import-queue-reader/DefaultPolicy/Resource: Trying to resolve() a Construct at /policyDocument/Statement/0/Resource
    --- resource created at ---
    at new Policy (/private/tmp/jsii-kernel-aRMZR8/node_modules/@aws-cdk/aws-iam/lib/policy.js:22:26)
    at Role.addToPolicy (/private/tmp/jsii-kernel-aRMZR8/node_modules/@aws-cdk/aws-iam/lib/role.js:39:34)
    at _wrapSandboxCode (/private/var/folders/x4/0pn8hl6x4kz135bdrmgdlk7xmh2tn2/T/jsii-java-runtime6462445815484053124/jsii-runtime.js:1:84495)
@millems
Copy link
Author

millems commented Aug 23, 2018

Removing the line: policyStatement.addResource(queue); resolves the issue.

@millems
Copy link
Author

millems commented Aug 23, 2018

policyStatement.addResource(queue.getQueueArn()); removes the exception, but I haven't tested to verify it actually works.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 23, 2018

Yeah, that is the way.

Another unfortunate case of a too broad type declaration?

@millems
Copy link
Author

millems commented Aug 23, 2018

Seems unintuitive, since I can do bucket.onObjectCreated(queue);, but I guess I understand the distinction.

@RomainMuller
Copy link
Contributor

I guess right now you have to be unfortunately aware of whether you're using a low-level class (will need you to use Ref or GetAtt accessors) or a higher-level class (will just get the object). That's not ideal 😔. Hopefully this fades out significantly as our L2 construct coverage expands and is not much of an issue anymore by then.

rix0rrr pushed a commit that referenced this issue Aug 27, 2018
A number methods were allowing 'any's in places where they easily lead
to passing the wrong object.

- `role.attachManagedPolicy`
- Various methods on `PolicyStatement`.

By restrictinig the types to what we actually expect (or `string`s)
these mistakes will be harder to make.

Fixes #622, doesn't completely resolve but helps with #621.
rix0rrr added a commit that referenced this issue Aug 27, 2018
A number methods were allowing `any`s in places where they easily led
to passing the wrong object.

- `role.attachManagedPolicy()`, expected an `Arn` but it was possible to
  pass it a `Policy` object by mistake.
- Various methods on `PolicyStatement`. Notably: `addResource()` which
  is supposed to take an `Arn` but it was too easy to pass it a
  resource object by mistake.

By restrictinig the types to what we actually expect these mistakes will
be harder to make (at the expense of slightly more code when passing raw
strings).

Most likely breakage consumers will see:

- `addResource("*")`, replace with `addAllResources()`.
- `addResource("some-arn")`, replace with `new Arn("some-arn")`.
- `new ServicePrincipal(new FnConcat(...))`, replace with 
  `new ServicePrincipal(new FnConcat(...).toString())`.

Fixes #622, doesn't completely resolve but helps with #621.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants