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

feat: add api id and amplify environment name to stash #2273

Merged
merged 16 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
6 changes: 6 additions & 0 deletions .changeset/strong-toes-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@aws-amplify/backend-data': minor
'@aws-amplify/backend': minor
---
Amplifiyer marked this conversation as resolved.
Show resolved Hide resolved

Add GraphQL API ID and Amplify environment name to custom JS resolver stash
1 change: 1 addition & 0 deletions .eslint_dictionary.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"hotswappable",
"hotswapped",
"hotswapping",
"href",
"iamv2",
"identitypool",
"idps",
Expand Down
4 changes: 3 additions & 1 deletion packages/backend-data/src/assets/js_resolver_handler.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/**
* Pipeline resolver request handler
*/
export const request = () => {
export const request = (ctx: Record<string, Record<string, string>>) => {
ctx.stash.awsAppsyncApiId = '${amplifyApiId}';
ctx.stash.amplifyBranchName = '${amplifyEnvironmentName}';
return {};
};
/**
Expand Down
86 changes: 82 additions & 4 deletions packages/backend-data/src/convert_js_resolvers.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
import { Template } from 'aws-cdk-lib/assertions';
import { Match, Template } from 'aws-cdk-lib/assertions';
import assert from 'node:assert';
import { beforeEach, describe, it } from 'node:test';
import { App, Duration, Stack } from 'aws-cdk-lib';
import {
AmplifyData,
AmplifyDataDefinition,
} from '@aws-amplify/data-construct';
import { resolve } from 'path';
import { fileURLToPath } from 'url';
import { convertJsResolverDefinition } from './convert_js_resolvers.js';
import { join, resolve } from 'path';
import { tmpdir } from 'os';
import { fileURLToPath, pathToFileURL } from 'url';
import {
convertJsResolverDefinition,
defaultJsResolverCode,
} from './convert_js_resolvers.js';
import { a } from '@aws-amplify/data-schema';
import { writeFileSync } from 'node:fs';

// stub schema for the AmplifyApi construct
// not relevant to this test suite
Expand All @@ -28,6 +33,31 @@ const createStackAndSetContext = (): Stack => {
return stack;
};

void describe('defaultJsResolverCode', () => {
void it('returns the default JS resolver code with api id and env name in valid JS', async () => {
const code = defaultJsResolverCode('testApiId', 'testEnvName');
assert(code.includes("ctx.stash.awsAppsyncApiId = 'testApiId';"));
assert(code.includes("ctx.stash.amplifyBranchName = 'testEnvName';"));

const tempDir = tmpdir();
const filename = join(tempDir, 'js_resolver_handler.js');
writeFileSync(filename, code);

github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
Dismissed Show dismissed Hide dismissed
// windows requires dynamic imports to use file urls
const fileUrl = pathToFileURL(filename).href;
const resolver = await import(fileUrl);
const context = { stash: {}, prev: { result: 'result' } };
assert.deepEqual(resolver.request(context), {});

// assert api id and env name are added to the context stash
assert.deepEqual(context.stash, {
awsAppsyncApiId: 'testApiId',
amplifyBranchName: 'testEnvName',
});
assert.equal(resolver.response(context), 'result');
});
});

void describe('convertJsResolverDefinition', () => {
let stack: Stack;
let amplifyApi: AmplifyData;
Expand Down Expand Up @@ -158,4 +188,52 @@ void describe('convertJsResolverDefinition', () => {

template.resourceCountIs('AWS::AppSync::Resolver', 1);
});

void it('adds api id and environment name to stash', () => {
const absolutePath = resolve(
fileURLToPath(import.meta.url),
'../../lib/assets',
'js_resolver_handler.js'
);

const schema = a.schema({
customQuery: a
.query()
.authorization((allow) => allow.publicApiKey())
.returns(a.string())
.handler(
a.handler.custom({
entry: absolutePath,
})
),
});
const { jsFunctions } = schema.transform();
convertJsResolverDefinition(stack, amplifyApi, jsFunctions);

const template = Template.fromStack(stack);
template.hasResourceProperties('AWS::AppSync::Resolver', {
Runtime: {
Name: 'APPSYNC_JS',
RuntimeVersion: '1.0.0',
},
Kind: 'PIPELINE',
TypeName: 'Query',
FieldName: 'customQuery',
Code: {
'Fn::Join': [
'',
[
"/**\n * Pipeline resolver request handler\n */\nexport const request = (ctx) => {\n ctx.stash.awsAppsyncApiId = '",
{
'Fn::GetAtt': [
Match.stringLikeRegexp('amplifyDataGraphQLAPI.*'),
'ApiId',
],
},
"';\n ctx.stash.amplifyBranchName = 'NONE';\n return {};\n};\n/**\n * Pipeline resolver response handler\n */\nexport const response = (ctx) => {\n return ctx.prev.result;\n};\n",
],
],
},
});
});
});
24 changes: 17 additions & 7 deletions packages/backend-data/src/convert_js_resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -18,17 +19,25 @@ 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(new RegExp(/\$\{amplifyApiId\}/, 'g'), amplifyApiId)
.replace(
new RegExp(/\$\{amplifyEnvironmentName\}/, 'g'),
amplifyEnvironmentName
);
};

/**
Expand All @@ -44,8 +53,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}`;
Expand All @@ -71,12 +78,15 @@ export const convertJsResolverDefinition = (

const resolverName = `Resolver_${resolver.typeName}_${resolver.fieldName}`;

const amplifyEnvironmentName =
scope.node.tryGetContext('amplifyEnvironmentName') ?? 'NONE';
Copy link
Contributor

Choose a reason for hiding this comment

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

This context var amplifyEnvironmentName looks like is set by data construct exclusively? Is this the branchName for CI/CD deployments and NONE for sandbox?

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

than we are using the wrong naming here. amplifyEnvironmentName suggests amplify wide name for the environment which is not the case here. May I suggest amplifyApiEnvironmentName?

Copy link
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 backend-data to which this value is assigned. As well as the new variable set in the convert_js_resolvers.ts. It's not a branch name since it's applicable for sandboxes as well.

Copy link
Member Author

@dpilch dpilch Dec 18, 2024

Choose a reason for hiding this comment

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

Oh sorry, you mean const amplifyEnvironmentName not what is in context?

Copy link
Contributor

Choose a reason for hiding this comment

The 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 amplifyBranchName)

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,
Expand Down
Loading