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

s3: bucketUrl and urlForObject(key) #370

Merged
merged 3 commits into from
Jul 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions packages/@aws-cdk/core/lib/cloudformation/pseudo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ export class AwsDomainSuffix extends PseudoParameter {
}
}

export class AwsURLSuffix extends PseudoParameter {
constructor() {
super('AWS::URLSuffix');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

}
}

export class AwsNotificationARNs extends PseudoParameter {
constructor() {
super('AWS::NotificationARNs');
Expand Down
11 changes: 10 additions & 1 deletion packages/@aws-cdk/iam/lib/policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import { Role } from './role';
import { User } from './user';
import { generatePolicyName, undefinedIfEmpty } from './util';

export interface IIdentityResource {
/**
* A construct that represents an IAM principal, such as a user, group or role.
*/
export interface IPrincipal {
/**
* The IAM principal of this identity (i.e. AWS principal, service principal, etc).
*/
Expand All @@ -31,6 +34,12 @@ export interface IIdentityResource {
attachManagedPolicy(arn: any): void;
}

/**
* @deprecated Use IPrincipal
*/
// tslint:disable-next-line:no-empty-interface
export interface IIdentityResource extends IPrincipal { }

export interface PolicyProps {
/**
* The name of the policy. If you specify multiple policies for an entity,
Expand Down
13 changes: 13 additions & 0 deletions packages/@aws-cdk/s3/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,19 @@ Define an unencrypted S3 bucket.
new Bucket(this, 'MyFirstBucket');
```

`Bucket` constructs expose the following deploy-time attributes:

* `bucketArn` - the ARN of the bucket (i.e. `arn:aws:s3:::bucket_name`)
* `bucketName` - the name of the bucket (i.e. `bucket_name`)
* `bucketUrl` - the URL of the bucket (i.e.
`https://s3.us-west-1.amazonaws.com/onlybucket`)
* `arnForObjects(...pattern)` - the ARN of an object or objects within the
bucket (i.e.
`arn:aws:s3:::my_corporate_bucket/exampleobject.png` or
`arn:aws:s3:::my_corporate_bucket/Development/*`)
* `urlForObject(key)` - the URL of an object within the bucket (i.e.
`https://s3.cn-north-1.amazonaws.com.cn/china-bucket/mykey`)

### Encryption

Define a KMS-encrypted bucket:
Expand Down
90 changes: 68 additions & 22 deletions packages/@aws-cdk/s3/lib/bucket.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { applyRemovalPolicy, Arn, Construct, FnConcat, Output, PolicyStatement, RemovalPolicy, Token } from '@aws-cdk/core';
import { IIdentityResource } from '@aws-cdk/iam';
import * as kms from '@aws-cdk/kms';
import cdk = require('@aws-cdk/core');
import iam = require('@aws-cdk/iam');
import kms = require('@aws-cdk/kms');
import { BucketPolicy } from './bucket-policy';
import * as perms from './perms';
import { LifecycleRule } from './rule';
Expand Down Expand Up @@ -45,7 +45,7 @@ export interface BucketRefProps {
* BucketRef.import(this, 'MyImportedBucket', ref);
*
*/
export abstract class BucketRef extends Construct {
export abstract class BucketRef extends cdk.Construct {
/**
* Creates a Bucket construct that represents an external bucket.
*
Expand All @@ -54,7 +54,7 @@ export abstract class BucketRef extends Construct {
* @param ref A BucketRefProps object. Can be obtained from a call to
* `bucket.export()`.
*/
public static import(parent: Construct, name: string, props: BucketRefProps): BucketRef {
public static import(parent: cdk.Construct, name: string, props: BucketRefProps): BucketRef {
return new ImportedBucketRef(parent, name, props);
}

Expand Down Expand Up @@ -92,8 +92,8 @@ export abstract class BucketRef extends Construct {
*/
public export(): BucketRefProps {
return {
bucketArn: new Output(this, 'BucketArn', { value: this.bucketArn }).makeImportValue(),
bucketName: new Output(this, 'BucketName', { value: this.bucketName }).makeImportValue(),
bucketArn: new cdk.Output(this, 'BucketArn', { value: this.bucketArn }).makeImportValue(),
bucketName: new cdk.Output(this, 'BucketName', { value: this.bucketName }).makeImportValue(),
};
}

Expand All @@ -103,7 +103,7 @@ export abstract class BucketRef extends Construct {
* contents. Use `bucketArn` and `arnForObjects(keys)` to obtain ARNs for
* this bucket or objects.
*/
public addToResourcePolicy(permission: PolicyStatement) {
public addToResourcePolicy(permission: cdk.PolicyStatement) {
if (!this.policy && this.autoCreatePolicy) {
this.policy = new BucketPolicy(this, 'Policy', { bucket: this });
}
Expand All @@ -113,6 +113,38 @@ export abstract class BucketRef extends Construct {
}
}

/**
* The https:// URL of this bucket.
* @example https://s3.us-west-1.amazonaws.com/onlybucket
* Similar to calling `urlForObject` with no object key.
*/
public get bucketUrl() {
return this.urlForObject();
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have expected the dependency to be in the reverse order of this...

Also, any reason to not use the DomainName attribute of AWS::S3::Bucket? Do we actually need to have a top-level S3 namespace rooted URL in some cases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, in order for this to be available for BucketRefs, which don't have the DomainName information on them.

Copy link
Contributor

Choose a reason for hiding this comment

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

The BucketRef could return the Fn::Concat stuff, and the Bucket could use the attribute directly. Isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would have expected the dependency to be in the reverse order of this...

This will result in FnConcat(FnConcat(...), key)) which is kind of ugly

Copy link
Contributor

Choose a reason for hiding this comment

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

Only when using a BucketRef. But whatever, that is not crazy important.

}

/**
* The https URL of an S3 object. For example:
* @example https://s3.us-west-1.amazonaws.com/onlybucket
* @example https://s3.us-west-1.amazonaws.com/bucket/key
* @example https://s3.cn-north-1.amazonaws.com.cn/china-bucket/mykey
* @param key The S3 key of the object. If not specified, the URL of the
* bucket is returned.
* @returns an ObjectS3Url token
*/
public urlForObject(key?: any): S3Url {
const components = [ 'https://', 's3.', new cdk.AwsRegion(), '.', new cdk.AwsURLSuffix(), '/', this.bucketName ];
if (key) {
// trim prepending '/'
if (typeof key === 'string' && key.startsWith('/')) {
key = key.substr(1);
}
components.push('/');
components.push(key);
}

return new cdk.FnConcat(...components);
}

/**
* Returns an ARN that represents all objects within the bucket that match
* the key pattern specified. To represent all keys, specify ``"*"``.
Expand All @@ -122,8 +154,8 @@ export abstract class BucketRef extends Construct {
* arnForObjects('home/', team, '/', user, '/*')
*
*/
public arnForObjects(...keyPattern: any[]): Arn {
return new FnConcat(this.bucketArn, '/', ...keyPattern);
public arnForObjects(...keyPattern: any[]): cdk.Arn {
return new cdk.FnConcat(this.bucketArn, '/', ...keyPattern);
}

/**
Expand All @@ -133,7 +165,7 @@ export abstract class BucketRef extends Construct {
* If an encryption key is used, permission to ues the key to decrypt the
* contents of the bucket will also be granted.
*/
public grantRead(identity?: IIdentityResource, objectsKeyPattern = '*') {
public grantRead(identity?: iam.IPrincipal, objectsKeyPattern: any = '*') {
if (!identity) {
return;
}
Expand All @@ -147,7 +179,7 @@ export abstract class BucketRef extends Construct {
* If an encryption key is used, permission to use the key for
* encrypt/decrypt will also be granted.
*/
public grantReadWrite(identity?: IIdentityResource, objectsKeyPattern = '*') {
public grantReadWrite(identity?: iam.IPrincipal, objectsKeyPattern: any = '*') {
if (!identity) {
return;
}
Expand All @@ -156,24 +188,24 @@ export abstract class BucketRef extends Construct {
this.grant(identity, objectsKeyPattern, bucketActions, keyActions);
}

private grant(identity: IIdentityResource, objectsKeyPattern: string, bucketActions: string[], keyActions: string[]) {
private grant(identity: iam.IPrincipal, objectsKeyPattern: any, bucketActions: string[], keyActions: string[]) {
const resources = [
this.bucketArn,
this.arnForObjects(objectsKeyPattern)
];

identity.addToPolicy(new PolicyStatement()
identity.addToPolicy(new cdk.PolicyStatement()
.addResources(...resources)
.addActions(...bucketActions));

// grant key permissions if there's an associated key.
if (this.encryptionKey) {
// KMS permissions need to be granted both directions
identity.addToPolicy(new PolicyStatement()
identity.addToPolicy(new cdk.PolicyStatement()
.addResource(this.encryptionKey.keyArn)
.addActions(...keyActions));

this.encryptionKey.addToResourcePolicy(new PolicyStatement()
this.encryptionKey.addToResourcePolicy(new cdk.PolicyStatement()
.addResource('*')
.addPrincipal(identity.principal)
.addActions(...keyActions));
Expand Down Expand Up @@ -216,7 +248,7 @@ export interface BucketProps {
*
* @default By default, the bucket will be destroyed if it is removed from the stack.
*/
removalPolicy?: RemovalPolicy;
removalPolicy?: cdk.RemovalPolicy;

/**
* The bucket policy associated with this bucket.
Expand Down Expand Up @@ -258,7 +290,7 @@ export class Bucket extends BucketRef {
private readonly lifecycleRules: LifecycleRule[] = [];
private readonly versioned?: boolean;

constructor(parent: Construct, name: string, props: BucketProps = {}) {
constructor(parent: cdk.Construct, name: string, props: BucketProps = {}) {
super(parent, name);

validateBucketName(props && props.bucketName);
Expand All @@ -269,10 +301,10 @@ export class Bucket extends BucketRef {
bucketName: props && props.bucketName,
bucketEncryption,
versioningConfiguration: props.versioned ? { status: 'Enabled' } : undefined,
lifecycleConfiguration: new Token(() => this.parseLifecycleConfiguration()),
lifecycleConfiguration: new cdk.Token(() => this.parseLifecycleConfiguration()),
});

applyRemovalPolicy(resource, props.removalPolicy);
cdk.applyRemovalPolicy(resource, props.removalPolicy);

this.versioned = props.versioned;
this.policy = props.policy;
Expand Down Expand Up @@ -435,7 +467,21 @@ export enum BucketEncryption {
/**
* The name of the bucket.
*/
export class BucketName extends Token {
export class BucketName extends cdk.Token {

}

/**
* A key to an S3 object.
*/
export class ObjectKey extends cdk.Token {

}

/**
* The web URL (https://s3.us-west-1.amazonaws.com/bucket/key) of an S3 object.
*/
export class S3Url extends cdk.Token {

}

Expand All @@ -447,7 +493,7 @@ class ImportedBucketRef extends BucketRef {
protected policy?: BucketPolicy;
protected autoCreatePolicy: boolean;

constructor(parent: Construct, name: string, props: BucketRefProps) {
constructor(parent: cdk.Construct, name: string, props: BucketRefProps) {
super(parent, name);

this.bucketArn = parseBucketArn(props);
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/s3/test/integ.bucket.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"MyBucketKeyC17130CF": {
"Type": "AWS::KMS::Key",
"Properties": {
"Description": "Created by aws-cdk-s3/MyBucket",
"KeyPolicy": {
"Statement": [
{
Expand Down Expand Up @@ -63,7 +62,8 @@
}
],
"Version": "2012-10-17"
}
},
"Description": "Created by aws-cdk-s3/MyBucket"
},
"DeletionPolicy": "Retain"
},
Expand Down
61 changes: 61 additions & 0 deletions packages/@aws-cdk/s3/test/integ.bucket.url.lit.expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
{
"Resources": {
"MyBucketF68F3FF0": {
"Type": "AWS::S3::Bucket"
}
},
"Outputs": {
"BucketURL": {
"Value": {
"Fn::Join": [
"",
[
"https://",
"s3.",
{
"Ref": "AWS::Region"
},
".",
{
"Ref": "AWS::URLSuffix"
},
"/",
{
"Ref": "MyBucketF68F3FF0"
}
]
]
},
"Export": {
"Name": "aws-cdk-s3-urls:BucketURL"
}
},
"ObjectURL": {
"Value": {
"Fn::Join": [
"",
[
"https://",
"s3.",
{
"Ref": "AWS::Region"
},
".",
{
"Ref": "AWS::URLSuffix"
},
"/",
{
"Ref": "MyBucketF68F3FF0"
},
"/",
"myfolder/myfile.txt"
]
]
},
"Export": {
"Name": "aws-cdk-s3-urls:ObjectURL"
}
}
}
}
19 changes: 19 additions & 0 deletions packages/@aws-cdk/s3/test/integ.bucket.url.lit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import cdk = require('@aws-cdk/core');
import s3 = require('../lib');

class TestStack extends cdk.Stack {
constructor(parent: cdk.App, id: string) {
super(parent, id);

/// !show
const bucket = new s3.Bucket(this, 'MyBucket');

new cdk.Output(this, 'BucketURL', { value: bucket.bucketUrl });
new cdk.Output(this, 'ObjectURL', { value: bucket.urlForObject('myfolder/myfile.txt') });
/// !hide
}
}

const app = new cdk.App(process.argv);
new TestStack(app, 'aws-cdk-s3-urls');
process.stdout.write(app.run());
Loading