-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(location): support RouteCalculator #30682
Changes from 15 commits
0212a23
68be912
21e2488
3183553
4260c94
056ffb9
0c6608d
1e01265
3e99d7c
f23e19b
3841404
7f12eec
15c105a
3e3df50
e10a710
6c8dff4
5761669
ebf3bd2
10380f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
export * from './place-index'; | ||
export * from './route-calculator'; | ||
|
||
// AWS::Location CloudFormation Resources: |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
import * as iam from 'aws-cdk-lib/aws-iam'; | ||
import { ArnFormat, IResource, Lazy, Resource, Stack, Token } from 'aws-cdk-lib/core'; | ||
import { Construct } from 'constructs'; | ||
import { CfnRouteCalculator } from 'aws-cdk-lib/aws-location'; | ||
import { DataSource } from './place-index'; | ||
import { generateUniqueId } from './util'; | ||
|
||
/** | ||
* A Route Calculator | ||
*/ | ||
export interface IRouteCalculator extends IResource { | ||
/** | ||
* The name of the route calculator | ||
* | ||
* @attribute | ||
*/ | ||
readonly routeCalculatorName: string; | ||
|
||
/** | ||
* The Amazon Resource Name (ARN) of the route calculator resource | ||
* | ||
* @attribute Arn,CalculatorArn | ||
*/ | ||
readonly routeCalculatorArn: string; | ||
} | ||
|
||
/** | ||
* Properties for a route calculator | ||
*/ | ||
export interface RouteCalculatorProps { | ||
/** | ||
* A name for the route calculator | ||
* | ||
* Must be between 1 and 100 characters and contain only alphanumeric characters, | ||
* hyphens, periods and underscores. | ||
* | ||
* @default - A name is automatically generated | ||
*/ | ||
readonly routeCalculatorName?: string; | ||
|
||
/** | ||
* Data source for the route calculator | ||
* | ||
* @default DataSource.ESRI | ||
*/ | ||
readonly dataSource?: DataSource; | ||
|
||
/** | ||
* A description for the route calculator | ||
* | ||
* @default - no description | ||
*/ | ||
readonly description?: string; | ||
} | ||
|
||
abstract class RouteCalculatorBase extends Resource implements IRouteCalculator { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment to another PR, don't think Base construct class is needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I've removed base class. |
||
public abstract readonly routeCalculatorName: string; | ||
public abstract readonly routeCalculatorArn: string; | ||
|
||
/** | ||
* Grant the given principal identity permissions to perform the actions on this route calculator. | ||
*/ | ||
public grant(grantee: iam.IGrantable, ...actions: string[]): iam.Grant { | ||
return iam.Grant.addToPrincipal({ | ||
grantee: grantee, | ||
actions: actions, | ||
resourceArns: [this.routeCalculatorArn], | ||
}); | ||
} | ||
|
||
/** | ||
* Grant the given identity permissions to access to a route calculator resource to calculate a route. | ||
* | ||
* @see https://docs.aws.amazon.com/location/latest/developerguide/security_iam_id-based-policy-examples.html#security_iam_id-based-policy-examples-calculate-route | ||
*/ | ||
public grantRead(grantee: iam.IGrantable): iam.Grant { | ||
return this.grant(grantee, | ||
'geo:CalculateRoute', | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* A Route Calculator | ||
* | ||
* @see https://docs.aws.amazon.com/location/latest/developerguide/places-concepts.html | ||
*/ | ||
export class RouteCalculator extends RouteCalculatorBase { | ||
/** | ||
* Use an existing route calculator by name | ||
*/ | ||
public static fromRouteCalculatorName(scope: Construct, id: string, routeCalculatorName: string): IRouteCalculator { | ||
const routeCalculatorArn = Stack.of(scope).formatArn({ | ||
service: 'geo', | ||
resource: 'route-calculator', | ||
resourceName: routeCalculatorName, | ||
}); | ||
|
||
return RouteCalculator.fromRouteCalculatorArn(scope, id, routeCalculatorArn); | ||
} | ||
|
||
/** | ||
* Use an existing route calculator by ARN | ||
*/ | ||
public static fromRouteCalculatorArn(scope: Construct, id: string, routeCalculatorArn: string): IRouteCalculator { | ||
const parsedArn = Stack.of(scope).splitArn(routeCalculatorArn, ArnFormat.SLASH_RESOURCE_NAME); | ||
|
||
if (!parsedArn.resourceName) { | ||
throw new Error(`Route Calculator Arn ${routeCalculatorArn} does not have a resource name.`); | ||
} | ||
|
||
class Import extends RouteCalculatorBase { | ||
public readonly routeCalculatorName = parsedArn.resourceName!; | ||
public readonly routeCalculatorArn = routeCalculatorArn; | ||
} | ||
|
||
return new Import(scope, id, { | ||
account: parsedArn.account, | ||
region: parsedArn.region, | ||
}); | ||
} | ||
|
||
public readonly routeCalculatorName: string; | ||
|
||
public readonly routeCalculatorArn: string; | ||
|
||
/** | ||
* The timestamp for when the route calculator resource was created in ISO 8601 format | ||
* | ||
* @attribute | ||
*/ | ||
public readonly routeCalculatorCreateTime: string; | ||
|
||
/** | ||
* The timestamp for when the route calculator resource was last updated in ISO 8601 format | ||
* | ||
* @attribute | ||
*/ | ||
public readonly routeCalculatorUpdateTime: string; | ||
|
||
constructor(scope: Construct, id: string, props: RouteCalculatorProps = {}) { | ||
|
||
if (props.description && !Token.isUnresolved(props.description) && props.description.length > 1000) { | ||
throw new Error(`\`description\` must be between 0 and 1000 characters. Received: ${props.description.length} characters`); | ||
} | ||
|
||
if (props.routeCalculatorName && !Token.isUnresolved(props.routeCalculatorName) && !/^[-.\w]{1,100}$/.test(props.routeCalculatorName)) { | ||
throw new Error(`Invalid route calculator name. The route calculator name must be between 1 and 100 characters and contain only alphanumeric characters, hyphens, periods and underscores. Received: ${props.routeCalculatorName}`); | ||
} | ||
|
||
mazyu36 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
super(scope, id, { | ||
physicalName: props.routeCalculatorName ?? Lazy.string({ produce: () => generateUniqueId(this) }), | ||
}); | ||
|
||
const routeCalculator = new CfnRouteCalculator(this, 'Resource', { | ||
calculatorName: this.physicalName, | ||
dataSource: props.dataSource ?? DataSource.ESRI, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think CFN doc didn't specify a default value. I'm not familiar with this service/resource, can you briefly explain the reasoning of choosing ESRI as the default behaviour? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I aligned it with the already implemented PlaceIndex class. However, since it's listed as a required item in the CFn documentation, I've adjusted it to match that. I think the DataSource for PlaceIndex should also be required, but I haven't made that change this time. |
||
description: props.description, | ||
}); | ||
|
||
this.routeCalculatorName = routeCalculator.ref; | ||
this.routeCalculatorArn = routeCalculator.attrArn; | ||
this.routeCalculatorCreateTime = routeCalculator.attrCreateTime; | ||
this.routeCalculatorUpdateTime = routeCalculator.attrUpdateTime; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { Names } from 'aws-cdk-lib/core'; | ||
import { IConstruct } from 'constructs'; | ||
|
||
export function generateUniqueId(context: IConstruct): string { | ||
const name = Names.uniqueId(context); | ||
if (name.length > 100) { | ||
return name.substring(0, 50) + name.substring(name.length - 50); | ||
} | ||
return name; | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
{ | ||
"Resources": { | ||
"RouteCalculator0F2D109D": { | ||
"Type": "AWS::Location::RouteCalculator", | ||
"Properties": { | ||
"CalculatorName": "cdkinteglocationroutecalculatorRouteCalculator916939B5", | ||
"DataSource": "Esri" | ||
} | ||
} | ||
}, | ||
"Parameters": { | ||
"BootstrapVersion": { | ||
"Type": "AWS::SSM::Parameter::Value<String>", | ||
"Default": "/cdk-bootstrap/hnb659fds/version", | ||
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]" | ||
} | ||
}, | ||
"Rules": { | ||
"CheckBootstrapVersion": { | ||
"Assertions": [ | ||
{ | ||
"Assert": { | ||
"Fn::Not": [ | ||
{ | ||
"Fn::Contains": [ | ||
[ | ||
"1", | ||
"2", | ||
"3", | ||
"4", | ||
"5" | ||
], | ||
{ | ||
"Ref": "BootstrapVersion" | ||
} | ||
] | ||
} | ||
] | ||
}, | ||
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI." | ||
} | ||
] | ||
} | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, the DataSource for
PlaceIndex
andRouteCalculator
are identical.Therefore, I decided to reference the
DataSource
of PlaceIndex.However, if there's a better approach, please share your opinion.
Ref:
I also considered defining a completely separate Enum like
RouteDataSource
, but decided against it as I thought it would be redundant.I also thought about moving PlaceIndex's
DataSource
to a separate file, but I think it would result in breaking changes...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Let's see if the core team has a different opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see in another PR of yours that you added a
util
file. I think it would be nice to move PlaceIndex'sDataSource
to somewhere more generic. Based on the CFN documentation, https://docs.aws.amazon.com/location/latest/developerguide/what-is-data-provider.html, it seems that all these three resources should re-use theDataSource
prop.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I misunderstood the breaking change. Although I needed to fix unit test within the package, it doesn't actually affect the usage of the package.
I've moved the DataSource to the util module.