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: Multi bucket support #1742

Merged
merged 102 commits into from
Aug 7, 2024
Merged

feat: Multi bucket support #1742

merged 102 commits into from
Aug 7, 2024

Conversation

0618
Copy link
Contributor

@0618 0618 commented Jul 15, 2024

Problem

This PR allow users to create more than one S3 bucket via Amplify Storage.

User Story

  • In amplify/storage/resource.ts...
import { defineStorage } from '@aws-amplify/backend';

export const storageOne = defineStorage({
  name: 'amplifyTeamDrive-one'
  isDefault: true
    access: (allow) => ({
    'profile-pictures/{entity_id}/*': [
      allow.guest.to(['read']),
      allow.entity('identity').to(['read', 'write', 'delete'])
    ],
    'picture-submissions/*': [
      allow.authenticated.to(['read','write']),
      allow.guest.to(['read', 'write'])
    ],
  })
});

export const storageTwo = defineStorage({
  name: 'amplifyTeamDrive-two'
});
export const storageThree = defineStorage({
  name: 'amplifyTeamDrive-three'
});
  • In amplify/backend.ts...
import { defineBackend } from '@aws-amplify/backend';
import { auth } from './auth/resource';
import { storageOne, storageTwo, StorageThree } from './storage/resource';

defineBackend({
  auth,
  storageOne,
  storageTwo,
  storageThree,
});
  • throw error if no default and multiple default buckets

Screenshot 2024-07-15 at 21 38 17

Screenshot 2024-07-15 at 21 43 23

Issue number, if available:
fixes #1699

Changes

  • update schema_v1 and cliend_config_v1 to v1.1
  • modify BackendOutput type
  • rename policy to add more policies
  • allow more buckets to be added

Corresponding docs PR, if applicable:

Validation

Checklist

  • If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
  • If this PR requires a change to the Project Architecture README, I have included that update in this PR.
  • If this PR requires a docs update, I have linked to that docs PR above.
  • If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

changeset-bot bot commented Jul 15, 2024

🦋 Changeset detected

