sid88in / serverless-appsync-plugin

serverless plugin for appsync
MIT License
952 stars 182 forks source link

[RFC] [V2] What do you like/dislike about the plugin? What would you like to see next? #378

Open bboure opened 3 years ago

bboure commented 3 years ago

Over the past few years, this plugin received a good acceptance from the AppSync community (with ~20k downloads a week!). We also received numerous PRs to continually improve it, add new features as AWS releases them, fix bugs, or just to make the AppSync experience better overall. We truly appreciate that!

All of it came at a cost though. In order to maintain backward compatibility, we often ended up having to patch things up. I was thinking that maybe it's time to do a complete (or at least a partial) rewrite in order to make things a bit more tidy, and take the opportunity to add new features as well.

Here are a few things that came to my mind based on my experience over the past few years

0. Re-write the plugin

The first goal would be to re-write the plugin and make it a bit more maintainable. Probably switch to Typescript too, for a better developer experience, improve test coverage, etc.

1. Re-think resolver and mapping templates declaration.

When working with AppSync, I often end up copy pasting a lot of code, especially when working with Lambdas. I need to declare a function, then add data source that point to that function, and yet again a mapping template that uses the data source. To make things worst, all of this happens at the different place. This is somewhat cumbersome. We should try to automate or centralize everything.

One approach could be to use Directives at the schema level a la Amplify. The plugin would then just parse the schema and auto-generate the resolvers and data sources.

2. API key auto-renewal

API keys in AppSync have a max validity of 365 days. You can extend the lifetime of your API keys, and this plugin actually does that for you automatically after each re-deploy. However, what if you don't re-deploy? API keys will end up expiring, maybe without you noticing. That can lead to bad surprises.

The idea would be to add an opt-in option that would automatically deploy a cron lambda that renews API keys periodically.

~# 3. Custom domain support~

~AppSync doesn't currently support custom domains "natively". There is a workaround though. We could see how we can integrate with foxxor/serverless-cloudfront-plugin and amplify-education/serverless-domain-manager in order to automate the procedure easily.~

This is now supported!


Are there other features you would like to see? Drop your comments below! All feedbacks are welcome.

Thanks!

meije702 commented 3 years ago

I just wanted to take this opportunity to thank you, for all the work you have done on this plugin. Everything can always be improved and nothing is perfect but I have been using this plugin to run a production AppSync API for two years now. You'r comments on further improving sound like music to my ears. For me the main thing that maybe could be improved is the schema stitching, or splitting your API for multiple teams to work on one big API. Again thank you, and all contributors for that matter, for all the work so far.

duwerq commented 3 years ago

I’d like to echo the statement above. A couple years ago I was horshoeing a single table design into AppSync and failed miserably with Amplify. Eventually had to eject everything to serverless and this plugin saved the day!

duwerq commented 3 years ago

I’ve been doing a lot of consulting over the last year and I find the autogen of templates and CloudFormation resources, from the schema directives, invaluable for developer velocity. I’d love to see the same autogen feature not only for mapping templates but also for the serverless templates themselves.

Negan1911 commented 3 years ago

Hope to not came too late to the party but... It would pay to migrate to serverless components? Haven't used it before because I heavily invested on this plugin, but the main idea of components is to help circumvent the slow deployment + resource limitations of cloudformation. It may also be beneficial to us on this plugin

bboure commented 3 years ago

@Negan1911 FYI, there is already a separate project for appsync serverless components. This plugin should stick to CloudFormation.

Oh, and by the way, Cfn recently increased the resource limit from 200 to 500.

akzincsystems commented 3 years ago

Having only picked this up in the last few days, cloudwatch logging seems to be an issue:

Logging is such an important part of any production delivery, that perhaps beefing up this area of the plugin would be of benefit to your users. [edit] Thinking about it, if the plugin can honour extensions, then it is possible that a number of these issues may go away with suitable documentation perhaps?

rehrumesh commented 3 years ago

Ability to stream logs to local terminal via a cli command. something like,

sls appsync logs -q <QUERY NAME> --stage <STAGE NAME>
sls appsync logs -m <MUTATION NAME> --stage <STAGE NAME>
sls appsync logs -s <SUBSCRIPTION NAME> --stage <STAGE NAME>

currently serverless supports function logging via sls logs -f <function name> It will save time if I can log streams from a request, VTL transformations, what happened in he data source, response transformation and response. that would save my dev and test time.

