sid88in / serverless-appsync-plugin

serverless plugin for appsync
MIT License
951 stars 189 forks source link

Upgrading from V1 > V2 drops + creates new API (breaks existing integrations) #617

Open manwaring opened 1 year ago

manwaring commented 1 year ago

First of all thank you so much for creating + maintaining this plugin, it's been so helpful!

I went through the steps to upgrade from V1 to V2 and was able to deploy successfully, however during the deployment the prior Appsync API is destroyed and a new one is created.

I found this issue about v2.3.0 dropping + recreating the API but I don't think that's related to what I'm seeing, as I'm not making any changes to my schema or configurations other than those required for going from V1 to V2.

Is this a known issue, and/or did I miss a step in the upgrade guide?

bboure commented 1 year ago

Hi,

In order to understand what is going on we'd need to check a few things

  1. Would you be able to share your v1 and v2 serverless.yml (the relevant parts. i.e. AppSync)
  2. Check the generated CloudFormation template from v1 and v2 and look for differences (e.g. different logical id).
manwaring commented 1 year ago

Just created a minimal example reproducing the issue: https://github.com/manwaring/appsync-test

manwaring commented 1 year ago

It looks like the Logical ID does in fact change from v1 to v2

v1 Logical ID for GraphQLApi

  "AppsynctestGraphQlApi": {
    "Type": "AWS::AppSync::GraphQLApi",
    "Properties": {
      "Name": "appsync-test",
      "AuthenticationType": "AMAZON_COGNITO_USER_POOLS",
      "AdditionalAuthenticationProviders": [
        {
          "AuthenticationType": "AWS_IAM"
        }
      ],
      "UserPoolConfig": {
        "AwsRegion": "us-east-1",
        "UserPoolId": {
          "Ref": "UserPool"
        },
        "DefaultAction": "ALLOW"
      },
      "LogConfig": {
        "CloudWatchLogsRoleArn": {
          "Fn::GetAtt": [
            "AppsynctestGraphQlApiCloudWatchLogsRole",
            "Arn"
          ]
        },
        "FieldLogLevel": "ERROR",
        "ExcludeVerboseContent": false
      },
      "XrayEnabled": false
    }
  }

v2 Logical ID for GraphQLApi

  "GraphQlApi": {
    "Type": "AWS::AppSync::GraphQLApi",
    "Properties": {
      "Name": "appsync-test",
      "XrayEnabled": false,
      "AuthenticationType": "AMAZON_COGNITO_USER_POOLS",
      "UserPoolConfig": {
        "AwsRegion": "us-east-2",
        "UserPoolId": {
          "Ref": "UserPool"
        },
        "DefaultAction": "ALLOW"
      },
      "AdditionalAuthenticationProviders": [
        {
          "AuthenticationType": "AWS_IAM"
        }
      ],
      "LogConfig": {
        "CloudWatchLogsRoleArn": {
          "Fn::GetAtt": [
            "GraphQlApiLogGroupRole",
            "Arn"
          ]
        },
        "FieldLogLevel": "ERROR",
        "ExcludeVerboseContent": false
      }
    }
  }
bboure commented 1 year ago

It looks like you were using the multiple APIs feature? (maybe without knowing it).

in v1, was your custom.appSync config passed as an array?

manwaring commented 1 year ago

yeah it was, not intentionally (I didn't have multiple, it was just an array with 1 entry) just because that's the way I must have set it up initially (and in this example app to reproduce)

Edit: I was curious and found the config I used as reference, was the Appsync MasterClass repo which passes in appsync configs as an array with the single entry

bboure commented 1 year ago

OK. So that's the issue.

Unfortunately, there is no easy way out of this as of today.

Are you using a custom domain by any chance? That would be the simplest solution: deploy a new API and point your custom domain to it, then remove the old one.

Another workaround would be to add an escape hatch by allowing a custom LogicalId for the main API resource, and maybe other things like API keys, etc. But this is trickier

manwaring commented 1 year ago

shoot, I wish I could go back in time a year ago and configure it differently 😅

I'd like to avoid using custom domains as i've had config issues with it in the past that have been difficult to resolve (so prefer to use generated domain if at all possible).

Would you be open to a PR that exposes an optional parameter for naming the appsync logical id?

manwaring commented 1 year ago

This is a simple implementation with updated tests in case seeing it helps

https://github.com/sid88in/serverless-appsync-plugin/pull/618

terrancej commented 1 year ago

Is anyone looking at this, im also in the same boat

bboure commented 1 year ago

Sorry, I'll start looking at this starting next week. Thank you for your patience

eddiesmithjr commented 1 year ago

I will close my issue as this is the root of the same issue I'm having here