Skip to content
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

Feature/upgrade codebuild http2 #4571

Merged
merged 5 commits into from
Mar 21, 2024
Merged

Conversation

dhaley
Copy link
Collaborator

@dhaley dhaley commented Mar 18, 2024

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)

@dhaley dhaley added the Maintenance Tag as maintenance if the issue relates to general cleanup, maintenance, etc. Do not delete label. label Mar 18, 2024
@dhaley dhaley requested a review from kflemin March 18, 2024 20:09
@axelstudios
Copy link
Member

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
Copy link
Collaborator Author

dhaley commented Mar 19, 2024

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

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

@kflemin kflemin merged commit bbf7518 into develop Mar 21, 2024
8 checks passed
@kflemin kflemin deleted the feature/upgrade-codebuild-http2 branch March 21, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Maintenance Tag as maintenance if the issue relates to general cleanup, maintenance, etc. Do not delete label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants