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 serverSnippets to TransportServer #1413

Merged
merged 1 commit into from
Mar 11, 2021
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
2 changes: 1 addition & 1 deletion cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ func main() {
templateExecutorV2, *nginxPlus, isWildcardEnabled, plusCollector, *enablePrometheusMetrics, latencyCollector, *enableLatencyMetrics)
controllerNamespace := os.Getenv("POD_NAMESPACE")

transportServerValidator := cr_validation.NewTransportServerValidator(*enableTLSPassthrough)
transportServerValidator := cr_validation.NewTransportServerValidator(*enableTLSPassthrough, *enableSnippets)
virtualServerValidator := cr_validation.NewVirtualServerValidator(*nginxPlus)

lbcInput := k8s.NewLoadBalancerControllerInput{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ spec:
type: string
protocol:
type: string
serverSnippets:
type: string
sessionParameters:
description: SessionParameters defines session parameters.
type: object
Expand Down
2 changes: 2 additions & 0 deletions deployments/common/crds/k8s.nginx.org_transportservers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ spec:
type: string
protocol:
type: string
serverSnippets:
type: string
sessionParameters:
description: SessionParameters defines session parameters.
type: object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ spec:
type: string
protocol:
type: string
serverSnippets:
type: string
sessionParameters:
description: SessionParameters defines session parameters.
type: object
Expand Down
36 changes: 36 additions & 0 deletions docs-web/configuration/transportserver-resource.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This document is the reference documentation for the TransportServer resource. T
- [SessionParameters](#sessionparameters)
- [Action](#action)
- [Using TransportServer](#using-transportserver)
- [Usings Snippets](#using-snippets)
- [Validation](#validation)
- [Structural Validation](#structural-validation)
- [Comprehensive Validation](#comprehensive-validation)
Expand Down Expand Up @@ -370,6 +371,41 @@ secure-app 46sm

In the kubectl get and similar commands, you can also use the short name `ts` instead of `transportserver`.

### Using Snippets
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved

Snippets allow you to insert raw NGINX config into different contexts of NGINX configuration. In the example below, we use snippets to configure [access control](http://nginx.org/en/docs/stream/ngx_stream_access_module.html) in a TransportServer:

```yaml
apiVersion: k8s.nginx.org/v1alpha1
kind: TransportServer
metadata:
name: cafe
spec:
host: cafe.example.com
serverSnippets: |
deny 192.168.1.1;
allow 192.168.1.0/24;
upstreams:
- name: tea
service: tea-svc
port: 80
```

Snippets are intended to be used by advanced NGINX users who need more control over the generated NGINX configuration.

However, because of the disadvantages described below, snippets are disabled by default. To use snippets, set the [`enable-snippets`](/nginx-ingress-controller/configuration/global-configuration/command-line-arguments#cmdoption-enable-snippets) command-line argument.

Disadvantages of using snippets:
* *Complexity*. To use snippets, you will need to:
* Understand NGINX configuration primitives and implement a correct NGINX configuration.
* Understand how the IC generates NGINX configuration so that a snippet doesn't interfere with the other features in the configuration.
* *Decreased robustness*. An incorrect snippet makes the NGINX config invalid which will lead to a failed reload. This will prevent any new configuration updates, including updates for the other TransportServer resource until the snippet is fixed.
* *Security implications*. Snippets give access to NGINX configuration primitives and those primitives are not validated by the Ingress Controller.


> Note that during a period when the NGINX config includes an invalid snippet, NGINX will continue to operate with the latest valid configuration.


### Validation

Two types of validation are available for the TransportServer resource:
Expand Down
3 changes: 2 additions & 1 deletion internal/configs/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *Transport

tsCfg := generateTransportServerConfig(transportServerEx, transportServerEx.ListenerPort, cnf.isPlus)

content, err := cnf.templateExecutorV2.ExecuteTransportServerTemplate(&tsCfg)
content, err := cnf.templateExecutorV2.ExecuteTransportServerTemplate(tsCfg)
if err != nil {
return fmt.Errorf("Error generating TransportServer config %v: %v", name, err)
}
Expand Down Expand Up @@ -1073,6 +1073,7 @@ func (cnf *Configurator) UpdateConfig(cfgParams *ConfigParams, ingExes []*Ingres
return allWarnings, nil
}

// UpdateTransportServers updates TransportServers.
func (cnf *Configurator) UpdateTransportServers(updatedTSExes []*TransportServerEx, deletedKeys []string) error {
for _, tsEx := range updatedTSExes {
err := cnf.addOrUpdateTransportServer(tsEx)
Expand Down
13 changes: 8 additions & 5 deletions internal/configs/transportserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (tsEx *TransportServerEx) String() string {
}

// generateTransportServerConfig generates a full configuration for a TransportServer.
func generateTransportServerConfig(transportServerEx *TransportServerEx, listenerPort int, isPlus bool) version2.TransportServerConfig {
func generateTransportServerConfig(transportServerEx *TransportServerEx, listenerPort int, isPlus bool) *version2.TransportServerConfig {
upstreamNamer := newUpstreamNamerForTransportServer(transportServerEx.TransportServer)

upstreams := generateStreamUpstreams(transportServerEx, upstreamNamer, isPlus)
Expand Down Expand Up @@ -59,14 +59,14 @@ func generateTransportServerConfig(transportServerEx *TransportServerEx, listene
proxyTimeout = transportServerEx.TransportServer.Spec.SessionParameters.Timeout
}

statusZone := ""
serverSnippets := generateSnippets(true, transportServerEx.TransportServer.Spec.ServerSnippets, []string{})

statusZone := transportServerEx.TransportServer.Spec.Listener.Name
if transportServerEx.TransportServer.Spec.Listener.Name == conf_v1alpha1.TLSPassthroughListenerName {
statusZone = transportServerEx.TransportServer.Spec.Host
} else {
statusZone = transportServerEx.TransportServer.Spec.Listener.Name
}

return version2.TransportServerConfig{
tsConfig := &version2.TransportServerConfig{
Server: version2.StreamServer{
TLSPassthrough: transportServerEx.TransportServer.Spec.Listener.Name == conf_v1alpha1.TLSPassthroughListenerName,
UnixSocket: generateUnixSocket(transportServerEx),
Expand All @@ -84,9 +84,12 @@ func generateTransportServerConfig(transportServerEx *TransportServerEx, listene
ProxyNextUpstreamTimeout: generateTimeWithDefault(nextUpstreamTimeout, "0s"),
ProxyNextUpstreamTries: nextUpstreamTries,
HealthCheck: healthCheck,
Snippets: serverSnippets,
},
Upstreams: upstreams,
}

return tsConfig
}

func generateUnixSocket(transportServerEx *TransportServerEx) string {
Expand Down
108 changes: 92 additions & 16 deletions internal/configs/transportserver_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package configs

import (
"reflect"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -62,6 +61,82 @@ func TestTransportServerExString(t *testing.T) {
}
}

func TestGenerateTransportServerConfigForTCPSnippets(t *testing.T) {
LorcanMcVeigh marked this conversation as resolved.
Show resolved Hide resolved
transportServerEx := TransportServerEx{
TransportServer: &conf_v1alpha1.TransportServer{
ObjectMeta: meta_v1.ObjectMeta{
Name: "tcp-server",
Namespace: "default",
},
Spec: conf_v1alpha1.TransportServerSpec{
Listener: conf_v1alpha1.TransportServerListener{
Name: "tcp-listener",
Protocol: "TCP",
},
Upstreams: []conf_v1alpha1.Upstream{
{
Name: "tcp-app",
Service: "tcp-app-svc",
Port: 5001,
},
},
Action: &conf_v1alpha1.Action{
Pass: "tcp-app",
},
ServerSnippets: "deny 192.168.1.1;\nallow 192.168.1.0/24;",
},
},
Endpoints: map[string][]string{
"default/tcp-app-svc:5001": {
"10.0.0.20:5001",
},
},
}

listenerPort := 2020

expected := &version2.TransportServerConfig{
Upstreams: []version2.StreamUpstream{
{
Name: "ts_default_tcp-server_tcp-app",
Servers: []version2.StreamUpstreamServer{
{
Address: "10.0.0.20:5001",
MaxFails: 1,
FailTimeout: "10s",
},
},
UpstreamLabels: version2.UpstreamLabels{
ResourceName: "tcp-server",
ResourceType: "transportserver",
ResourceNamespace: "default",
Service: "tcp-app-svc",
},
},
},
Server: version2.StreamServer{
Port: listenerPort,
UDP: false,
StatusZone: "tcp-listener",
ProxyPass: "ts_default_tcp-server_tcp-app",
Name: "tcp-server",
Namespace: "default",
ProxyConnectTimeout: "60s",
ProxyNextUpstream: false,
ProxyNextUpstreamTries: 0,
ProxyNextUpstreamTimeout: "0s",
ProxyTimeout: "10m",
HealthCheck: nil,
Snippets: []string{"deny 192.168.1.1;", "allow 192.168.1.0/24;"},
},
}

result := generateTransportServerConfig(&transportServerEx, listenerPort, true)
if diff := cmp.Diff(expected, result); diff != "" {
t.Errorf("generateTransportServerConfig() mismatch (-want +got):\n%s", diff)
}
}

func TestGenerateTransportServerConfigForTCP(t *testing.T) {
transportServerEx := TransportServerEx{
TransportServer: &conf_v1alpha1.TransportServer{
Expand Down Expand Up @@ -104,7 +179,7 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) {

listenerPort := 2020

expected := version2.TransportServerConfig{
expected := &version2.TransportServerConfig{
Upstreams: []version2.StreamUpstream{
{
Name: "ts_default_tcp-server_tcp-app",
Expand Down Expand Up @@ -136,14 +211,15 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) {
ProxyNextUpstreamTimeout: "0s",
ProxyTimeout: "50s",
HealthCheck: nil,
Snippets: []string{},
},
}

isPlus := true
result := generateTransportServerConfig(&transportServerEx, listenerPort, isPlus)
if !reflect.DeepEqual(result, expected) {
t.Errorf("generateTransportServerConfig() returned \n%+v but expected \n%+v", result, expected)
result := generateTransportServerConfig(&transportServerEx, listenerPort, true)
if diff := cmp.Diff(expected, result); diff != "" {
t.Errorf("generateTransportServerConfig() mismatch (-want +got):\n%s", diff)
}

}

func TestGenerateTransportServerConfigForTLSPasstrhough(t *testing.T) {
Expand Down Expand Up @@ -186,7 +262,7 @@ func TestGenerateTransportServerConfigForTLSPasstrhough(t *testing.T) {

listenerPort := 2020

expected := version2.TransportServerConfig{
expected := &version2.TransportServerConfig{
Upstreams: []version2.StreamUpstream{
{
Name: "ts_default_tcp-server_tcp-app",
Expand Down Expand Up @@ -220,13 +296,13 @@ func TestGenerateTransportServerConfigForTLSPasstrhough(t *testing.T) {
ProxyNextUpstreamTries: 0,
ProxyTimeout: "10m",
HealthCheck: nil,
Snippets: []string{},
},
}

isPlus := true
result := generateTransportServerConfig(&transportServerEx, listenerPort, isPlus)
if !reflect.DeepEqual(result, expected) {
t.Errorf("generateTransportServerConfig() returned \n%+v but expected \n%+v", result, expected)
result := generateTransportServerConfig(&transportServerEx, listenerPort, true)
if diff := cmp.Diff(expected, result); diff != "" {
t.Errorf("generateTransportServerConfig() mismatch (-want +got):\n%s", diff)
}
}

Expand Down Expand Up @@ -275,7 +351,7 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {

listenerPort := 2020

expected := version2.TransportServerConfig{
expected := &version2.TransportServerConfig{
Upstreams: []version2.StreamUpstream{
{
Name: "ts_default_udp-server_udp-app",
Expand Down Expand Up @@ -309,13 +385,13 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) {
ProxyNextUpstreamTries: 0,
ProxyTimeout: "10m",
HealthCheck: nil,
Snippets: []string{},
},
}

isPlus := true
result := generateTransportServerConfig(&transportServerEx, listenerPort, isPlus)
if !reflect.DeepEqual(result, expected) {
t.Errorf("generateTransportServerConfig() returned \n%+v but expected \n%+v", result, expected)
result := generateTransportServerConfig(&transportServerEx, listenerPort, true)
if diff := cmp.Diff(expected, result); diff != "" {
t.Errorf("generateTransportServerConfig() mismatch (-want +got):\n%s", diff)
}
}

Expand Down
4 changes: 4 additions & 0 deletions internal/configs/version2/nginx-plus.transportserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ server {
proxy_responses {{ $s.ProxyResponses }};
{{ end }}

{{ range $snippet := $s.Snippets }}
{{- $snippet }}
{{ end }}

proxy_pass {{ $s.ProxyPass }};

{{ if $s.HealthCheck }}
Expand Down
4 changes: 4 additions & 0 deletions internal/configs/version2/nginx.transportserver.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ server {
proxy_responses {{ $s.ProxyResponses }};
{{ end }}

{{ range $snippet := $s.Snippets }}
{{- $snippet }}
{{ end }}

proxy_pass {{ $s.ProxyPass }};

proxy_timeout {{ $s.ProxyTimeout }};
Expand Down
1 change: 1 addition & 0 deletions internal/configs/version2/stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ type StreamServer struct {
ProxyNextUpstreamTimeout string
ProxyNextUpstreamTries int
HealthCheck *StreamHealthCheck
Snippets []string
}

// StreamHealthCheck defines a health check for a StreamUpstream in a StreamServer.
Expand Down
10 changes: 5 additions & 5 deletions internal/configs/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS
}
}

httpSnippets := generateSnippets(vsc.enableSnippets, vsEx.VirtualServer.Spec.HTTPSnippets, []string{""})
httpSnippets := generateSnippets(vsc.enableSnippets, vsEx.VirtualServer.Spec.HTTPSnippets, []string{})
serverSnippets := generateSnippets(
vsc.enableSnippets,
vsEx.VirtualServer.Spec.ServerSnippets,
Expand Down Expand Up @@ -1410,11 +1410,11 @@ func generateTimeWithDefault(value string, defaultValue string) string {
return generateTime(value)
}

func generateSnippets(enableSnippets bool, s string, defaultS []string) []string {
if !enableSnippets || s == "" {
return defaultS
func generateSnippets(enableSnippets bool, snippet string, defaultSnippets []string) []string {
if !enableSnippets || snippet == "" {
return defaultSnippets
}
return strings.Split(s, "\n")
return strings.Split(snippet, "\n")
}

func generateBuffers(s *conf_v1.UpstreamBuffers, defaultS string) string {
Expand Down
Loading