From 36ac2ef7e0fedb685a68667a97153ffe3b569f32 Mon Sep 17 00:00:00 2001 From: Chase Kiefer <112438922+chase-kiefer@users.noreply.github.com> Date: Fri, 14 Apr 2023 11:22:03 -0500 Subject: [PATCH] Egress via Ingress VirtualServer Resource (#3491) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add ability for nginx service mesh to egress through a virtualserver resource - added internalRoute field to the virtualserver CRD - added templates for internal routes in virtualserver templates for n+ and oss - added unit test to validate virtualserver internal routes - added enableInternalRoutes boolean to virtualServerConfigurator type - updated virtualserver configuration items to include internRoute docs * Add a description for the InternalRoute field in the VS CRD * Add test case for nsmEgress being true in TestIsTLSEnabled * Update the isTLSEnabled function for clarity * Reverse function params for isTLSEnabled * Add virtual server internal route validation and warning - Add warning to catch cases where a virtual server internal route should not be created - Switch variable names to match ingress naming scheme * Add refactored VS templates to avoid duplicate listen blocks * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Add conditional to prevent SpiffeClientCerts being set for internal routes * Fix unit tests --------- Co-authored-by: Ciara Stacke <18287516+ciarams87@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Tomás Ó hAodha <86358393+tomasohaodha@users.noreply.github.com> Co-authored-by: Venktesh Shivam Patel --- .../crds/k8s.nginx.org_virtualservers.yaml | 3 + .../crds/k8s.nginx.org_virtualservers.yaml | 3 + ...server-and-virtualserverroute-resources.md | 1 + internal/configs/version2/http.go | 17 +- .../version2/nginx-plus.virtualserver.tmpl | 14 +- .../configs/version2/nginx.virtualserver.tmpl | 13 +- internal/configs/virtualserver.go | 28 +- internal/configs/virtualserver_test.go | 241 +++++++++++++++++- pkg/apis/configuration/v1/types.go | 2 + 9 files changed, 303 insertions(+), 19 deletions(-) diff --git a/deployments/common/crds/k8s.nginx.org_virtualservers.yaml b/deployments/common/crds/k8s.nginx.org_virtualservers.yaml index b6fe476eab..3dbdea9786 100644 --- a/deployments/common/crds/k8s.nginx.org_virtualservers.yaml +++ b/deployments/common/crds/k8s.nginx.org_virtualservers.yaml @@ -93,6 +93,9 @@ spec: type: string ingressClassName: type: string + internalRoute: + description: InternalRoute allows for the configuration of internal routing. + type: boolean policies: type: array items: diff --git a/deployments/helm-chart/crds/k8s.nginx.org_virtualservers.yaml b/deployments/helm-chart/crds/k8s.nginx.org_virtualservers.yaml index b6fe476eab..3dbdea9786 100644 --- a/deployments/helm-chart/crds/k8s.nginx.org_virtualservers.yaml +++ b/deployments/helm-chart/crds/k8s.nginx.org_virtualservers.yaml @@ -93,6 +93,9 @@ spec: type: string ingressClassName: type: string + internalRoute: + description: InternalRoute allows for the configuration of internal routing. + type: boolean policies: type: array items: diff --git a/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md b/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md index 432450f1c8..dab909e831 100644 --- a/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md @@ -59,6 +59,7 @@ spec: |``upstreams`` | A list of upstreams. | [[]upstream](#upstream) | No | |``routes`` | A list of routes. | [[]route](#virtualserverroute) | No | |``ingressClassName`` | Specifies which Ingress Controller must handle the VirtualServer resource. | ``string`` | No | +|``internalRoute`` | Specifies if the VirtualServer resource is an internal route or not. | ``boolean`` | No | |``http-snippets`` | Sets a custom snippet in the http context. | ``string`` | No | |``server-snippets`` | Sets a custom snippet in server context. Overrides the ``server-snippets`` ConfigMap key. | ``string`` | No | {{% /table %}} diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 29fc7310ff..8da6938a79 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -12,14 +12,15 @@ type UpstreamLabels struct { // VirtualServerConfig holds NGINX configuration for a VirtualServer. type VirtualServerConfig struct { - HTTPSnippets []string - LimitReqZones []LimitReqZone - Maps []Map - Server Server - SpiffeCerts bool - SplitClients []SplitClient - StatusMatches []StatusMatch - Upstreams []Upstream + HTTPSnippets []string + LimitReqZones []LimitReqZone + Maps []Map + Server Server + SpiffeCerts bool + SpiffeClientCerts bool + SplitClients []SplitClient + StatusMatches []StatusMatch + Upstreams []Upstream } // Upstream defines an upstream. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 846320e58a..72648d11f9 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -107,11 +107,21 @@ server { {{ if $ssl.RejectHandshake }} ssl_reject_handshake on; + {{ else if $.SpiffeCerts }} + ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; + ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; {{ else }} ssl_certificate {{ $ssl.Certificate }}; ssl_certificate_key {{ $ssl.CertificateKey }}; {{ end }} - {{ end }} + {{ else }} + {{ if $.SpiffeCerts }} + listen 443 ssl; + {{if not $s.DisableIPV6}}listen [::]:443 ssl;{{end}} + ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; + ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + {{ end }} + {{ end }} {{ with $s.IngressMTLS }} ssl_client_certificate {{ .ClientCert }}; @@ -584,7 +594,7 @@ server { {{ range $h := $l.AddHeaders }} add_header {{ $h.Name }} "{{ $h.Value }}" {{ if $h.Always }}always{{ end }}; {{ end }} - {{ if $.SpiffeCerts }} + {{ if $.SpiffeClientCerts }} {{ $proxyOrGRPC }}_ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; {{ $proxyOrGRPC }}_ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; {{ $proxyOrGRPC }}_ssl_trusted_certificate /etc/nginx/secrets/spiffe_rootca.pem; diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 2f2d604cb7..0ea8e47d51 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -62,10 +62,20 @@ server { {{ if $ssl.RejectHandshake }} ssl_reject_handshake on; + {{ else if $.SpiffeCerts }} + ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; + ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; {{ else }} ssl_certificate {{ $ssl.Certificate }}; ssl_certificate_key {{ $ssl.CertificateKey }}; {{ end }} + {{ else }} + {{ if $.SpiffeCerts }} + listen 443 ssl; + {{if not $s.DisableIPV6}}listen [::]:443 ssl;{{end}} + ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; + ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; + {{ end }} {{ end }} {{ with $s.IngressMTLS }} @@ -313,7 +323,6 @@ server { {{ if $l.ProxyBufferSize }} {{ $proxyOrGRPC }}_buffer_size {{ $l.ProxyBufferSize }}; {{ end }} - {{ if not $l.GRPCPass }} proxy_http_version 1.1; set $default_connection_header {{ if $l.HasKeepalive }}""{{ else }}close{{ end }}; @@ -360,7 +369,7 @@ server { {{ range $h := $l.AddHeaders }} add_header {{ $h.Name }} "{{ $h.Value }}" {{ if $h.Always }}always{{ end }}; {{ end }} - {{ if $.SpiffeCerts }} + {{ if $.SpiffeClientCerts }} {{ $proxyOrGRPC }}_ssl_certificate /etc/nginx/secrets/spiffe_cert.pem; {{ $proxyOrGRPC }}_ssl_certificate_key /etc/nginx/secrets/spiffe_key.pem; {{ $proxyOrGRPC }}_ssl_trusted_certificate /etc/nginx/secrets/spiffe_rootca.pem; diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index fc41497d87..4d2c3e95fb 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -232,6 +232,7 @@ type virtualServerConfigurator struct { enableSnippets bool warnings Warnings spiffeCerts bool + enableInternalRoutes bool oidcPolCfg *oidcPolicyCfg isIPV6Disabled bool } @@ -272,6 +273,7 @@ func newVirtualServerConfigurator( enableSnippets: staticParams.EnableSnippets, warnings: make(map[runtime.Object][]string), spiffeCerts: staticParams.NginxServiceMesh, + enableInternalRoutes: staticParams.EnableInternalRoutes, oidcPolCfg: &oidcPolicyCfg{}, isIPV6Disabled: staticParams.DisableIPV6, } @@ -327,6 +329,13 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( dosCfg := generateDosCfg(dosResources[""]) + // enabledInternalRoutes controls if a virtual server is configured as an internal route. + enabledInternalRoutes := vsEx.VirtualServer.Spec.InternalRoute + if vsEx.VirtualServer.Spec.InternalRoute && !vsc.enableInternalRoutes { + vsc.addWarningf(vsEx.VirtualServer, "Internal Route cannot be configured for virtual server %s. Internal Routes can be enabled by setting the enable-internal-routes flag", vsEx.VirtualServer.Name) + enabledInternalRoutes = false + } + // crUpstreams maps an UpstreamName to its conf_v1.Upstream as they are generated // necessary for generateLocation to know what Upstream each Location references crUpstreams := make(map[string]conf_v1.Upstream) @@ -355,7 +364,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( ups := vsc.generateUpstream(vsEx.VirtualServer, upstreamName, u, isExternalNameSvc, endpoints) upstreams = append(upstreams, ups) - u.TLS.Enable = isTLSEnabled(u, vsc.spiffeCerts) + u.TLS.Enable = isTLSEnabled(u, vsc.spiffeCerts, vsEx.VirtualServer.Spec.InternalRoute) crUpstreams[upstreamName] = u if hc := generateHealthCheck(u, upstreamName, vsc.cfgParams); hc != nil { @@ -384,7 +393,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( _, isExternalNameSvc := vsEx.ExternalNameSvcs[GenerateExternalNameSvcKey(upstreamNamespace, u.Service)] ups := vsc.generateUpstream(vsr, upstreamName, u, isExternalNameSvc, endpoints) upstreams = append(upstreams, ups) - u.TLS.Enable = isTLSEnabled(u, vsc.spiffeCerts) + u.TLS.Enable = isTLSEnabled(u, vsc.spiffeCerts, vsEx.VirtualServer.Spec.InternalRoute) crUpstreams[upstreamName] = u if hc := generateHealthCheck(u, upstreamName, vsc.cfgParams); hc != nil { @@ -676,7 +685,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( VSName: vsEx.VirtualServer.Name, DisableIPV6: vsc.isIPV6Disabled, }, - SpiffeCerts: vsc.spiffeCerts, + SpiffeCerts: enabledInternalRoutes, + SpiffeClientCerts: vsc.spiffeCerts && !enabledInternalRoutes, } return vsCfg, vsc.warnings @@ -2474,8 +2484,16 @@ func generateProxySSLName(svcName, ns string) string { return fmt.Sprintf("%s.%s.svc", svcName, ns) } -func isTLSEnabled(u conf_v1.Upstream, spiffeCerts bool) bool { - return u.TLS.Enable || spiffeCerts +// isTLSEnabled checks whether TLS is enabled for the given upstream, taking into account the configuration +// of the NGINX Service Mesh and the presence of SPIFFE certificates. +func isTLSEnabled(upstream conf_v1.Upstream, hasSpiffeCerts, isInternalRoute bool) bool { + if isInternalRoute { + // Internal routes in the NGINX Service Mesh do not require TLS. + return false + } + + // TLS is enabled if explicitly configured for the upstream or if SPIFFE certificates are present. + return upstream.TLS.Enable || hasSpiffeCerts } func isGRPC(protocolType string) bool { diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index d42f538d8f..933d4931d1 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -1256,7 +1256,7 @@ func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { }, }, }, - SpiffeCerts: true, + SpiffeClientCerts: true, } isPlus := false @@ -1275,6 +1275,232 @@ func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { } } +func TestGenerateVirtualServerConfigWithInternalRoutes(t *testing.T) { + t.Parallel() + virtualServerEx := VirtualServerEx{ + VirtualServer: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cafe", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerSpec{ + Host: "cafe.example.com", + Upstreams: []conf_v1.Upstream{ + { + Name: "tea", + Service: "tea-svc", + Port: 80, + TLS: conf_v1.UpstreamTLS{Enable: false}, + }, + }, + Routes: []conf_v1.Route{ + { + Path: "/", + Action: &conf_v1.Action{ + Pass: "tea", + }, + }, + }, + InternalRoute: true, + }, + }, + Endpoints: map[string][]string{ + "default/tea-svc:80": { + "10.0.0.20:80", + }, + }, + } + + baseCfgParams := ConfigParams{ + ServerTokens: "off", + Keepalive: 16, + ServerSnippets: []string{"# server snippet"}, + ProxyProtocol: true, + SetRealIPFrom: []string{"0.0.0.0/0"}, + RealIPHeader: "X-Real-IP", + RealIPRecursive: true, + } + + expected := version2.VirtualServerConfig{ + Upstreams: []version2.Upstream{ + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_tea", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.20:80", + }, + }, + Keepalive: 16, + }, + }, + HTTPSnippets: []string{}, + LimitReqZones: []version2.LimitReqZone{}, + Server: version2.Server{ + ServerName: "cafe.example.com", + StatusZone: "cafe.example.com", + VSNamespace: "default", + VSName: "cafe", + ProxyProtocol: true, + ServerTokens: "off", + SetRealIPFrom: []string{"0.0.0.0/0"}, + RealIPHeader: "X-Real-IP", + RealIPRecursive: true, + Snippets: []string{"# server snippet"}, + TLSPassthrough: true, + Locations: []version2.Location{ + { + Path: "/", + ProxyPass: "http://vs_default_cafe_tea", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: true, + ProxySSLName: "tea-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "tea-svc", + }, + }, + }, + SpiffeCerts: true, + SpiffeClientCerts: false, + } + + isPlus := false + isResolverConfigured := false + staticConfigParams := &StaticConfigParams{TLSPassthrough: true, NginxServiceMesh: true, EnableInternalRoutes: true} + isWildcardEnabled := false + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled) + + result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) + if diff := cmp.Diff(expected, result); diff != "" { + t.Errorf("GenerateVirtualServerConfig() mismatch (-want +got):\n%s", diff) + } + + if len(warnings) != 0 { + t.Errorf("GenerateVirtualServerConfig returned warnings: %v", vsc.warnings) + } +} + +func TestGenerateVirtualServerConfigWithInternalRoutesWarning(t *testing.T) { + t.Parallel() + virtualServerEx := VirtualServerEx{ + VirtualServer: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cafe", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerSpec{ + Host: "cafe.example.com", + Upstreams: []conf_v1.Upstream{ + { + Name: "tea", + Service: "tea-svc", + Port: 80, + TLS: conf_v1.UpstreamTLS{Enable: false}, + }, + }, + Routes: []conf_v1.Route{ + { + Path: "/", + Action: &conf_v1.Action{ + Pass: "tea", + }, + }, + }, + InternalRoute: true, + }, + }, + Endpoints: map[string][]string{ + "default/tea-svc:80": { + "10.0.0.20:80", + }, + }, + } + + baseCfgParams := ConfigParams{ + ServerTokens: "off", + Keepalive: 16, + ServerSnippets: []string{"# server snippet"}, + ProxyProtocol: true, + SetRealIPFrom: []string{"0.0.0.0/0"}, + RealIPHeader: "X-Real-IP", + RealIPRecursive: true, + } + + expected := version2.VirtualServerConfig{ + Upstreams: []version2.Upstream{ + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_tea", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.20:80", + }, + }, + Keepalive: 16, + }, + }, + HTTPSnippets: []string{}, + LimitReqZones: []version2.LimitReqZone{}, + Server: version2.Server{ + ServerName: "cafe.example.com", + StatusZone: "cafe.example.com", + VSNamespace: "default", + VSName: "cafe", + ProxyProtocol: true, + ServerTokens: "off", + SetRealIPFrom: []string{"0.0.0.0/0"}, + RealIPHeader: "X-Real-IP", + RealIPRecursive: true, + Snippets: []string{"# server snippet"}, + TLSPassthrough: true, + Locations: []version2.Location{ + { + Path: "/", + ProxyPass: "http://vs_default_cafe_tea", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: true, + ProxySSLName: "tea-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "tea-svc", + }, + }, + }, + SpiffeCerts: true, + SpiffeClientCerts: true, + } + + isPlus := false + isResolverConfigured := false + staticConfigParams := &StaticConfigParams{TLSPassthrough: true, NginxServiceMesh: true, EnableInternalRoutes: false} + isWildcardEnabled := false + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled) + + result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) + if diff := cmp.Diff(expected, result); diff == "" { + t.Errorf("GenerateVirtualServerConfig() should not configure internal route") + } + + if len(warnings) != 1 { + t.Errorf("GenerateVirtualServerConfig should return warning to enable internal routing") + } +} + func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { t.Parallel() virtualServerEx := VirtualServerEx{ @@ -8251,6 +8477,7 @@ func TestIsTLSEnabled(t *testing.T) { tests := []struct { upstream conf_v1.Upstream spiffeCert bool + nsmEgress bool expected bool }{ { @@ -8289,10 +8516,20 @@ func TestIsTLSEnabled(t *testing.T) { spiffeCert: false, expected: true, }, + { + upstream: conf_v1.Upstream{ + TLS: conf_v1.UpstreamTLS{ + Enable: true, + }, + }, + nsmEgress: true, + spiffeCert: false, + expected: false, + }, } for _, test := range tests { - result := isTLSEnabled(test.upstream, test.spiffeCert) + result := isTLSEnabled(test.upstream, test.spiffeCert, test.nsmEgress) if result != test.expected { t.Errorf("isTLSEnabled(%v, %v) returned %v but expected %v", test.upstream, test.spiffeCert, result, test.expected) } diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index f994afcda3..c6a134383a 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -46,6 +46,8 @@ type VirtualServerSpec struct { ServerSnippets string `json:"server-snippets"` Dos string `json:"dos"` ExternalDNS ExternalDNS `json:"externalDNS"` + // InternalRoute allows for the configuration of internal routing. + InternalRoute bool `json:"internalRoute"` } // ExternalDNS defines externaldns sub-resource of a virtual server.