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: need a well-supported way to depend on added permissions #7236

Closed
blimmer opened this issue Apr 7, 2020 · 7 comments
Closed

iam: need a well-supported way to depend on added permissions #7236

blimmer opened this issue Apr 7, 2020 · 7 comments
Assignees
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management @aws-cdk/aws-kinesis Related to Amazon Kinesis effort/medium Medium work item – several days of effort feature-request A feature should be added or improved.

Comments

@blimmer
Copy link
Contributor

blimmer commented Apr 7, 2020

I'm manually creating an IAM role for my Kinesis Data Stream -> Kinesis Data Firehose infrastructure. However, when I use either role.addToPolicy or stream.grantRead(myRole), it doesn't wait for generated policies to be attached to the created role.

Reproduction Steps

import * as cdk from "@aws-cdk/core";
import * as kinesis from "@aws-cdk/aws-kinesis";
import * as kinesisfirehose from "@aws-cdk/aws-kinesisfirehose";
import * as iam from "@aws-cdk/aws-iam";
import * as s3 from "@aws-cdk/aws-s3";

export class MyStack extends cdk.Stack {
  readonly dataStream: kinesis.Stream;
  readonly s3Bucket: s3.Bucket;
  readonly s3DeliveryStream: kinesisfirehose.CfnDeliveryStream;

  private constructName: string;

  constructor(scope: cdk.Construct, id: string, props: cdk.StackProps) {
    super(scope, id, props);
    this.constructName = "ExampleStack";

    this.dataStream = this.createDataStream();
    this.s3Bucket = this.createS3Bucket();
    this.s3DeliveryStream = this.createDeliveryStream(this.s3Bucket, this.dataStream);
  }

  private createDataStream(): kinesis.Stream {
    return new kinesis.Stream(this, `${this.constructName}DataStream`, {
      shardCount: 1,
    });
  }

  private createS3Bucket(): s3.Bucket {
    return new s3.Bucket(this, `${this.constructName}Bucket`, {
      bucketName: "my-example-bucket-fjdklsajklfdsa123124",
      blockPublicAccess: s3.BlockPublicAccess.BLOCK_ALL,
    });
  }

  private createDeliveryStream(bucket: s3.Bucket, dataStream: kinesis.Stream): kinesisfirehose.CfnDeliveryStream {
    const deliveryStreamRole = new iam.Role(this, `${this.constructName}DeliveryStreamRole`, {
      assumedBy: new iam.ServicePrincipal("firehose.amazonaws.com"),
    });
    dataStream.grantRead(deliveryStreamRole);

    deliveryStreamRole.addToPolicy(
      new iam.PolicyStatement({
        resources: [bucket.bucketArn, `${bucket.bucketArn}/*`],
        actions: [
          "s3:AbortMultipartUpload",
          "s3:GetBucketLocation",
          "s3:GetObject",
          "s3:ListBucket",
          "s3:ListBucketMultipartUploads",
          "s3:PutObject",
        ],
      }),
    );

    const deliveryStream = new kinesisfirehose.CfnDeliveryStream(this, `${this.constructName}DeliveryStream`, {
      deliveryStreamName: `${this.constructName}-delivery-stream`,
      deliveryStreamType: "KinesisStreamAsSource",
      kinesisStreamSourceConfiguration: {
        kinesisStreamArn: dataStream.streamArn,
        roleArn: deliveryStreamRole.roleArn,
      },
      s3DestinationConfiguration: {
        bucketArn: bucket.bucketArn,
        bufferingHints: {
          intervalInSeconds: 300,
          sizeInMBs: 2,
        },
        compressionFormat: "GZIP",
        roleArn: deliveryStreamRole.roleArn,
      },
    });
    // I tried to do this to make it wait for the policies, but it doesn't seem to
    // work.
    deliveryStream.addDependsOn(deliveryStreamRole.node.defaultChild as iam.CfnRole);
    return deliveryStream;
  }
}

Error Log

cdk deploy 'EventBusTestCDKIssue101F1437'
This deployment will make potentially sensitive changes according to your current security approval level (--require-approval broadening).
Please confirm you intend to make the following modifications:

