Conversation
|
|
||
| # Update sudoers | ||
| # Using tee to append to sudoers to handle permissions, and ensuring lines are added only once. | ||
| if ! sudo grep -q "www-data ALL=(ALL) NOPASSWD: /usr/sbin/apachectl configtest" /etc/sudoers; then |
There was a problem hiding this comment.
Just a nitpick, but everywhere else you've been good to wrap IF conditions with brackets- why not here? also, Probably better to compare the actual return code -ne 0 or something.
| required_providers { | ||
| aws = { | ||
| source = "hashicorp/aws" | ||
| version = "~> 5.0" |
There was a problem hiding this comment.
Can this be pinned more specifically? I'm just worried about the edge-case of some other terraform code trying to interact with resources created by this choking on a version difference (and yes, that happens, and is why unity-proxy is very specifically pinned)
| required_version = ">= 1.0" | ||
|
|
||
| # Local backend for state management | ||
| backend "local" { |
There was a problem hiding this comment.
why not a S3 backend? As a more fault-tolerant option when our instances evaporate
| } | ||
|
|
||
| # Download files from S3 | ||
| aws s3 sync s3://${S3_BUCKET}/ /etc/apache2/venues.d/ --exclude "*" --include "*.conf" --delete --quiet |
There was a problem hiding this comment.
For this, it doesn't seem like we have a nice failure state- that is, if the configtest fails, we still have a broken configuration in-place, right?
Could we have something to copy-out the original config, then pull the new config, and copyback on failure?
(something as simple as rsync -ax --delete /etc/apache2/venues.d/ /etc/apache2/venues.d.reload_safe/ and vice versa)
jpl-btlunsfo
left a comment
There was a problem hiding this comment.
I have some questions/nitpicks, but overall approve.
Purpose
Proposed Changes
Issues
Testing