tuukkafc commented 3 years ago

Generally speaking I agree with others here, thank you for putting together this plugin, it has been great.

The custom domain issue is actually solved pretty easily already with serverless-appsync-cloudfront and if you want to automate the certificate with serverless-certificate-creator

One thing that is slightly annoying to me is that the plugin doesn't seem to pick up the tags from the provider automatically.

vicary commented 3 years ago

I am the author of appsync-schema-converter and I'd like to see my package being superseded by the next major version here. If that's possible I am more than happy to deprecate my package and contribute here instead.

AppSync is great because it is one of the first usable serverless products.

But at the same time, AppSync is bad because it still works with a legacy version of GraphQL schema. For example, it takes comment descriptions instead of string descriptions.

What's worse is that even if I correctly translates all of my modern schemas into legacy format, serverless-appsync-plugin still strips away all of my comments before deploy for no reason.

I'd like the next major version to focus on developer experience and maintainability, much more than fancy features that does the job for AWS.

  1. Re-write the plugin

This is great.

  1. Re-think resolver and mapping templates declaration.

I strongly oppose the Amplify approach because even Amplify itself prohibits mid-large sized projects, it does not offer a nice integration with CI/CD pipelines, it does not offer low-level integrations directly with AppSync such as custom authenticators.

Custom local directives is yet another domain specific syntax that unnecessarily steepen the learning curve and lengthened the onboarding period of new recruitments.

  1. API key auto-renewal

Nice to have.

  1. Custom domain support

I would ask for this here only when AWS officially supports it. Anything home-baked is doomed to be a trap for future refactoring; it affects all downstream projects depends on it.

EDIT: I released new versions to add coverage for ENUM descriptions, which is why you are removing the comments in the first place. I think my package is ready to be integrated, having the following things to keep in mind:

  1. Copying schemaPrinter.js from graphql@14 and printSchema.js from grapqhl@15 really is a sub-optimal solution here, but until https://github.com/graphql/graphql-js/issues/2020 is resolved I couldn't think of a better solution.
  2. To rethink what validate-schema means when we have to choose between modern style and AppSync style schemas. If users actually implement multiple interfaces with commas, shall we accept it?
bboure commented 2 years ago

Thank you all for all the feedback. I thoughtfully read them all as you posted them and deliberately omitted to respond in most cases to avoid "polluting" the thread and risking getting into deep debates.

I recently started working on v2 of this plugin. It is still very WIP but I hope I can have an alpha version ready very soon.

Meanwhile, I would love some additional feedback on a few things that I considered for the new version:

New API

For version 2, I completely reviewed the API. I won't go into details here (it would be too long), but the idea is to make it more developer-friendly and easy to use. There will be some shorthands to declare resolvers and data sources easier. And in some cases, even declare them all in one place.

I did NOT plan on adding resource inference from the schema or any custom directive, etc. As some pointed out, AppSync has some issues with the schema format being outdated, which already adds enough complexity.

Dropping support for a few features

This is where I'd like most of the feedback. There are a few features that I am not convinced that they were useful, good practice, or even widely used. Namely:

I have never seen any other plugin, let alone Serverless Framework support anything like that (please correct me if I'm wrong). I understand the need when you have it, but it is also probably bad practice. With custom domains in place, I would instead advocate for setting up a custom domain, deploying a brand new API, and pointing the domain to the new API (though I understand it' snot always possible)

Also, AppSync APIs are known to be very "resource-consuming" from a CloudFormatio point of view. (ie: it is easy to reach the 500 resources limit). With multiple APIs, you'll hit that limit even faster.

From a point of view of the plugin, it will also become easier to manage all these resources internally when generating the Cfn template.

So, I am considering dropping support for that too, and instead, recommend using different stacks. As a comparison, I don't think that Serverless Framework supports several API Gateway APIs (at least, I could not find a way to do it).


Everything else is probably going to remain, and more 👇

More

As you probably already all know, custom domains are now supported "natively". I want to support this as well, of course.

There is nothing official yet, but as we know, they are coming (we don't know when yet). When this becomes official, the goal is to support them as well. (Hopefully, this will come soon, or at least it won't require too much refactoring or break anything in v2 🤞)