IAM Statement Changes
┌───┬────────────────────────┬────────┬────────────────────────┬─────────────────────────┬───────────┐
│   │ Resource               │ Effect │ Action                 │ Principal               │ Condition │
├───┼────────────────────────┼────────┼────────────────────────┼─────────────────────────┼───────────┤
│ + │ ${TestCDKIssue/Example │ Allow  │ s3:AbortMultipartUploa │ AWS:${TestCDKIssue/Exam │           │
│   │ StackBucket.Arn}       │        │ d                      │ pleStackDeliveryStreamR │           │
│   │ ${TestCDKIssue/Example │        │ s3:GetBucketLocation   │ ole}                    │           │
│   │ StackBucket.Arn}/*     │        │ s3:GetObject           │                         │           │
│   │                        │        │ s3:ListBucket          │                         │           │
│   │                        │        │ s3:ListBucketMultipart │                         │           │
│   │                        │        │ Uploads                │                         │           │
│   │                        │        │ s3:PutObject           │                         │           │
├───┼────────────────────────┼────────┼────────────────────────┼─────────────────────────┼───────────┤
│ + │ ${TestCDKIssue/Example │ Allow  │ kinesis:DescribeStream │ AWS:${TestCDKIssue/Exam │           │
│   │ StackDataStream.Arn}   │        │ kinesis:GetRecords     │ pleStackDeliveryStreamR │           │
│   │                        │        │ kinesis:GetShardIterat │ ole}                    │           │
│   │                        │        │ or                     │                         │           │
├───┼────────────────────────┼────────┼────────────────────────┼─────────────────────────┼───────────┤
│ + │ ${TestCDKIssue/Example │ Allow  │ sts:AssumeRole         │ Service:firehose.amazon │           │
│   │ StackDeliveryStreamRol │        │                        │ aws.com                 │           │
│   │ e.Arn}                 │        │                        │                         │           │
└───┴────────────────────────┴────────┴────────────────────────┴─────────────────────────┴───────────┘
(NOTE: There may be security-related changes not in this list. See https://github.com/aws/aws-cdk/issues/1299)

Do you wish to deploy these changes (y/n)? y
EventBusTestCDKIssue101F1437: deploying...
EventBusTestCDKIssue101F1437: creating CloudFormation changeset...
 0/7 | 11:37:19 AM | CREATE_IN_PROGRESS   | AWS::Kinesis::Stream                 | EventBus/TestCDKIssue/ExampleStackDataStream (ExampleStackDataStream6000A15C)
 0/7 | 11:37:19 AM | CREATE_IN_PROGRESS   | AWS::CDK::Metadata                   | CDKMetadata
 0/7 | 11:37:19 AM | CREATE_IN_PROGRESS   | AWS::S3::Bucket                      | EventBus/TestCDKIssue/ExampleStackBucket (ExampleStackBucket87307B5D)
 0/7 | 11:37:19 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role                       | EventBus/TestCDKIssue/ExampleStackDeliveryStreamRole (ExampleStackDeliveryStreamRole99360ADD)
 0/7 | 11:37:19 AM | CREATE_IN_PROGRESS   | AWS::Kinesis::Stream                 | EventBus/TestCDKIssue/ExampleStackDataStream (ExampleStackDataStream6000A15C) Resource creation Initiated
 0/7 | 11:37:20 AM | CREATE_IN_PROGRESS   | AWS::IAM::Role                       | EventBus/TestCDKIssue/ExampleStackDeliveryStreamRole (ExampleStackDeliveryStreamRole99360ADD) Resource creation Initiated
 0/7 | 11:37:21 AM | CREATE_IN_PROGRESS   | AWS::S3::Bucket                      | EventBus/TestCDKIssue/ExampleStackBucket (ExampleStackBucket87307B5D) Resource creation Initiated
 0/7 | 11:37:21 AM | CREATE_IN_PROGRESS   | AWS::CDK::Metadata                   | CDKMetadata Resource creation Initiated
 1/7 | 11:37:21 AM | CREATE_COMPLETE      | AWS::CDK::Metadata                   | CDKMetadata
 2/7 | 11:37:35 AM | CREATE_COMPLETE      | AWS::IAM::Role                       | EventBus/TestCDKIssue/ExampleStackDeliveryStreamRole (ExampleStackDeliveryStreamRole99360ADD)
 3/7 | 11:37:42 AM | CREATE_COMPLETE      | AWS::S3::Bucket                      | EventBus/TestCDKIssue/ExampleStackBucket (ExampleStackBucket87307B5D)
3/7 Currently in progress: ExampleStackDataStream6000A15C
 4/7 | 11:38:20 AM | CREATE_COMPLETE      | AWS::Kinesis::Stream                 | EventBus/TestCDKIssue/ExampleStackDataStream (ExampleStackDataStream6000A15C)
 4/7 | 11:38:22 AM | CREATE_IN_PROGRESS   | AWS::IAM::Policy                     | EventBus/TestCDKIssue/ExampleStackDeliveryStreamRole/DefaultPolicy (ExampleStackDeliveryStreamRoleDefaultPolicy9AA6D67D)
 4/7 | 11:38:22 AM | CREATE_IN_PROGRESS   | AWS::KinesisFirehose::DeliveryStream | EventBus/TestCDKIssue/ExampleStackDeliveryStream (ExampleStackDeliveryStream)
 5/7 | 11:38:23 AM | CREATE_FAILED        | AWS::KinesisFirehose::DeliveryStream | EventBus/TestCDKIssue/ExampleStackDeliveryStream (ExampleStackDeliveryStream) Role arn:aws:iam::<MYACCOUNTNUMBER>:role/EventBusTestCDKIssue101F1-ExampleStackDeliveryStre-QQQLVXOG8HIC is not authorized to perform: kinesis:DescribeStream on resource arn:aws:kinesis:us-west-2:<MYACCOUNTNUMBER>:stream/EventBusTestCDKIssue101F1437-ExampleStackDataStream6000A15C-1XGB6GV60GFKN. (Service: AmazonKinesisFirehose; Status Code: 400; Error Code: InvalidArgumentException; Request ID: da4a0ca8-b564-7949-8d4c-b06783a4b8f2)
	EventBusS3DeliveryStack.createDeliveryStream (/Users/blimmer/code/cdk-example/src/stacks/event-bus-s3-delivery.ts:56:28)
	\_ new EventBusS3DeliveryStack (/Users/blimmer/code/cdk-example/src/stacks/event-bus-s3-delivery.ts:20:34)
	\_ EventBusStack.createS3DeliveryStream (/Users/blimmer/code/cdk-example/src/stacks/event-bus.ts:111:5)
	\_ new EventBusStack (/Users/blimmer/code/cdk-example/src/stacks/event-bus.ts:51:10)
	\_ Object.<anonymous> (/Users/blimmer/code/cdk-example/src/index.ts:16:22)
	\_ Module._compile (internal/modules/cjs/loader.js:1158:30)
	\_ Module.m._compile (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/index.ts:836:23)
	\_ Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
	\_ Object.require.extensions.<computed> [as .ts] (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/index.ts:839:12)
	\_ Module.load (internal/modules/cjs/loader.js:1002:32)
	\_ Function.Module._load (internal/modules/cjs/loader.js:901:14)
	\_ Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12)
	\_ main (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/bin.ts:226:14)
	\_ Object.<anonymous> (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/bin.ts:485:3)
	\_ Module._compile (internal/modules/cjs/loader.js:1158:30)
	\_ Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
	\_ Module.load (internal/modules/cjs/loader.js:1002:32)
	\_ Function.Module._load (internal/modules/cjs/loader.js:901:14)
	\_ Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12)
	\_ /Users/blimmer/.nvm/versions/node/v12.16.1/lib/node_modules/npm/node_modules/libnpx/index.js:268:14
 5/7 | 11:38:23 AM | CREATE_IN_PROGRESS   | AWS::IAM::Policy                     | EventBus/TestCDKIssue/ExampleStackDeliveryStreamRole/DefaultPolicy (ExampleStackDeliveryStreamRoleDefaultPolicy9AA6D67D) Resource creation Initiated
 6/7 | 11:38:23 AM | CREATE_FAILED        | AWS::IAM::Policy                     | EventBus/TestCDKIssue/ExampleStackDeliveryStreamRole/DefaultPolicy (ExampleStackDeliveryStreamRoleDefaultPolicy9AA6D67D) Resource creation cancelled
	new Policy (/Users/blimmer/code/cdk-example/node_modules/@aws-cdk/aws-iam/lib/policy.ts:114:22)
	\_ Role.addToPolicy (/Users/blimmer/code/cdk-example/node_modules/@aws-cdk/aws-iam/lib/role.ts:353:28)
	\_ Function.addToPrincipal (/Users/blimmer/code/cdk-example/node_modules/@aws-cdk/aws-iam/lib/grant.ts:147:61)
	\_ Stream.grant (/Users/blimmer/code/cdk-example/node_modules/@aws-cdk/aws-kinesis/lib/stream.ts:168:22)
	\_ Stream.grantRead (/Users/blimmer/code/cdk-example/node_modules/@aws-cdk/aws-kinesis/lib/stream.ts:118:22)
	\_ EventBusS3DeliveryStack.createDeliveryStream (/Users/blimmer/code/cdk-example/src/stacks/event-bus-s3-delivery.ts:40:16)
	\_ new EventBusS3DeliveryStack (/Users/blimmer/code/cdk-example/src/stacks/event-bus-s3-delivery.ts:20:34)
	\_ EventBusStack.createS3DeliveryStream (/Users/blimmer/code/cdk-example/src/stacks/event-bus.ts:111:5)
	\_ new EventBusStack (/Users/blimmer/code/cdk-example/src/stacks/event-bus.ts:51:10)
	\_ Object.<anonymous> (/Users/blimmer/code/cdk-example/src/index.ts:16:22)
	\_ Module._compile (internal/modules/cjs/loader.js:1158:30)
	\_ Module.m._compile (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/index.ts:836:23)
	\_ Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
	\_ Object.require.extensions.<computed> [as .ts] (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/index.ts:839:12)
	\_ Module.load (internal/modules/cjs/loader.js:1002:32)
	\_ Function.Module._load (internal/modules/cjs/loader.js:901:14)
	\_ Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12)
	\_ main (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/bin.ts:226:14)
	\_ Object.<anonymous> (/Users/blimmer/code/cdk-example/node_modules/ts-node/src/bin.ts:485:3)
	\_ Module._compile (internal/modules/cjs/loader.js:1158:30)
	\_ Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
	\_ Module.load (internal/modules/cjs/loader.js:1002:32)
	\_ Function.Module._load (internal/modules/cjs/loader.js:901:14)
	\_ Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12)
	\_ /Users/blimmer/.nvm/versions/node/v12.16.1/lib/node_modules/npm/node_modules/libnpx/index.js:268:14
 6/7 | 11:38:24 AM | ROLLBACK_IN_PROGRESS | AWS::CloudFormation::Stack           | EventBusTestCDKIssue101F1437 The following resource(s) failed to create: [ExampleStackDeliveryStream, ExampleStackDeliveryStreamRoleDefaultPolicy9AA6D67D]. . Rollback requested by user.

Environment

  • CLI Version : 1.31.0
  • Framework Version: 1.31.0
  • OS : MacOS
  • Language : English

Other

If I look at the console during the creation of this stack, I can see that the Role is created, but it has no policies attached. The way I've worked around this is by creating the role separately during a cdk deploy, then create the stream. Then, everything works.


This is 🐛 Bug Report

@blimmer blimmer added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 7, 2020
@andrestone
Copy link
Contributor

andrestone commented Apr 9, 2020

Hi,

The scope the resource dependency is restricted to its state at creation.

Since you're creating the IAM Role and adding the policies later, the dependency is fulfilled when the Role is created, without the later added policies.

The Role construct allows you to pass inlinePolicies as props, fulfilling your dependency at creation time.

I'd also cast the defaultChild as CfnResource.

Fix those and your code will work.

@SomayaB SomayaB added @aws-cdk/aws-iam Related to AWS Identity and Access Management @aws-cdk/aws-kinesis Related to Amazon Kinesis labels Apr 9, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 9, 2020

    deliveryStream.addDependsOn(deliveryStreamRole.node.defaultChild as iam.CfnRole);

Try this instead:

deliveryStream.addDependency(deliveryStreamRole);

It's not ideal, but it will fix your issue.

@rix0rrr rix0rrr changed the title Generated IAM Role default policy should be created and attached before using the Role in other constructs iam: need a well-supported way to depend on added permissions Apr 9, 2020
@rix0rrr rix0rrr added feature-request A feature should be added or improved. effort/medium Medium work item – several days of effort and removed bug This issue is a bug. labels Apr 9, 2020
@andrestone
Copy link
Contributor

andrestone commented Apr 9, 2020

    deliveryStream.addDependsOn(deliveryStreamRole.node.defaultChild as iam.CfnRole);

Try this instead:

deliveryStream.addDependency(deliveryStreamRole);

It's not ideal, but it will fix your issue.

Hi @rix0rrr, I can confirm this doesn't work:

    // @ts-ignore
    deliveryStream.addDependsOn(deliveryStreamRole);

Maybe because those dependencies are ultimately scoped to the underlying CfnResources, and not the constructs?

// stack.ts
// ...
    // Resource dependencies
    for (const dependency of this.node.dependencies) {
      for (const target of findCfnResources([ dependency.target ])) {
        for (const source of findCfnResources([ dependency.source ])) {
          source.addDependsOn(target);
        }
      }
    }

Also, isn't the dependency issue already pretty reasonably covered by the construct?

  /**
   * A list of named policies to inline into this role. These policies will be
   * created with the role, whereas those added by ``addToPolicy`` are added
   * using a separate CloudFormation resource (allowing a way around circular
   * dependencies that could otherwise be introduced).
   *
   * @default - No policy is inlined in the Role resource.
   */
  readonly inlinePolicies?: { [name: string]: PolicyDocument };

