-
Notifications
You must be signed in to change notification settings - Fork 102
feat: add api id and amplify environment name to stash #2273
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
Changes from 9 commits
2d1aa93
263efd0
59102df
304d3c1
bc9b37c
9c6ab19
80684b5
9ad666c
9aff173
e0d795c
e50269c
ec2c184
389b1a6
cd166ed
a5ca531
8987f6e
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,6 @@ | ||
--- | ||
'@aws-amplify/backend-data': minor | ||
'@aws-amplify/backend': minor | ||
--- | ||
|
||
Add GraphQL API ID and Amplify environment name to custom JS resolver stash |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,7 @@ | |
"hotfix", | ||
"hotswap", | ||
"hotswapped", | ||
"href", | ||
"iamv2", | ||
"identitypool", | ||
"idps", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import { CfnFunctionConfiguration, CfnResolver } from 'aws-cdk-lib/aws-appsync'; | |
import { JsResolver } from '@aws-amplify/data-schema-types'; | ||
import { resolve } from 'path'; | ||
import { fileURLToPath } from 'node:url'; | ||
import { readFileSync } from 'fs'; | ||
import { Asset } from 'aws-cdk-lib/aws-s3-assets'; | ||
import { resolveEntryPath } from './resolve_entry_path.js'; | ||
|
||
|
@@ -18,17 +19,22 @@ const JS_PIPELINE_RESOLVER_HANDLER = './assets/js_resolver_handler.js'; | |
* It's required for defining a pipeline resolver. The only purpose it serves is returning the output of the last function in the pipeline back to the client. | ||
* | ||
* Customer-provided handlers are added as a Functions list in `pipelineConfig.functions` | ||
* | ||
* Add Amplify API ID and environment name to the context stash for use in the customer-provided handlers. | ||
*/ | ||
const defaultJsResolverAsset = (scope: Construct): Asset => { | ||
export const defaultJsResolverCode = ( | ||
amplifyApiId: string, | ||
amplifyEnvironmentName: string | ||
): string => { | ||
const resolvedTemplatePath = resolve( | ||
fileURLToPath(import.meta.url), | ||
'../../lib', | ||
JS_PIPELINE_RESOLVER_HANDLER | ||
); | ||
|
||
return new Asset(scope, 'default_js_resolver_handler_asset', { | ||
path: resolveEntryPath(resolvedTemplatePath), | ||
}); | ||
return readFileSync(resolvedTemplatePath, 'utf-8') | ||
.replace('${amplifyApiId}', amplifyApiId) | ||
.replace('${amplifyEnvironmentName}', amplifyEnvironmentName); | ||
dpilch marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
}; | ||
|
||
/** | ||
|
@@ -44,8 +50,6 @@ export const convertJsResolverDefinition = ( | |
return; | ||
} | ||
|
||
const jsResolverTemplateAsset = defaultJsResolverAsset(scope); | ||
|
||
for (const resolver of jsResolvers) { | ||
const functions: string[] = resolver.handlers.map((handler, idx) => { | ||
const fnName = `Fn_${resolver.typeName}_${resolver.fieldName}_${idx + 1}`; | ||
|
@@ -71,12 +75,15 @@ export const convertJsResolverDefinition = ( | |
|
||
const resolverName = `Resolver_${resolver.typeName}_${resolver.fieldName}`; | ||
|
||
const amplifyEnvironmentName = | ||
scope.node.tryGetContext('amplifyEnvironmentName') ?? 'NONE'; | ||
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. This context var 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. Yes, this is this the branchName for CI/CD deployments and NONE for sandbox. 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. than we are using the wrong naming here. 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. The value is already used in the data construct. To use the new name without a breaking change we would need to set both variables, then it might be confusing why there are two variables with the same value. I think it is a better option to continue using the existing one. 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'm not asking to change in the data construct but rather the new variables created in the 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. Oh sorry, you mean 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. yes, I understand context vars are harder to change, but we should not create more vars with this naming (such as |
||
new CfnResolver(scope, resolverName, { | ||
apiId: amplifyApi.apiId, | ||
fieldName: resolver.fieldName, | ||
typeName: resolver.typeName, | ||
kind: APPSYNC_PIPELINE_RESOLVER, | ||
codeS3Location: jsResolverTemplateAsset.s3ObjectUrl, | ||
// Uses synth-time inline code to avoid circular dependency when adding the API ID as an environment variable. | ||
code: defaultJsResolverCode(amplifyApi.apiId, amplifyEnvironmentName), | ||
runtime: { | ||
name: APPSYNC_JS_RUNTIME_NAME, | ||
runtimeVersion: APPSYNC_JS_RUNTIME_VERSION, | ||
|
Uh oh!
There was an error while loading. Please reload this page.