Skip to content

SS Proxy Update #132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

SS Proxy Update #132

wants to merge 18 commits into from

Conversation

jl-0
Copy link

@jl-0 jl-0 commented Jun 9, 2025

Purpose

  • Refactor the SS Proxy to use multiple config files for each venue/project

Proposed Changes

  • [ADD] scripts, terraform, and documentation to deploy the SS proxy

Issues

  • Saw cases of a malformed .conf file previously with duplicated sections. Seemed likely that race conditions had been experienced.

Testing

  • Implemented a second proxy on port 8443, added endpoint to Congnito and ensured routes were working when added to S3 bucket, also generated a test script to hit the bucket in random patterns and make sure the Apache server was synchronized.


# 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor

@jpl-btlunsfo jpl-btlunsfo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some questions/nitpicks, but overall approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants