aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.47k stars 3.83k forks source link

aws-appsync: javascript resolver support #22921

Closed mtliendo closed 1 year ago

mtliendo commented 1 year ago

Describe the feature

Based on this RFC, it would nice to explore what a CDK supported candidate would look like.

Use Case

I'd like the experimental L2 construct to support the JS runtime for AppSync resolvers

Proposed Solution

the datasource.createResolver() method should accept a code argument, a specified runtime, and make the mappingTemplates optional.

From this:

const appsyncFunction = new appsync.AppsyncFunction(this, 'function', {
  name: 'appsync_function',
  api,
  dataSource: api.addNoneDataSource('none'),
  requestMappingTemplate: appsync.MappingTemplate.fromFile('request.vtl'),
  responseMappingTemplate: appsync.MappingTemplate.fromFile('response.vtl'),
});

const pipelineResolver = new appsync.Resolver(this, 'pipeline', {
  api,
  dataSource: api.addNoneDataSource('none'),
  typeName: 'Query',
  fieldName: 'getDemos',
  requestMappingTemplate: appsync.MappingTemplate.fromFile('beforeRequest.vtl'),
  pipelineConfig: [appsyncFunction],
  responseMappingTemplate: appsync.MappingTemplate.fromFile('afterResponse.vtl'),
});

To this:

const appsyncFunction = new appsync.AppsyncFunction(this, 'function', {
  name: 'appsync_function',
  api, 
  dataSource: api.addNoneDataSource('none'),
  runtime:  appsync.Runtime.APPSYNC_JS,
  code: appsync.Code.fromAsset(path.join(__dirname, 'fn1.js'))
});

const pipelineResolver = new appsync.Resolver(this, 'pipeline', {
  api,
  dataSource: api.addNoneDataSource('none'),
  typeName: 'Query',
  fieldName: 'getDemos',
  code: appsync.Code.fromAsset(path.join(__dirname, 'pipelineMappings.js'))
  pipelineConfig: [appsyncFunction],
});

Other Information

No response

Acknowledgements

CDK version used

2.50.0

Environment details (OS name and version, etc.)

macOS

peterwoodworth commented 1 year ago

We're bound by the CloudFormation API which requires a template within the CloudFormation template, or requires the template to reside in S3. I'm not sure how exactly we'd be able to implement this feature, or how this would be useful given the CloudFormation requirements. Could you explain exactly how this would be useful and how we might be able to accomplish this feature?

garretcharp commented 1 year ago

Not really sure how cloudformation actually works but this would be the absolute most useful feature for appsync. VTL is really annoying to work with compared to javascript

mtliendo commented 1 year ago

We on the AppSync team would be happy to help push this PR through based on our new release today 😊

https://aws.amazon.com/blogs/aws/aws-appsync-graphql-apis-supports-javascript-resolvers/

piotrmoszkowicz commented 1 year ago

According to https://github.com/aws/aws-cdk/blob/04baba88b4e27e19b7d0ab237ff641043d089edc/packages/%40aws-cdk/cfnspec/spec-source/specification/000_cfn/000_official/000_AWS_AppSync.json which is currently being merged, there's Code, CodeS3Location and Runtime (with Name and RuntimeVersion properties), which could be used for that. I assume we can implement it. I can volunteer for that, but could do Sunday / beginning of next week, but would love to help!

piotrmoszkowicz commented 1 year ago

Also for Code - we could partially take a look on https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda.Code.html and copy part of it (ie. fromInline, fromAsset, fromDockerImage and fromBucket). Also would be nice to take a look at https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_lambda_nodejs-readme.html and enable bundling via esbuild - I would love to have that feature!

fab-mindflow commented 1 year ago

👍 strongly interested in this support at our startup already heavily using AppSync w/ CDK.

mtliendo commented 1 year ago

@piotrmoszkowicz I have a CDK repo up (can finally make public 😅 ) that uses the L1 construct in conjunction with the addOverrides property to enable support for JS resolvers. The overrides are only needed due to the latest CloudFormation updates not being merged into the CDK yet (I think this may happen some time next week)

The main points to call out are when I create a function, add the overrides and then create create the pipeline, then and those overrides.

Would you be interested in making this contribution to the L2 alpha construct?

piotrmoszkowicz commented 1 year ago

@mtliendo Yes I will be interested! Could you pass me possible values for runtime (both name and version)? As CF docs are not yet published I can't look it up 😅

mtliendo commented 1 year ago

APPSYNC_JS is a string and the only runtime at the moment, but should be an enum to support other things that may come later 😉