🤔

@blimmer
Copy link
Contributor Author

blimmer commented Apr 12, 2020

@rix0rrr and @andrestone - thanks for the continued diagnosis. I also confirmed that this didn't fix the problem:

// @ts-ignore
deliveryStream.addDependsOn(deliveryStreamRole);

Because constructs like the Kinesis stream grantRead method exist and rely on the default policy logic, it feels like using addDependsOn with the Role should, indeed, add transitive dependencies on the default role being created and attached.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Apr 14, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Apr 24, 2020

I actually meant

deliveryStream.node.addDependency(...)

Forgot the .node. in the example. addDependsOn will not work.

rix0rrr added a commit that referenced this issue Apr 24, 2020
Lambda Targets can get registered into a TargetGroup before the
`lambda:Invoke` permissions are added. Since the TargetGroup does
a permission test, deployment will fail.

Fixes #4663.

Adding a long-overdue mechanism to the IAM library to depend on Grants
that just got created, to ensure permissions are created before the
resources that depend on them (in effect: `Grant`s are made
`IDependable`).

Relates to the root cause of #7236.
@logemann
Copy link

logemann commented May 5, 2020

i have quite a similar issue. Take this code:

        const serviceLinkedRole = new CfnServiceLinkedRole(this, "esServiceLinkedRole", {
            awsServiceName: "es.amazonaws.com",
            description: "Role for ES to access resources in my VPC"
        })

        let esDomain = new CfnDomain(this, "ElasticSearchDomain", this.createCfnDomainProps(domain, props.vpc));
        esDomain.node.addDependency(serviceLinkedRole);