Latest commit: 079fd73

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@aws-amplify/deployed-backend-client Minor
@aws-amplify/backend-output-schemas Minor
@aws-amplify/backend-output-storage Minor
@aws-amplify/backend-storage Minor
@aws-amplify/client-config Minor
@aws-amplify/plugin-types Minor
@aws-amplify/backend Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@0618 0618 marked this pull request as ready for review July 16, 2024 16:01
@0618 0618 requested review from a team as code owners July 16, 2024 16:01
@@ -275,24 +275,29 @@ export const unifiedBackendOutputSchema: z.ZodObject<{
payload: z.ZodObject<{
bucketName: z.ZodString;
storageRegion: z.ZodString;
buckets: z.ZodOptional<z.ZodArray<z.ZodRecord<z.ZodString, z.ZodString>, "many">>;
Copy link
Member

Choose a reason for hiding this comment

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

why typing for buckets is different here than in other places ?

additionally. should we use records here or should be come up with subtype with named properties? (this applies to all buckets introduced in this file)

Copy link
Contributor Author

@0618 0618 Jul 19, 2024

Choose a reason for hiding this comment

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

Done d67854e

@@ -1,9 +1,12 @@
import { z } from 'zod';

const bucketSchema = z.record(z.string(), z.string());
Copy link
Member

Choose a reason for hiding this comment

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

should we type this stronger as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done d67854e

Comment on lines 28 to 31
outputStorageStrategy?: BackendOutputStorageStrategy<{
version: '1';
payload: Record<string, string>;
}>;
Copy link
Member

Choose a reason for hiding this comment

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

why this change?
Shouldn't we change StorageOutput type ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

StorageOutput is from schema, which is the final output, but the initial output we stored in stack metadata is different, so I introduced a new type, StorageOutputToStore in a new commit f6a1c12

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See how the new commit f6a1c12 looks?

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why do we need type StorageOutputToStore .

Auth and Functions are just using their respective outputs, see:

outputStorageStrategy: BackendOutputStorageStrategy<AuthOutput> = new StackMetadataBackendOutputStorageStrategy(

and
private readonly outputStorageStrategy: BackendOutputStorageStrategy<FunctionOutput>

Copy link
Member

Choose a reason for hiding this comment

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

StorageOutput is from schema

StorageOutput is a shape that we store in CFN metadata/outputs. This is not related to client config schemas - that translation happens when client config package reads stacks and transforms it to amplify outputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There would be some mis-match after CfnOutput sending the output (code), then the output we get from CFN would be like the following

{
 version: "1",
 payload: {
   name: [..., ..., ...],
   bucketName: [..., ..., ...],
   storageRegion: [..., ..., ...],
 }
}

So we need to change the type, and modify the output when parsing to amplify_outputs.json (code).

Is there a way to avoid the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tl;dr -- When we store output, we store the individual bucket (code) (which has name, bucketName, and storageRegion), but when generate the amplify_outputs.json, we also need to add buckets, and remove name etc. So, there are two different storageOutput .

There might be a way to avoid the complex, and I missed it.

Copy link
Member

Choose a reason for hiding this comment

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

The way we should evolve this is
Change

export const storageOutputSchema = z.object({
  version: z.literal('1'),
  payload: z.object({
    bucketName: z.string(),
    storageRegion: z.string(),
  }),
});

To

export const storageOutputSchema = z.object({
  version: z.literal('1'),
  payload: z.object({
    bucketName: z.string(),
    storageRegion: z.string(),
    buckets: z.string(), // JSON array as string (serialized buckets, like in functions output), this should likely be optional.
  }),
});

And then in order for this to be backwards compatible we should populate bucketName and storageRegion with the one that's 'isDefault = true'.

Copy link
Member

Choose a reason for hiding this comment

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

And then use this trick

outputStorageStrategy.appendToBackendOutputList(functionOutputKey, {
version: '1',
payload: {
definedFunctions: this.resources.lambda.functionName,
},
});
to append to buckets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done d67854e

Comment on lines 39 to 51
const metadataValue =
this.stack.templateOptions.metadata &&
this.stack.templateOptions.metadata[keyName]
? this.stack.templateOptions.metadata[keyName]
: { stackOutputs: [] };

this.stack.addMetadata(keyName, {
...metadataValue,
version: backendOutputEntry.version,
stackOutputs: Object.keys(backendOutputEntry.payload),
stackOutputs: [
...metadataValue.stackOutputs,
...Object.keys(backendOutputEntry.payload),
],
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain this change?

Edit: after reading rest of the code I'm guessing this is to solve a problem of "multiple defineStorage contributing to same stack metadata/output key".
I was facing similar problem in the POC I was building for custom client config. This wasn't merged as final approach was different but you might find it useful for this PR, see #946 (comment) (i.e. addBackendOutputEntry and logic around it).

Copy link
Contributor

@rtpascual rtpascual Jul 17, 2024

Choose a reason for hiding this comment

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

Could we maybe use appendToBackendOutputList (see here) to lazily append multiple buckets to the same output key similar to what is done for functions? See here where we are lazily adding function names to a list as they are synthesized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point @rtpascual! I somehow didn't see appendToBackendOutputList in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done e8548cf

@@ -122,7 +127,7 @@ export class AmplifyStorage
},
};

this.storeOutput(props.outputStorageStrategy);
this.storeOutput(props.outputStorageStrategy, props.isDefault || false);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.storeOutput(props.outputStorageStrategy, props.isDefault || false);
this.storeOutput(props.outputStorageStrategy, props.isDefault ?? false);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isDefault can't be 0 or "" since it's set to be boolean type, so I think || is appropriate here. Even if it was 0 or "", I think we can assume it means false.

Comment on lines 160 to 164
/* The following code can guarantee there's only one `isDefault`
* because if we can only create one construct with the same `storageRegion` and `bucketName` name.
* The default bucket takes the `storageRegion` and `bucketName` name without a number post-fix.
*/
const num = isDefault ? '' : Math.floor(Math.random() * 100);
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't use random generator here. This will cause re-deployments of storage as outputs are going to change every time impacting performance (e.g. this will cause sandbox never hotswapping).
Instead of this we should either apply consistent hashing OR have global counter for storage instances.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Yea, I'm looking for some suggestions to replace the random generator. I'll try those.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should do the same thing that we do for auth and data:

keep a static counter in the factory class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done cddcd97

});
return new Policy(
this.stack,
`${this.bucket.bucketName.slice(0, 6)}${Math.floor(
Copy link
Member

Choose a reason for hiding this comment

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

same here, this needs consistent hashing or global counter.

Copy link
Contributor Author

@0618 0618 Jul 19, 2024

Choose a reason for hiding this comment

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

done 2f932ec

Copy link
Contributor Author

@0618 0618 Jul 19, 2024

Choose a reason for hiding this comment

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

  • need to revisit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Using global counter ec113d0

Comment on lines 33 to 34
...(output[storageOutputKey] && {
[storageOutputKey]: this.parseStorageOutput(output[storageOutputKey]),
Copy link
Member

Choose a reason for hiding this comment

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

it is odd to have storage output parsing being so specific.
can't this be solved by modeling schema properly?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 this should all be handled by zod if we model the type properly

Copy link
Contributor

@edwardfoyle edwardfoyle Jul 17, 2024

Choose a reason for hiding this comment

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

Coming back to this, I think you may be conflating reading backend output and formatting client config. This class should only be responsible for reading stack output and metadata and reconstituting it into a validated and typed object. Then the client config contributors take that object and format it however is needed for the frontend. That happens here for storage:

Comment on lines 55 to 62
const hasDefaultBucket = bucketNames.includes('bucketName');
if (!hasDefaultBucket && bucketNames.length > 1) {
throw new AmplifyUserError('NoDefaultBucketError', {
message: 'No default bucket set in the Amplify project.',
resolution:
'Add `isDefault: true` to one of the buckets `defineStorage` in the Amplify project.',
});
}
Copy link
Member

Choose a reason for hiding this comment

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

this validation should happen at synthesis time. not here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. This is the comment I've been waiting for :p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 1b08c09

@@ -74,10 +74,10 @@ export type BackendIdentifier = {
};

// @public (undocumented)
export type BackendOutput = Record<string, BackendOutputEntry>;
export type BackendOutput<T = Record<string, string>> = Record<string, BackendOutputEntry<T>>;
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because buckets is Record<string, string>[], but I can't set it in the type since it breaks other categories.

Parameter T was introduced so that we can restrict it only for Storage.

Copy link
Member

Choose a reason for hiding this comment

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

I think by adapting the concept suggested here #1742 (comment) we could nest buckets under single storage output and avoid this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The doesn't affect here because the buckets array will be under single storage, which is not acceptable for BackendOutput before the current change with T. I saw the following error:

Type '{ name: string; bucket_name: string; aws_region: string; }[]' is not assignable to type 'string'.

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should do something like this

definedFunctions: z.string(), // JSON array as string
.

Copy link
Contributor Author

@0618 0618 Jul 19, 2024

Choose a reason for hiding this comment

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

  • need to figure out why API.md doesn't update properly

@@ -32,6 +29,11 @@ const storageStackType = 'storage-S3';
export type AmplifyStorageTriggerEvent = 'onDelete' | 'onUpload';

export type AmplifyStorageProps = {
/**
* Whether this storage resource is the default storage resource for the backend
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: also add some wording here explaining that it's required if there are multiple storage definitions in the backend

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done
bc4d412

@@ -32,6 +29,11 @@ const storageStackType = 'storage-S3';
export type AmplifyStorageTriggerEvent = 'onDelete' | 'onUpload';

export type AmplifyStorageProps = {
/**
* Whether this storage resource is the default storage resource for the backend
* @default false
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't the default true? ie if I just have one storage instance and I don't define this field, then it's the default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I don't know if the default should be true or false because it's conditional. If there's only one bucket, then it's true. If multiple buckets, it's false. Maybe I should update the JSdoc to clarify the conditions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done
bc4d412

Comment on lines 48 to 49
version: '1';
payload: Record<string, string>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think @sobolk had a similar comment elsewhere, but we should keep this strongly typed, even if that means updating the type

Comment on lines 160 to 164
/* The following code can guarantee there's only one `isDefault`
* because if we can only create one construct with the same `storageRegion` and `bucketName` name.
* The default bucket takes the `storageRegion` and `bucketName` name without a number post-fix.
*/
const num = isDefault ? '' : Math.floor(Math.random() * 100);
Copy link
Contributor

Choose a reason for hiding this comment

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

We should do the same thing that we do for auth and data:

keep a static counter in the factory class

Comment on lines 33 to 34
...(output[storageOutputKey] && {
[storageOutputKey]: this.parseStorageOutput(output[storageOutputKey]),
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 this should all be handled by zod if we model the type properly

Comment on lines 33 to 34
...(output[storageOutputKey] && {
[storageOutputKey]: this.parseStorageOutput(output[storageOutputKey]),
Copy link
Contributor

@edwardfoyle edwardfoyle Jul 17, 2024

Choose a reason for hiding this comment

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

Coming back to this, I think you may be conflating reading backend output and formatting client config. This class should only be responsible for reading stack output and metadata and reconstituting it into a validated and typed object. Then the client config contributors take that object and format it however is needed for the frontend. That happens here for storage:

/**
* StorageValidator class implements the IAspect interface.
*/
export class StorageOutputsAspect implements IAspect {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: update comment and move to a separate file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. 0940ed6

});
return new Policy(
this.stack,
`storageAccess${this.stack.node.children.length}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

just to make sure I'm understanding this correctly, this length is guaranteed to be different each time this method is called because more nodes will have been added to the stack in-between each call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, that's right.

@@ -535,22 +535,37 @@ void describe('storage client config contributor v1', () => {

void it('returns translated config when output has auth', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not strictly part of this PR, but looks like this test name needs an update

@@ -12,7 +12,7 @@ import {
clientConfigTypesV1,
} from '../client-config-types/client_config.js';
import { ModelIntrospectionSchemaAdapter } from '../model_introspection_schema_adapter.js';
import { AwsAppsyncAuthorizationType } from '../client-config-schema/client_config_v1.js';
import { AwsAppsyncAuthorizationType } from '../client-config-schema/client_config_v1.1.js';
Copy link
Contributor

Choose a reason for hiding this comment

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

I probably don't have all the context around this, but why are we renaming the schema file to v1.1? It doesn't look like we are changing the version number in the schema, so it seems like this file name should stay the same as well

Copy link
Contributor Author

@0618 0618 Aug 6, 2024

Choose a reason for hiding this comment

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

We are changing the schema version from v1 to v1.1 in this PR as well.

@0618 0618 dismissed stale reviews from Amplifiyer and sobolk via 9ade76a August 6, 2024 18:12
@0618 0618 closed this Aug 7, 2024
@0618 0618 reopened this Aug 7, 2024
@0618
Copy link
Contributor Author

0618 commented Aug 7, 2024

@0618 0618 enabled auto-merge (squash) August 7, 2024 18:27
@0618 0618 merged commit d8b43d2 into aws-amplify:main Aug 7, 2024
83 of 105 checks passed
@0618 0618 deleted the multi-bucket-support branch August 7, 2024 22:49
@0618 0618 restored the multi-bucket-support branch August 8, 2024 18:51
0618 added a commit that referenced this pull request Aug 8, 2024
sobolk pushed a commit that referenced this pull request Aug 8, 2024
* Revert "feat: Multi bucket support (#1742)"

This reverts commit d8b43d2.

* add changeset
sobolk added a commit that referenced this pull request Aug 12, 2024
* update cdk error mapper to catch all deployment errors (#1826)

* update cdk error mapper to catch all deployment errors

* add package-loc

* prevent ctrl+c handling for pnpm (#1818)

* feat: Multi bucket support (#1742)

* feat: init multi-bucket support

* refactor: BackendOutput type

* update package-lock

* temp

* feat: update schema and client_config

* feat: increment schema and client_config versions to 1.1

* rename to buckets and remove friendlyName

* chore: add changeset

* chore: update API.md

* fix: remove duplicated code

* fix: construct test

* feat: validate isDefault

* chore: remove as any

* test: fix client_config_contributor

* chore: update package-lock

* fix: getOutput

* refactor: backend_output_client

* refactor: DefaultBackendOutputClient

* revert changes on addBackendOutputEntry, use appendToBackendOutputList

* update JSdocs for construct

* make storageOutput more specific

* add factoryCounter and hasDefault

* use factoryCounter as postfix

* update changeset

* fix: buckets output schema

* feat: add name to buckets

* feat: throw no isdefault error before deploy

* chore: remove post-deploy error

* refactor: use factoryCounter for policy

* refactor: pass buckets to appendToBackendOutputList

* remove StorageOutputPayloadToStore

* fix construct test

* revert BackendOutput type

* chore: update changeset

* fix: client_config_contributer_v1 and test

* fix: pin api-extractor to 7.40.0

* chore: update API.md

* refactor: use global factoryCounter for policy

* refactor: use addBackendOutputEntry for default

* test: fix storage construct test

* this works

* move DeepPartial

* remove StorageBucketsPayload

* refactor: use Aspects to validate storage

* fix: add buckets to metadata

* fix: factory unit test

* fix: metadata_output_storage_strategy unit test

* fix: not export AmplifyStorage

* refactor: addOrUpdateMetadata

* update package-lock

* update package-lock

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* refactor: remove redundant code in StorageClientConfigContributor

* test: add unit tests for construct and factory

* fix: one bucket no default case

* fix: unit test

* test log currentCodebaseOutputs and npmOutputs

* fix: convert bucketName, storageRegion into bucket_name, aws_region

* fix: client_config_contributor_v1.test

* refactor: use Aspect to replact factory static

* fix: buckets keys

* fix: unit test

* fix MultipleDefaultBucketError and add more unit tests

* remove policyCount static

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* refactor StorageValidator

* Refactor: storeOutput to factory

* remove comment

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/construct.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Revert "test log currentCodebaseOutputs and npmOutputs"

This reverts commit 4ce8db5.

* rename defaultStorageFound

* remove firstStorage from getInstance

* rename buckets to storage, and fix unit tests

* use isStorageProcessed

* fix construct test

* fix StackMetadataBackendOutputStorageStrategy unit test

* fix a typo to make lint happy

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* Update packages/client-config/src/client-config-contributor/client_config_contributor_v1.ts

Co-authored-by: Edward Foyle <[email protected]>

* fix type error in client_config_contributer_v1

* refactor: use early return

* test: fix test name

* refactor: move Aspects to another file

* refactor: remove this.node

* Update packages/backend-storage/src/storage_outputs_aspect.ts

Co-authored-by: Amplifiyer <[email protected]>

* add isMatch to match outputs

* add unit test for aspects

---------

Co-authored-by: Kamil Sobol <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Kamil Sobol <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>

* chore(deps): bump fast-xml-parser, @aws-sdk/client-amplify, @aws-sdk/client-cloudformation, @aws-sdk/client-cognito-identity-provider, @aws-sdk/client-dynamodb, @aws-sdk/client-iam, @aws-sdk/client-s3 and @aws-sdk/client-ssm (#1836)

Bumps [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser) to 4.4.1 and updates ancestor dependencies [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser), [@aws-sdk/client-amplify](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-amplify), [@aws-sdk/client-cloudformation](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-cloudformation), [@aws-sdk/client-cognito-identity-provider](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-cognito-identity-provider), [@aws-sdk/client-dynamodb](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-dynamodb), [@aws-sdk/client-iam](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-iam), [@aws-sdk/client-s3](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-s3) and [@aws-sdk/client-ssm](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-ssm). These dependencies need to be updated together.


Updates `fast-xml-parser` from 4.1.2 to 4.4.1
- [Release notes](https://github.com/NaturalIntelligence/fast-xml-parser/releases)
- [Changelog](https://github.com/NaturalIntelligence/fast-xml-parser/blob/master/CHANGELOG.md)
- [Commits](NaturalIntelligence/fast-xml-parser@v4.1.2...v4.4.1)

Updates `@aws-sdk/client-amplify` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-amplify/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-amplify)

Updates `@aws-sdk/client-cloudformation` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-cloudformation/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-cloudformation)

Updates `@aws-sdk/client-cognito-identity-provider` from 3.614.0 to 3.625.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-cognito-identity-provider/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.625.0/clients/client-cognito-identity-provider)

Updates `@aws-sdk/client-dynamodb` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-dynamodb/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-dynamodb)

Updates `@aws-sdk/client-iam` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-iam/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-iam)

Updates `@aws-sdk/client-s3` from 3.614.0 to 3.626.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-s3/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.626.0/clients/client-s3)

Updates `@aws-sdk/client-ssm` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-ssm/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-ssm)

---
updated-dependencies:
- dependency-name: fast-xml-parser
  dependency-type: indirect
- dependency-name: "@aws-sdk/client-amplify"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-cloudformation"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-cognito-identity-provider"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-dynamodb"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-iam"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-s3"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-ssm"
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "feat: Multi bucket support" (#1837)

* Revert "feat: Multi bucket support (#1742)"

This reverts commit d8b43d2.

* add changeset

* upgrade AWS SDK packages to latest (#1839)

* Split sandbox e2e tests (#1841)

* Handle nested namespaces in api check (#1842)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Roshane Pascual <[email protected]>
Co-authored-by: MJ Zhang <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
sobolk added a commit that referenced this pull request Sep 4, 2024
… conversation handler (#1948)

* Add `ai-constructs` and default implementation of conversation handler (#1822)

* add backend-ai package

* commonjs

* default handler - wip

* fix bedrock version

* fix that

* e2e test

* update response type

* update response type

* make code testable

* some PR feedback

* rename construct package

* top level try catch

* construct unit tests

* lint

* more tests

* ehhh

* adapter test

* more tests

* addressing TODOs

* readme

* undo that

* Add Bedrock tools to conversation handler (#1830)

* add tools

* changeset

* add e2e test for programmatic tool

* add e2e test for data tool

* lint

* adapter tests

* refactor event tools

* refactor event tools

* gql tool test

* provider test

* factory test

* detect all duplicates

* pr feedback.

* Merge main to conversation handler (#1846)

* update cdk error mapper to catch all deployment errors (#1826)

* update cdk error mapper to catch all deployment errors

* add package-loc

* prevent ctrl+c handling for pnpm (#1818)

* feat: Multi bucket support (#1742)

* feat: init multi-bucket support

* refactor: BackendOutput type

* update package-lock

* temp

* feat: update schema and client_config

* feat: increment schema and client_config versions to 1.1

* rename to buckets and remove friendlyName

* chore: add changeset

* chore: update API.md

* fix: remove duplicated code

* fix: construct test

* feat: validate isDefault

* chore: remove as any

* test: fix client_config_contributor

* chore: update package-lock

* fix: getOutput

* refactor: backend_output_client

* refactor: DefaultBackendOutputClient

* revert changes on addBackendOutputEntry, use appendToBackendOutputList

* update JSdocs for construct

* make storageOutput more specific

* add factoryCounter and hasDefault

* use factoryCounter as postfix

* update changeset

* fix: buckets output schema

* feat: add name to buckets

* feat: throw no isdefault error before deploy

* chore: remove post-deploy error

* refactor: use factoryCounter for policy

* refactor: pass buckets to appendToBackendOutputList

* remove StorageOutputPayloadToStore

* fix construct test

* revert BackendOutput type

* chore: update changeset

* fix: client_config_contributer_v1 and test

* fix: pin api-extractor to 7.40.0

* chore: update API.md

* refactor: use global factoryCounter for policy

* refactor: use addBackendOutputEntry for default

* test: fix storage construct test

* this works

* move DeepPartial

* remove StorageBucketsPayload

* refactor: use Aspects to validate storage

* fix: add buckets to metadata

* fix: factory unit test

* fix: metadata_output_storage_strategy unit test

* fix: not export AmplifyStorage

* refactor: addOrUpdateMetadata

* update package-lock

* update package-lock

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* refactor: remove redundant code in StorageClientConfigContributor

* test: add unit tests for construct and factory

* fix: one bucket no default case

* fix: unit test

* test log currentCodebaseOutputs and npmOutputs

* fix: convert bucketName, storageRegion into bucket_name, aws_region

* fix: client_config_contributor_v1.test

* refactor: use Aspect to replact factory static

* fix: buckets keys

* fix: unit test

* fix MultipleDefaultBucketError and add more unit tests

* remove policyCount static

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* refactor StorageValidator

* Refactor: storeOutput to factory

* remove comment

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/construct.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Revert "test log currentCodebaseOutputs and npmOutputs"

This reverts commit 4ce8db5.

* rename defaultStorageFound

* remove firstStorage from getInstance

* rename buckets to storage, and fix unit tests

* use isStorageProcessed

* fix construct test

* fix StackMetadataBackendOutputStorageStrategy unit test

* fix a typo to make lint happy

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* Update packages/client-config/src/client-config-contributor/client_config_contributor_v1.ts

Co-authored-by: Edward Foyle <[email protected]>

* fix type error in client_config_contributer_v1

* refactor: use early return

* test: fix test name

* refactor: move Aspects to another file

* refactor: remove this.node

* Update packages/backend-storage/src/storage_outputs_aspect.ts

Co-authored-by: Amplifiyer <[email protected]>

* add isMatch to match outputs

* add unit test for aspects

---------

Co-authored-by: Kamil Sobol <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Kamil Sobol <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>

* chore(deps): bump fast-xml-parser, @aws-sdk/client-amplify, @aws-sdk/client-cloudformation, @aws-sdk/client-cognito-identity-provider, @aws-sdk/client-dynamodb, @aws-sdk/client-iam, @aws-sdk/client-s3 and @aws-sdk/client-ssm (#1836)

Bumps [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser) to 4.4.1 and updates ancestor dependencies [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser), [@aws-sdk/client-amplify](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-amplify), [@aws-sdk/client-cloudformation](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-cloudformation), [@aws-sdk/client-cognito-identity-provider](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-cognito-identity-provider), [@aws-sdk/client-dynamodb](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-dynamodb), [@aws-sdk/client-iam](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-iam), [@aws-sdk/client-s3](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-s3) and [@aws-sdk/client-ssm](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-ssm). These dependencies need to be updated together.


Updates `fast-xml-parser` from 4.1.2 to 4.4.1
- [Release notes](https://github.com/NaturalIntelligence/fast-xml-parser/releases)
- [Changelog](https://github.com/NaturalIntelligence/fast-xml-parser/blob/master/CHANGELOG.md)
- [Commits](NaturalIntelligence/fast-xml-parser@v4.1.2...v4.4.1)

Updates `@aws-sdk/client-amplify` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-amplify/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-amplify)

Updates `@aws-sdk/client-cloudformation` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-cloudformation/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-cloudformation)

Updates `@aws-sdk/client-cognito-identity-provider` from 3.614.0 to 3.625.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-cognito-identity-provider/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.625.0/clients/client-cognito-identity-provider)

Updates `@aws-sdk/client-dynamodb` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-dynamodb/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-dynamodb)

Updates `@aws-sdk/client-iam` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-iam/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-iam)

Updates `@aws-sdk/client-s3` from 3.614.0 to 3.626.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-s3/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.626.0/clients/client-s3)

Updates `@aws-sdk/client-ssm` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-ssm/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-ssm)

---
updated-dependencies:
- dependency-name: fast-xml-parser
  dependency-type: indirect
- dependency-name: "@aws-sdk/client-amplify"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-cloudformation"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-cognito-identity-provider"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-dynamodb"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-iam"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-s3"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-ssm"
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "feat: Multi bucket support" (#1837)

* Revert "feat: Multi bucket support (#1742)"

This reverts commit d8b43d2.

* add changeset

* upgrade AWS SDK packages to latest (#1839)

* Split sandbox e2e tests (#1841)

* Handle nested namespaces in api check (#1842)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Roshane Pascual <[email protected]>
Co-authored-by: MJ Zhang <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make conversation response selection set dynamic (#1848)

* Make conversation selection set dynamic

* grrr

* UI tools (#1853)

* add client tools to schema

* api

* logic and unit tests

* e2e

* pr feedback

* Add `defineConversationHandlerFunction` and `@aws-amplify/backend-ai` package. (#1858)

* Add backend-ai

* factory

* this works

* changeset

* api

* fix that

* boop

* add unit test

* readme

* some pr feedback

* Serialize tool us blocks (#1866)

* stringify tool input

* stringify tool input

* Adjust API.md generation in backend-ai for api check to work. (#1869)

* Adjust API.md generation in backend-ai for api check to work.

* changeset

* Add main entry point. (#1870)

* fix e2e test

* Add ability to pass inference configuration (#1878)

* Add ability to pass inference configuration

* Add ability to pass inference configuration

* fix(ai-constructs): support no input graphql query tools (#1907)

* fix(ai-constructs): no input graphql query tools

* add changeset

* empty

* fix api diff

* fix tool query assertion

* lets try this again

* Api adjustments for conversation handler (#1938)

* accept models from schema def

* api updates

* api updates

* Squash changeset on feature/conversation-handler branch and fix api check (#1949)

* squash changeset

* fix api check

* grrr

* handle usage of subnamespaced types in api check (#1952)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Roshane Pascual <[email protected]>
Co-authored-by: MJ Zhang <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ian Saultz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-e2e Label that will include e2e tests in PR checks workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There is already a Construct with name 'storageRegion' in AmplifyStack
5 participants