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

fix: Lambda client env var name issue #2324

Merged
merged 13 commits into from
Dec 17, 2024
7 changes: 7 additions & 0 deletions .changeset/warm-sloths-tickle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@aws-amplify/backend-function': patch
'@aws-amplify/backend-data': patch
'@aws-amplify/platform-core': patch
stocaaro marked this conversation as resolved.
Show resolved Hide resolved
---

Lambda client env var name issue
stocaaro marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 11 additions & 4 deletions packages/backend-data/src/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import { Bucket } from 'aws-cdk-lib/aws-s3';
import { BucketDeployment, Source } from 'aws-cdk-lib/aws-s3-deployment';

const modelIntrospectionSchemaKey = 'modelIntrospectionSchema.json';
const defaultName = 'amplifyData';

/**
* Singleton factory for AmplifyGraphqlApi constructs that can be used in Amplify project files.
Expand Down Expand Up @@ -127,7 +128,7 @@ class DataGenerator implements ConstructContainerEntryGenerator {
private readonly getInstanceProps: ConstructFactoryGetInstanceProps,
private readonly outputStorageStrategy: BackendOutputStorageStrategy<GraphqlOutput>
) {
this.name = props.name ?? 'amplifyData';
this.name = props.name ?? defaultName;
}

generateContainerEntry = ({
Expand Down Expand Up @@ -307,14 +308,20 @@ class DataGenerator implements ConstructContainerEntryGenerator {

convertJsResolverDefinition(scope, amplifyApi, schemasJsFunctions);

const namePrefix = this.name === defaultName ? '' : defaultName;

const ssmEnvironmentEntries =
ssmEnvironmentEntriesGenerator.generateSsmEnvironmentEntries({
[`${this.name}_GRAPHQL_ENDPOINT`]:
[`${namePrefix}${this.name}_GRAPHQL_ENDPOINT`]:
amplifyApi.resources.cfnResources.cfnGraphqlApi.attrGraphQlUrl,
[`${this.name}_MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME`]:
[`${namePrefix}${this.name}_MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME`]:
modelIntrospectionSchemaBucket.bucketName,
[`${this.name}_MODEL_INTROSPECTION_SCHEMA_KEY`]:
[`${namePrefix}${this.name}_MODEL_INTROSPECTION_SCHEMA_KEY`]:
modelIntrospectionSchemaKey,
['AMPLIFY_DATA_DEFAULT_NAME']: `${namePrefix}${this.name}`,
// @deprecated
[`${this.name}_GRAPHQL_ENDPOINT`]:
Copy link
Contributor

Choose a reason for hiding this comment

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

will this not create a duplicate when namePrefix is empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was worried about the same thing. Up on line 311, name prefix is left blank if the name is defaulted.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not expected, we should avoid adding it. Also wanted to ask what's special about GRAPHQL_ENDPOINT that this behavior is not replicated for the other _MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME and _MODEL_INTROSPECTION_SCHEMA_KEY

Copy link
Member Author

Choose a reason for hiding this comment

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

GRAPHQL_ENDPOINT predates the MIS fields and customers may be depending on it directly. Since the other two are newer, @sobolk advised that they can be modified without concern for breaking change.

Also GRAPHQL_ENDPOINT is included in the prefixing, however, we are leaving an unprefixed version in place with a @Deprecation flag to enable removal with the next major version change.

amplifyApi.resources.cfnResources.cfnGraphqlApi.attrGraphQlUrl,
});

const policyGenerator = new AppSyncPolicyGenerator(
Expand Down
8 changes: 3 additions & 5 deletions packages/backend-function/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,12 @@ type DataClientConfig = {

// @public (undocumented)
type DataClientEnv = {
AMPLIFY_DATA_GRAPHQL_ENDPOINT: string;
AMPLIFY_DATA_MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME: string;
AMPLIFY_DATA_MODEL_INTROSPECTION_SCHEMA_KEY: string;
AWS_ACCESS_KEY_ID: string;
AWS_SECRET_ACCESS_KEY: string;
AWS_SESSION_TOKEN: string;
AWS_REGION: string;
};
AMPLIFY_DATA_DEFAULT_NAME: string;
} & Record<string, unknown>;

// @public (undocumented)
type DataClientError = {
Expand Down Expand Up @@ -111,7 +109,7 @@ const getAmplifyDataClientConfig: <T>(env: T, s3Client?: S3Client) => Promise<Da

// @public (undocumented)
type InvalidConfig = unknown & {
invalidType: 'This function needs to be granted `authorization((allow) => [allow.resource(fcn)])` on the data schema.';
invalidType: 'Some of the AWS environment variables needed to configure Amplify are missing.';
};

// @public (undocumented)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { NoSuchKey, S3, S3ServiceException } from '@aws-sdk/client-s3';

import { getAmplifyDataClientConfig } from './get_amplify_clients_configuration.js';

const validEnv = {
const validDefaultEnv = {
AMPLIFY_DATA_MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME:
'TEST_VALUE for AMPLIFY_DATA_MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME',
AMPLIFY_DATA_MODEL_INTROSPECTION_SCHEMA_KEY:
Expand All @@ -14,6 +14,21 @@ const validEnv = {
AWS_SESSION_TOKEN: 'TEST_VALUE for AWS_SESSION_TOKEN',
AWS_REGION: 'TEST_VALUE for AWS_REGION',
AMPLIFY_DATA_GRAPHQL_ENDPOINT: 'TEST_VALUE for AMPLIFY_DATA_GRAPHQL_ENDPOINT',
AMPLIFY_DATA_DEFAULT_NAME: 'AmplifyData',
};

const validNamedEnv = {
AMPLIFY_DATA_TEST_NAME_MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME:
'TEST_VALUE for AMPLIFY_DATA_MODEL_INTROSPECTION_SCHEMA_BUCKET_NAME',
AMPLIFY_DATA_TEST_NAME_MODEL_INTROSPECTION_SCHEMA_KEY:
'TEST_VALUE for AMPLIFY_DATA_MODEL_INTROSPECTION_SCHEMA_KEY',
AWS_ACCESS_KEY_ID: 'TEST_VALUE for AWS_ACCESS_KEY_ID',
AWS_SECRET_ACCESS_KEY: 'TEST_VALUE for AWS_SECRET_ACCESS_KEY',
AWS_SESSION_TOKEN: 'TEST_VALUE for AWS_SESSION_TOKEN',
AWS_REGION: 'TEST_VALUE for AWS_REGION',
AMPLIFY_DATA_TEST_NAME_GRAPHQL_ENDPOINT:
'TEST_VALUE for AMPLIFY_DATA_GRAPHQL_ENDPOINT',
AMPLIFY_DATA_DEFAULT_NAME: 'AmplifyDataTestName',
};

let mockS3Client: S3;
Expand All @@ -23,110 +38,146 @@ void describe('getAmplifyDataClientConfig', () => {
mockS3Client = new S3();
});

Object.keys(validEnv).forEach((envFieldToExclude) => {
void it(`returns empty config objects when ${envFieldToExclude} is not included`, async () => {
const env = { ...validEnv } as Record<string, string>;
delete env[envFieldToExclude];
assert.deepEqual(await getAmplifyDataClientConfig(env), {
resourceConfig: {},
libraryOptions: {},
[
{
name: 'no set name',
dataBackendName: 'AMPLIFY_DATA',
validEnv: validDefaultEnv,
},
{
name: 'an explicit name',
dataBackendName: 'AMPLIFY_DATA_TEST_NAME',
validEnv: validNamedEnv,
},
].forEach(({ name, dataBackendName, validEnv }) => {
void describe(`env variable with ${name} for the data backend`, () => {
Object.keys(validEnv)
.filter((k) => k !== 'AMPLIFY_DATA_DEFAULT_NAME')
.forEach((envFieldToExclude) => {
if (envFieldToExclude.includes(dataBackendName)) {
void it(`throws error when ${envFieldToExclude} is not included`, async () => {
const env = { ...validEnv } as Record<string, string>;
delete env[envFieldToExclude];
await assert.rejects(
async () => await getAmplifyDataClientConfig(env),
/The data environment variables are malformed/
);
});

void it(`throws error when ${envFieldToExclude} is not a string`, async () => {
const env = { ...validEnv } as Record<string, unknown>;
env[envFieldToExclude] = 123;
await assert.rejects(
async () => await getAmplifyDataClientConfig(env),
/The data environment variables are malformed/
);
});
} else {
void it(`returns empty config objects when ${envFieldToExclude} is not included`, async () => {
const env = { ...validEnv } as Record<string, string>;
delete env[envFieldToExclude];
assert.deepEqual(await getAmplifyDataClientConfig(env), {
resourceConfig: {},
libraryOptions: {},
});
});

void it(`returns empty config objects when ${envFieldToExclude} is not a string`, async () => {
const env = { ...validEnv } as Record<string, unknown>;
env[envFieldToExclude] = 123;
assert.deepEqual(await getAmplifyDataClientConfig(env), {
resourceConfig: {},
libraryOptions: {},
});
});
}
});

void it('raises a custom error message when the model introspection schema is missing from the s3 bucket', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', async () => {
throw new NoSuchKey({ message: 'TEST_ERROR', $metadata: {} });
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

await assert.rejects(
async () => await getAmplifyDataClientConfig(validEnv, mockS3Client),
new Error(
'Error retrieving the schema from S3. Please confirm that your project has a `defineData` included in the `defineBackend` definition.'
)
);
});
});

void it(`returns empty config objects when ${envFieldToExclude} is not a string`, async () => {
const env = { ...validEnv } as Record<string, unknown>;
env[envFieldToExclude] = 123;
assert.deepEqual(await getAmplifyDataClientConfig(env), {
resourceConfig: {},
libraryOptions: {},
void it('raises a custom error message when there is a S3ServiceException error retrieving the model introspection schema from the s3 bucket', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', async () => {
throw new S3ServiceException({
name: 'TEST_ERROR',
message: 'TEST_MESSAGE',
$fault: 'server',
$metadata: {},
});
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

await assert.rejects(
async () => await getAmplifyDataClientConfig(validEnv, mockS3Client),
new Error(
'Error retrieving the schema from S3. You may need to grant this function authorization on the schema. TEST_ERROR: TEST_MESSAGE.'
)
);
});
});
});

void it('raises a custom error message when the model introspection schema is missing from the s3 bucket', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', async () => {
throw new NoSuchKey({ message: 'TEST_ERROR', $metadata: {} });
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

await assert.rejects(
async () => await getAmplifyDataClientConfig(validEnv, mockS3Client),
new Error(
'Error retrieving the schema from S3. Please confirm that your project has a `defineData` included in the `defineBackend` definition.'
)
);
});
void it('re-raises a non-S3 error received when retrieving the model introspection schema from the s3 bucket', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', async () => {
throw new Error('Test Error');
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

void it('raises a custom error message when there is a S3ServiceException error retrieving the model introspection schema from the s3 bucket', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', async () => {
throw new S3ServiceException({
name: 'TEST_ERROR',
message: 'TEST_MESSAGE',
$fault: 'server',
$metadata: {},
await assert.rejects(
async () => await getAmplifyDataClientConfig(validEnv, mockS3Client),
new Error('Test Error')
);
});
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

await assert.rejects(
async () => await getAmplifyDataClientConfig(validEnv, mockS3Client),
new Error(
'Error retrieving the schema from S3. You may need to grant this function authorization on the schema. TEST_ERROR: TEST_MESSAGE.'
)
);
});

void it('re-raises a non-S3 error received when retrieving the model introspection schema from the s3 bucket', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', async () => {
throw new Error('Test Error');
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

await assert.rejects(
async () => await getAmplifyDataClientConfig(validEnv, mockS3Client),
new Error('Test Error')
);
});

void it('returns the expected libraryOptions and resourceConfig values in the happy case', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', () => {
return Promise.resolve({
Body: {
transformToString: () => JSON.stringify({ testSchema: 'TESTING' }),
},
void it('returns the expected libraryOptions and resourceConfig values in the happy case', async () => {
const s3ClientSendMock = mock.method(mockS3Client, 'send', () => {
return Promise.resolve({
Body: {
transformToString: () =>
JSON.stringify({ testSchema: 'TESTING' }),
},
});
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

const { resourceConfig, libraryOptions } =
await getAmplifyDataClientConfig(validEnv, mockS3Client);

assert.deepEqual(
await libraryOptions.Auth.credentialsProvider.getCredentialsAndIdentityId?.(),
{
credentials: {
accessKeyId: 'TEST_VALUE for AWS_ACCESS_KEY_ID',
secretAccessKey: 'TEST_VALUE for AWS_SECRET_ACCESS_KEY',
sessionToken: 'TEST_VALUE for AWS_SESSION_TOKEN',
},
}
);
assert.deepEqual(
await libraryOptions.Auth.credentialsProvider.clearCredentialsAndIdentityId?.(),
undefined
);

assert.deepEqual(resourceConfig, {
API: {
GraphQL: {
endpoint: 'TEST_VALUE for AMPLIFY_DATA_GRAPHQL_ENDPOINT',
region: 'TEST_VALUE for AWS_REGION',
defaultAuthMode: 'iam',
modelIntrospection: { testSchema: 'TESTING' },
},
},
});
});
});
mock.method(mockS3Client, 'send', s3ClientSendMock);

const { resourceConfig, libraryOptions } = await getAmplifyDataClientConfig(
validEnv,
mockS3Client
);

assert.deepEqual(
await libraryOptions.Auth.credentialsProvider.getCredentialsAndIdentityId?.(),
{
credentials: {
accessKeyId: 'TEST_VALUE for AWS_ACCESS_KEY_ID',
secretAccessKey: 'TEST_VALUE for AWS_SECRET_ACCESS_KEY',
sessionToken: 'TEST_VALUE for AWS_SESSION_TOKEN',
},
}
);
assert.deepEqual(
await libraryOptions.Auth.credentialsProvider.clearCredentialsAndIdentityId?.(),
undefined
);

assert.deepEqual(resourceConfig, {
API: {
GraphQL: {
endpoint: 'TEST_VALUE for AMPLIFY_DATA_GRAPHQL_ENDPOINT',
region: 'TEST_VALUE for AWS_REGION',
defaultAuthMode: 'iam',
modelIntrospection: { testSchema: 'TESTING' },
},
},
});
});
});
Loading
Loading