Skip to content

Commit

Permalink
Support custom listener ports (#745)
Browse files Browse the repository at this point in the history
Problem: NKG only supported listener ports 80 and 443. This limitation prevents the conformance tests from running because the conformance tests create a base Gateway that listens on port 8080.

Solution: Allow users to specify any port between 1-65535.
  • Loading branch information
kate-osborn committed Jun 20, 2023
1 parent f64d045 commit 2079725
Show file tree
Hide file tree
Showing 22 changed files with 793 additions and 347 deletions.
7 changes: 4 additions & 3 deletions conformance/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ List available commands:
```bash
$ make

build-test-image Build conformance test image
build-test-runner-image Build conformance test runner image
create-kind-cluster Create a kind cluster
delete-kind-cluster Delete kind cluster
help Display this help
install-nkg Install NKG on configured kind cluster
install-nkg Install NKG with provisioner on configured kind cluster
prepare-nkg Build and load NKG container on configured kind cluster
run-conformance-tests Run conformance tests
uninstall-nkg Uninstall NKG from configured kind cluster
uninstall-nkg Uninstall NKG on configured kind cluster
update-test-kind-config Update kind config
```
### Step 1 - Create a kind Cluster
Expand Down
6 changes: 5 additions & 1 deletion deploy/manifests/service/loadbalancer-aws-nlb.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ metadata:
service.beta.kubernetes.io/aws-load-balancer-type: "nlb"
spec:
type: LoadBalancer
ports:
ports: # Update the following ports to match your Gateway Listener ports
- port: 80
targetPort: 80
protocol: TCP
name: http
- port: 443
targetPort: 443
protocol: TCP
name: https
selector:
app: nginx-gateway
2 changes: 1 addition & 1 deletion deploy/manifests/service/loadbalancer.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ metadata:
spec:
externalTrafficPolicy: Local
type: LoadBalancer
ports:
ports: # Update the following ports to match your Gateway Listener ports
- port: 80
targetPort: 80
protocol: TCP
Expand Down
6 changes: 5 additions & 1 deletion deploy/manifests/service/nodeport.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,14 @@ metadata:
namespace: nginx-gateway
spec:
type: NodePort
ports:
ports: # Update the following ports to match your Gateway Listener ports
- port: 80
targetPort: 80
protocol: TCP
name: http
- port: 443
targetPort: 443
protocol: TCP
name: https
selector:
app: nginx-gateway
7 changes: 3 additions & 4 deletions docs/gateway-api-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Fields:
* `listeners`
* `name` - supported.
* `hostname` - partially supported. Wildcard hostnames like `*.example.com` are not yet supported.
* `port` - partially supported. Allowed values: `80` for HTTP listeners and `443` for HTTPS listeners.
* `port` - supported.
* `protocol` - partially supported. Allowed values: `HTTP`, `HTTPS`.
* `tls`
* `mode` - partially supported. Allowed value: `Terminate`.
Expand Down Expand Up @@ -86,13 +86,12 @@ Fields:
* `Accepted/True/Accepted`
* `Accepted/False/UnsupportedProtocol`
* `Accepted/False/InvalidCertificateRef`
* `Accepted/False/HostnameConflict`
* `Accepted/False/PortUnavailable`
* `Accepted/False/ProtocolConflict`
* `Accepted/False/UnsupportedValue`: Custom reason for when a value of a field in a Listener is invalid or not supported.
* `Accepted/False/GatewayConflict`: Custom reason for when the Gateway is ignored due to a conflicting Gateway. NKG only supports a single Gateway.
* `ResolvedRefs/True/ResolvedRefs`
* `ResolvedRefs/False/InvalidCertificateRef`
* `Conflicted/True/HostnameConflict`
* `Conflicted/True/ProtocolConflict`
* `Conflicted/False/NoConflicts`

### HTTPRoute
Expand Down
5 changes: 5 additions & 0 deletions docs/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ This guide walks you through how to install NGINX Kubernetes Gateway on a generi

You can gain access to NGINX Kubernetes Gateway by creating a `NodePort` Service or a `LoadBalancer` Service.

> Important
>
> The Service manifests expose NGINX Kubernetes Gateway on ports 80 and 443, which exposes any Gateway [Listener](https://gateway-api.sigs.k8s.io/references/spec/#gateway.networking.k8s.io/v1beta1.Listener) configured for those ports. If you'd like to use different ports in your listeners,
> update the manifests accordingly.
### Create a NodePort Service

Create a Service with type `NodePort`:
Expand Down
3 changes: 3 additions & 0 deletions internal/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ func Start(cfg config.Config) error {
options := manager.Options{
Scheme: scheme,
Logger: logger,
// We disable the metrics server because we reserve all ports (1-65535) for the data plane.
// Once we add support for Prometheus, we can make this port configurable by the user.
MetricsBindAddress: "0",
}

eventCh := make(chan interface{})
Expand Down
27 changes: 11 additions & 16 deletions internal/nginx/config/generator_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package config_test

import (
"strings"
"testing"

. "github.com/onsi/gomega"
"k8s.io/apimachinery/pkg/types"

"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/config"
Expand All @@ -26,20 +26,24 @@ func TestGenerate(t *testing.T) {
HTTPServers: []dataplane.VirtualServer{
{
IsDefault: true,
Port: 80,
},
{
Hostname: "example.com",
Port: 80,
},
},
SSLServers: []dataplane.VirtualServer{
{
IsDefault: true,
Port: 443,
},
{
Hostname: "example.com",
SSL: &dataplane.SSL{
CertificatePath: "/etc/nginx/secrets/default",
},
Port: 443,
},
},
Upstreams: []dataplane.Upstream{
Expand All @@ -50,22 +54,13 @@ func TestGenerate(t *testing.T) {
},
BackendGroups: []dataplane.BackendGroup{bg},
}
g := NewGomegaWithT(t)

generator := config.NewGeneratorImpl()
cfg := string(generator.Generate(conf))

if !strings.Contains(cfg, "listen 80") {
t.Errorf("Generate() did not generate a config with a default HTTP server; config: %s", cfg)
}

if !strings.Contains(cfg, "listen 443") {
t.Errorf("Generate() did not generate a config with an SSL server; config: %s", cfg)
}

if !strings.Contains(cfg, "upstream") {
t.Errorf("Generate() did not generate a config with an upstream block; config: %s", cfg)
}

if !strings.Contains(cfg, "split_clients") {
t.Errorf("Generate() did not generate a config with an split_clients block; config: %s", cfg)
}
g.Expect(cfg).To(ContainSubstring("listen 80"))
g.Expect(cfg).To(ContainSubstring("listen 443"))
g.Expect(cfg).To(ContainSubstring("upstream"))
g.Expect(cfg).To(ContainSubstring("split_clients"))
}
1 change: 1 addition & 0 deletions internal/nginx/config/http/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ type Server struct {
Locations []Location
IsDefaultHTTP bool
IsDefaultSSL bool
Port int32
}

// Location holds all configuration for an HTTP location.
Expand Down
30 changes: 15 additions & 15 deletions internal/nginx/config/servers.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ func createServers(httpServers, sslServers []dataplane.VirtualServer) []http.Ser

func createSSLServer(virtualServer dataplane.VirtualServer) http.Server {
if virtualServer.IsDefault {
return createDefaultSSLServer()
return http.Server{
IsDefaultSSL: true,
Port: virtualServer.Port,
}
}

return http.Server{
Expand All @@ -51,22 +54,27 @@ func createSSLServer(virtualServer dataplane.VirtualServer) http.Server {
Certificate: virtualServer.SSL.CertificatePath,
CertificateKey: virtualServer.SSL.CertificatePath,
},
Locations: createLocations(virtualServer.PathRules, 443),
Locations: createLocations(virtualServer.PathRules, virtualServer.Port),
Port: virtualServer.Port,
}
}

func createServer(virtualServer dataplane.VirtualServer) http.Server {
if virtualServer.IsDefault {
return createDefaultHTTPServer()
return http.Server{
IsDefaultHTTP: true,
Port: virtualServer.Port,
}
}

return http.Server{
ServerName: virtualServer.Hostname,
Locations: createLocations(virtualServer.PathRules, 80),
Locations: createLocations(virtualServer.PathRules, virtualServer.Port),
Port: virtualServer.Port,
}
}

func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Location {
func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.Location {
lenPathRules := len(pathRules)

if lenPathRules == 0 {
Expand Down Expand Up @@ -171,15 +179,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo
return locs
}

func createDefaultSSLServer() http.Server {
return http.Server{IsDefaultSSL: true}
}

func createDefaultHTTPServer() http.Server {
return http.Server{IsDefaultHTTP: true}
}

func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int) *http.Return {
func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int32) *http.Return {
if filter == nil {
return nil
}
Expand All @@ -196,7 +196,7 @@ func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter,

port := listenerPort
if filter.Port != nil {
port = int(*filter.Port)
port = int32(*filter.Port)
}

scheme := "$scheme"
Expand Down
76 changes: 39 additions & 37 deletions internal/nginx/config/servers_template.go
Original file line number Diff line number Diff line change
@@ -1,57 +1,59 @@
package config

var serversTemplateText = `
{{ range $s := . -}}
{{ if $s.IsDefaultSSL -}}
{{- range $s := . -}}
{{ if $s.IsDefaultSSL -}}
server {
listen 443 ssl default_server;
listen {{ $s.Port }} ssl default_server;
ssl_reject_handshake on;
ssl_reject_handshake on;
}
{{- else if $s.IsDefaultHTTP }}
{{- else if $s.IsDefaultHTTP }}
server {
listen 80 default_server;
listen {{ $s.Port }} default_server;
default_type text/html;
return 404;
default_type text/html;
return 404;
}
{{- else }}
{{- else }}
server {
{{- if $s.SSL }}
listen 443 ssl;
ssl_certificate {{ $s.SSL.Certificate }};
ssl_certificate_key {{ $s.SSL.CertificateKey }};
{{- if $s.SSL }}
listen {{ $s.Port }} ssl;
ssl_certificate {{ $s.SSL.Certificate }};
ssl_certificate_key {{ $s.SSL.CertificateKey }};
if ($ssl_server_name != $host) {
return 421;
}
{{- end }}
if ($ssl_server_name != $host) {
return 421;
}
{{- else }}
listen {{ $s.Port }};
{{- end }}
server_name {{ $s.ServerName }};
server_name {{ $s.ServerName }};
{{ range $l := $s.Locations }}
location {{ if $l.Exact }}= {{ end }}{{ $l.Path }} {
{{ if $l.Internal -}}
internal;
{{ end }}
{{ range $l := $s.Locations }}
location {{ if $l.Exact }}= {{ end }}{{ $l.Path }} {
{{ if $l.Internal -}}
internal;
{{ end }}
{{- if $l.Return -}}
return {{ $l.Return.Code }} "{{ $l.Return.Body }}";
{{ end }}
{{- if $l.Return -}}
return {{ $l.Return.Code }} "{{ $l.Return.Body }}";
{{ end }}
{{- if $l.HTTPMatchVar -}}
set $http_matches {{ $l.HTTPMatchVar | printf "%q" }};
js_content httpmatches.redirect;
{{ end }}
{{- if $l.HTTPMatchVar -}}
set $http_matches {{ $l.HTTPMatchVar | printf "%q" }};
js_content httpmatches.redirect;
{{ end }}
{{- if $l.ProxyPass -}}
proxy_set_header Host $host;
proxy_pass {{ $l.ProxyPass }}$request_uri;
{{- end }}
}
{{ end }}
{{- if $l.ProxyPass -}}
proxy_set_header Host $host;
proxy_pass {{ $l.ProxyPass }}$request_uri;
{{- end }}
}
{{ end }}
}
{{- end }}
{{- end }}
{{ end }}
server {
listen unix:/var/lib/nginx/nginx-502-server.sock;
Expand Down
Loading

0 comments on commit 2079725

Please sign in to comment.