aws-amplify / amplify-cli

The AWS Amplify CLI is a toolchain for simplifying serverless web and mobile development.
Apache License 2.0
2.82k stars 820 forks source link

Amplify crash due to a cyclic dependency #6526

Closed sytolk closed 3 years ago

sytolk commented 3 years ago

Note: If your issue/bug is regarding the AWS Amplify Console service, please log it in the Amplify Console GitHub Issue Tracker

Describe the bug Crash when I try to update functions permissions

Amplify CLI Version 4.41.2

To Reproduce amplify update function

Desktop (please complete the following information):

Log output Include any relevant log output under ~/.amplify/logs/amplify-cli-<issue-date>.log

? Which setting do you want to update? Resource access permissions
? Select the category storage
? Storage has 3 resources in this project. Select the one you would like your Lambda to access tagspacedata
? Select the operations you want to permit for tagspacedata create, read, update, delete

You can access the following resource attributes as environment variables from your Lambda function
        STORAGE_TAGSPACEDATA_BUCKETNAME
Error: Cannot add S3Trigger388cd038 due to a cyclic dependency
    at checkForCyclicDependencies (/Users/sytolk/.nvm/versions/node/v12.13.0/lib/node_modules/@aws-amplify/cli/lib/extensions/amplify-helpers/update-amplify-meta.js:210:15)
    at AmplifyToolkit.updateamplifyMetaAfterResourceUpdate [as _updateamplifyMetaAfterResourceUpdate] (/Users/sytolk/.nvm/versions/node/v12.13.0/lib/node_modules/@aws-amplify/cli/lib/extensions/amplify-helpers/update-amplify-meta.js:129:9)
    at updateFunctionResource (/Users/sytolk/.nvm/versions/node/v12.13.0/lib/node_modules/@aws-amplify/cli/node_modules/amplify-category-function/lib/provider-utils/awscloudformation/index.js:144:29)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async Object.executeAmplifyCommand (/Users/sytolk/.nvm/versions/node/v12.13.0/lib/node_modules/@aws-amplify/cli/node_modules/amplify-category-function/lib/index.js:195:5)
    at async executePluginModuleCommand (/Users/sytolk/.nvm/versions/node/v12.13.0/lib/node_modules/@aws-amplify/cli/lib/execution-manager.js:166:5)
    at async Object.executeCommand (/Users/sytolk/.nvm/versions/node/v12.13.0/lib/node_modules/@aws-amplify/cli/lib/execution-manager.js:35:9)
    at async Object.run (/Users/sytolk/.nvm/versions/node/v12.13.0/lib/node_modules/@aws-amplify/cli/lib/index.js:92:9)
There was an error adding the function resource
jhockett commented 3 years ago

Hi @sytolk, this could definitely use a better error message, but this is intentional behavior to prevent cyclic dependencies between resources.

Edit: Marking this as a bug until we improve the error message

sytolk commented 3 years ago

Hi @jhockett I have clean setup and push from a cloud and try to update storage (remove guest access and to use trigger function) but the same error appears. What can be cyclic in dependencies?? A24AF240-21FF-4E25-AE7A-38F954F979E3

jhockett commented 3 years ago

@sytolk I see you chose an existing Lambda function above. Does that function have additional access permissions added to it?

sytolk commented 3 years ago

Yes its have Read/Write permissions to tagspacedata storage

"function": {
  "createTagspaceIndex": {
      "build": true,
      "providerPlugin": "awscloudformation",
      "service": "Lambda",
      "dependsOn": [
        {
          "category": "storage",
          "resourceName": "tagspacedata",
          "attributes": [
            "BucketName"
          ]
        }
      ]
    }
}
"storage": {
    "tagspacedata": {
      "service": "S3",
      "providerPlugin": "awscloudformation",
      "dependsOn": []
    }
  }

this is the cycling dependency but how can I set trigger function to the storage with Read/Write permissions for the same storage?

sytolk commented 3 years ago

If I remove Reac/Write permissions to function the cycling dependency not appears but I cannot access storage from the trigger function:

