Skip to content

Commit

Permalink
Merge branch 'master' into lambda-nodejs-local
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Aug 17, 2020
2 parents 606af99 + e9a4102 commit 32a4e7a
Show file tree
Hide file tree
Showing 17 changed files with 219 additions and 17 deletions.
7 changes: 5 additions & 2 deletions packages/@aws-cdk/aws-apigateway/lib/stage.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, Duration, IResource, Resource, Stack } from '@aws-cdk/core';
import { Construct, Duration, IResource, Resource, Stack, Token } from '@aws-cdk/core';
import { AccessLogFormat, IAccessLogDestination } from './access-log';
import { CfnStage } from './apigateway.generated';
import { Deployment } from './deployment';
Expand Down Expand Up @@ -210,7 +210,10 @@ export class Stage extends Resource implements IStage {
if (!accessLogDestination && !accessLogFormat) {
accessLogSetting = undefined;
} else {
if (accessLogFormat !== undefined && !/.*\$context.requestId.*/.test(accessLogFormat.toString())) {
if (accessLogFormat !== undefined &&
!Token.isUnresolved(accessLogFormat.toString()) &&
!/.*\$context.requestId.*/.test(accessLogFormat.toString())) {

throw new Error('Access log must include at least `AccessLogFormat.contextRequestId()`');
}
if (accessLogFormat !== undefined && accessLogDestination === undefined) {
Expand Down
21 changes: 21 additions & 0 deletions packages/@aws-cdk/aws-apigateway/test/test.stage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,27 @@ export = {
test.done();
},

'does not fail when access log format is a token'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false });
const deployment = new apigateway.Deployment(stack, 'my-deployment', { api });
api.root.addMethod('GET');

// WHEN
const testLogGroup = new logs.LogGroup(stack, 'LogGroup');
const testFormat = apigateway.AccessLogFormat.custom(cdk.Lazy.stringValue({ produce: () => 'test' }));

// THEN
test.doesNotThrow(() => new apigateway.Stage(stack, 'my-stage', {
deployment,
accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup),
accessLogFormat: testFormat,
}));

test.done();
},

