Egress via Ingress VirtualServer Resource (#3491)

* 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 <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Tomás Ó hAodha <[email protected]> Co-authored-by: Venktesh Shivam Patel <[email protected]>
nginx · Apr 14, 2023 · 36ac2ef · 36ac2ef
1 parent 96d28b2
commit 36ac2ef
Show file tree

Hide file tree

Showing 9 changed files with 303 additions and 19 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 {