AWScliSetup.sh: Support for dedicated installation of AWS RPMs

plus3it / amigen7

Set of tools to provide automation of tasks for creating STIG-partitioned EL7 AMIs

Apache License 2.0

33 stars 27 forks source link

AWScliSetup.sh: Support for dedicated installation of AWS RPMs #90

Closed Rendanic closed 4 years ago

Rendanic commented 4 years ago

The script has been refactored for suppoting the installation of dedicated RPMs from an environment variable.

The commits are splitted to show the changes and enhancements. A support for setting the list of RPMs with parameters during execution is planned for a later time.

lorengordon commented 4 years ago

@Rendanic I haven't been able to do a thorough review, but there are a couple things we try to be very careful with these days:

run the script with bash -eu -o pipefail to be sure errors are not masked
test the yum install to be sure the requested packages are actually present
- yum has a bad habit of returning a zero exit code even when it does not actually install the requested packages for some reason
- yum also has a bad habit of exiting non-zero when a single-requested package is already installed
- this is why the epel-release yum install was so complex... i think your alternative approach is equivalent, but we'll need some time to really exercise it

Rendanic commented 4 years ago

test the yum install to be sure the requested packages are actually present

I tried some ideas but I didn't found s nice lookung working solution. => I ddidn't change the code in this point. It was bad in the past and is still an issue...

yum also has a bad habit of exiting non-zero when a single-requested package is already installed

I did multiple test and cannot confirm this behavior. Maybe that bug has been fixed?

this is why the epel-release yum install was so complex... i think your alternative approach is equivalent, but we'll need some time to really exercise it

I still have a new idea and try to write some code for it.

Rendanic commented 4 years ago

run the script with bash -eu -o pipefail to be sure errors are not masked

This point should be adressed in a new PR. No bash script in this repo has these settings and I think it is a bad idea, starting with this during major changes in the same script.

ferricoxide commented 4 years ago

run the script with bash -eu -o pipefail to be sure errors are not masked

This point should be adressed in a new PR. No bash script in this repo has these settings and I think it is a bad idea, starting with this during major changes in the same script.

No, none of the scripts in the repo are executed that way, it's simply how other projects leveraging the scripts invoke them (to ensure those wrappers bail on any errors that aren't otherwise explicitly trapped for).

Rendanic commented 4 years ago

I did a push --force with signed commits.