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
845 stars 361 forks source link

Usage of `sed` in `build-s3-dist.sh` not cross-platform compatible #100

Closed aldegoeij closed 1 year ago

aldegoeij commented 4 years ago

The current deployment/build-s3-dist.sh build file is using sed in MacOS style, not GNU style, resulting in compatibility issues when running in pipelines (see: https://github.com/awslabs/aws-waf-security-automations/issues/32, https://github.com/awslabs/aws-waf-security-automations/issues/30)

For example sed is used here: https://github.com/awslabs/aws-waf-security-automations/blob/e504013c87bde6d6af434097f0c1147a4c1f86c0/deployment/build-s3-dist.sh#L57-L60 uses MacOS -i '' notation which fails on GNU Linux (CentOS, Ubuntu, etc):

Works with default MacOS, but not on any Linux flavour:

sed -i '' -e "s/%%BUCKET_NAME%%/$1/g" target.template

Works on Linux but fails on MacOS:

sed -i -e "s/%%BUCKET_NAME%%/$1/g" target.template

The best solution is for the AWS dev team to stop using MacOS default cli tools and do a brew install sed to fix Apple's mistakes 😜, however, the simplest solution is to move from sed to perl.

The above snippet becomes:

echo "Updating code source bucket in template with '$1'"
replace="s,%%BUCKET_NAME%%,$1,g"
echo "perl -pi -e $replace $dist_dir/aws-waf-security-automations.template"
perl -pi -e "$replace" "$dist_dir"/aws-waf-security-automations.template

The replacement string is almost identical as is the command itself, and perl comes preinstalled on all flavours of MacOS, BSD, GNU Linux, etc.

PR shortly.

aldegoeij commented 4 years ago

100th issue 🎉

beomseoklee commented 4 years ago

Thanks for your input. As you said, if you run sed -i '' -e "s/%%BUCKET_NAME%%/$1/g" target.template on Mac, it would create aws-waf-security-automations.template-e file.

We would consider the better way as you mentioned.

aijunpeng commented 1 year ago

Closing the old ticket due to inactivity