-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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 GeofenceCollection #30711
Changes from 5 commits
4b80527
5e5d121
4b22485
9324d26
baf15dd
1a715e2
df19851
bb82a5e
1eb8ff9
ff62d88
a063feb
9068e77
a9eb231
a624296
0270867
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 | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,165 @@ | ||||||||
import * as iam from 'aws-cdk-lib/aws-iam'; | ||||||||
import * as kms from 'aws-cdk-lib/aws-kms'; | ||||||||
import { ArnFormat, IResource, Lazy, Resource, Stack, Token } from 'aws-cdk-lib/core'; | ||||||||
import { Construct } from 'constructs'; | ||||||||
import { CfnGeofenceCollection } from 'aws-cdk-lib/aws-location'; | ||||||||
import { generateUniqueId } from './util'; | ||||||||
|
||||||||
/** | ||||||||
* A Geofence Collection | ||||||||
*/ | ||||||||
export interface IGeofenceCollection extends IResource { | ||||||||
/** | ||||||||
* The name of the geofence collection | ||||||||
* | ||||||||
* @attribute | ||||||||
*/ | ||||||||
readonly geofenceCollectionName: string; | ||||||||
|
||||||||
/** | ||||||||
* The Amazon Resource Name (ARN) of the geofence collection resource | ||||||||
* | ||||||||
* @attribute Arn, CollectionArn | ||||||||
*/ | ||||||||
readonly geofenceCollectionArn: string; | ||||||||
} | ||||||||
|
||||||||
/** | ||||||||
* Properties for a geofence collection | ||||||||
*/ | ||||||||
export interface GeofenceCollectionProps { | ||||||||
/** | ||||||||
* A name for the geofence collection | ||||||||
* | ||||||||
* @default - A name is automatically generated | ||||||||
*/ | ||||||||
readonly geofenceCollectionName?: string; | ||||||||
|
||||||||
/** | ||||||||
* A description for the geofence collection | ||||||||
* | ||||||||
* @default - no description | ||||||||
*/ | ||||||||
readonly description?: string; | ||||||||
|
||||||||
/** | ||||||||
* The customer managed to encrypt your data. | ||||||||
* | ||||||||
* @default - Use an AWS managed key | ||||||||
* @see https://docs.aws.amazon.com/location/latest/developerguide/encryption-at-rest.html | ||||||||
*/ | ||||||||
readonly kmsKey?: kms.IKey; | ||||||||
} | ||||||||
|
||||||||
abstract class GeofenceCollectionBase extends Resource implements IGeofenceCollection { | ||||||||
public abstract readonly geofenceCollectionName: string; | ||||||||
public abstract readonly geofenceCollectionArn: string; | ||||||||
|
||||||||
/** | ||||||||
* Grant the given principal identity permissions to perform the actions on this geofence collection. | ||||||||
*/ | ||||||||
public grant(grantee: iam.IGrantable, ...actions: string[]): iam.Grant { | ||||||||
return iam.Grant.addToPrincipal({ | ||||||||
grantee: grantee, | ||||||||
actions: actions, | ||||||||
resourceArns: [this.geofenceCollectionArn], | ||||||||
}); | ||||||||
} | ||||||||
|
||||||||
/** | ||||||||
* Grant the given identity permissions to read this geofence collection | ||||||||
* | ||||||||
* See https://docs.aws.amazon.com/location/latest/developerguide/security_iam_id-based-policy-examples.html#security_iam_id-based-policy-examples-read-only-geofences | ||||||||
mazyu36 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
*/ | ||||||||
public grantRead(grantee: iam.IGrantable): iam.Grant { | ||||||||
return this.grant(grantee, | ||||||||
'geo:ListGeofences', | ||||||||
'geo:GetGeofence', | ||||||||
); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
/** | ||||||||
* A Geofence Collection | ||||||||
* | ||||||||
* @see https://docs.aws.amazon.com/location/latest/developerguide/geofence-tracker-concepts.html#geofence-overview | ||||||||
*/ | ||||||||
export class GeofenceCollection extends GeofenceCollectionBase { | ||||||||
/** | ||||||||
* Use an existing geofence collection by name | ||||||||
*/ | ||||||||
public static fromGeofenceCollectionName(scope: Construct, id: string, geofenceCollectionName: string): IGeofenceCollection { | ||||||||
const geofenceCollectionArn = Stack.of(scope).formatArn({ | ||||||||
service: 'geo', | ||||||||
resource: 'geofence-collection', | ||||||||
resourceName: geofenceCollectionName, | ||||||||
}); | ||||||||
|
||||||||
return GeofenceCollection.fromGeofenceCollectionArn(scope, id, geofenceCollectionArn); | ||||||||
} | ||||||||
|
||||||||
/** | ||||||||
* Use an existing geofence collection by ARN | ||||||||
*/ | ||||||||
public static fromGeofenceCollectionArn(scope: Construct, id: string, geofenceCollectionArn: string): IGeofenceCollection { | ||||||||
const parsedArn = Stack.of(scope).splitArn(geofenceCollectionArn, ArnFormat.SLASH_RESOURCE_NAME); | ||||||||
|
||||||||
if (!parsedArn.resourceName) { | ||||||||
throw new Error(`Geofence Collection Arn ${geofenceCollectionArn} does not have a resource name.`); | ||||||||
} | ||||||||
|
||||||||
class Import extends GeofenceCollectionBase { | ||||||||
public readonly geofenceCollectionName = parsedArn.resourceName!; | ||||||||
public readonly geofenceCollectionArn = geofenceCollectionArn; | ||||||||
} | ||||||||
|
||||||||
return new Import(scope, id, { | ||||||||
account: parsedArn.account, | ||||||||
region: parsedArn.region, | ||||||||
}); | ||||||||
} | ||||||||
|
||||||||
public readonly geofenceCollectionName: string; | ||||||||
|
||||||||
public readonly geofenceCollectionArn: string; | ||||||||
|
||||||||
/** | ||||||||
* The timestamp for when the geofence collection resource was created in ISO 8601 format | ||||||||
* | ||||||||
* @attribute | ||||||||
*/ | ||||||||
public readonly geofenceCollectionCreateTime: string; | ||||||||
|
||||||||
/** | ||||||||
* The timestamp for when the geofence collection resource was last updated in ISO 8601 format | ||||||||
* | ||||||||
* @attribute | ||||||||
*/ | ||||||||
public readonly geofenceCollectionUpdateTime: string; | ||||||||
|
||||||||
constructor(scope: Construct, id: string, props: GeofenceCollectionProps = {}) { | ||||||||
|
||||||||
if (props.description && !Token.isUnresolved(props.geofenceCollectionName) && props.description.length > 1000) { | ||||||||
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.
Suggested change
Your implementation is not validating 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.
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. You're right, I messed up 😄 Thanks for double-checking! |
||||||||
throw new Error(`\`description\` must be between 0 and 1000 characters. Received: ${props.description.length} characters` ); | ||||||||
} | ||||||||
|
||||||||
if (props.geofenceCollectionName && !Token.isUnresolved(props.geofenceCollectionName) && !/^[-.\w]{1,100}$/.test(props.geofenceCollectionName)) { | ||||||||
throw new Error(`Invalid geofence collection name. The geofence collection name must be between 1 and 100 characters and contain only alphanumeric characters, hyphens, periods and underscores. Received: ${props.geofenceCollectionName}`); | ||||||||
} | ||||||||
|
||||||||
mazyu36 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
super(scope, id, { | ||||||||
physicalName: props.geofenceCollectionName ?? Lazy.string({ produce: () => generateUniqueId(this) }), | ||||||||
}); | ||||||||
|
||||||||
const geofenceCollection = new CfnGeofenceCollection(this, 'Resource', { | ||||||||
collectionName: this.physicalName, | ||||||||
description: props.description, | ||||||||
kmsKeyId: props.kmsKey?.keyArn, | ||||||||
}); | ||||||||
|
||||||||
this.geofenceCollectionName = geofenceCollection.ref; | ||||||||
this.geofenceCollectionArn = geofenceCollection.attrArn; | ||||||||
this.geofenceCollectionCreateTime = geofenceCollection.attrCreateTime; | ||||||||
this.geofenceCollectionUpdateTime = geofenceCollection.attrUpdateTime; | ||||||||
} | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
export * from './geofence-collection'; | ||
export * from './place-index'; | ||
|
||
// AWS::Location CloudFormation Resources: |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { Names } from 'aws-cdk-lib/core'; | ||
|
||
export function generateUniqueId(context: any): string { | ||
mazyu36 marked this conversation as resolved.
Show resolved
Hide resolved
GavinZZ marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const name = Names.uniqueId(context); | ||
if (name.length > 100) { | ||
return name.substring(0, 50) + name.substring(name.length - 50); | ||
} | ||
return name; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
import { Match, Template } from 'aws-cdk-lib/assertions'; | ||
import * as iam from 'aws-cdk-lib/aws-iam'; | ||
import * as kms from 'aws-cdk-lib/aws-kms'; | ||
import { Stack } from 'aws-cdk-lib'; | ||
import { GeofenceCollection } from '../lib/geofence-collection'; | ||
|
||
let stack: Stack; | ||
beforeEach(() => { | ||
stack = new Stack(); | ||
}); | ||
|
||
test('create a geofence collecction', () => { | ||
new GeofenceCollection(stack, 'GeofenceCollection', { description: 'test' }); | ||
|
||
Template.fromStack(stack).hasResourceProperties('AWS::Location::GeofenceCollection', { | ||
CollectionName: 'GeofenceCollection', | ||
Description: 'test', | ||
}); | ||
}); | ||
|
||
test('throws with invalid description', () => { | ||
expect(() => new GeofenceCollection(stack, 'GeofenceCollection', { | ||
description: 'a'.repeat(1001), | ||
})).toThrow('`description` must be between 0 and 1000 characters. Received: 1001 characters'); | ||
}); | ||
|
||
test('throws with invalid name', () => { | ||
expect(() => new GeofenceCollection(stack, 'GeofenceCollection', { | ||
geofenceCollectionName: 'inv@lid', | ||
})).toThrow('Invalid geofence collection name. The geofence collection name must be between 1 and 100 characters and contain only alphanumeric characters, hyphens, periods and underscores. Received: inv@lid'); | ||
}); | ||
|
||
test('grant read actions', () => { | ||
const geofenceCollection = new GeofenceCollection(stack, 'GeofenceCollection', { | ||
}); | ||
|
||
const role = new iam.Role(stack, 'Role', { | ||
assumedBy: new iam.ServicePrincipal('foo'), | ||
}); | ||
|
||
geofenceCollection.grantRead(role); | ||
|
||
Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', Match.objectLike({ | ||
PolicyDocument: Match.objectLike({ | ||
Statement: [ | ||
{ | ||
Action: [ | ||
'geo:ListGeofences', | ||
'geo:GetGeofence', | ||
], | ||
Effect: 'Allow', | ||
Resource: { | ||
'Fn::GetAtt': [ | ||
'GeofenceCollection6FAC681F', | ||
'Arn', | ||
], | ||
}, | ||
}, | ||
], | ||
}), | ||
})); | ||
}); | ||
|
||
test('import from arn', () => { | ||
const geofenceCollectionArn = stack.formatArn({ | ||
service: 'geo', | ||
resource: 'geofence-collection', | ||
resourceName: 'MyGeofenceCollection', | ||
}); | ||
const geofenceCollection = GeofenceCollection.fromGeofenceCollectionArn(stack, 'GeofenceCollection', geofenceCollectionArn); | ||
|
||
// THEN | ||
expect(geofenceCollection.geofenceCollectionName).toEqual('MyGeofenceCollection'); | ||
expect(geofenceCollection.geofenceCollectionArn).toEqual(geofenceCollectionArn); | ||
}); | ||
|
||
test('import from name', () => { | ||
// WHEN | ||
const geofenceCollectionName = 'MyGeofenceCollection'; | ||
const geofenceCollection = GeofenceCollection.fromGeofenceCollectionName(stack, 'GeofenceCollection', geofenceCollectionName); | ||
|
||
// THEN | ||
expect(geofenceCollection.geofenceCollectionName).toEqual(geofenceCollectionName); | ||
expect(geofenceCollection.geofenceCollectionArn).toEqual(stack.formatArn({ | ||
service: 'geo', | ||
resource: 'geofence-collection', | ||
resourceName: 'MyGeofenceCollection', | ||
})); | ||
}); | ||
|
||
test('create a geofence collection with a customer managed key)', () => { | ||
// GIVEN | ||
const kmsKey = new kms.Key(stack, 'Key'); | ||
|
||
// WHEN | ||
new GeofenceCollection(stack, 'GeofenceCollection', | ||
{ kmsKey }, | ||
); | ||
|
||
// THEN | ||
Template.fromStack(stack).hasResourceProperties('AWS::Location::GeofenceCollection', { | ||
KmsKeyId: stack.resolve(kmsKey.keyArn), | ||
}); | ||
}); |
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.
What's the reasoning behind creating a base class when it's only inherited by one class? Do you foresee another class extending this abstract class?
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.
This is because the already implemented PlaceIndex class has a base class. I don't know if more classes will be added in the future, but I'm doing this to maintain consistency within the package.
I'd appreciate your opinion on this if you have any.
https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-location-alpha/lib/place-index.ts#L95
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 personally don't see the purpose of having a separate base class. I would recommend to just use one class instead. Otherwise the PR LGTM.
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.
@GavinZZ
Thanks. I've removed the based class.
Would you please review again?