'fails when access log destination is empty'(test: Test) {
// GIVEN
const stack = new cdk.Stack();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export enum PredefinedMetric {
*/
ECS_SERVICE_AVERAGE_CPU_UTILIZATION = 'ECSServiceAverageCPUUtilization',
/**
* ECS_SERVICE_AVERAGE_CPU_UTILIZATION
* ECS_SERVICE_AVERAGE_MEMORY_UTILIZATION
* @see https://docs.aws.amazon.com/autoscaling/application/APIReference/API_PredefinedMetricSpecification.html
*/
ECS_SERVICE_AVERAGE_MEMORY_UTILIZATION = 'ECSServiceAverageMemoryUtilization',
Expand Down
3 changes: 0 additions & 3 deletions packages/@aws-cdk/aws-codebuild/lib/source.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,6 @@ export class FilterGroup {
}

private addFilePathFilter(pattern: string, include: boolean): FilterGroup {
if (this.actions.size !== 1 || !this.actions.has(EventAction.PUSH)) {
throw new Error('A file path condition cannot be added if a Group contains any event action other than PUSH');
}
return this.addFilter(FILE_PATH_WEBHOOK_COND, pattern, include);
}

Expand Down
12 changes: 5 additions & 7 deletions packages/@aws-cdk/aws-codebuild/test/test.codebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1631,13 +1631,11 @@ export = {
test.done();
},

'cannot have file path conditions if the Group contains any action other than PUSH'(test: Test) {
const filterGroup = codebuild.FilterGroup.inEventOf(codebuild.EventAction.PULL_REQUEST_CREATED,
codebuild.EventAction.PUSH);

test.throws(() => {
filterGroup.andFilePathIsNot('.*\\.java');
}, /A file path condition cannot be added if a Group contains any event action other than PUSH/);
'can have FILE_PATH filters if the Group contains PUSH and PR_CREATED events'(test: Test) {
codebuild.FilterGroup.inEventOf(
codebuild.EventAction.PULL_REQUEST_CREATED,
codebuild.EventAction.PUSH)
.andFilePathIsNot('.*\\.java');

test.done();
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cognito/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ can either be email and/or SMS. Read more at [Recovering User Accounts](https://
```ts
new UserPool(this, 'UserPool', {
...,
accountRecoverySettings: AccountRecovery.EMAIL_ONLY,
accountRecovery: AccountRecovery.EMAIL_ONLY,
})
```

Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-ecs-patterns/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ You can customize the health check for your target group; otherwise it defaults

Fargate services will use the `LATEST` platform version by default, but you can override by providing a value for the `platformVersion` property in the constructor.

By setting `redirectHTTP` to true, CDK will automatically create a listener on port 80 that redirects HTTP traffic to the HTTPS port.

Additionally, if more than one application target group are needed, instantiate one of the following:

* `ApplicationMultipleTargetGroupsEc2Service`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { IVpc } from '@aws-cdk/aws-ec2';
import { AwsLogDriver, BaseService, CloudMapOptions, Cluster, ContainerImage, ICluster, LogDriver, PropagatedTagSource, Secret } from '@aws-cdk/aws-ecs';
import {
ApplicationListener, ApplicationLoadBalancer, ApplicationProtocol, ApplicationTargetGroup,
IApplicationLoadBalancer, ListenerCertificate,
IApplicationLoadBalancer, ListenerCertificate, ListenerAction,
} from '@aws-cdk/aws-elasticloadbalancingv2';
import { IRole } from '@aws-cdk/aws-iam';
import { ARecord, IHostedZone, RecordTarget } from '@aws-cdk/aws-route53';
Expand Down Expand Up @@ -168,6 +168,14 @@ export interface ApplicationLoadBalancedServiceBaseProps {
* @default - AWS Cloud Map service discovery is not enabled.
*/
readonly cloudMapOptions?: CloudMapOptions;

/**
* Specifies whether the load balancer should redirect traffic on port 80 to port 443 to support HTTP->HTTPS redirects
* This is only valid if the protocol of the ALB is HTTPS
*
* @default false
*/
readonly redirectHTTP?: boolean;
}

export interface ApplicationLoadBalancedTaskImageOptions {
Expand Down Expand Up @@ -276,6 +284,11 @@ export abstract class ApplicationLoadBalancedServiceBase extends cdk.Construct {
*/
public readonly listener: ApplicationListener;

/**
* The redirect listener for the service if redirectHTTP is enabled.
*/
public readonly redirectListener?: ApplicationListener;

/**
* The target group for the service.
*/
Expand Down Expand Up @@ -322,6 +335,9 @@ export abstract class ApplicationLoadBalancedServiceBase extends cdk.Construct {
if (props.certificate !== undefined && props.protocol !== undefined && props.protocol !== ApplicationProtocol.HTTPS) {
throw new Error('The HTTPS protocol must be used when a certificate is given');
}
if (props.protocol !== ApplicationProtocol.HTTPS && props.redirectHTTP === true) {
throw new Error('The HTTPS protocol must be used when redirecting HTTP traffic');
}
const protocol = props.protocol !== undefined ? props.protocol :
(props.certificate ? ApplicationProtocol.HTTPS : ApplicationProtocol.HTTP);

Expand Down Expand Up @@ -353,6 +369,18 @@ export abstract class ApplicationLoadBalancedServiceBase extends cdk.Construct {
if (this.certificate !== undefined) {
this.listener.addCertificates('Arns', [ListenerCertificate.fromCertificateManager(this.certificate)]);
}
if (props.redirectHTTP) {
this.redirectListener = loadBalancer.addListener('PublicRedirectListener', {
protocol: ApplicationProtocol.HTTP,
port: 80,
open: true,
defaultAction: ListenerAction.redirect({
port: props.listenerPort?.toString() || '443',
protocol: ApplicationProtocol.HTTPS,
permanent: true,
}),
});
}

let domainName = loadBalancer.loadBalancerDnsName;
if (typeof props.domainName !== 'undefined') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,13 @@
"FromPort": 443,
"IpProtocol": "tcp",
"ToPort": 443
},
{
"CidrIp": "0.0.0.0/0",
"Description": "Allow from anyone on port 80",
"FromPort": 80,
"IpProtocol": "tcp",
"ToPort": 80
}
],
"VpcId": {
Expand Down Expand Up @@ -460,6 +467,26 @@
}
}
},
"myServiceLBPublicRedirectListener0EEF9DCA": {
"Type": "AWS::ElasticLoadBalancingV2::Listener",
"Properties": {
"DefaultActions": [
{
"RedirectConfig": {
"Port": "443",
"Protocol": "HTTPS",
"StatusCode": "HTTP_301"
},
"Type": "redirect"
}
],
"LoadBalancerArn": {
"Ref": "myServiceLB168895E1"
},
"Port": 80,
"Protocol": "HTTP"
}
},
"myServiceCertificate152F9DDA": {
"Type": "AWS::CertificateManager::Certificate",
"Properties": {
Expand All @@ -480,7 +507,7 @@
"Type": "A",
"AliasTarget": {
"DNSName": {
"Fn::Join":
"Fn::Join":
[
"",
[
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ new ApplicationLoadBalancedFargateService(stack, 'myService', {
stack,
node: stack.node,
},
redirectHTTP: true,
});

app.synth();
7 changes: 7 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/lib/diff/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,20 @@
* object prototype into account for the purpose of the comparison, only the values of
* properties reported by +Object.keys+.
*
* If both operands can be parsed to equivalent numbers, will return true.
* This makes diff consistent with CloudFormation, where a numeric 10 and a literal "10"
* are considered equivalent.
*
* @param lvalue the left operand of the equality comparison.
* @param rvalue the right operand of the equality comparison.
*
* @returns +true+ if both +lvalue+ and +rvalue+ are equivalent to each other.
*/
export function deepEqual(lvalue: any, rvalue: any): boolean {
if (lvalue === rvalue) { return true; }
// allows a numeric 10 and a literal "10" to be equivalent;
// this is consistent with CloudFormation.
if (parseFloat(lvalue) === parseFloat(rvalue)) { return true; }
if (typeof lvalue !== typeof rvalue) { return false; }
if (Array.isArray(lvalue) !== Array.isArray(rvalue)) { return false; }
if (Array.isArray(lvalue) /* && Array.isArray(rvalue) */) {
Expand Down
53 changes: 53 additions & 0 deletions packages/@aws-cdk/cloudformation-diff/test/diff-template.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -321,3 +321,56 @@ test('resource replacement is tracked through references', () => {
// THEN
expect(differences.resources.differenceCount).toBe(3);
});

test('adding and removing quotes from a numeric property causes no changes', () => {
const currentTemplate = {
Resources: {
Bucket: {
Type: 'AWS::S3::Bucket',
Properties: {
CorsConfiguration: {
CorsRules: [
{
AllowedMethods: [
'GET',
],
AllowedOrigins: [
'*',
],
MaxAge: 10,
},
],
},
},
},
},
};

const newTemplate = {
Resources: {
Bucket: {
Type: 'AWS::S3::Bucket',
Properties: {
CorsConfiguration: {
CorsRules: [
{
AllowedMethods: [
'GET',
],
AllowedOrigins: [
'*',
],
MaxAge: '10',
},
],
},
},
},
},
};
let differences = diffTemplate(currentTemplate, newTemplate);
expect(differences.resources.differenceCount).toBe(0);

differences = diffTemplate(newTemplate, currentTemplate);
expect(differences.resources.differenceCount).toBe(0);
});
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,14 @@ describe('CDK Include', () => {
includeTestTemplate(stack, 'fn-sub-${}-only.json');
}).toThrow(/Element referenced in Fn::Sub expression with logical ID: '' was not found in the template/);
});

test('throws an error when a template supplies an invalid string to a number parameter', () => {
includeTestTemplate(stack, 'alphabetical-string-passed-to-number.json');

expect(() => {
SynthUtils.synthesize(stack);
}).toThrow(/"abc" should be a number/);
});
});

function includeTestTemplate(scope: core.Construct, testTemplate: string): inc.CfnInclude {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"Resources": {
"Bucket": {
"Type": "AWS::S3::Bucket",
"Properties": {
"CorsConfiguration": {
"CorsRules": [
{
"AllowedMethods": ["GET"],
"AllowedOrigins": ["*"],
"MaxAge": "abc"
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"Resources": {
"Bucket": {
"Type": "AWS::S3::Bucket",
"Properties": {
"CorsConfiguration": {
"CorsRules": [
{
"AllowedMethods": ["GET"],
"AllowedOrigins": ["*"],
"MaxAge": "10"
}
]
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ describe('CDK Include', () => {
);
});

test('correctly parse strings as integers as needed', () => {
includeTestTemplate(stack, 'parsing-as-numbers.json');

expect(stack).toMatchTemplate(
loadTestFileToJsObject('parsing-as-numbers.json'),
);
});

xtest('correctly changes the logical IDs, including references, if imported with preserveLogicalIds=false', () => {
const cfnTemplate = includeTestTemplate(stack, 'bucket-with-encryption-key.json', {
preserveLogicalIds: false,
Expand Down
Loading

0 comments on commit 32a4e7a

Please sign in to comment.