I am still not sure if or how this can work. But I was considering supporting deploying resolvers without having to re-deploy the whole stack (just like SF supports sls deploy function -f) There are many things that are not clear yet on how to achieve this, but hopefully, it is possible. The goal is to increase the dev loop speed by allowing to sync code in the cloud (ie: not rely on simulators).

Note that it will probably remain very basic. eg: Update a resolver's mapping template. So nothing that requires creating new resources (DataSources, resolvers, etc). (Just like deploy function is only capable of updating existing functions)

Obviously, this would remain a developer tool and not something to use on production.

Suggested by @rehrumesh, I think this is a great idea!! I will definitely consider it.

@akzincsystems I will have a look at your suggestions. The last time I checked, custom names for AppSync logs were not supported (it has to be the API id), but I will check again. I'll also check the other suggestions. If you had more context here, that'd be great!

@vicary thank you for the great work trying to make AppSync schemas more "compliant". I think the goal would be to fully support "modern" schemas and transform them as needed for AppSync. This is a tricky subject though.

I haven't thought about it yet. The minimum is to maintain a sliding window style renewal of the keys each time a deployment is done (unless you use a specific expiresAt). Auto-renewing keys in a cron Lambda or something might be an issue as it is usually bad practice to edit CloudFormation-created resources by other means. (eg: the console or the SDK)

Backward compatibility

Last but not least, except for the breaking changes above, the goal is to make the resulting CloudFormation template backward compatible. This will allow users to migrate to the new version, and after a few adjustments to match the new API, the resulting deployed resources should remain unchanged (eg: no replacement needed).

There is obviously some trickiness to this. eg: if support for multiple APIs is dropped. Possibly, I would consider an "escape hatch" for those cases, but ideally, I'd like to avoid that to avoid falling into a rabbit hole.

v1 Support

Given the above (about backward compatibility), after v2 becomes production-ready, there will probably be a maintenance window to keep supporting v1 for a while for people who might be stuck with it for some reason. There will probably not be new features though (unless critical), and only bug fixes.

Contributors wanted!

Of course, if anyone has some spare time to dedicate to this project, I'd be more than happy to receive help in order to achieve this. Feel free to reach out to me.

Thank you!

bboure commented 2 years ago

Additional thoughts.

AppSync currently supports an old version of GraphQL specs.

In v2, I want to support and probably enforce modern schema specs. EIther the plugin or AppSync itself will ensure that the delivered schema is compatible with AppSync by converting it, but the plugin might not accept "legacy schemas". The reason is to try to be future-proof and be able to remove any conversion later if we can.

More details in #453 and https://twitter.com/Benoit_Boure/status/1482416292216950792

bboure commented 2 years ago

Serverless Framework Compatibility

SF is going to release a V3 soon. I originally planned on supporting the SF v2 but since there are significant differences in the API, the code is starting to become a mess. So, I think that it actually makes more sense to drop support for SF V2 in this new major of the plugin.

I think that it should be fine for most use cases. Users willing to take time and migrate to the newer version of this plugin will not have any issues doing the same for the SF anyways and greenfield projects should start using v3 + v2 from day one.

Let me know if you see any issue with that.

NevRA commented 2 years ago

Thanks for the great plugin!

I would miss multiple APIs. The reason why we are using it is that we want to separate public and private APIs:

  1. Have the same models with different fields set and additionally no need to name it XxxPublic XxxPrivate for type generators
  2. Different waf configs per endpoint (like rate limiter for the public / private api)
  3. Strictly separate authorization, if the API is mixed, you can forget to set the attribute
  4. Using different endpoint we don't expose irrelevant queries / mutations
  5. "One should probably deploy them in separate stacks". It is much more convenient to work with one stack, rather than having multiple stacks (shared resources & CI/CD logic)
  6. "it is easy to reach the 500 resources limit". We don't heat 500 resources limit because we are using stack split plugin with custom resolver for Appsync
  7. We can have a lot of S3, Dynamodb etc but not Appsync which seems like an artificial limitation :)
bboure commented 2 years ago

Thanks for the feedback @NevRA

I am trying to gather the pros and the cons so we can make a decision. As far as I know, Amplify does not allow it either.

Side note: The new Serverless Framework Compose could possibly be a workaround here too.

mleziva commented 1 year ago

We have been using this plugin successfully, but we are looking to use the new private APIs feature of AppSync. Any idea when this feature will be integrated into a release?

https://aws.amazon.com/blogs/mobile/introducing-private-apis-on-aws-appsync/