sinofseven / serverless-s3-remover

this is plugin of serverless, before "sls remove", this empties the s3 bucket.
MIT License
39 stars 11 forks source link

Unable to pass a CloudFormation Ref as bucket name #3

Open guillaumervls opened 6 years ago

guillaumervls commented 6 years ago

Hi and thanks for this plugin,

However it does not seem to work when passed a CloudFormation "Ref", like this :

remover:
   buckets:
     - Ref: S3Bucket

It then says (in the logs) :

Serverless: make buckets empty
Serverless: faild: [object Object] may not be empty.

Am I missing something ?

Cheers

sinofseven commented 6 years ago

Thank you, use this plugin! I may not be bothered by English because I am not good at it, but do not mind.

To put it briefly, the way of writing is wrong, the correct way to write is as follows.

custom:
  s3BucketName: (your bucket name)
  remover:
    buckets:
      - ${self:custom.s3BucketName}
resources:
  Resources:
    S3Bucket:
      Type: AWS::S3::Bucket
      Properties:
        BucketName: ${self:custom.s3BucketName}

Instead of emptying the S3 bucket with CloudFormation, this plugin empties it with Serverless Framework (Node.js) before CloudFormation deletes the stack. Therefore, you can not use the CloudFormation function to set the plugin. Instead, you can use the Serverless Framework variables (written in the example), so use this one.

guillaumervls commented 6 years ago

Thanks for your answer. That's what I was thinking about. But I was wondering if you had an idea when you don't want to name the bucket, because you want to leave the naming to AWS (since bucket names must be unique among all S3). 😉

sinofseven commented 6 years ago

If CloudFormation does not specify S3's bucket name, it will automatically be named. Thank you, I knew for the first time.

Then you can use buffet name from the stack information of CloudFormation using service and stage of serverless.yml. However, since it is not implemented by the current version of the plugin, it is necessary to update it.

tommedema commented 6 years ago

@sinofseven it is bad practice to hardcode your s3 bucket names. You should be able to deploy numerous times, without being aware of which bucket names are already reserved. This allows for developers to easily deploy to review stages and for CI processes to easily spawn a new infrastructure for automated tests etc. Therefore the request made here is quite a good one and I would say that such references are a must for a plugin like this to be used in production environments.

sinofseven commented 6 years ago

@tommedema thank you, your suggestion!
Even if "Ref" is written in short form, can you meet your request?
I am planning to adapt the code to only short forms.

tommedema commented 6 years ago

@sinofseven I'm not currently using this plugin, was just trying to backup @guillaumervls's request :)

I'm not sure I understand your question. Let me know if I can be of any help.

sinofseven commented 6 years ago

CloudFormation's Ref function has two notations for YAML.

# full function
Ref: logicalName
# short form
!Ref logicalName

I tried to make it possible to use only short form for this plugin. Because it thought that correspondence is easy because it is a character string rather than a map unlike ordinary writing method if it is here. It is a previous comment that whether there is any inconvenience that it corresponds to only short form without corresponding to ordinary writing style.

tommedema commented 6 years ago

@sinofseven imo it should support normal Ref: syntax, because this is the ordinary way of doing it in serverless. !Ref is cloudformation only syntax that does not work out of the box in serverless.yml.

simlu commented 5 years ago

This would be great to have supported!

m-radzikowski commented 4 years ago

I solved it for myself to have unique bucket names generated by CF and exposing it in CF Outputs, and then referencing it in the plugin config:

custom:
  baseName: someName-${opt:stage}

  remover:
    buckets:
      - ${cf:${self:custom.baseName}.MyBucketToRemoveName, ''}

provider:
  name: aws
  stage: ${opt:stage}
  stackName: ${self:custom.baseName}

resources:
  Resources:

    # name of this bucket will have a random suffix for uniqueness
    MyBucketToRemove:
      Type: AWS::S3::Bucket

  Outputs:

    MyBucketToRemoveName:
      Value: !Ref MyBucketToRemove
      Export:
        Name: ${self:custom.baseName}-MyBucketToRemoveName