'1.0.0' for the version. It's represented as a string and should stay a string.

Also, as seen in the repo, JS files have to be passed as string, so I'm using fs.readFile but if there's a more elegant way (like how the graphql file gets turned into a string) then that would be great to have.

You're awesome, thanks so much for helping to bring this in and please let me know how I can help!

piotrmoszkowicz commented 1 year ago

@mtliendo So we just keep Runtime Name as ENUM, how about VTL? Because I would love to have statically typed solution, which basically tells you to specify runtime when you specify code and same approach for VTL 😊 I guess for VTLs nothing have change, yes?

mtliendo commented 1 year ago

You got it :) For VTL the process is the same. JS resolver support is opt-in by specifying a JS file to the Code property.

But your right, it would be nice to get type checking on that 🤔 I could be wrong, but this may actually be a good use case for the never keyword based off of what I've seen others do.

mtliendo commented 1 year ago

Cloudformation docs are updated :) Im told an automated PR should be available in less than a week to update the L1 construct.

piotrmoszkowicz commented 1 year ago

Awesome, will check that!

riywo commented 1 year ago

Any chance to get this update? L1 is just fine for me.

piotrmoszkowicz commented 1 year ago

I hope to work on that during this weekend :)

MrArnoldPalmer commented 1 year ago

L1 support is landed and myself and @mtliendo are working on an implementation for the L2s. PRs also welcome if anyone gets to it in the meantime.

mattiLeBlanc commented 1 year ago

Firstly, thank you for this implementation of this great new improvement of Appsync.

Can anyone give some examples for L1 in TS code for adding a resolver? And also for L2 when ready? I assume it will take some time for docs to be updated.

mtliendo commented 1 year ago

@mattiLeBlanc I got you! https://github.com/mtliendo/appsync-jsresolver-test/blob/main/lib/appsync-jsresolver-test-stack.ts

mtliendo commented 1 year ago

@mattiLeBlanc keep in mind that it’s still possible to use the L2 + the escape hatches: https://github.com/mtliendo/reinvent-chat/tree/main/lib/graphql

mattiLeBlanc commented 1 year ago

@mtliendo Wonderful! Thank you so much.

mattiLeBlanc commented 1 year ago

@mtliendo So just to be clear, is this production ready or should I rely on VTL resolvers for now and wait for this to mature a bit more?

mtliendo commented 1 year ago

JS resolvers shipped production ready with no loss in functionality or increase in latency 🙂

the L1 construct is updated however many folks use the L2 construct ( rightfully so), and that’s what this weeks PR will be targeting.

github-actions[bot] commented 1 year ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

mattiLeBlanc commented 1 year ago

@mtliendo Hi, thanks for you hard work! Can you explain to me the difference with L1 and L2 constructs? Which one are the higher level ones? I one of the examples (https://github.com/mtliendo/reinvent-chat/blob/main/lib/graphql/utils/index.ts) you reference L1 constructs as an escape hatch and having to define the JS Runtime. It looks like L1 is the lower level construct and we should use the higher level, new AppsyncFunction() ? right?

MrArnoldPalmer commented 1 year ago

@mattiLeBlanc L2s are higher level than L1s. L1s are direct 1:1 representations of the underlying cloudformation resources. Escape hatches are a mechanism for usually used to manipulate the L1 constructs that are composed within an L2.

Here are some docs explaining the relevant concepts: constructs escape hatches

L2s are generally recommended but they can be opinionated so if you disagree with those built in opinions, L1s can sometimes be better for your use case. In the case of AppSync though the L2s don't really layer in too much beyond just convenience, IE bundling your resolver code etc.

danrivett commented 1 year ago

Following on from a great article from @bboure about a possible way to share common local functions across multiple JavaScript resolvers using esbuild, I was wondering how feasible it is to do with a CDK deployed Javascript Resolver?

It would be fantastic to support this, as it's really the main missing piece I am facing. I'm currently copying and pasting common functions between resolvers as I'm not sure how else to automate running esbuild on a cdk deploy (#17377 ?).

Given Lambda functions (NodejsFunction) has built in support to configure esbuild bundling, I'm hopefuly my request isn't too far off base, but I welcome alternative suggestions.

Update: I see #24548 has been created already which looks to have raised this question already.

bboure commented 1 year ago

Thanks @danrivett for the mention and reference. While this isn't supported in the CDK officially, I have a follow up article that I will publish hopefully soon (it's not quite ready yet) that covers this.

