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

feat(aws-iam): grants support non-identity principals #1623

Merged
merged 40 commits into from
Apr 3, 2019
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
19ad316
Introduce Role -> IIdentity -> IPrincipal
Jan 27, 2019
be0ec5d
feat(aws-iam): grants support non-identity principals
Jan 28, 2019
fcce210
Forgot to add new file
Jan 28, 2019
f3c7827
Merge remote-tracking branch 'origin/master' into huijbers/iam-refactor
rix0rrr Feb 11, 2019
d53d794
Undo move of principals to their own file
rix0rrr Feb 11, 2019
a445780
Change grants API
rix0rrr Feb 12, 2019
a43bf24
Make key interface work with jsii
rix0rrr Feb 13, 2019
2a75620
Splat in the consumer
rix0rrr Feb 13, 2019
dd03daf
Merge remote-tracking branch 'origin/master' into huijbers/iam-refactor
rix0rrr Feb 13, 2019
09cffbd
Can't have the same function in 2 interfaces in C#
rix0rrr Feb 13, 2019
295d698
Merge remote-tracking branch 'origin/master' into huijbers/iam-refactor
rix0rrr Feb 13, 2019
554816d
Respect the refactoring
rix0rrr Feb 13, 2019
cf68f7d
Add awslint rule to force grant() methods to use helpers
rix0rrr Feb 27, 2019
df46221
WIP
rix0rrr Feb 27, 2019
8428937
Update API
rix0rrr Feb 28, 2019
81de979
Update ECS expectations
rix0rrr Feb 28, 2019
0d4ac85
Merge remote-tracking branch 'origin/master' into huijbers/iam-refactor
Feb 28, 2019
28945ee
Make statics also return a Grant
Feb 28, 2019
c7391f4
Review comments
rix0rrr Mar 1, 2019
53f103b
Merge branch 'huijbers/iam-refactor' of github.com:awslabs/aws-cdk in…
rix0rrr Mar 1, 2019
2006891
Review comments
rix0rrr Mar 1, 2019
3323f08
Merge remote-tracking branch 'origin/master' into huijbers/iam-refactor
rix0rrr Mar 1, 2019
b195ac3
IRole implementing both IConstruct and IIdentity leads to a C# build …
rix0rrr Mar 1, 2019
49f996e
Fix unused import
rix0rrr Mar 1, 2019
a65d805
awslint should also find indirect interface extensions
rix0rrr Mar 4, 2019
f2eab4e
Merge remote-tracking branch 'origin/master' into huijbers/iam-refactor
rix0rrr Mar 20, 2019
77865f5
Fixes
rix0rrr Mar 20, 2019
0192f0f
Merge remote-tracking branch 'origin/master' into huijbers/iam-refactor
rix0rrr Apr 1, 2019
c9349f6
Make it build
rix0rrr Apr 1, 2019
929c6fb
Fix failing tests
rix0rrr Apr 2, 2019
753eb84
Make principal in grant methods nonoptional
rix0rrr Apr 2, 2019
c2ca705
Adding IGrantable (WIP)
rix0rrr Apr 2, 2019
e53474b
Adding IGrantable (WIP)
rix0rrr Apr 2, 2019
1b707b1
Merge branch 'huijbers/iam-refactor' of github.com:awslabs/aws-cdk in…
Apr 3, 2019
74bbf35
Finish introduction of IGrantable, rename Grant.withResource -> Grant…
rix0rrr Apr 3, 2019
71964e7
Rename grant methods to be more explicit
rix0rrr Apr 3, 2019
361a92e
Remove dynamodb global
rix0rrr Apr 3, 2019
3772ba4
Update IParameter
rix0rrr Apr 3, 2019
0d24421
Fix stray unrenamed call
rix0rrr Apr 3, 2019
b51c76d
Make sure JSON.stringify(principal) doesn't recurse indefinitely
rix0rrr Apr 3, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions design/aws-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,9 @@ export interface IFoo extends cdk.IConstruct, ISomething {
readonly connections: ec2.Connections;

// permission grants (adds statements to the principal's policy)
grant(principal?: iam.IPrincipal, ...actions: string[]): void;
grantFoo(principal?: iam.IPrincipal): void;
grantBar(principal?: iam.IPrincipal): void;
grant(grantee?: iam.IGrantable, ...actions: string[]): void;
grantFoo(grantee?: iam.IGrantable): void;
grantBar(grantee?: iam.IGrantable): void;

// resource policy (if applicable)
addToResourcePolicy(statement: iam.PolicyStatement): void;
Expand Down Expand Up @@ -364,7 +364,7 @@ export abstract class FooBase extends cdk.Construct implements IFoo {
public abstract export(): FooAttributes;

// grants can usually be shared
public grantYyy(principal?: iam.IPrincipal) {
public grantYyy(grantee?: iam.IGrantable) {
// ...
}

Expand Down
13 changes: 2 additions & 11 deletions packages/@aws-cdk/assets-docker/test/test.image-asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,22 +61,13 @@ export = {
":",
{ "Ref": "AWS::AccountId" },
":repository/",
{
"Fn::GetAtt": [
"ImageAdoptRepositoryE1E84E35",
"RepositoryName"
]
}
{ "Fn::GetAtt": [ "ImageAdoptRepositoryE1E84E35", "RepositoryName" ] }
]
]
}
},
{
"Action": [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense that we lost these permissions?

Copy link
Contributor Author

@rix0rrr rix0rrr Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the user pulling from ECR doesn't need to create logs (they should have never have been part of that grant in the first place).

"ecr:GetAuthorizationToken",
"logs:CreateLogStream",
"logs:PutLogEvents"
],
"Action": "ecr:GetAuthorizationToken",
"Effect": "Allow",
"Resource": "*"
}
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/assets/lib/asset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export interface GenericAssetProps {
* A list of principals that should be able to read this asset from S3.
* You can use `asset.grantRead(principal)` to grant read permissions later.
*/
readonly readers?: iam.IPrincipal[];
readonly readers?: iam.IGrantable[];
}

