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

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

Open
bboure opened this issue Dec 13, 2020 · 15 comments

Comments

@bboure
Copy link
Collaborator

bboure commented Dec 13, 2020

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!

@bboure bboure pinned this issue Dec 13, 2020
@bboure bboure changed the title [RFC] What do you like/dislike about the plugin? What would you like to see next? [V2?] [RFC] [V2] What do you like/dislike about the plugin? What would you like to see next? Dec 13, 2020
@meije702
Copy link

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
Copy link

duwerq commented Dec 23, 2020

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
Copy link

duwerq commented Dec 23, 2020

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
Copy link

Negan1911 commented Dec 29, 2020

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
Copy link
Collaborator Author

bboure commented Dec 29, 2020

@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
Copy link

akzincsystems commented Feb 15, 2021

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

  • cannot name log after function/service name. My view is it should be named after the appSync.name like Lambdas do
  • stack generation ignores extensions to log group e.g.
  extensions:
    GraphQlApiLogGroup:
      Properties:
        RetentionInDays: ${self:custom.logRetentionDays}
  • doesn't play with serverless-plugin-log-subscription (probably for same reason as previous item)

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
Copy link

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
Copy link

tuukkafc commented Apr 15, 2021

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
Copy link
Contributor

vicary commented Jun 27, 2021

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 Allow schemaPrinter to be customized / extended. graphql/graphql-js#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
Copy link
Collaborator Author

bboure commented Dec 27, 2021

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)

  • Graphiql Playground
    Personally, I have never used it. There are already great tools out there like Insomnia or Postman that support Graphql very well. I don't think this is something that should be embedded in this plugin.

  • multiple APIs in one stack
    I'm kind of on the fence on this one. I personally think that it is probably a bad practice too to deploy several APIs in the same stack. One should probably deploy them in separate stacks. I understand that some resources might be shared across several APIs (eg: a DynamoDb table), but there are other solutions for that. (eg: CloudFormation cross-references).

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

  • Custom Domains

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

  • JS resolvers

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 🤞)

  • sls deploy resolver -r

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.

  • stream logs

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

  • log groups

@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!

  • Schema

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

  • API renewal

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
Copy link
Collaborator Author

bboure commented Jan 16, 2022

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
Copy link
Collaborator Author

bboure commented Jan 16, 2022

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
Copy link

NevRA commented Apr 18, 2022

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
Copy link
Collaborator Author

bboure commented May 1, 2022

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
Copy link

mleziva commented Jun 13, 2023

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/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants