aws-amplify / amplify-category-api

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development. This plugin provides functionality for the API category, allowing for the creation and management of GraphQL and REST based backends for your amplify project.
https://docs.amplify.aws/
Apache License 2.0
89 stars 79 forks source link

Removing @function directive from a @model field creates a CF conflict #106

Open dhruvbansal2 opened 2 years ago

dhruvbansal2 commented 2 years ago

Before opening, please confirm:

How did you install the Amplify CLI?

curl

If applicable, what version of Node.js are you using?

v16.13.0

Amplify CLI Version

7.6.21

What operating system are you using?

Mac

Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.

No manual changes made

Amplify Categories

api

Amplify Commands

push

Describe the bug

Currently, I have a @function directive applied to a model field (refer to example schema). Removing the @function/removing the entire field & running amplify push -y results in the following error. I also see the same error when I upgrade to the new transformer & run amplify push -y (after cleaning up the schema & runningamplify api migrate)

Error:

UPDATE_FAILED FunctionDirectiveStack AWS::CloudFormation::Stack
Embedded stack xxxxx was not successfully updated. Currently in UPDATE_ROLLBACK_IN_PROGRESS with reason:
Export xxxxxx:GetAtt:CustomLambdaDataSource:FunctionId cannot be deleted as it is in use by xxxxx-ConnectionStack-xxxx

Expected behavior

amplify push -y should succeed

Reproduction steps

Steps to reproduce:

  1. Upgrade to amplify cli 7.6.21 (from 6.4.0)
  2. amplify push the Example schema attached to this issue
  3. Remove the exampleField line and amplify push

GraphQL schema(s)

```graphql # Put schemas below this line type Example @model(timestamps: { createdAt: "createdAt" }) @auth(rules: []) @key(...) { id: ID! name: String exampleField: [Example!] @function(name: "customName-{env}") } ```

Log output

``` # Put your logs below this line UPDATE_FAILED FunctionDirectiveStack AWS::CloudFormation::Stack Embedded stack xxxxx was not successfully updated. Currently in UPDATE_ROLLBACK_IN_PROGRESS with reason: Export xxxxxx:GetAtt:CustomLambdaDataSource:FunctionId cannot be deleted as it is in use by xxxxx-ConnectionStack-xxxx ```

Additional information

1) This occurs for both versions of the transformer 2) I've seen similar issues with @connection, which seem to have been resolved a while back: https://github.com/aws-amplify/amplify-cli/issues/1425, https://github.com/aws-amplify/amplify-cli/pull/1182 3) That specific field doesn't actually get used anymore. At some point, we did have two vtl files for it but those files have been deleted & this issue blocks our ability to migrate to v2 of the graphql transformer.

sundersc commented 2 years ago

@dhruvbansal2 - I'm unable to reproduce the issue using the given reproduction steps. If you have any additional details that would help to reproduce the scenario, please add it to the issue.

dhruvbansal2 commented 2 years ago

Interesting, so I'm also seeing the following in our transform.conf.json.

    "StackMapping": {
        "ExampelexampleFieldResolver": "ConnectionStack"
    }

I'm wondering if this could be causing the issue on my end @sundersc? (I'm pretty new to amplify & amplify-generated resources so I'm still learning how these resources integrate).

sundersc commented 2 years ago

Is that the complete schema? I'm trying to understand how your application end up in this state.

dhruvbansal2 commented 2 years ago

That isn't the entire schema @sundersc . Essentially, we have connections to a couple other models, mutations, queries, & enums. The mutations & queries reference a custom lambda function.

# Put schemas below this line
type Example
  @model(timestamps: { createdAt: "createdAt" })
  @auth(rules: [])
  @key(...)
  {
   id: ID!
   name: String
   exampleTwoID: ID
   exampleTwo: ExampleTwo @connection(fields: ["exampleTwoID"])
   exampleField: [Example!] @function(name: "customName-{env}")
  }

type ExampleTwo
  @model(timestamps: { createdAt: "createdAt" })
  @auth(rules: [])
  {
   id: ID!
   organization: String!
   examples: [Example!]  @connection(keyName: "byExampleByDate", fields: ["id"])
  }

enum ExampleType {
  IS_TEST
}

type ExampleInput {
 ...
}

type ExampleResponse {
 ...
}

type AvailableExample @aws_iam @aws_cognito_user_pools {
 ...
}

type Mutation {
  modifyExample(input: ExampleInput!): ExampleResponse
    @function(name: "customName-${env}")
    @auth(
      rules: [
        { allow: private, provider: iam }
        { allow: groups, groups: [...] }
      ]
    )
}

type Query {
  listAvailableExamples(
    exampleID: ID
  ): [AvailableExample]
    @function(name: "customName-${env}")
    @auth(
      rules: [
        { allow: public, provider: iam }
        { allow: groups, groups: [...] }
      ]
    )
}
SaileshKumar commented 2 years ago

I am having the same problem - do you have any workarounds @dhruvbansal2 ?

dhruvbansal2 commented 2 years ago

@SaileshKumar unfortunately, the only work arounds I found were either

1) Delete the model entirely and recreate it (without the field with the @function directive) 2) Creating a new amplify environment without the field (or in our case, because we were trying to migrate to the new transformer, we kept the field and creating a new environment worked as the CF ConnectionStack didn't exist on the initial deployment)

Neither option is ideal for production amplify environments though.

@sundersc based on the larger schema provided above, do you have any ideas/recommendations on what could be causing this? I am still running into this issue.

dhruvbansal2 commented 2 years ago

From my understanding, the ConnectionStack in CloudFormation is unable to update (when migrating) as it references the CustomLambdaDataSource FunctionID.

"ExampleexampleFieldResolver": {
            "Type": "AWS::AppSync::Resolver",
            "Properties": {
                "ApiId": {
                    "Ref": "AppSyncApiId"
                },
                "TypeName": "Example",
                "FieldName": "exampleField",
                "Kind": "PIPELINE",
                "PipelineConfig": {
                    "Functions": [
                        {
                            "Fn::ImportValue": {
                                "Fn::Join": [
                                    ":",
                                    [
                                        {
                                            "Ref": "AppSyncApiId"
                                        },
                                        "GetAtt",
                                        "CustomLambdaDataSource",
                                        "FunctionId"
                                    ]
                                ]
                            }
                        }
                    ]
                },
                "RequestMappingTemplateS3Location": {
                    "Fn::Sub": [
                        "s3://${S3DeploymentBucket}/${S3DeploymentRootKey}/resolvers/${ResolverFileName}",
                        {
                            "S3DeploymentBucket": {
                                "Ref": "S3DeploymentBucket"
                            },
                            "S3DeploymentRootKey": {
                                "Ref": "S3DeploymentRootKey"
                            },
                            "ResolverFileName": {
                                "Fn::Join": [
                                    ".",
                                    [
                                        "Example",
                                        "exampleField",
                                        "req",
                                        "vtl"
                                    ]
                                ]
                            }
                        }
                    ]
                },
                "ResponseMappingTemplateS3Location": {
                    "Fn::Sub": [
                        "s3://${S3DeploymentBucket}/${S3DeploymentRootKey}/resolvers/${ResolverFileName}",
                        {
                            "S3DeploymentBucket": {
                                "Ref": "S3DeploymentBucket"
                            },
                            "S3DeploymentRootKey": {
                                "Ref": "S3DeploymentRootKey"
                            },
                            "ResolverFileName": {
                                "Fn::Join": [
                                    ".",
                                    [
                                        "Example",
                                        "exampleField",
                                        "res",
                                        "vtl"
                                    ]
                                ]
                            }
                        }
                    ]
                }
            }
        }
sundersc commented 2 years ago

@dhruvbansal2 - I'm still unable to reproduce it. I will continue to test few more scenarios to reproduce it. But it looks like it has to do with the sequence the nested stacks are getting deployed. Could you try this and let us know if it works?

  1. Clone a new environment for testing.
  2. Push the new environment to cloud (amplify push).
  3. Remove the @function directive and amplify push. This should throw the error as you reported.
  4. Now, go to AppSync console. On schema tab - right section, remove the resolver attached to the field. (Click on pipeline and select Delete Resolver). image
  5. Delete the corresponding lambda data source.
  6. Now try amplify push again.

Once testing is done, make sure to clean up the environment.

Another option is to update the connectionstack template manually in cloudformation console to remove the reference to lambda datasource.

sundersc commented 2 years ago

In the sample app I have tested, I see that the resolver for the function directive field actually exists on the FunctionDirectiveStack.json file. However based on the information provided, it looks like in your case the resolver resource is defined on a different nested stack. I have went as back as version 5.x and don't see that behavior.

SaileshKumar commented 2 years ago

In my case my model looks like this:

type ClassRegistration
  @model
  @key(name: "byClass", fields: ["class_id"])
  @key(name: "byStudent", fields: ["student_id"])
  @auth(
    rules: [
      { allow: groups, groups: ["Admins"] }
      { allow: owner }
      { allow: private, provider: iam }
      { allow: private, operations: [read] }
      { allow: public, operations: [read] }
    ]
  ) {
  id: ID!
  class_id: ID
  student_id: ID
  student: User @connection(fields: ["student_id"])
  class: Class @connection(fields: ["class_id"])
  owner: String
  stripe_session_id: String
  payment_status: PaymentStatus
    @auth(rules: [{ allow: private, operations: [read] }])
    @function(name: "getRegistrationPaymentStatus-${env}")
  confirmation_email_sent: Boolean
}

@function(name: "getRegistrationPaymentStatus-${env}") is the one I am trying to remove

AFAICT - the model stack for ClassRegistration is dependent on the output of the FunctionDirectiveStack, but the Amplify CLI is attempting to deploy the updated FunctionDirectiveStack prior to the ClassRegistration stack.

sundersc commented 2 years ago

@SaileshKumar - Could you try this workaround in cloudformation console?

  1. Find the model stack (in this case ClassRegistration) in cloudformation console.
  2. Copy the template and save it to a file.
  3. Remove the problematic resolver resource from the file and save it. (it should look like something below)
  4. On the cloudformation console, create a new changeset and upload the updated file.
  5. Cloudformation should run a check and show the results that the resolver will be removed. (Note: that should be the only change)
  6. Click "Execute" to deploy the change.
  7. Now go back to your project and do amplify push. This time it should go through as we removed the reference manually.
"ClassRegistrationpaymentstatus63c5Resolver": {
  "Type": "AWS::AppSync::Resolver",
  "Properties": {
      "ApiId": {
          "Ref": "AppSyncApiId"
      },
      "TypeName": "ClassRegistration",
      "FieldName": "payment_status",
      "Kind": "PIPELINE",
      "PipelineConfig": {
          "Functions": [
              {
                  "Fn::ImportValue": {
                      "Fn::Join": [
                          ":",
                          [
                              {
                                  "Ref": "AppSyncApiId"
                              },
                              "GetAtt",
                              "InvokeGetRegistrationPaymentStatusLambdaDataSource",
                              "FunctionId"
                          ]
                      ]
                  }
              }
          ]
      },
      "RequestMappingTemplateS3Location": {
          "Fn::Sub": [
              "s3://${S3DeploymentBucket}/${S3DeploymentRootKey}/resolvers/${ResolverFileName}",
              {
                  "S3DeploymentBucket": {
                      "Ref": "S3DeploymentBucket"
                  },
                  "S3DeploymentRootKey": {
                      "Ref": "S3DeploymentRootKey"
                  },
                  "ResolverFileName": {
                      "Fn::Join": [
                          ".",
                          [
                              "ClassRegistration",
                              "payment_status",
                              "req",
                              "vtl"
                          ]
                      ]
                  }
              }
          ]
      },
      "ResponseMappingTemplateS3Location": {
          "Fn::Sub": [
              "s3://${S3DeploymentBucket}/${S3DeploymentRootKey}/resolvers/${ResolverFileName}",
              {
                  "S3DeploymentBucket": {
                      "Ref": "S3DeploymentBucket"
                  },
                  "S3DeploymentRootKey": {
                      "Ref": "S3DeploymentRootKey"
                  },
                  "ResolverFileName": {
                      "Fn::Join": [
                          ".",
                          [
                              "ClassRegistration",
                              "payment_status",
                              "res",
                              "vtl"
                          ]
                      ]
                  }
              }
          ]
      }
  }
}

Let me know if there are any problems in following the steps, we can schedule a call to walkthrough the steps.

SaileshKumar commented 2 years ago

@sundersc would love to do it on a call if possible :), I tried something similar on my own and got my stack in a really weird state (and it took an AWS support person over an hour to help me get to a good state), but I'm quite sure I did something incorrectly. How can I schedule it with you? Thank you in advance!!

