Skip to content

Commit

Permalink
chore(s3): use ICfnBucket wherever possible (#28178)
Browse files Browse the repository at this point in the history
This is a best-effort migration of `IBucket` usage in aws-cdk-lib to
`ICfnBucket`. In places where we rely on a property specific to
`IBucket`, we use `Bucket.fromCfnBucket(iCfnBucket)` to generate an
`IBucket` to use.

----

<img width="285" alt="image"
src="https://github.com/aws/aws-cdk/assets/36202692/7a86396b-8193-4df6-a145-c5d5126dc8f1">



----

*By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license*
  • Loading branch information
kaizencc authored Dec 15, 2023
1 parent 18877f3 commit 6f9d970
Show file tree
Hide file tree
Showing 74 changed files with 381 additions and 267 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export interface S3AssertProps {
/**
* The s3 bucket to query.
*/
readonly bucket: s3.IBucket;
readonly bucket: s3.ICfnBucket;

/**
* The object key.
Expand All @@ -31,15 +31,14 @@ export interface S3AssertProps {
* Code is written in Python because why not.
*/
export class S3Assert extends Construct {

constructor(scope: Construct, id: string, props: S3AssertProps) {
super(scope, id);

new CustomResource(this, 'Resource', {
serviceToken: S3AssertProvider.getOrCreate(this),
resourceType: 'Custom::S3Assert',
properties: {
BucketName: props.bucket.bucketName,
BucketName: props.bucket.attrBucketName,
ObjectKey: props.objectKey,
ExpectedContent: props.expectedContent,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ interface S3FileProps {
/**
* The bucket in which the file will be created.
*/
readonly bucket: s3.IBucket;
readonly bucket: s3.ICfnBucket;

/**
* The object key.
Expand Down Expand Up @@ -46,7 +46,7 @@ export class S3File extends Construct {
serviceToken: S3FileProvider.getOrCreate(this),
resourceType: 'Custom::S3File',
properties: {
[api.PROP_BUCKET_NAME]: props.bucket.bucketName,
[api.PROP_BUCKET_NAME]: props.bucket.attrBucketName,
[api.PROP_CONTENTS]: props.contents,
[api.PROP_OBJECT_KEY]: props.objectKey,
[api.PROP_PUBLIC]: props.public,
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-appconfig-alpha/lib/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -916,9 +916,9 @@ export abstract class ConfigurationSource {
* @param objectKey The path to the configuration
* @param key The KMS Key that the bucket is encrypted with
*/
public static fromBucket(bucket: s3.IBucket, objectKey: string, key?: kms.IKey): ConfigurationSource {
public static fromBucket(bucket: s3.ICfnBucket, objectKey: string, key?: kms.IKey): ConfigurationSource {
return {
locationUri: bucket.s3UrlForObject(objectKey),
locationUri: s3.Bucket.fromCfnBucket(bucket).s3UrlForObject(objectKey),
type: ConfigurationSourceType.S3,
key,
};
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-codestar-alpha/lib/github-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export interface GitHubRepositoryProps {
/**
* The name of the Amazon S3 bucket that contains the ZIP file with the content to be committed to the new repository
*/
readonly contentsBucket: s3.IBucket;
readonly contentsBucket: s3.ICfnBucket;

/**
* The S3 object key or file name for the ZIP file
Expand Down Expand Up @@ -97,7 +97,7 @@ export class GitHubRepository extends cdk.Resource implements IGitHubRepository
repositoryAccessToken: props.accessToken.unsafeUnwrap(), // Safe usage
code: {
s3: {
bucket: props.contentsBucket.bucketName,
bucket: props.contentsBucket.attrBucketName,
key: props.contentsKey,
objectVersion: props.contentsS3Version,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-gamelift-alpha/lib/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export class Build extends BuildBase {
/**
* Create a new Build from s3 content
*/
static fromBucket(scope: Construct, id: string, bucket: s3.IBucket, key: string, objectVersion?: string) {
static fromBucket(scope: Construct, id: string, bucket: s3.ICfnBucket, key: string, objectVersion?: string) {
return new Build(scope, id, {
content: Content.fromBucket(bucket, key, objectVersion),
});
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-gamelift-alpha/lib/content.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export abstract class Content {
* @param key The object key
* @param objectVersion Optional S3 ob ject version
*/
public static fromBucket(bucket: s3.IBucket, key: string, objectVersion?: string): S3Content {
public static fromBucket(bucket: s3.ICfnBucket, key: string, objectVersion?: string): S3Content {
return new S3Content(bucket, key, objectVersion);
}

Expand Down Expand Up @@ -48,9 +48,9 @@ export interface ContentConfig {
* Game content from an S3 archive.
*/
export class S3Content extends Content {
constructor(private readonly bucket: s3.IBucket, private key: string, private objectVersion?: string) {
constructor(private readonly bucket: s3.ICfnBucket, private key: string, private objectVersion?: string) {
super();
if (!bucket.bucketName) {
if (!bucket.attrBucketName) {
throw new Error('bucketName is undefined for the provided bucket');
}
}
Expand All @@ -59,13 +59,13 @@ export class S3Content extends Content {
// Adding permission to access specific content
role.addToPrincipalPolicy(new iam.PolicyStatement({
effect: iam.Effect.ALLOW,
resources: [this.bucket.arnForObjects(this.key)],
resources: [s3.Bucket.fromCfnBucket(this.bucket).arnForObjects(this.key)],
actions: ['s3:GetObject', 's3:GetObjectVersion'],
}));

return {
s3Location: {
bucketName: this.bucket.bucketName,
bucketName: this.bucket.attrBucketName,
objectKey: this.key,
objectVersion: this.objectVersion,
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-gamelift-alpha/lib/script.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class Script extends ScriptBase {
/**
* Create a new realtime server script from s3 content
*/
static fromBucket(scope: Construct, id: string, bucket: s3.IBucket, key: string, objectVersion?: string) {
static fromBucket(scope: Construct, id: string, bucket: s3.ICfnBucket, key: string, objectVersion?: string) {
return new Script(scope, id, {
content: Content.fromBucket(bucket, key, objectVersion),
});
Expand Down
10 changes: 6 additions & 4 deletions packages/@aws-cdk/aws-glue-alpha/lib/code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,12 @@ import * as constructs from 'constructs';
* Represents a Glue Job's Code assets (an asset can be a scripts, a jar, a python file or any other file).
*/
export abstract class Code {

/**
* Job code as an S3 object.
* @param bucket The S3 bucket
* @param key The object key
*/
public static fromBucket(bucket: s3.IBucket, key: string): S3Code {
public static fromBucket(bucket: s3.ICfnBucket, key: string): S3Code {
return new S3Code(bucket, key);
}

Expand All @@ -39,15 +38,18 @@ export abstract class Code {
* Glue job Code from an S3 bucket.
*/
export class S3Code extends Code {
constructor(private readonly bucket: s3.IBucket, private readonly key: string) {
private readonly bucket: s3.IBucket;
constructor(bucket: s3.ICfnBucket, private readonly key: string) {
super();

this.bucket = s3.Bucket.fromCfnBucket(bucket);
}

public bind(_scope: constructs.Construct, grantable: iam.IGrantable): CodeConfig {
this.bucket.grantRead(grantable, this.key);
return {
s3Location: {
bucketName: this.bucket.bucketName,
bucketName: this.bucket.attrBucketName,
objectKey: this.key,
},
};
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-glue-alpha/lib/job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -379,14 +379,14 @@ export interface SparkUIProps {
/**
* Enable Spark UI.
*/
readonly enabled: boolean
readonly enabled: boolean;

/**
* The bucket where the Glue job stores the logs.
*
* @default - a new bucket will be created.
*/
readonly bucket?: s3.IBucket;
readonly bucket?: s3.ICfnBucket;

/**
* The path inside the bucket (objects prefix) where the Glue job stores the logs.
Expand Down Expand Up @@ -818,7 +818,7 @@ export class Job extends JobBase {
}

this.validatePrefix(props.prefix);
const bucket = props.bucket ?? new s3.Bucket(this, 'SparkUIBucket');
const bucket = props.bucket ? s3.Bucket.fromCfnBucket(props.bucket) : new s3.Bucket(this, 'SparkUIBucket');
bucket.grantReadWrite(role, this.cleanPrefixForGrant(props.prefix));
const args = {
'--enable-spark-ui': 'true',
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-glue-alpha/lib/s3-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,16 @@ export enum TableEncryption {
*
* @see https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingClientSideEncryption.html
*/
CLIENT_SIDE_KMS = 'CSE-KMS'
CLIENT_SIDE_KMS = 'CSE-KMS',
}

export interface S3TableProps extends TableBaseProps {
/**
* S3 bucket in which to store data.
*
* @default one is created for you
* @default - one is created for you
*/
readonly bucket?: s3.IBucket;
readonly bucket?: s3.ICfnBucket;

/**
* S3 prefix under which table objects are stored.
Expand All @@ -71,7 +71,7 @@ export interface S3TableProps extends TableBaseProps {
*
* The `encryption` property must be `SSE-KMS` or `CSE-KMS`.
*
* @default key is managed by KMS.
* @default - key is managed by KMS.
*/
readonly encryptionKey?: kms.IKey;
}
Expand Down Expand Up @@ -250,7 +250,7 @@ const encryptionMappings = {

// create the bucket to store a table's data depending on the `encryption` and `encryptionKey` properties.
function createBucket(table: S3Table, props: S3TableProps) {
let bucket = props.bucket;
let bucket = props.bucket ? s3.Bucket.fromCfnBucket(props.bucket) : undefined;

if (bucket && (props.encryption !== undefined && props.encryption !== TableEncryption.CLIENT_SIDE_KMS)) {
throw new Error('you can not specify encryption settings if you also provide a bucket');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class S3PutObjectAction implements iot.IAction {
* @param bucket The Amazon S3 bucket to which to write data.
* @param props Optional properties to not use default
*/
constructor(private readonly bucket: s3.IBucket, props: S3PutObjectActionProps = {}) {
constructor(private readonly bucket: s3.ICfnBucket, props: S3PutObjectActionProps = {}) {
this.accessControl = props.accessControl;
this.key = props.key;
this.role = props.role;
Expand All @@ -53,13 +53,13 @@ export class S3PutObjectAction implements iot.IAction {
const role = this.role ?? singletonActionRole(rule);
role.addToPrincipalPolicy(new iam.PolicyStatement({
actions: ['s3:PutObject'],
resources: [this.bucket.arnForObjects('*')],
resources: [s3.Bucket.fromCfnBucket(this.bucket).arnForObjects('*')],
}));

return {
configuration: {
s3: {
bucketName: this.bucket.bucketName,
bucketName: this.bucket.attrBucketName,
cannedAcl: this.accessControl && toKebabCase(this.accessControl.toString()),
key: this.key ?? '${topic()}/${timestamp()}',
roleArn: role.roleArn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export abstract class ApplicationCode {
* @param fileKey - a key pointing to a Flink JAR file
* @param objectVersion - an optional version string for the provided fileKey
*/
public static fromBucket(bucket: s3.IBucket, fileKey: string, objectVersion?: string): ApplicationCode {
public static fromBucket(bucket: s3.ICfnBucket, fileKey: string, objectVersion?: string): ApplicationCode {
return new BucketApplicationCode({
bucket,
fileKey,
Expand All @@ -56,13 +56,13 @@ export abstract class ApplicationCode {
}

interface BucketApplicationCodeProps {
readonly bucket: s3.IBucket;
readonly bucket: s3.ICfnBucket;
readonly fileKey: string;
readonly objectVersion?: string;
}

class BucketApplicationCode extends ApplicationCode {
public readonly bucket?: s3.IBucket;
public readonly bucket?: s3.ICfnBucket;
public readonly fileKey: string;
public readonly objectVersion?: string;

Expand All @@ -79,15 +79,15 @@ class BucketApplicationCode extends ApplicationCode {
applicationCodeConfiguration: {
codeContent: {
s3ContentLocation: {
bucketArn: this.bucket!.bucketArn,
bucketArn: this.bucket!.attrArn,
fileKey: this.fileKey,
objectVersion: this.objectVersion,
},
},
codeContentType: 'ZIPFILE',
},
},
bucket: this.bucket!,
bucket: s3.Bucket.fromCfnBucket(this.bucket!),
};
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export interface DestinationS3BackupProps extends DestinationLoggingProps, Commo
*
* @default - If `mode` is set to `BackupMode.ALL` or `BackupMode.FAILED`, a bucket will be created for you.
*/
readonly bucket?: s3.IBucket;
readonly bucket?: s3.ICfnBucket;

/**
* Indicates the mode by which incoming records should be backed up to S3, if any.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export function createBackupConfig(scope: Construct, role: iam.IRole, props?: De
return undefined;
}

const bucket = props.bucket ?? new s3.Bucket(scope, 'BackupBucket');
const bucket = props.bucket ? s3.Bucket.fromCfnBucket(props.bucket) : new s3.Bucket(scope, 'BackupBucket');
const bucketGrant = bucket.grantReadWrite(role);

const { loggingOptions, dependables: loggingDependables } = createLoggingOptions(scope, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface S3BucketProps extends CommonDestinationS3Props, CommonDestinati
* An S3 bucket destination for data from a Kinesis Data Firehose delivery stream.
*/
export class S3Bucket implements firehose.IDestination {
constructor(private readonly bucket: s3.IBucket, private readonly props: S3BucketProps = {}) {
constructor(private readonly bucket: s3.ICfnBucket, private readonly props: S3BucketProps = {}) {
if (this.props.s3Backup?.mode === BackupMode.FAILED) {
throw new Error('S3 destinations do not support BackupMode.FAILED');
}
Expand All @@ -26,7 +26,8 @@ export class S3Bucket implements firehose.IDestination {
assumedBy: new iam.ServicePrincipal('firehose.amazonaws.com'),
});

const bucketGrant = this.bucket.grantReadWrite(role);
const bucket = s3.Bucket.fromCfnBucket(this.bucket);
const bucketGrant = bucket.grantReadWrite(role);

const { loggingOptions, dependables: loggingDependables } = createLoggingOptions(scope, {
logging: this.props.logging,
Expand All @@ -44,7 +45,7 @@ export class S3Bucket implements firehose.IDestination {
s3BackupConfiguration: backupConfig,
s3BackupMode: this.getS3BackupMode(),
bufferingHints: createBufferingHints(this.props.bufferingInterval, this.props.bufferingSize),
bucketArn: this.bucket.bucketArn,
bucketArn: bucket.attrArn,
compressionFormat: this.props.compression?.value,
encryptionConfiguration: createEncryptionConfig(role, this.props.encryptionKey),
errorOutputPrefix: this.props.errorOutputPrefix,
Expand Down
5 changes: 2 additions & 3 deletions packages/@aws-cdk/aws-msk-alpha/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,7 @@ export interface S3LoggingConfiguration {
/**
* The S3 bucket that is the destination for broker logs.
*/
readonly bucket: s3.IBucket;

readonly bucket: s3.ICfnBucket;
/**
* The S3 prefix that is the destination for broker logs.
*
Expand Down Expand Up @@ -578,7 +577,7 @@ export class Cluster extends ClusterBase {
}
: undefined;

const loggingBucket = props.logging?.s3?.bucket;
const loggingBucket = props.logging?.s3?.bucket ? s3.Bucket.fromCfnBucket(props.logging.s3.bucket) : undefined;
if (loggingBucket && FeatureFlags.of(this).isEnabled(S3_CREATE_DEFAULT_LOGGING_POLICY)) {
const stack = core.Stack.of(this);
loggingBucket.addToResourcePolicy(new iam.PolicyStatement({
Expand Down
11 changes: 6 additions & 5 deletions packages/@aws-cdk/aws-redshift-alpha/lib/cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export interface LoggingProperties {
* Bucket to send logs to.
* Logging information includes queries and connection attempts, for the specified Amazon Redshift cluster.
*/
readonly loggingBucket: s3.IBucket;
readonly loggingBucket: s3.ICfnBucket;

/**
* Prefix used for logging.
Expand Down Expand Up @@ -535,20 +535,21 @@ export class Cluster extends ClusterBase {

let loggingProperties;
if (props.loggingProperties) {
const loggingBucket = s3.Bucket.fromCfnBucket(props.loggingProperties.loggingBucket);
loggingProperties = {
bucketName: props.loggingProperties.loggingBucket.bucketName,
bucketName: loggingBucket.attrBucketName,
s3KeyPrefix: props.loggingProperties.loggingKeyPrefix,
};
props.loggingProperties.loggingBucket.addToResourcePolicy(
loggingBucket.addToResourcePolicy(
new iam.PolicyStatement(
{
actions: [
's3:GetBucketAcl',
's3:PutObject',
],
resources: [
props.loggingProperties.loggingBucket.arnForObjects('*'),
props.loggingProperties.loggingBucket.bucketArn,
loggingBucket.arnForObjects('*'),
loggingBucket.attrArn,
],
principals: [
new iam.ServicePrincipal('redshift.amazonaws.com'),
Expand Down
Loading

0 comments on commit 6f9d970

Please sign in to comment.