-
Notifications
You must be signed in to change notification settings - Fork 104
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
Support NginxProxy at the Gateway level #3058
Support NginxProxy at the Gateway level #3058
Conversation
This doc: https://github.com/nginx/nginx-gateway-fabric/blob/main/site/content/how-to/data-plane-configuration.md needs to be updated to reflect the new API, but I decided to add this in a separate PR since this PR is already pretty large. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## change/control-data-plane-split #3058 +/- ##
===================================================================
- Coverage 89.74% 88.77% -0.98%
===================================================================
Files 109 111 +2
Lines 11150 11495 +345
Branches 50 50
===================================================================
+ Hits 10007 10205 +198
- Misses 1083 1235 +152
+ Partials 60 55 -5 ☔ View full report in Codecov by Sentry. |
Looks like codecov is reporting a few misses in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm except for compatibility doc mentioning the version change.
I know you are handling that in another PR?
Good callout! I totally forgot about the compat doc. I'll update that as part of the docs PR |
Co-authored-by: bjee19 <139261241+bjee19@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
54acfb4
into
change/control-data-plane-split
Problem: When the control plane and data planes are split, the user will need the ability to specify data plane settings on a per-Gateway basis. To allow this, we need to support NginxProxy at the Gateway level in addition the the GatewayClass level. In practice, this means a user can reference an NginxProxy resource via the spec.infrastructure.parametersRef field on the Gateway resource. We still want to support referencing an NginxProxy at the GatewayClass level. If a Gateway and its GatewayClass reference distinct NginxProxy resources, the settings must be merged. Settings specified on a Gateway NginxProxy must override those set on the GatewayClass NginxProxy. Solution: To support NginxProxy at the Gateway level several changes were made to the API. As a result, the API is now at version v1alpha2. Breaking Changes: * Change the scope of the CRD to Namespaced. The parametersRef.namespace field on the GatewayClass is now required. * Make DisableHTTP2 and Telemetry.Exporter.Endpoint optional. New fields: * Telemetry.DisabledFeatures: allows users to explicitly disable telemetry features. It is a list with one supported entry: DisableTracing. More features may be added in future releases. Other changes: * Remove the listType=Map kubebuilder annotation from the RewriteClientIP.TrustedAddresses field. This listType is incorrect since TrustedAddresses can have duplicate keys. The graph now stores NginxProxies that are referenced by the winning GatewayClass and Gateway. This will need to be updated once we support multiple Gateways. The graph is also responsible for merging the NginxProxies when necessary. The result of this is stored on the graph's Gateway object in the field EffectiveNginxProxy. The EffectiveNginxProxy on the Gateway is used to build the NGINX configuration.
Proposed changes
Problem: When the control plane and data planes are split, the user will need the ability to specify data plane settings on a per-Gateway basis. To allow this, we need to support NginxProxy at the Gateway level in addition the the GatewayClass level. In practice, this means a user can reference an NginxProxy resource via the
spec.infrastructure.parametersRef
field on the Gateway resource. We still want to support referencing an NginxProxy at the GatewayClass level. If a Gateway and its GatewayClass reference distinct NginxProxy resources, the settings must be merged. Settings specified on a Gateway NginxProxy must override those set on the GatewayClass NginxProxy.Solution:
NginxProxy API Changes
The NginxProxy API is now at version v1alpha2.
Breaking Changes:
parametersRef.namespace
field on the GatewayClass required. If not specified, the NginxProxy resource will not be configured and an error message is left on the GatewayClass status.DisableHTTP2
andTelemetry.Exporter.Endpoint
optional.New fields:
Telemetry.DisabledFeatures
: allows users to explicitly disable telemetry features. It is a list with one supported entry:DisableTracing
. More features may be added in future releases.Other changes:
listType=Map
kubebuilder annotation from theRewriteClientIP.TrustedAddresses
field. This listType is incorrect since TrustedAddresses can have duplicate keys.Code changes
The graph now stores NginxProxies that are referenced by the winning GatewayClass and Gateway. This will need to be updated once we support multiple Gateways. The graph is also responsible for merging the NginxProxies when necessary. The result of this is stored on the graph's Gateway object in the field
EffectiveNginxProxy
. TheEffectiveNginxProxy
on the Gateway is used to build the NGINX configuration.Testing:
Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.
Closes #2990
Checklist
Before creating a PR, run through this checklist and mark each as complete.
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.