aws-solutions / aws-waf-security-automations

This solution automatically deploys a single web access control list (web ACL) with a set of AWS WAF rules designed to filter common web-based attacks.
https://aws.amazon.com/solutions/aws-waf-security-automations
Apache License 2.0
854 stars 365 forks source link

README is junk #111

Closed scastria closed 4 years ago

scastria commented 4 years ago
  1. Prerequisites: It says Node.js 10.x, but then says the latest version has only been tested with Node.js 8.10

  2. Declare environment variables: It says to make variables TEMPLATE_BUCKET_NAME and DIST_BUCKET_NAME, but then in step 04, it references TEMPLATE_OUTPUT_BUCKET and DIST_OUTPUT_BUCKET

  3. Upload deployment assets to your Amazon S3 bucket: It says to copy ./dist to the S3 bucket, but after running the build script, it doesn't generate a dist directory.

ryanvito commented 4 years ago

I second this. Trying to walk through this readme.md is like falling down a flight of stairs. I appreciate the effort put into making this repo but I'm completely baffled how to use it when it is one error after the other. Issue number 3 is the biggest concern for me since I'm trying to figure this stuff out on the fly.

  1. Same issue but I was easily able to figure it out and fix the variable names

3. Same issue: I am assuming it means the stuff created in the regional-s3-assets folder?

  1. The pip installs are not multi-platform. I needed to change it to pip3 and got the error: "must supply either home or prefix/exec-prefix -- not both". I had to add a setup.cfg to each folder with a requirements.txt with [install] prefix= in it to make it work.
ghost commented 4 years ago

Hate to be that guy, but the readme really is broken. This is because every "minor" update overhauls the entire project (see #116). It would be really great if the maintainers could establish something a little more rigorous so we customers don't have to re-implement everything anytime there's an update.

georgebearden commented 4 years ago

The README has been updated in the latest version. Please let us know if additional clarification is needed. Thank you.