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

chore(core): change helper functions to return response objects #904

Merged
merged 28 commits into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b5b8398
Changed API functions to return response objects
biffgaut Feb 14, 2023
3fb1fb8
Fix remaining references to API helper functions
biffgaut Feb 14, 2023
c2f9d84
One more reference to API Helper function
biffgaut Feb 14, 2023
b8516cb
Merge branch 'main' into Issue853
biffgaut Feb 14, 2023
e667e89
CloudFront and lambda-sagemakerendpoint
biffgaut Feb 15, 2023
7a8f9d8
updated dynamodb
biffgaut Feb 15, 2023
3a88f07
Merge branch 'Issue853' of https://github.com/awslabs/aws-solutions-c…
biffgaut Feb 15, 2023
e74093d
Format errors
biffgaut Feb 15, 2023
20ca283
Adjust Elasticsearch
biffgaut Feb 15, 2023
e656aaa
Includes S3, apigateway
biffgaut Feb 16, 2023
e8937ad
Missed Apigateway reference
biffgaut Feb 17, 2023
ba253ec
Upated fargate-helper
biffgaut Feb 17, 2023
527eb04
One missed fargate reference
biffgaut Feb 17, 2023
3b03dbe
One more fargate
biffgaut Feb 17, 2023
b8b031d
Fargate existing resource misses
biffgaut Feb 17, 2023
7e9d682
More fargate references
biffgaut Feb 17, 2023
085e098
missing whitespace
biffgaut Feb 17, 2023
ea61071
Merge branch 'main' into Issue853
biffgaut Feb 17, 2023
0851826
all sns changes
biffgaut Feb 17, 2023
17b3476
step functions
biffgaut Feb 17, 2023
1826fe3
sqs referencex
biffgaut Feb 17, 2023
c9b1e7e
Another sqs reference
biffgaut Feb 17, 2023
d35a88b
lambda-sqs-lambda
biffgaut Feb 17, 2023
ad4e3c9
one more
biffgaut Feb 17, 2023
dbaa181
nettlesome flummery
biffgaut Feb 18, 2023
c5c048b
Sagemaker Return values
biffgaut Feb 18, 2023
a39986f
Clean up, ensure consistency
biffgaut Feb 19, 2023
a85f6ae
Address Review Issues
biffgaut Feb 20, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ export class AlbToFargate extends Construct {
// CheckFargateProps confirms that the container is provided
this.container = props.existingContainerDefinitionObject!;
} else {
[this.service, this.container] = defaults.CreateFargateService(
const createFargateServiceResponse = defaults.CreateFargateService(
scope,
id,
this.vpc,
Expand All @@ -218,6 +218,8 @@ export class AlbToFargate extends Construct {
props.containerDefinitionProps,
props.fargateServiceProps
);
this.service = createFargateServiceResponse.service;
this.container = createFargateServiceResponse.containerDefinition;
}
// Add the Fargate Service to the
// to the ALB Listener we set up earlier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ test('Test existing load balancer, vpc, service', () => {

const existingVpc = defaults.getTestVpc(stack);

const [testService, testContainer] = defaults.CreateFargateService(stack,
const createFargateServiceResponse = defaults.CreateFargateService(stack,
'test',
existingVpc,
undefined,
Expand All @@ -145,8 +145,8 @@ test('Test existing load balancer, vpc, service', () => {
const testProps: AlbToFargateProps = {
existingVpc,
publicApi: true,
existingFargateServiceObject: testService,
existingContainerDefinitionObject: testContainer,
existingFargateServiceObject: createFargateServiceResponse.service,
existingContainerDefinitionObject: createFargateServiceResponse.containerDefinition,
existingLoadBalancerObj: existingAlb,
listenerProps: {
protocol: 'HTTP'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const image = ecs.ContainerImage.fromRegistry('nginx');

const testExistingVpc = defaults.getTestVpc(stack);

const [testService, testContainer] = defaults.CreateFargateService(stack,
const createFargateServiceResponse = defaults.CreateFargateService(stack,
'test',
testExistingVpc,
undefined,
Expand All @@ -48,8 +48,8 @@ const existingAlb = new elb.ApplicationLoadBalancer(stack, 'test-alb', {
const testProps: AlbToFargateProps = {
existingVpc: testExistingVpc,
publicApi: true,
existingFargateServiceObject: testService,
existingContainerDefinitionObject: testContainer,
existingFargateServiceObject: createFargateServiceResponse.service,
existingContainerDefinitionObject: createFargateServiceResponse.containerDefinition,
existingLoadBalancerObj: existingAlb,
listenerProps: {
protocol: 'HTTP'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,15 +239,18 @@ export class ApiGatewayToDynamoDB extends Construct {

// Since we are only invoking this function with an existing Table or tableProps,
// (not a table interface), we know that the implementation will always return
// a Table object and we can safely cast away the optional aspect of the type.
this.dynamoTable = defaults.buildDynamoDBTable(this, {
// a Table object and we can force assignment to the dynamoTable property.
const buildDynamoDBTableResponse = defaults.buildDynamoDBTable(this, {
existingTableObj: props.existingTableObj,
dynamoTableProps: props.dynamoTableProps
})[1] as dynamodb.Table;
});
this.dynamoTable = buildDynamoDBTableResponse.tableObject!;
Copy link
Contributor

Choose a reason for hiding this comment

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

When I first read this, I asked myself "If we know the tableObject is never null, why not change the helper to return that". After going through the whole review it makes sense why this was done with the ! operator where possible. Nice check!


// Setup the API Gateway
[this.apiGateway, this.apiGatewayCloudWatchRole, this.apiGatewayLogGroup] = defaults.GlobalRestApi(this,
props.apiGatewayProps, props.logGroupProps);
const globalRestApiResponse = defaults.GlobalRestApi(this, props.apiGatewayProps, props.logGroupProps);
this.apiGateway = globalRestApiResponse.api;
this.apiGatewayCloudWatchRole = globalRestApiResponse.role;
this.apiGatewayLogGroup = globalRestApiResponse.logGroup;

// Setup the API Gateway role
this.apiGatewayRole = new iam.Role(this, 'api-gateway-role', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,10 @@ export class ApiGatewayToIot extends Construct {
}

// Setup the API Gateway
[this.apiGateway, this.apiGatewayCloudWatchRole,
this.apiGatewayLogGroup] = defaults.GlobalRestApi(this, extraApiGwProps, props.logGroupProps);
const globalRestApiResponse = defaults.GlobalRestApi(this, extraApiGwProps, props.logGroupProps);
this.apiGateway = globalRestApiResponse.api;
this.apiGatewayCloudWatchRole = globalRestApiResponse.role;
this.apiGatewayLogGroup = globalRestApiResponse.logGroup;

// Validate the Query Params
const requestValidatorProps: api.RequestValidatorProps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,10 @@ export class ApiGatewayToKinesisStreams extends Construct {
});

// Setup the API Gateway
[this.apiGateway, this.apiGatewayCloudWatchRole, this.apiGatewayLogGroup] = defaults.GlobalRestApi(this,
props.apiGatewayProps, props.logGroupProps);
const globalRestApiResponse = defaults.GlobalRestApi(this, props.apiGatewayProps, props.logGroupProps);
this.apiGateway = globalRestApiResponse.api;
this.apiGatewayCloudWatchRole = globalRestApiResponse.role;
this.apiGatewayLogGroup = globalRestApiResponse.logGroup;

// Setup the API Gateway role
this.apiGatewayRole = new iam.Role(this, 'api-gateway-role', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ export class ApiGatewayToLambda extends Construct {
});

// Setup the API Gateway
[this.apiGateway, this.apiGatewayCloudWatchRole,
this.apiGatewayLogGroup] = defaults.GlobalLambdaRestApi(this, this.lambdaFunction, props.apiGatewayProps, props.logGroupProps);
const globalRestApiResponse = defaults.GlobalLambdaRestApi(this, this.lambdaFunction, props.apiGatewayProps, props.logGroupProps);
this.apiGateway = globalRestApiResponse.api;
this.apiGatewayCloudWatchRole = globalRestApiResponse.role;
this.apiGatewayLogGroup = globalRestApiResponse.group;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,10 @@ export class ApiGatewayToSageMakerEndpoint extends Construct {
defaults.CheckProps(props);

// Setup the API Gateway
[this.apiGateway, this.apiGatewayCloudWatchRole, this.apiGatewayLogGroup] = defaults.GlobalRestApi(this,
props.apiGatewayProps, props.logGroupProps);
const globalRestApiResponse = defaults.GlobalRestApi(this, props.apiGatewayProps, props.logGroupProps);
this.apiGateway = globalRestApiResponse.api;
this.apiGatewayCloudWatchRole = globalRestApiResponse.role;
this.apiGatewayLogGroup = globalRestApiResponse.logGroup;

// Setup the API Gateway role
if (props.apiGatewayExecutionRole !== undefined) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,18 +223,21 @@ export class ApiGatewayToSqs extends Construct {
});

// Setup the queue
[this.sqsQueue] = defaults.buildQueue(this, 'queue', {
const buildQueueResponse = defaults.buildQueue(this, 'queue', {
existingQueueObj: props.existingQueueObj,
queueProps: props.queueProps,
deadLetterQueue: this.deadLetterQueue,
enableEncryptionWithCustomerManagedKey: props.enableEncryptionWithCustomerManagedKey,
encryptionKey: props.encryptionKey,
encryptionKeyProps: props.encryptionKeyProps
});
this.sqsQueue = buildQueueResponse.queue;

// Setup the API Gateway
[this.apiGateway, this.apiGatewayCloudWatchRole, this.apiGatewayLogGroup] = defaults.GlobalRestApi(this,
props.apiGatewayProps, props.logGroupProps);
const globalRestApiResponse = defaults.GlobalRestApi(this, props.apiGatewayProps, props.logGroupProps);
this.apiGateway = globalRestApiResponse.api;
this.apiGatewayCloudWatchRole = globalRestApiResponse.role;
this.apiGatewayLogGroup = globalRestApiResponse.logGroup;

// Setup the API Gateway role
this.apiGatewayRole = new iam.Role(this, 'api-gateway-role', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ const app = new App();
const stack = new Stack(app, generateIntegStackName(__filename));
stack.templateOptions.description = 'Integration Test for aws-apigateway-sqs';

const [existingQueueObj] = buildQueue(stack, 'existing-queue', {});
const buildQueueResponse = buildQueue(stack, 'existing-queue', {});

new ApiGatewayToSqs(stack, 'test-api-gateway-sqs-existing-queue', {
existingQueueObj
existingQueueObj: buildQueueResponse.queue
});

// Synth
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ export class CloudFrontToApiGatewayToLambda extends Construct {
lambdaFunctionProps: props.lambdaFunctionProps
});

[this.apiGateway, this.apiGatewayCloudWatchRole, this.apiGatewayLogGroup] =
defaults.RegionalLambdaRestApi(this, this.lambdaFunction, props.apiGatewayProps, props.logGroupProps);
const regionalLambdaRestApiResponse = defaults.RegionalLambdaRestApi(this, this.lambdaFunction, props.apiGatewayProps, props.logGroupProps);
this.apiGateway = regionalLambdaRestApiResponse.api;
this.apiGatewayCloudWatchRole = regionalLambdaRestApiResponse.role;
this.apiGatewayLogGroup = regionalLambdaRestApiResponse.group;

this.apiGateway.methods.forEach((apiMethod) => {
// Override the API Gateway Authorization Type from AWS_IAM to NONE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,16 @@ export class CloudFrontToApiGateway extends Construct {

this.apiGateway = props.existingApiGatewayObj;

[this.cloudFrontWebDistribution, this.cloudFrontFunction, this.cloudFrontLoggingBucket] =
defaults.CloudFrontDistributionForApiGateway(
this,
props.existingApiGatewayObj,
props.cloudFrontDistributionProps,
props.insertHttpSecurityHeaders,
props.cloudFrontLoggingBucketProps,
props.responseHeadersPolicyProps
);
const cloudFrontDistributionForApiGatewayResponse = defaults.CloudFrontDistributionForApiGateway(
this,
props.existingApiGatewayObj,
props.cloudFrontDistributionProps,
props.insertHttpSecurityHeaders,
props.cloudFrontLoggingBucketProps,
props.responseHeadersPolicyProps
);
this.cloudFrontWebDistribution = cloudFrontDistributionForApiGatewayResponse.distribution;
this.cloudFrontFunction = cloudFrontDistributionForApiGatewayResponse.cloudfrontFunction;
this.cloudFrontLoggingBucket = cloudFrontDistributionForApiGatewayResponse.loggingBucket;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ const inProps: lambda.FunctionProps = {

const func = defaults.deployLambdaFunction(stack, inProps);

const [_api] = defaults.RegionalLambdaRestApi(stack, func);
const regionalLambdaRestApiResponse = defaults.RegionalLambdaRestApi(stack, func);

_api.methods.forEach((apiMethod) => {
regionalLambdaRestApiResponse.api.methods.forEach((apiMethod) => {
// Override the API Gateway Authorization Type from AWS_IAM to NONE
const child = apiMethod.node.findChild('Resource') as api.CfnMethod;
if (child.authorizationType === 'AWS_IAM') {
Expand All @@ -52,7 +52,7 @@ _api.methods.forEach((apiMethod) => {
});

new CloudFrontToApiGateway(stack, 'cf-apigw', {
existingApiGatewayObj: _api,
existingApiGatewayObj: regionalLambdaRestApiResponse.api,
cloudFrontLoggingBucketProps: {
removalPolicy: RemovalPolicy.DESTROY,
autoDeleteObjects: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ const inProps: lambda.FunctionProps = {

const func = defaults.deployLambdaFunction(stack, inProps);

const [_api] = defaults.RegionalLambdaRestApi(stack, func);
const regionalLambdaRestApiResponse = defaults.RegionalLambdaRestApi(stack, func);

_api.methods.forEach((apiMethod) => {
regionalLambdaRestApiResponse.api.methods.forEach((apiMethod) => {
// Override the API Gateway Authorization Type from AWS_IAM to NONE
const child = apiMethod.node.findChild('Resource') as api.CfnMethod;
if (child.authorizationType === 'AWS_IAM') {
Expand All @@ -51,7 +51,7 @@ _api.methods.forEach((apiMethod) => {
});

new CloudFrontToApiGateway(stack, 'test-cloudfront-apigateway', {
existingApiGatewayObj: _api,
existingApiGatewayObj: regionalLambdaRestApiResponse.api,
cloudFrontLoggingBucketProps: {
removalPolicy: RemovalPolicy.DESTROY,
autoDeleteObjects: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ function deploy(stack: cdk.Stack) {

const func = defaults.deployLambdaFunction(stack, inProps);

const [api] = defaults.RegionalLambdaRestApi(stack, func);
const regionalLambdaRestApiResponse = defaults.RegionalLambdaRestApi(stack, func);

return new CloudFrontToApiGateway(stack, 'test-cloudfront-apigateway', {
existingApiGatewayObj: api
existingApiGatewayObj: regionalLambdaRestApiResponse.api
});
}

Expand Down Expand Up @@ -172,8 +172,8 @@ function createApi() {

const func = defaults.deployLambdaFunction(stack, inProps);

const [api] = defaults.RegionalLambdaRestApi(stack, func);
return {stack, api};
const regionalLambdaRestApiResponse = defaults.RegionalLambdaRestApi(stack, func);
return {stack, api: regionalLambdaRestApiResponse.api};
}

// --------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,17 @@ export class CloudFrontToMediaStore extends Construct {
this.mediaStoreContainer = defaults.MediaStoreContainer(this, mediaStoreProps);
}

[this.cloudFrontWebDistribution, this.cloudFrontLoggingBucket, this.cloudFrontOriginRequestPolicy, this.cloudFrontFunction]
= defaults.CloudFrontDistributionForMediaStore(
this,
this.mediaStoreContainer,
cloudFrontDistributionProps,
props.insertHttpSecurityHeaders,
props.cloudFrontLoggingBucketProps,
props.responseHeadersPolicyProps
);
const DistributionResponse = defaults.CloudFrontDistributionForMediaStore(
this,
this.mediaStoreContainer,
cloudFrontDistributionProps,
props.insertHttpSecurityHeaders,
props.cloudFrontLoggingBucketProps,
props.responseHeadersPolicyProps
);
this.cloudFrontWebDistribution = DistributionResponse.distribution;
this.cloudFrontLoggingBucket = DistributionResponse.loggingBucket;
this.cloudFrontOriginRequestPolicy = DistributionResponse.requestPolicy;
this.cloudFrontFunction = DistributionResponse.cloudfrontFunction;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,31 @@ export class CloudFrontToS3 extends Construct {
let bucket: s3.IBucket;

if (!props.existingBucketObj) {
[this.s3Bucket, this.s3LoggingBucket] = defaults.buildS3Bucket(this, {
const buildS3BucketResponse = defaults.buildS3Bucket(this, {
bucketProps: props.bucketProps,
loggingBucketProps: props.loggingBucketProps,
logS3AccessLogs: props.logS3AccessLogs
});
this.s3Bucket = buildS3BucketResponse.bucket;
bucket = this.s3Bucket;
} else {
bucket = props.existingBucketObj;
}

this.s3BucketInterface = bucket;

[this.cloudFrontWebDistribution, this.cloudFrontFunction, this.cloudFrontLoggingBucket] =
defaults.CloudFrontDistributionForS3(
this,
this.s3BucketInterface,
props.cloudFrontDistributionProps,
props.insertHttpSecurityHeaders,
props.originPath,
props.cloudFrontLoggingBucketProps,
props.responseHeadersPolicyProps
);
const cloudFrontDistributionForS3Response = defaults.CloudFrontDistributionForS3(
this,
this.s3BucketInterface,
props.cloudFrontDistributionProps,
props.insertHttpSecurityHeaders,
props.originPath,
props.cloudFrontLoggingBucketProps,
props.responseHeadersPolicyProps
);
this.cloudFrontWebDistribution = cloudFrontDistributionForS3Response.distribution;
this.cloudFrontFunction = cloudFrontDistributionForS3Response.cloudfrontFunction;
this.cloudFrontLoggingBucket = cloudFrontDistributionForS3Response.loggingBucket;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,11 @@ export class CognitoToApiGatewayToLambda extends Construct {
existingLambdaObj: props.existingLambdaObj,
lambdaFunctionProps: props.lambdaFunctionProps
});
[this.apiGateway, this.apiGatewayCloudWatchRole, this.apiGatewayLogGroup] =
defaults.GlobalLambdaRestApi(this, this.lambdaFunction, props.apiGatewayProps, props.logGroupProps);
const globalLambdaRestApiResponse = defaults.GlobalLambdaRestApi(this, this.lambdaFunction, props.apiGatewayProps, props.logGroupProps);
this.apiGateway = globalLambdaRestApiResponse.api;
this.apiGatewayCloudWatchRole = globalLambdaRestApiResponse.role;
this.apiGatewayLogGroup = globalLambdaRestApiResponse.group;

this.userPool = defaults.buildUserPool(this, props.cognitoUserPoolProps);
this.userPoolClient = defaults.buildUserPoolClient(this, this.userPool, props.cognitoUserPoolClientProps);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,12 @@ export class DynamoDBStreamsToLambda extends Construct {
lambdaFunctionProps: props.lambdaFunctionProps
});

[this.dynamoTableInterface, this.dynamoTable] = defaults.buildDynamoDBTableWithStream(this, {
const buildDynamoDBTableWithStreamResponse = defaults.buildDynamoDBTableWithStream(this, {
dynamoTableProps: props.dynamoTableProps,
existingTableInterface: props.existingTableInterface
});
this.dynamoTableInterface = buildDynamoDBTableWithStreamResponse.tableInterface;
this.dynamoTable = buildDynamoDBTableWithStreamResponse.tableObject;

// Grant DynamoDB Stream read perimssion for lambda function
this.dynamoTableInterface.grantStreamRead(this.lambdaFunction.grantPrincipal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,16 @@ export class EventbridgeToSns extends Construct {
}

// Setup the sns topic.
[this.snsTopic, this.encryptionKey] = defaults.buildTopic(this, {
const buildTopicResponse = defaults.buildTopic(this, {
existingTopicObj: props.existingTopicObj,
topicProps: props.topicProps,
enableEncryptionWithCustomerManagedKey: enableEncryptionParam,
encryptionKey: props.encryptionKey,
encryptionKeyProps: props.encryptionKeyProps
});

this.snsTopic = buildTopicResponse.topic;
this.encryptionKey = buildTopicResponse.key;
// Setup the event rule target as sns topic.
const topicEventTarget: events.IRuleTarget = {
bind: () => ({
Expand Down
Loading