Skip to content

Commit

Permalink
Add field to NginxProxy to allow disabling HTTP2 (#1925)
Browse files Browse the repository at this point in the history
* Add field to NginxProxy to allow disabling HTTP2
  • Loading branch information
ciarams87 committed May 16, 2024
1 parent 869ac38 commit 250792c
Show file tree
Hide file tree
Showing 20 changed files with 278 additions and 62 deletions.
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"`
}

// 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) {
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
}

0 comments on commit 250792c

Please sign in to comment.