2021-02-01T18:41:51.420Z    deafcbcd-4a46-4ffc-b3c0-fcf6791b0de5    WARN Error listing directory / AccessDenied: Access Denied
    at Request.extractError (/var/runtime/node_modules/aws-sdk/lib/services/s3.js:700:35)
    at Request.callListeners (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
    at Request.emit (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
    at Request.emit (/var/runtime/node_modules/aws-sdk/lib/request.js:688:14)
    at Request.transition (/var/runtime/node_modules/aws-sdk/lib/request.js:22:10)
    at AcceptorStateMachine.runTo (/var/runtime/node_modules/aws-sdk/lib/state_machine.js:14:12)
    at /var/runtime/node_modules/aws-sdk/lib/state_machine.js:26:10
    at Request.<anonymous> (/var/runtime/node_modules/aws-sdk/lib/request.js:38:9)
    at Request.<anonymous> (/var/runtime/node_modules/aws-sdk/lib/request.js:690:12)
    at Request.callListeners (/var/runtime/node_modules/aws-sdk/lib/sequential_executor.js:116:18) {
  code: 'AccessDenied',
  region: 'eu-central-1',
  time: 2021-02-01T18:41:51.359Z,
  requestId: '787679BB1264CA66',
  extendedRequestId: 'kA0orJAtNgABPeMfubAqAbrddjbSZ/I+xu4EmIpoP5w3h+6Fa26iW6ptc1o+BdlTw36ZGgpX1DU=',
  cfId: undefined,
  statusCode: 403,
  retryable: false,
  retryDelay: 85.77381851989487
}

@jhockett Maybe you can add question tag too: How can I list bucket from a triggered function?

s3.listObjectsV2(params, (error, data) => {
 if (error) {
               console.warn('Error listing directory ' + path, error);
}
akshbhu commented 3 years ago

Hi @sytolk

To add the Trigger you can do 2 ways:

1) Go through add storage flow and add the trigger from there. Make sure either its a new Trigger or existing Trigger that doesn't have dependency on storage(this means you haven't selected resource access permissions for same storage on the function.)

2) Go through update/add function flow and in the advanced workflows : Resource access permissions , select category storage, this will give your function access to storage.

I guess in this comment https://github.com/aws-amplify/amplify-cli/issues/6526#issuecomment-771072732 , you have removed the resources access permissions from the S3 Trigger you created via amplify add storage. This resolved your circular dependency.

For this error we have a PR out for list object actions for S3 which will be available in next release https://github.com/aws-amplify/amplify-cli/pull/6497 , you can try GET/PUT actions which can work.

Let me know if you have any questions

sytolk commented 3 years ago

@akshbhu thanks but after 2. it will have circular dependency too. look at the log from my https://github.com/aws-amplify/amplify-cli/issues/6526#issue-798562357 Tomorrow I will check with the next release.

akshbhu commented 3 years ago

HI @sytolk

I think you are doing amplify function update on a S3 Trigger already created via amplify add storage flow.

Apologies for the confusion. Those were two different ways to get a lambda access to storage.What I mean is either go through amplify add function flow and give access to storage or go through amplify add storage and select trigger from there.

sytolk commented 3 years ago

Hi @akshbhu Yes I have done amlify update function and amlify update storage and I'm expecting that I can update function storage permissions later after amplify add function/storage and if this is not true? this is a new issue.. This is from amplify CLI help file:

  <category> add      Adds a resource for an Amplify category in your local backend                                                                                
  <category> update   Update resource for an Amplify category in your local backend.       

Correct me if something miss me.. but I have not read that triggers and permissions of storage or function created with amplify add cannot be changed later with amplify update ??

p.s. I think it's not enough to simple try/catch cyclic dependencies and change the error message.. amplify can handle circular dependency better. Its need additional questions for this case: amplify update storage -> add Trigger The function have dependency to this storage do you want to remove this dependency to prevent cyclic dependencies ? Do you want to set read/write/delete function permissions for this storage?

sytolk commented 3 years ago

For this error we have a PR out for list object actions for S3 which will be available in next release #6497 , you can try GET/PUT actions which can work.

I can confirm that GET/PUT actions work with the S3 trigger function. The permissions problem is only with list object actions. @akshbhu thanks that pointing me out of the cyclic dependencies :) and is there temporary fix before new release for this access issue maybe I can change cloudformation script and set - 's3:ListBucket' function access to S3 ?

akshbhu commented 3 years ago

Hi @sytolk

As a workaround, you can add this to the S3 Template. Make sure to add the diff as, PUT/GET actions might already be present in the template.

{
  "Effect": "Allow",
  "Action": [
    "s3:PutObject",
    "s3:GetObject"
  ],
  "Resource": [
    {
      "Fn::Join": [
        "",
        [
          "arn:aws:s3:::",
          {
            "Ref": "yourBucketName"
          },
          "/*"
        ]
      ]
    }
  ]
},
{
  "Effect": "Allow",
  "Action": "s3:ListBucket",
  "Resource": [
    {
      "Fn::Join": [
        "",
        [
          "arn:aws:s3:::",
          {
            "Ref": "yourBucketName"
          }
        ]
      ]
    }
  ]
}

Also check this comment: https://github.com/aws-amplify/amplify-cli/issues/5528#issue-717324358 for more details.

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels for those types of questions.