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

Add field to NginxProxy to allow disabling HTTP2 #1925

Merged
merged 4 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions apis/v1alpha1/nginxproxy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ type NginxProxySpec struct {
//
// +optional
Telemetry *Telemetry `json:"telemetry,omitempty"`
// DisableHTTP2 defines if http2 should be disabled for all servers.
// Default is false, meaning http2 will be enabled for all servers.
//
// +optional
DisableHTTP2 bool `json:"disableHTTP2,omitempty"`
pleshakov marked this conversation as resolved.
Show resolved Hide resolved
}

// Telemetry specifies the OpenTelemetry configuration.
Expand Down
1 change: 1 addition & 0 deletions charts/nginx-gateway-fabric/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ nginx:

## The configuration for the data plane that is contained in the NginxProxy resource.
config: {}
# disableHTTP2: false
# telemetry:
# exporter:
# endpoint: otel-collector.default.svc:4317
Expand Down
5 changes: 5 additions & 0 deletions config/crd/bases/gateway.nginx.org_nginxproxies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,11 @@ spec:
spec:
description: Spec defines the desired state of the NginxProxy.
properties:
disableHTTP2:
description: |-
DisableHTTP2 defines if http2 should be disabled for all servers.
Default is false, meaning http2 will be enabled for all servers.
type: boolean
telemetry:
description: Telemetry specifies the OpenTelemetry configuration.
properties:
Expand Down
5 changes: 5 additions & 0 deletions deploy/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,11 @@ spec:
spec:
description: Spec defines the desired state of the NginxProxy.
properties:
disableHTTP2:
description: |-
DisableHTTP2 defines if http2 should be disabled for all servers.
Default is false, meaning http2 will be enabled for all servers.
type: boolean
telemetry:
description: Telemetry specifies the OpenTelemetry configuration.
properties:
Expand Down
2 changes: 0 additions & 2 deletions internal/mode/static/nginx/conf/nginx-plus.conf
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ http {
sendfile on;
tcp_nopush on;

http2 on;

server {
listen 127.0.0.1:8765;
root /usr/share/nginx/html;
Expand Down
2 changes: 0 additions & 2 deletions internal/mode/static/nginx/conf/nginx.conf
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ http {
sendfile on;
tcp_nopush on;

http2 on;

server {
listen unix:/var/run/nginx/nginx-status.sock;
access_log off;
Expand Down
18 changes: 18 additions & 0 deletions internal/mode/static/nginx/config/base_http_config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package config

import (
gotemplate "text/template"

"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
)

var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTPTemplateText))

func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult {
result := executeResult{
dest: httpConfigFile,
data: execute(baseHTTPTemplate, conf.BaseHTTPConfig),
}

return []executeResult{result}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package config

const baseHTTPTemplateText = `
{{- if .HTTP2 }}http2 on;{{ end }}
`
52 changes: 52 additions & 0 deletions internal/mode/static/nginx/config/base_http_config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package config

import (
"strings"
"testing"

. "github.com/onsi/gomega"

"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane"
)

func TestExecuteBaseHttp(t *testing.T) {
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
confOn := dataplane.Configuration{
BaseHTTPConfig: dataplane.BaseHTTPConfig{
HTTP2: true,
},
}

confOff := dataplane.Configuration{
BaseHTTPConfig: dataplane.BaseHTTPConfig{
HTTP2: false,
},
}

expSubStr := "http2 on;"

tests := []struct {
name string
conf dataplane.Configuration
expCount int
}{
{
name: "http2 on",
conf: confOn,
expCount: 1,
},
{
name: "http2 off",
expCount: 0,
conf: confOff,
},
}

for _, test := range tests {

g := NewWithT(t)

res := executeBaseHTTPConfig(test.conf)
g.Expect(res).To(HaveLen(1))
g.Expect(test.expCount).To(Equal(strings.Count(string(res[0].data), expSubStr)))
}
}
1 change: 1 addition & 0 deletions internal/mode/static/nginx/config/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ func (g GeneratorImpl) generateHTTPConfig(conf dataplane.Configuration) []file.F

func (g GeneratorImpl) getExecuteFuncs() []executeFunc {
return []executeFunc{
executeBaseHTTPConfig,
executeServers,
g.executeUpstreams,
executeSplitClients,
Expand Down
4 changes: 4 additions & 0 deletions internal/mode/static/nginx/config/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ func TestGenerate(t *testing.T) {
BatchSize: 512,
BatchCount: 4,
},
BaseHTTPConfig: dataplane.BaseHTTPConfig{
HTTP2: true,
},
}
g := NewWithT(t)

Expand Down Expand Up @@ -104,6 +107,7 @@ func TestGenerate(t *testing.T) {
g.Expect(httpCfg).To(ContainSubstring("batch_size 512;"))
g.Expect(httpCfg).To(ContainSubstring("batch_count 4;"))
g.Expect(httpCfg).To(ContainSubstring("otel_service_name ngf:gw-ns:gw-name:my-name;"))
g.Expect(httpCfg).To(ContainSubstring("http2 on;"))

g.Expect(files[2].Path).To(Equal("/etc/nginx/conf.d/matches.json"))

Expand Down
15 changes: 15 additions & 0 deletions internal/mode/static/state/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ const (
// Used with Accepted (false).
RouteReasonGatewayNotProgrammed v1.RouteConditionReason = "GatewayNotProgrammed"

// RouteReasonUnsupportedConfiguration is used when the associated Gateway does not support the Route.
// Used with Accepted (false).
RouteReasonUnsupportedConfiguration v1.RouteConditionReason = "UnsupportedConfiguration"

// GatewayReasonGatewayConflict indicates there are multiple Gateway resources to choose from,
// and we ignored the resource in question and picked another Gateway as the winner.
// This reason is used with GatewayConditionAccepted (false).
Expand Down Expand Up @@ -241,6 +245,17 @@ func NewRouteNoMatchingParent() conditions.Condition {
}
}

// NewRouteUnsupportedConfiguration returns a Condition that indicates that the Route is not Accepted because
// it is incompatible with the Gateway's configuration.
func NewRouteUnsupportedConfiguration(msg string) conditions.Condition {
return conditions.Condition{
Type: string(v1.RouteConditionAccepted),
Status: metav1.ConditionFalse,
Reason: string(RouteReasonUnsupportedConfiguration),
Message: msg,
}
}

// NewRouteGatewayNotProgrammed returns a Condition that indicates that the Gateway it references is not programmed,
// which does not guarantee that the Route has been configured.
func NewRouteGatewayNotProgrammed(msg string) conditions.Condition {
Expand Down
35 changes: 27 additions & 8 deletions internal/mode/static/state/dataplane/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,18 @@ func BuildConfiguration(
keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners)
certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups)
telemetry := buildTelemetry(g)
baseHTTPConfig := buildBaseHTTPConfig(g)

config := Configuration{
HTTPServers: httpServers,
SSLServers: sslServers,
Upstreams: upstreams,
BackendGroups: backendGroups,
SSLKeyPairs: keyPairs,
Version: configVersion,
CertBundles: certBundles,
Telemetry: telemetry,
HTTPServers: httpServers,
SSLServers: sslServers,
Upstreams: upstreams,
BackendGroups: backendGroups,
SSLKeyPairs: keyPairs,
Version: configVersion,
CertBundles: certBundles,
Telemetry: telemetry,
BaseHTTPConfig: baseHTTPConfig,
}

return config
Expand Down Expand Up @@ -619,3 +621,20 @@ func buildTelemetry(g *graph.Graph) Telemetry {

return tel
}

// buildBaseHTTPConfig generates the base http context config that should be applied to all servers.
func buildBaseHTTPConfig(g *graph.Graph) BaseHTTPConfig {
baseConfig := BaseHTTPConfig{
// HTTP2 should be enabled by default
HTTP2: true,
}
if g.NginxProxy == nil {
return baseConfig
}

if g.NginxProxy.Spec.DisableHTTP2 {
baseConfig.HTTP2 = false
}

return baseConfig
}
Loading
Loading