Skip to content

Conversation

ciarams87
Copy link
Contributor

@ciarams87 ciarams87 commented May 2, 2024

Proposed changes

Problem: We need a way to allow a user to disable HTTP2

Solution: Extend the NginxProxy CRD to include a DisableHTTP2 option which will apply to all servers

Testing: Unit and local testing, confirmed that the http2 directive is set correctly unless disabled. If disabled, confirmed that a condition is added to any GRPCRoutes.

Closes #1858

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Added configuration option to disable HTTP2 to the NginxProxy CRD

@github-actions github-actions bot added enhancement New feature or request helm-chart Relates to helm chart labels May 2, 2024
Copy link

codecov bot commented May 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.96%. Comparing base (869ac38) to head (1e02137).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1925      +/-   ##
==========================================
+ Coverage   86.90%   86.96%   +0.05%     
==========================================
  Files          88       89       +1     
  Lines        6056     6083      +27     
  Branches       50       50              
==========================================
+ Hits         5263     5290      +27     
  Misses        741      741              
  Partials       52       52              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ciarams87 ciarams87 force-pushed the feat/add-disable-http2-option branch 3 times, most recently from 5abffd1 to dc4828e Compare May 15, 2024 09:51
@ciarams87 ciarams87 marked this pull request as ready for review May 15, 2024 10:11
@ciarams87 ciarams87 requested a review from a team as a code owner May 15, 2024 10:11
@ciarams87 ciarams87 changed the title Add baseHTTPConfig to NginxProxy to allow disabling HTTP2 Add option to NginxProxy to allow disabling HTTP2 May 15, 2024
@ciarams87 ciarams87 changed the title Add option to NginxProxy to allow disabling HTTP2 Add field to NginxProxy to allow disabling HTTP2 May 15, 2024
Copy link
Contributor

@kate-osborn kate-osborn left a comment

Choose a reason for hiding this comment

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

Just a couple nits

@ciarams87 ciarams87 force-pushed the feat/add-disable-http2-option branch from 3b07215 to 8ba7bb1 Compare May 16, 2024 09:52
@ciarams87 ciarams87 requested a review from kate-osborn May 16, 2024 14:10
@ciarams87 ciarams87 force-pushed the feat/add-disable-http2-option branch from 8ba7bb1 to 9e09fbd Compare May 16, 2024 15:48
@ciarams87 ciarams87 force-pushed the feat/add-disable-http2-option branch from 9e09fbd to 1e02137 Compare May 16, 2024 16:30
@ciarams87 ciarams87 enabled auto-merge (squash) May 16, 2024 16:31
@ciarams87 ciarams87 merged commit 250792c into nginx:main May 16, 2024
miledxz added a commit to miledxz/nginx-gateway-fabric that referenced this pull request Jan 14, 2025
* Add field to NginxProxy to allow disabling HTTP2
@ciarams87 ciarams87 deleted the feat/add-disable-http2-option branch April 12, 2025 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request helm-chart Relates to helm chart release-notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add mechanism for user to disable HTTP/2 if required
4 participants