esDomain.node.addDependency(..) AND esDomain.addDependsOn(..) dont work. Always get

ElasticSearchDomain
4/5 | 01:12:00 | CREATE_FAILED | AWS::Elasticsearch::Domain | ElasticSearchDomain Before you can proceed, you must enable a service-linked role to give Amazon ES permissions to access your VPC

If i run this code without the ESDomain creation (only the CfnServiceLinkedRole creation) and run it again this time WITH esDomain creation, it works. So why do all those depend* methods dont work?

Funny enough... according to logs, the ServiceRole was created before the Domain creation started but nevertheless the error appeared.

2/5 | 01:11:54 | CREATE_COMPLETE | AWS::IAM::ServiceLinkedRole | esServiceLinkedRole
2/5 | 01:11:57 | CREATE_IN_PROGRESS | AWS::EC2::SecurityGroup | ElasticSearchSecurityGroup (ElasticSearchSecurityGroup19ECC607) Resource creation Initiated
3/5 | 01:11:58 | CREATE_COMPLETE | AWS::EC2::SecurityGroup | ElasticSearchSecurityGroup (ElasticSearchSecurityGroup19ECC607)
3/5 | 01:11:59 | CREATE_IN_PROGRESS | AWS::Elasticsearch::Domain | ElasticSearchDomain
4/5 | 01:12:00 | CREATE_FAILED | AWS::Elasticsearch::Domain | ElasticSearchDomain Before you can