TL;DR; you can bundle the code yourself (That's an example with TypeScript, but it should work just fine with JS too)

danrivett commented 1 year ago

That's fantastic @bboure, thank you so much for pointing me to your example, that is really helpful. I'll give it a go later today and also look forward to your blog in the future.

We largely use TypeScript over JavaScript, but I reverted to JavaScript for our AppSync JavaScript resolvers as I wasn't clear how best to support transpiling TS to JS via our CDK-managed stack.

Your solution looks really simple. I see it uses Code.fromInline(). I think that has a limitation that the code is maximum 4KiB in size (see here).

That likely isn't a huge issue, but I wondered if there is a simple way to use Code.fromAsset() instead? I assume just having esbuild write to a local dist directory and referencing that would be all that's needed.

In that case I hope your example will also provide inspiration to AWS to to officially support this, as it would be great to simplify and standardize that support because I think it would really super-charge AppSync JavaScript resolvers adding both local imports and TypeScript support.

Perhaps they could add Code.buildAsset(filePath, esBuildOptions), that would be great.

bboure commented 1 year ago

Thanks @danrivett I wasn't aware of this limit.

If this becomes an issue, an alternative it to pre-build the resolvers and use fromAsset(), yes.

esbuild src/*.ts \
  --bundle \
  --outdir=build \
  --external:"@aws-appsync/utils" \
  --format=esm
  --target=es2020

then point fromAsset to the build directory.

 code: Code.fromAsset('build/resolver.js'),

tip: change cdk.json to

"app": "esbuild src/*.ts ..... && npx ts-node --prefer-ts-exts --project=tsconfig.json ./src/index.ts ",
lysachok commented 1 year ago

any ideas why this approach (code: Code.fromAsset('build/resolver.js')) leads to the error:

The specified bucket does not exist (Service: Amazon S3; Status Code: 404; Error Code: NoSuchBucket;

?

MrArnoldPalmer commented 1 year ago

@lysachok usually this is caused by the account/region not being bootstrapped. https://docs.aws.amazon.com/cdk/v2/guide/bootstrapping.html

lysachok commented 1 year ago

@lysachok usually this is caused by the account/region not being bootstrapped.

https://docs.aws.amazon.com/cdk/v2/guide/bootstrapping.html

@MrArnoldPalmer right, but I was trying to do that in an Amplify project so it didn't help anyway.

Thank you for replying!

mattiLeBlanc commented 1 year ago

That's fantastic @bboure, thank you so much for pointing me to your example, that is really helpful. I'll give it a go later today and also look forward to your blog in the future.

We largely use TypeScript over JavaScript, but I reverted to JavaScript for our AppSync JavaScript resolvers as I wasn't clear how best to support transpiling TS to JS via our CDK-managed stack.

Your solution looks really simple. I see it uses Code.fromInline(). I think that has a limitation that the code is maximum 4KiB in size (see here).

That likely isn't a huge issue, but I wondered if there is a simple way to use Code.fromAsset() instead? I assume just having esbuild write to a local dist directory and referencing that would be all that's needed.

In that case I hope your example will also provide inspiration to AWS to to officially support this, as it would be great to simplify and standardize that support because I think it would really super-charge AppSync JavaScript resolvers adding both local imports and TypeScript support.

Perhaps they could add Code.buildAsset(filePath, esBuildOptions), that would be great.

This brilliant example also has an sync esbuild during CDK deployment: https://github.com/graphboltdev/appsync-typescript-resolvers/blob/master/lib/helpers.ts https://github.com/graphboltdev/appsync-typescript-resolvers/blob/master/lib/cdk-stack.ts

mattiLeBlanc commented 1 year ago

@MrArnoldPalmer Hi,

I am trying to implement support for JS resolvers into my Appsync CDK script. I am having a lot of issues converting my Typescript templates to Javascript and make them acceptable to the CDK. Long story short, AmazonDeepdish is giving me a hard time on code: The code contains one or more errors. (Service: AmazonDeepdish; Status Code: 400;

I tried multiple things like building the TS in my dist but it becomes es6 (with arrow functions etc) and AmazonDeepdish doesn't like it. Then I tried the esbuild solution, to convert the TS into JS on the fly, but esbuild doesn't support Es5 anymore (to get rid of imports and arrow functions). Now I am running @swc's transformSync like

    module: {
      type: 'commonjs'
    },
    jsc: {
      parser: {
        syntax: 'typescript',

      },
      target: 'es5',
    }
  });

which I hope is acceptable code, but still Deepdish doesn;t like it.

Code generated by CDK Synth:

Name: testFacility
      Code: |
        "use strict";
        Object.defineProperty(exports, "__esModule", {
            value: true
        });
        var _utils = require("@aws-appsync/utils");
        function _class_call_check(instance, Constructor) {
            if (!(instance instanceof Constructor)) {
                throw new TypeError("Cannot call a class as a function");
            }
        }
        function _defineProperties(target, props) {
            for(var i = 0; i < props.length; i++){
                var descriptor = props[i];
                descriptor.enumerable = descriptor.enumerable || false;
                descriptor.configurable = true;
                if ("value" in descriptor) descriptor.writable = true;
                Object.defineProperty(target, descriptor.key, descriptor);
            }
        }
        function _create_class(Constructor, protoProps, staticProps) {
            if (protoProps) _defineProperties(Constructor.prototype, protoProps);
            if (staticProps) _defineProperties(Constructor, staticProps);
            return Constructor;
        }
        var Resolver = /*#__PURE__*/ function() {
            "use strict";
            function Resolver() {
                _class_call_check(this, Resolver);
            }
            _create_class(Resolver, null, [
                {
                    key: "request",
                    value: function request(ctx) {
                        return {
                            operation: "Query",
                            query: {
                                expression: "sk = :sk",
                                index: "sk-pk-index",
                                expressionValues: {
                                    ":sk": _utils.util.dynamodb.toDynamoDB("FACILITY:COUNTRY=".concat(ctx.args.country))
                                }
                            }
                        };
                    }
                },
                {
                    key: "response",
                    value: function response(ctx) {
                        console.log(ctx.args);
                        return ctx.result;
                    }
                }
            ]);
            return Resolver;
        }();
        exports.request = Resolver.request;
        exports.response = Resolver.response;
      FunctionVersion: "2018-05-29"
      Runtime:
        Name: APPSYNC_JS
        RuntimeVersion: 1.0.0
    DependsOn:

The TS source is:

import { Context, util } from '@aws-appsync/utils';
import { ResolverConfig } from '../../../../lib/helpers/appsync.helper';

@ResolverConfig({
  name: 'testFacility',
  datasource: 'DYNAMODB',
  datasourceName: 'facilities',
})
class Resolver {
  static request(ctx: Context<any>) {
    return {
      operation: 'Query',
      query: {
        expression: 'sk = :sk',
        index: 'sk-pk-index',
        expressionValues: {
          ':sk': util.dynamodb.toDynamoDB(`FACILITY:COUNTRY=${ctx.args.country}`),
        },
      },
    };
  }
  static response(ctx: Context<any, object, object, object, any>,) {
    console.log(ctx.args);
    return ctx.result;
  }
}
exports.Resolver = Resolver;
exports.request = Resolver.request;
exports.response = Resolver.response;

Yes, I know I am using a class, but that is because I try to use the Decorator to pass in config information for the Resolver (Angular style). I filter out the Decorator and its import before I convert to es5. It works, but maybe Deepdish doesn't like classes? Can you tell me what is wrong with my JS atm. Maybe I have to put the config in a comment block and parse that instead of using Class+Decorator.... sigh.

I must say, I like the JS support, but I find it very cumbersome; 1) you can't create a JS unit resolver, but it always have to be a pipeline resolver with a function resolver inside 2) JS support seems to be poor, how great would it be if we can just use Typescript from the start and have a helper function convert it using @swc or some other library.

I have to park my JS resolver implementation because it completely derailed me. I just got so excited.

bboure commented 1 year ago

@mattiLeBlanc APPSYNC_JS is very strict and you can't do many of the things you are doing. I recommend that you refer to the documentation and check the supported features (e.g. Classes are not supported)

I also advise that you use the @aws-appsync/eslint-plugin npm package to validate your JS (see here)

Finally, you can also use the evaluate-code command to test and get better feedback about errors than CloudFormation. Alternatively, GraphBolt also has a tool to evaluate code in a GUI 😛

hoegertn commented 1 year ago

Here is some code I am using to bundle AppSync functions: https://github.com/taimos/cdk-serverless/blob/main/src/constructs/graphql.ts#L279

Beware that I name the js files .jar to avoid a missing feature in CDK assets I will address here: https://github.com/aws/aws-cdk/pull/26106

sudokar commented 1 year ago

Built a custom AWS CDK construct to transpile and bundle Typescript to Javascript https://github.com/sudokar/cdk-appsync-typescript-resolver

samturner3 commented 1 year ago

One feature I feel is missing is being able to provide a dynamic value to the resolver from the cdk file. See: https://github.com/aws/aws-cdk/issues/26848