sundersc commented 2 years ago

Please send an email to amplify-cli@amazon.com with the issue# on subject.

dhruvbansal2 commented 2 years ago

If possible, could I also join in on this call or would you recommend I send a separate email @sundersc ?

sundersc commented 2 years ago

Please send an email, we will workout the schedule through email.

dhruvbansal2 commented 2 years ago

Perfect, reached out - thank you @sundersc !!

sundersc commented 2 years ago

Finally I was able to reproduce it, marking it as bug. Posting the reproduction steps here for reference.

  1. Init an amplify project and add a GraphQL api with API_KEY, Cognito and IAM authModes .
  2. Update the schema to the below and push the changes to the cloud.
type ClassRegistration
  @model
  @key(name: "byClass", fields: ["class_id"])
  @key(name: "byStudent", fields: ["student_id"])
  @auth(
    rules: [
      { allow: groups, groups: ["Admins"] }
      { allow: owner }
      { allow: private, provider: iam }
      { allow: private, operations: [read] }
      { allow: public, operations: [read] }
    ]
  ) {
  id: ID!
  class_id: ID
  student_id: ID
  owner: String
  stripe_session_id: String
  payment_status: PaymentStatus
    @auth(rules: [{ allow: private, operations: [read] }])
  payment_test_status: PaymentStatus
    @auth(rules: [{ allow: private, operations: [read] }])
  confirmation_email_sent: Boolean
}