synthesized file for stack looks good too. "ElasticSearchDomain" has the property:

      "DependsOn": [
        "esServiceLinkedRole"
      ],

mergify bot pushed a commit that referenced this issue May 12, 2020
Lambda Targets can get registered into a TargetGroup before the
`lambda:Invoke` permissions are added. Since the TargetGroup does
a permission test, deployment will fail.

Fixes #4663.

Adding a long-overdue mechanism to the IAM library to depend on Grants
that just got created, to ensure permissions are created before the
resources that depend on them (in effect: `Grant`s are made
`IDependable`).

Relates to the root cause of #7236.
karupanerura pushed a commit to karupanerura/aws-cdk that referenced this issue May 22, 2020
Lambda Targets can get registered into a TargetGroup before the
`lambda:Invoke` permissions are added. Since the TargetGroup does
a permission test, deployment will fail.

Fixes aws#4663.

Adding a long-overdue mechanism to the IAM library to depend on Grants
that just got created, to ensure permissions are created before the
resources that depend on them (in effect: `Grant`s are made
`IDependable`).

Relates to the root cause of aws#7236.
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 12, 2020

This has been added, Grants are now IDependable.

@rix0rrr rix0rrr closed this as completed Aug 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management @aws-cdk/aws-kinesis Related to Amazon Kinesis effort/medium Medium work item – several days of effort feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

5 participants