Skip to content

Commit

Permalink
feat(aws-cloudfront-s3): added loggingBucketProps for cloudfront-s3 (#…
Browse files Browse the repository at this point in the history 
…419)

* added loggingBucketProps for cloudfront-s3

* update design guidelines for s3 logging bucket prop

* updated description in design guideline for S3
  • Loading branch information
@mickychetta
mickychetta authored Oct 5, 2021
1 parent ed7bdfa commit 743c874
Show file tree
Hide file tree
Showing 6 changed files with 590 additions and 88 deletions.
63 changes: 32 additions & 31 deletions DESIGN_GUIDELINES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ To make a Construct as flexible as possible, it should perform a single architec

The construct should not create new classes or interfaces to describe services or resources. Although the new class may seem simpler now, as new capabilities are added to the construct the new class will acquire new properties – the ultimate result would be something equivalent to the CDK definition, but not compatible. The CDK definitions are well thought out and interact predictably with other CDK constructs, use them. If you want a client the ability to specify a few attributes of a ConstructProps without specifying every required value, then make the type of that attribute ConstructProps | any. This pattern exists several places in the Solutions Constructs library.

Another practice that this rule prohibits is putting specific attributes of sub resources in your Solutions Constructs Props object. For instance - if your VPC needs an Internet Gateway, then the client should send VPC Props that create the Internet Gateway, don't create a property at in your Construct Props object of InternetGateway: true.
Another practice that this rule prohibits is putting specific attributes of sub resources in your Solutions Constructs Props object. For instance - if your VPC needs an Internet Gateway, then the client should send VPC Props that create the Internet Gateway, don't create a property at in your Construct Props object of InternetGateway: true.

**The client should have the option (but not requirement) to provide any props used within the construct**

Expand Down Expand Up @@ -81,15 +81,15 @@ Existing Inconsistencies would not be published, that’s for our internal use


