search

SEED-platform / seed

Standard Energy Efficiency Data (SEED) Platform™ is a web-based application that helps organizations easily manage data on the energy performance of large groups of buildings.
Other
106 stars 55 forks source link

Feature/upgrade codebuild http2 #4571

Closed dhaley closed 3 months ago

dhaley commented 3 months ago

Support for multiple project builds and optional HTTP2

Adapt buildspec.yml and Makefile so that codebase can be built by multiple Codeubild jobs to produce Docker image artifacts in multiple repos.

Adds support for optionally using HTTP2 with environment variable at docker build time.

How should this be manually tested?

Test builds in Codebuild or manually.

docker build -t $(REPO):$(TAG) \ -f Dockerfile \ --build-arg NGINX_LISTEN_OPTS=$(NGINX_LISTEN_OPTS) \ ./

What are the relevant tickets?

Screenshots (if appropriate)

axelstudios commented 3 months ago

Copying this comment from the previous PR:

I don't think the http2 directive will do anything here, since HTTP/2 only works over HTTPS. This Nginx config is only used for uWSGI and special handling for a few routes - We have a separate Nginx container for HTTP/2 with SSL termination and security headers, but when we're not using that we just use CloudFlare in front of this config for HTTP/3

dhaley commented 3 months ago

Copying this comment from the previous PR:

I don't think the http2 directive will do anything here, since HTTP/2 only works over HTTPS. This Nginx config is only used for uWSGI and special handling for a few routes - We have a separate Nginx container for HTTP/2 with SSL termination and security headers, but when we're not using that we just use CloudFlare in front of this config for HTTP/3

@axelstudios , technically HTTP2 works without SSL termination in the AWS ECS environment as we have a lot of sites using that configuration. Your are right that SSL termination is recommended though when using HTTP2. The advantage of having NGINX configured with HTTP2 when the TargetGroup is also HTTP2 is that no errors are generated in the TargetGroup Healthcheck. Without nginx being configured for HTTP, then the TargetGroup Healthcheck receives a 400 error causing the healthcheck to fail.

The default on the container build is still to just use HTTP1.1 so this would be a non-breaking change. But the Docker ARG is needed for the Stratus Environment to allow the Healthcheck to pass now that all of our TargetGroups and Load-balancers have been upgraded to default to HTTP2.

I can supply more info or screenshots if you want to see more specifics of the Stratus production environment. If this change is controversial, I could break this pull request into two requests since there is also a commit to support multiple projects using this codebase in Codebuild. But taking the HTTP2 support out of this request will cause me to have to do significant work for the seedcerl project to create TargetGroups and load-balancers in Stratus that are downgraded to HTTP1.1.

axelstudios commented 3 months ago

@dhaley Gotcha, that's good enough for me if this is something that tangibly benefits the ECS setup