-
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(pipes-enrichments): support API destination enrichment #31312
Merged
mergify
merged 5 commits into
aws:main
from
mazyu36:pipes-enrichment-api-destination-29383
Oct 16, 2024
Merged
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
866d194
add API destination enrichment
mazyu36 f261c56
fix import order
mazyu36 a3f7ede
Merge branch 'main' into pipes-enrichment-api-destination-29383
mazyu36 929ea1a
Merge branch 'main' into pipes-enrichment-api-destination-29383
mazyu36 82570fb
Merge branch 'main' into pipes-enrichment-api-destination-29383
mergify[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
86 changes: 86 additions & 0 deletions
86
packages/@aws-cdk/aws-pipes-enrichments-alpha/lib/api-destination.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
import { EnrichmentParametersConfig, IEnrichment, IPipe, InputTransformation } from '@aws-cdk/aws-pipes-alpha'; | ||
import { IApiDestination } from 'aws-cdk-lib/aws-events'; | ||
import { IRole, PolicyStatement } from 'aws-cdk-lib/aws-iam'; | ||
import { CfnPipe } from 'aws-cdk-lib/aws-pipes'; | ||
|
||
/** | ||
* Properties for a ApiDestinationEnrichment | ||
*/ | ||
export interface ApiDestinationEnrichmentProps { | ||
/** | ||
* The input transformation for the enrichment | ||
* @see https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-pipes-input-transformation.html | ||
* @default - None | ||
*/ | ||
readonly inputTransformation?: InputTransformation; | ||
|
||
/** | ||
* The headers that need to be sent as part of request invoking the EventBridge ApiDestination. | ||
* | ||
* @default - none | ||
*/ | ||
readonly headerParameters?: Record<string, string>; | ||
|
||
/** | ||
* The path parameter values used to populate the EventBridge API destination path wildcards ("*"). | ||
* | ||
* @default - none | ||
*/ | ||
readonly pathParameterValues?: string[]; | ||
|
||
/** | ||
* The query string keys/values that need to be sent as part of request invoking the EventBridge API destination. | ||
* | ||
* @default - none | ||
*/ | ||
readonly queryStringParameters?: Record<string, string>; | ||
} | ||
|
||
/** | ||
* An API Destination enrichment for a pipe | ||
*/ | ||
export class ApiDestinationEnrichment implements IEnrichment { | ||
public readonly enrichmentArn: string; | ||
|
||
private readonly inputTransformation?: InputTransformation; | ||
private readonly headerParameters?: Record<string, string>; | ||
private readonly pathParameterValues?: string[]; | ||
private readonly queryStringParameters?: Record<string, string>; | ||
|
||
constructor(private readonly destination: IApiDestination, props?: ApiDestinationEnrichmentProps) { | ||
this.enrichmentArn = destination.apiDestinationArn; | ||
this.inputTransformation = props?.inputTransformation; | ||
this.headerParameters = props?.headerParameters; | ||
this.queryStringParameters = props?.queryStringParameters; | ||
this.pathParameterValues = props?.pathParameterValues; | ||
} | ||
|
||
bind(pipe: IPipe): EnrichmentParametersConfig { | ||
|
||
const httpParameters: CfnPipe.PipeEnrichmentHttpParametersProperty | undefined = | ||
this.headerParameters ?? | ||
this.pathParameterValues ?? | ||
this.queryStringParameters | ||
? { | ||
headerParameters: this.headerParameters, | ||
pathParameterValues: this.pathParameterValues, | ||
queryStringParameters: this.queryStringParameters, | ||
} | ||
: undefined; | ||
|
||
return { | ||
enrichmentParameters: { | ||
inputTemplate: this.inputTransformation?.bind(pipe).inputTemplate, | ||
httpParameters, | ||
}, | ||
}; | ||
} | ||
|
||
grantInvoke(pipeRole: IRole): void { | ||
pipeRole.addToPrincipalPolicy(new PolicyStatement({ | ||
resources: [this.destination.apiDestinationArn], | ||
actions: ['events:InvokeApiDestination'], | ||
})); | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
export * from './api-destination'; | ||
export * from './lambda'; | ||
export * from './stepfunctions'; |
1 change: 1 addition & 0 deletions
1
packages/@aws-cdk/aws-pipes-enrichments-alpha/rosetta/default.ts-fixture
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 changes: 54 additions & 0 deletions
54
...ages/@aws-cdk/aws-pipes-enrichments-alpha/test/__snapshots__/api-destination.test.ts.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`api-destination should grant pipe role invoke access 1`] = ` | ||
{ | ||
"MyPipeRoleCBC8E9AB": { | ||
"Properties": { | ||
"AssumeRolePolicyDocument": { | ||
"Statement": [ | ||
{ | ||
"Action": "sts:AssumeRole", | ||
"Effect": "Allow", | ||
"Principal": { | ||
"Service": "pipes.amazonaws.com", | ||
}, | ||
}, | ||
], | ||
"Version": "2012-10-17", | ||
}, | ||
}, | ||
"Type": "AWS::IAM::Role", | ||
}, | ||
} | ||
`; | ||
|
||
exports[`api-destination should grant pipe role invoke access 2`] = ` | ||
{ | ||
"MyPipeRoleDefaultPolicy31387C20": { | ||
"Properties": { | ||
"PolicyDocument": { | ||
"Statement": [ | ||
{ | ||
"Action": "events:InvokeApiDestination", | ||
"Effect": "Allow", | ||
"Resource": { | ||
"Fn::GetAtt": [ | ||
"ApiDestination3AB57A39", | ||
"Arn", | ||
], | ||
}, | ||
}, | ||
], | ||
"Version": "2012-10-17", | ||
}, | ||
"PolicyName": "MyPipeRoleDefaultPolicy31387C20", | ||
"Roles": [ | ||
{ | ||
"Ref": "MyPipeRoleCBC8E9AB", | ||
}, | ||
], | ||
}, | ||
"Type": "AWS::IAM::Policy", | ||
}, | ||
} | ||
`; |
124 changes: 124 additions & 0 deletions
124
packages/@aws-cdk/aws-pipes-enrichments-alpha/test/api-destination.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,124 @@ | ||
import { DynamicInput, InputTransformation, Pipe } from '@aws-cdk/aws-pipes-alpha'; | ||
import { App, Stack, SecretValue } from 'aws-cdk-lib'; | ||
import { Template } from 'aws-cdk-lib/assertions'; | ||
import * as events from 'aws-cdk-lib/aws-events'; | ||
import { Secret } from 'aws-cdk-lib/aws-secretsmanager'; | ||
import { TestSource, TestTarget } from './test-classes'; | ||
import { ApiDestinationEnrichment } from '../lib'; | ||
|
||
describe('api-destination', () => { | ||
let app: App; | ||
let stack: Stack; | ||
let secret: Secret; | ||
let connection: events.Connection; | ||
let apiDestination: events.ApiDestination; | ||
|
||
beforeEach(() => { | ||
app = new App(); | ||
stack = new Stack(app, 'TestStack'); | ||
secret = new Secret(stack, 'MySecret', { | ||
secretStringValue: SecretValue.unsafePlainText('abc123'), | ||
}); | ||
connection = new events.Connection(stack, 'MyConnection', { | ||
authorization: events.Authorization.apiKey('x-api-key', secret.secretValue), | ||
description: 'Connection with API Key x-api-key', | ||
connectionName: 'MyConnection', | ||
}); | ||
|
||
apiDestination = new events.ApiDestination(stack, 'ApiDestination', { | ||
apiDestinationName: 'ApiDestination', | ||
connection, | ||
description: 'ApiDestination', | ||
httpMethod: events.HttpMethod.GET, | ||
endpoint: 'someendpoint', | ||
rateLimitPerSecond: 60, | ||
}); | ||
}); | ||
|
||
it('should have only enrichment arn', () => { | ||
// ARRANGE | ||
const enrichment = new ApiDestinationEnrichment(apiDestination); | ||
|
||
new Pipe(stack, 'MyPipe', { | ||
source: new TestSource(), | ||
enrichment, | ||
target: new TestTarget(), | ||
}); | ||
|
||
// ACT | ||
const template = Template.fromStack(stack); | ||
|
||
// ASSERT | ||
template.hasResourceProperties('AWS::Pipes::Pipe', { | ||
Enrichment: { | ||
'Fn::GetAtt': [ | ||
'ApiDestination3AB57A39', | ||
'Arn', | ||
], | ||
}, | ||
EnrichmentParameters: {}, | ||
}); | ||
}); | ||
|
||
it('should have enrichment parameters', () => { | ||
// ARRANGE | ||
const enrichment = new ApiDestinationEnrichment(apiDestination, { | ||
inputTransformation: InputTransformation.fromObject({ | ||
body: DynamicInput.fromEventPath('$.body'), | ||
}), | ||
headerParameters: { | ||
headerParam: 'headerParam', | ||
}, | ||
pathParameterValues: ['pathParam'], | ||
queryStringParameters: { | ||
param: 'queryParam', | ||
}, | ||
}); | ||
|
||
new Pipe(stack, 'MyPipe', { | ||
source: new TestSource(), | ||
enrichment, | ||
target: new TestTarget(), | ||
}); | ||
|
||
// ACT | ||
const template = Template.fromStack(stack); | ||
|
||
// ASSERT | ||
template.hasResourceProperties('AWS::Pipes::Pipe', { | ||
EnrichmentParameters: { | ||
InputTemplate: '{"body":<$.body>}', | ||
HttpParameters: { | ||
HeaderParameters: { | ||
headerParam: 'headerParam', | ||
}, | ||
PathParameterValues: [ | ||
'pathParam', | ||
], | ||
QueryStringParameters: { | ||
param: 'queryParam', | ||
}, | ||
}, | ||
}, | ||
}); | ||
}); | ||
|
||
it('should grant pipe role invoke access', () => { | ||
// ARRANGE | ||
const enrichment = new ApiDestinationEnrichment(apiDestination); | ||
|
||
new Pipe(stack, 'MyPipe', { | ||
source: new TestSource(), | ||
enrichment, | ||
target: new TestTarget(), | ||
}); | ||
|
||
// ACT | ||
const template = Template.fromStack(stack); | ||
|
||
// ASSERT | ||
expect(template.findResources('AWS::IAM::Role')).toMatchSnapshot(); | ||
expect(template.findResources('AWS::IAM::Policy')).toMatchSnapshot(); | ||
}); | ||
}); | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Thanks for the PR @mazyu36 - overall looks good to me. I'm just wondering about the assignment of the
httpParameters
variable here - in practice, does it make sense to pass in HTTP parameters if one of the parameter values is undefined? (It might be nice to add a test for this, too?)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.
@sumupitchayan
Yes, I think it makes sense. For example, when calling an API that only requires path parameters.
This is in line with similar implementations as follows:
https://github.com/aws/aws-cdk/blob/main/packages/@aws-cdk/aws-pipes-targets-alpha/lib/api-destination.ts
https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-events-targets/lib/api-destination.ts
I think it depends on the design of the API being called, so it would be sufficient to confirm that the parameters are set in the unit test.
https://github.com/aws/aws-cdk/pull/31312/files#diff-0523525438cc22991d3e0555d86f9ec9c8a79cfa4a86599cdb2ea8d4075d4d84R63
I think adding integration tests unnecessary.
What do you think?
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.
@mazyu36 thanks for the clarification. Looks good to me then 👍