enum PaymentStatus {
  NONE
  PENDING
  CONFIRMED
}
  1. Add functions FunctionA and FunctionB and update the following fields in the schema as below, then amplify push.
    payment_status: PaymentStatus
    @auth(rules: [{ allow: private, operations: [read] }])
    @function(name: "FunctionA-${env}")
    payment_test_status: PaymentStatus
    @auth(rules: [{ allow: private, operations: [read] }])
    @function(name: "FunctionB-${env}")
  2. Remove @function directive on payment_status field and push again. This should throw the reported error.

The error is thrown because FunctionDirectiveStack is getting deployed first and trying to remove the export reference of FunctionA datasource but it is still used by the existing model stack ClassRegistration.

I can confirm that the workaround steps solved the problem.

dhruvbansal2 commented 2 years ago

@sundersc in my case, the issue is with the ConnectionStack & not the existing Model stack (i.e. the resolver reference is in the Connection CF stack & not the Model CF stack).

The workaround still works if I remove it from the ConnectionStack but I want to make sure the bug is resolved for both scenarios (thank you for the workaround & reproduction steps!)

SaileshKumar commented 2 years ago

@SaileshKumar - Could you try this workaround in cloudformation console?

  1. Find the model stack (in this case ClassRegistration) in cloudformation console.
  2. Copy the template and save it to a file.
  3. Remove the problematic resolver resource from the file and save it. (it should look like something below)
  4. On the cloudformation console, create a new changeset and upload the updated file.
  5. Cloudformation should run a check and show the results that the resolver will be removed. (Note: that should be the only change)
  6. Click "Execute" to deploy the change.
  7. Now go back to your project and do amplify push. This time it should go through as we removed the reference manually.