/**
Expand Down Expand Up @@ -171,12 +171,12 @@ export class Asset extends cdk.Construct {
/**
* Grants read permissions to the principal on the asset's S3 object.
*/
public grantRead(principal?: iam.IPrincipal) {
public grantRead(grantee: iam.IGrantable) {
// We give permissions on all files with the same prefix. Presumably
// different versions of the same file will have the same prefix
// and we don't want to accidentally revoke permission on old versions
// when deploying a new version.
this.bucket.grantRead(principal, `${this.s3Prefix}*`);
this.bucket.grantRead(grantee, `${this.s3Prefix}*`);
}
}

Expand All @@ -190,7 +190,7 @@ export interface FileAssetProps {
* A list of principals that should be able to read this file asset from S3.
* You can use `asset.grantRead(principal)` to grant read permissions later.
*/
readonly readers?: iam.IPrincipal[];
readonly readers?: iam.IGrantable[];
}

/**
Expand All @@ -212,7 +212,7 @@ export interface ZipDirectoryAssetProps {
* A list of principals that should be able to read this ZIP file from S3.
* You can use `asset.grantRead(principal)` to grant read permissions later.
*/
readonly readers?: iam.IPrincipal[];
readonly readers?: iam.IGrantable[];
}

/**
Expand Down
14 changes: 7 additions & 7 deletions packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ export class Metric {
/**
* Grant permissions to the given identity to write metrics.
*
* @param identity The IAM identity to give permissions to.
* @param grantee The IAM identity to give permissions to.
*/
public static grantPutMetricData(identity?: iam.IPrincipal) {
if (!identity) { return; }

identity.addToPolicy(new iam.PolicyStatement()
.addAllResources()
.addAction("cloudwatch:PutMetricData"));
public static grantPutMetricData(grantee: iam.IGrantable): iam.Grant {
return iam.Grant.onPrincipal({
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
grantee,
actions: ['cloudwatch:PutMetricData'],
resourceArns: ['*']
});
}

public readonly dimensions?: DimensionHash;
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface IProject extends cdk.IConstruct, events.IEventRuleTarget {
readonly projectName: string;

/** The IAM service Role of this Project. Undefined for imported Projects. */
readonly role?: iam.IRole;
readonly role: iam.IRole;
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved

/**
* Defines a CloudWatch event rule triggered when the build project state
Expand Down Expand Up @@ -162,8 +162,8 @@ export abstract class ProjectBase extends cdk.Construct implements IProject {
/** The human-visible name of this Project. */
public abstract readonly projectName: string;

/** The IAM service Role of this Project. Undefined for imported Projects. */
public abstract readonly role?: iam.IRole;
/** The IAM service Role of this Project. */
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
public abstract readonly role: iam.IRole;

/** A role used by CloudWatch events to trigger a build */
private eventsRole?: iam.Role;
Expand Down Expand Up @@ -371,7 +371,7 @@ export abstract class ProjectBase extends cdk.Construct implements IProject {
class ImportedProject extends ProjectBase {
public readonly projectArn: string;
public readonly projectName: string;
public readonly role?: iam.Role = undefined;
public readonly role: iam.Role = undefined;
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved

constructor(scope: cdk.Construct, id: string, private readonly props: ProjectImportProps) {
super(scope, id);
Expand Down Expand Up @@ -575,7 +575,7 @@ export class Project extends ProjectBase {
/**
* The IAM role for this project.
*/
public readonly role?: iam.IRole;
public readonly role: iam.IRole;

/**
* The ARN of the project.
Expand Down
16 changes: 8 additions & 8 deletions packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export class LambdaDeploymentGroup extends cdk.Construct implements ILambdaDeplo
throw new Error('A pre-hook function is already defined for this deployment group');
}
this.preHook = preHook;
this.grantPutLifecycleEventHookExecutionStatus(this.preHook.role);
this.grantPutLifecycleEventHookExecutionStatus(this.preHook);
this.preHook.grantInvoke(this.role);
}

Expand All @@ -208,7 +208,7 @@ export class LambdaDeploymentGroup extends cdk.Construct implements ILambdaDeplo
throw new Error('A post-hook function is already defined for this deployment group');
}
this.postHook = postHook;
this.grantPutLifecycleEventHookExecutionStatus(this.postHook.role);
this.grantPutLifecycleEventHookExecutionStatus(this.postHook);
this.postHook.grantInvoke(this.role);
}

Expand All @@ -217,12 +217,12 @@ export class LambdaDeploymentGroup extends cdk.Construct implements ILambdaDeplo
* on this deployment group resource.
* @param principal to grant permission to
*/
public grantPutLifecycleEventHookExecutionStatus(principal?: iam.IPrincipal): void {
if (principal) {
principal.addToPolicy(new iam.PolicyStatement()
.addResource(this.deploymentGroupArn)
.addAction('codedeploy:PutLifecycleEventHookExecutionStatus'));
}
public grantPutLifecycleEventHookExecutionStatus(grantee: iam.IGrantable): iam.Grant {
return iam.Grant.onPrincipal({
grantee,
resourceArns: [this.deploymentGroupArn],
actions: ['codedeploy:PutLifecycleEventHookExecutionStatus'],
});
}

public export(): LambdaDeploymentGroupImportProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,11 @@ class PipelineDouble extends cdk.Construct implements codepipeline.IPipeline {
throw new Error('asEventRuleTarget() is unsupported in PipelineDouble');
}

public grantBucketRead(_identity?: iam.IPrincipal): void {
public grantBucketRead(_identity?: iam.IGrantable): iam.Grant {
throw new Error('grantBucketRead() is unsupported in PipelineDouble');
}

public grantBucketReadWrite(_identity?: iam.IPrincipal): void {
public grantBucketReadWrite(_identity?: iam.IGrantable): iam.Grant {
throw new Error('grantBucketReadWrite() is unsupported in PipelineDouble');
}
}
Expand Down Expand Up @@ -369,9 +369,10 @@ class RoleDouble extends iam.Role {
super(scope, id, props);
}

public addToPolicy(statement: iam.PolicyStatement) {
public addToPolicy(statement: iam.PolicyStatement): boolean {
super.addToPolicy(statement);
this.statements.push(statement);
return true;
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codepipeline/lib/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ export interface IPipeline extends cdk.IConstruct, events.IEventRuleTarget {
*
* @param identity the IAM Identity to grant the permissions to
*/
grantBucketRead(identity?: iam.IPrincipal): void;
grantBucketRead(identity: iam.IGrantable): iam.Grant;

/**
* Grants read & write permissions to the Pipeline's S3 Bucket to the given Identity.
*
* @param identity the IAM Identity to grant the permissions to
*/
grantBucketReadWrite(identity?: iam.IPrincipal): void;
grantBucketReadWrite(identity: iam.IGrantable): iam.Grant;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,12 @@ export class Pipeline extends cdk.Construct implements IPipeline {
return this.stages.length;
}

public grantBucketRead(identity?: iam.IPrincipal): void {
this.artifactBucket.grantRead(identity);
public grantBucketRead(identity: iam.IGrantable): iam.Grant {
return this.artifactBucket.grantRead(identity);
}

public grantBucketReadWrite(identity?: iam.IPrincipal): void {
this.artifactBucket.grantReadWrite(identity);
public grantBucketReadWrite(identity: iam.IGrantable): iam.Grant {
return this.artifactBucket.grantReadWrite(identity);
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-dynamodb-global/.nycrc
98 changes: 98 additions & 0 deletions packages/@aws-cdk/aws-dynamodb-global/lib/aws-dynamodb-global.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import dynamodb = require("@aws-cdk/aws-dynamodb");
import cdk = require("@aws-cdk/cdk");
import { LambdaGlobalDynamoDBMaker } from "./lambda-global-dynamodb";
import { MultiDynamoDBStack } from "./multi-dynamodb-stack";
/**
* NOTE: These props should match dynamodb.TableProps exactly
* EXCEPT for tableName is now required (for global tables to work, the
* table name must match across regions)
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
*/
export interface GlobalDynamoDBProps {
/**
* Partition key attribute definition.
*/
partitionKey: dynamodb.Attribute;
/**
* Table sort key attribute definition.
*
* @default no sort key
*/
sortKey?: dynamodb.Attribute;
/**
* The read capacity for the table. Careful if you add Global Secondary Indexes, as
* those will share the table's provisioned throughput.
*
* Can only be provided if billingMode is Provisioned.
*
* @default 5
*/
readCapacity?: number;
/**
* The write capacity for the table. Careful if you add Global Secondary Indexes, as
* those will share the table's provisioned throughput.
*
* Can only be provided if billingMode is Provisioned.
*
* @default 5
*/
writeCapacity?: number;
/**
* Specify how you are charged for read and write throughput and how you manage capacity.
* @default Provisioned
*/
billingMode?: dynamodb.BillingMode;
/**
* Enforces a particular physical table name.
* @default <generated>
*/
tableName: string;
/**
* Whether point-in-time recovery is enabled.
* @default undefined, point-in-time recovery is disabled
*/
pitrEnabled?: boolean;
/**
* Whether server-side encryption with an AWS managed customer master key is enabled.
* @default undefined, server-side encryption is enabled with an AWS owned customer master key
*/
sseEnabled?: boolean;
/**
* When an item in the table is modified, StreamViewType determines what information
* is written to the stream for this table. Valid values for StreamViewType are:
* @default dynamodb.StreamViewType.NewAndOldImages, streams must be enabled
*/
streamSpecification?: dynamodb.StreamViewType;
/**
* The name of TTL attribute.
* @default undefined, TTL is disabled
*/
ttlAttributeName?: string;
}
/**
* Properties for the mutliple DynamoDB tables to mash together into a
* global table
*/
export interface DynamoDBGlobalStackProps extends cdk.StackProps {
/**
* Properties for DynamoDB Tables
* All the properties must be exactly the same
* to make the tables mesh together as a global table
*/
dynamoProps: GlobalDynamoDBProps;
/**
* Array of environments to create DynamoDB tables in
* Accounts should be omitted, or at least all identical
*/
regions: string[];
}
export declare class GlobalTable extends cdk.Construct {
/**
* Creates dynamoDB tables across regions that will be able to be globbed together into a global table
*/
tables: MultiDynamoDBStack[];
/**
* Creates the cloudformation custom resource that launches a lambda to tie it all together
*/
lambdaGlobalDynamodbMaker: LambdaGlobalDynamoDBMaker;
constructor(scope: cdk.Construct, id: string, props: DynamoDBGlobalStackProps);
}
Loading