## API Gateway
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| apiGatewayProps | api.RestApiProps | aws-cloudfront-apigateway is an exception (covered below) ||
| allow*Name*Operation/*name*OperationTemplate | | Required in pairs for integration with DDB and SQS |
| logGroupProps? | logs.LogGroupProps | |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -99,22 +99,22 @@ Existing Inconsistencies would not be published, that’s for our internal use
| apiGatewayRole | iam.Role ||

## CloudFront
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| cloudFrontDistributionProps? | cloudfront.CloudFront.WebDistributionProps ||
| insertHttpSecurityHeaders? | boolean ||

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| cloudFrontLoggingBucket? s3.Bucket ||
| cloudFrontWebDistribution cloudfront.CloudrontWebDistribution ||

## DynamoDB
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -123,38 +123,38 @@ Existing Inconsistencies would not be published, that’s for our internal use
| tablePermissions? | string | Only where DynamoDB is a data store being accessed by the construct|
| dynamoEventSourceProps? | aws-lambda-event-sources.DynamoEventSourceProps | Only where DynamoDB is invoking other services (dynamodb streams) |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| dynamoTable | dynamodb.Table ||

## ElasticSearch
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| esDomainProps? | elasticsearch.CfnDomainProps ||
| domainName | string ||


**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| elasticsearchDomain | elasticsearch.CfnDomain ||
| elasticsearchDomainRole | iam.Role ||

## Eventbridge
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| eventRuleProps | events.RuleProps ||
| existingEVentBusInterface? | events.IEventBus ||
| eventBusProps? | events.EventBusProps ||

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -163,13 +163,13 @@ Existing Inconsistencies would not be published, that’s for our internal use


## Firehose
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| kinesisFirehoseProps? | aws-kinesisfirehose.CfnDeliveryStreamProps ||

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -178,69 +178,70 @@ Existing Inconsistencies would not be published, that’s for our internal use
| kinesisFirehoseLogGroup | logs.LogGroup ||

## IoT
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| iotEndpoint | string | When IoT is *downstream* (e.g. – aws-apigateway-iot) |
| iotTopicRuleProps | iot.CfnTopicRuleProps | When iot is *upstream* (eg – aws-iot-lambda) |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| iotActionsRole | iam.Role | For upstream IoT|
| iotTopicRule | iot.CfnTopicRule | When iot is upstream |

## Kinesis Streams
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| existingStreamObj? | kinesis.Stream | |
| kinesisStreamProps? | kinesis.StreamProps ||
|createCloudWatchAlarms|`boolean`| |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| kinesisStream | kinesis.Stream ||
| kinesisStreamRole | iam.Role | Only when Kinesis is upstream (because then the role is important to the construct) |

## Lambda
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| existingLambdaObj? | lambda.Function ||
| lambdaFunctionProps? | lambda.FunctionProps ||

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| lambdaFunction | lambda.Function ||

## S3
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| existingBucketObj? | s3.Bucket | Either this or bucketProps must be provided |
| bucketProps? | s3.BucketProps | |
| loggingBucketProps? | s3.BucketProps | Creating an S3 Bucket will generate a Logging Bucket which users can provide props to override the default props |
| s3EventTypes? | s3.EventType | Only required when construct responds to S3 events |
| s3EventFilters? | s3.NotificationKeyFilter |Only required when construct responds to S3 events |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| s3Bucket | s3.Bucket ||
| s3LoggingBucket | s3.Bucket ||

## SNS
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -250,15 +251,15 @@ Existing Inconsistencies would not be published, that’s for our internal use
| encryptionKey? | kms.Key |
| encryptionKeyProps? | kms.KeyProps |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| snsTopic | sns.Topic | |
| encryptionKey | kms.Key | Only required when AWS service is writing to the SNS topic (similar to SQS) |

## SQS
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -270,7 +271,7 @@ Existing Inconsistencies would not be published, that’s for our internal use
| enableQueuePurging | boolean | This is only on 2 constructs, docs talk about a Lambda function role|
| encryptionKey? | kms.Key | Sending messages from an AWS service to an encrypted queue [requires a Customer Master key](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-key-management.html#compatibility-with-aws-services). Those constructs require these properties. |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -279,14 +280,14 @@ Existing Inconsistencies would not be published, that’s for our internal use
| encryptionKey | kms.Key | Only for service to SQS constructs that require a non-default CMK. |

## Step Functions
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| stateMachineProps | sfn.StateMachineProps ||
| createCloudWatchAlarms | boolean | |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
Expand All @@ -295,29 +296,29 @@ Existing Inconsistencies would not be published, that’s for our internal use
| cloudwatchAlarms? | cloudwatch.Alarm[] ||

## VPC
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| existingVpc? | ec2.IVpc | |
| deployVpc? | boolean| |
| vpcProps? | ec2.VpcProps| |

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
| vpc? | ec2.IVpc | |

## WAF WebACL
**Required Attributes on Props**
**Required Attributes on Props**

| Name | Type | Notes |
| --- | --- | --- |
| existingWebaclObj? | wafv2.CfnWebACL ||
| webaclProps? | wafv2.CfnWebACLProps ||

**Required Construct Properties**
**Required Construct Properties**

| Name | Type | Notes |
| --- | --- | --- |
Expand Down
5 changes: 3 additions & 2 deletions source/patterns/@aws-solutions-constructs/aws-cloudfront-s3/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@ _Parameters_
| **Name** | **Type** | **Description** |
|:-------------|:----------------|-----------------|
|existingBucketInterface?|[`s3.IBucket`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.IBucket.html)|Existing instance of S3 Bucket object or interface. If this is provided, then also providing bucketProps will cause an error. |
|bucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.BucketProps.html)|User provided props to override the default props for the S3 Bucket.|
|bucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.BucketProps.html)|Optional user provided props to override the default props for the S3 Bucket.|
|cloudFrontDistributionProps?|[`cloudfront.DistributionProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-cloudfront.DistributionProps.html)|Optional user provided props to override the default props for CloudFront Distribution|
|insertHttpSecurityHeaders?|`boolean`|Optional user provided props to turn on/off the automatic injection of best practice HTTP security headers in all responses from CloudFront|
|loggingBucketProps?|[`s3.BucketProps`](https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_aws-s3.BucketProps.html)|Optional user provided props to override the default props for the S3 Logging Bucket.|

## Pattern Properties

Expand All @@ -76,7 +77,7 @@ Out of the box implementation of the Construct without any override will set the
### Amazon S3 Bucket
* Configure Access logging for S3 Bucket
* Enable server-side encryption for S3 Bucket using AWS managed KMS Key
* Enforce encryption of data in transit
* Enforce encryption of data in transit
* Turn on the versioning for S3 Bucket
* Don't allow public access for S3 Bucket
* Retain the S3 Bucket when deleting the CloudFormation stack
Expand Down
Loading

0 comments on commit 743c874

Please sign in to comment.