"ClassRegistrationpaymentstatus63c5Resolver": {
  "Type": "AWS::AppSync::Resolver",
  "Properties": {
      "ApiId": {
          "Ref": "AppSyncApiId"
      },
      "TypeName": "ClassRegistration",
      "FieldName": "payment_status",
      "Kind": "PIPELINE",
      "PipelineConfig": {
          "Functions": [
              {
                  "Fn::ImportValue": {
                      "Fn::Join": [
                          ":",
                          [
                              {
                                  "Ref": "AppSyncApiId"
                              },
                              "GetAtt",
                              "InvokeGetRegistrationPaymentStatusLambdaDataSource",
                              "FunctionId"
                          ]
                      ]
                  }
              }
          ]
      },
      "RequestMappingTemplateS3Location": {
          "Fn::Sub": [
              "s3://${S3DeploymentBucket}/${S3DeploymentRootKey}/resolvers/${ResolverFileName}",
              {
                  "S3DeploymentBucket": {
                      "Ref": "S3DeploymentBucket"
                  },
                  "S3DeploymentRootKey": {
                      "Ref": "S3DeploymentRootKey"
                  },
                  "ResolverFileName": {
                      "Fn::Join": [
                          ".",
                          [
                              "ClassRegistration",
                              "payment_status",
                              "req",
                              "vtl"
                          ]
                      ]
                  }
              }
          ]
      },
      "ResponseMappingTemplateS3Location": {
          "Fn::Sub": [
              "s3://${S3DeploymentBucket}/${S3DeploymentRootKey}/resolvers/${ResolverFileName}",
              {
                  "S3DeploymentBucket": {
                      "Ref": "S3DeploymentBucket"
                  },
                  "S3DeploymentRootKey": {
                      "Ref": "S3DeploymentRootKey"
                  },
                  "ResolverFileName": {
                      "Fn::Join": [
                          ".",
                          [
                              "ClassRegistration",
                              "payment_status",
                              "res",
                              "vtl"
                          ]
                      ]
                  }
              }
          ]
      }
  }
}

Let me know if there are any problems in following the steps, we can schedule a call to walkthrough the steps.

This worked!!

ahtokca commented 2 years ago

Is there a fix coming? or CI way to workaround?

alharris-at commented 2 years ago

Hi @ahtokca, I'll be taking a look to see how we can resolve this bug and get rid of the need for workarounds here, based on the repro steps provided by @sundersc above.

alharris-at commented 2 years ago

Because of the way that functions get attached across child stacks within the amplify stack, it appears that what is happening here is quite similar to a bug we're currently looking into related to using StackMappings to move resources between stacks. Because CloudFormation PeerStacks have no concept of dependency or state between them, we end up in a situation where what would be an atomic operation within a single stack becomes a race condition between stacks.

There are two operations happening in this specific case: a. In FunctionStack, function export is removed, and function is subsequently deleted. b. In APIStack, function reference is removed, and resolvers are updated.

In the case that 'a' happens first, then we end up with the error message you're encountering. In the case that 'b' happens first, then there is no error, and things proceed as expected. This is likely why reproducibility was not straightforward here. Different deploy states will lead to different latencies before 'a' or 'b' occur in the deployment, leading to inconsistency.

Ultimately, I think we need to refactor a deployment that is removing a reference between stacks into two deployments, a first one which will remove the usage from the destination stack, and a second one which removes the export and updates relevant resources in the source stack.