diff --git a/apis/v1alpha2/nginxproxy_types.go b/apis/v1alpha2/nginxproxy_types.go index 192818cb3c..c5d9dfcef1 100644 --- a/apis/v1alpha2/nginxproxy_types.go +++ b/apis/v1alpha2/nginxproxy_types.go @@ -94,6 +94,11 @@ type NginxProxySpec struct { // +kubebuilder:validation:Minimum=1 // +kubebuilder:validation:Maximum=65535 WorkerConnections *int32 `json:"workerConnections,omitempty"` + // DNSResolver specifies the DNS resolver configuration for external name resolution. + // This enables support for routing to ExternalName Services. + // + // +optional + DNSResolver *DNSResolver `json:"dnsResolver,omitempty"` } // Telemetry specifies the OpenTelemetry configuration. @@ -355,6 +360,61 @@ type NginxPlus struct { AllowedAddresses []NginxPlusAllowAddress `json:"allowedAddresses,omitempty"` } +// DNSResolver specifies the DNS resolver configuration for NGINX. +// This enables dynamic DNS resolution for ExternalName Services. +// Corresponds to the NGINX resolver directive: https://nginx.org/en/docs/http/ngx_http_core_module.html#resolver +type DNSResolver struct { + // Timeout specifies the timeout for name resolution. + // + // +optional + Timeout *v1alpha1.Duration `json:"timeout,omitempty"` + + // CacheTTL specifies how long to cache DNS responses. + // + // +optional + CacheTTL *v1alpha1.Duration `json:"cacheTTL,omitempty"` + + // DisableIPv6 disables IPv6 lookups. + // If not specified, or set to false, IPv6 lookups will be enabled. + // + // +optional + DisableIPv6 *bool `json:"disableIPv6,omitempty"` + + // Addresses specifies the list of DNS server addresses. + // Each address can be an IP address or hostname. + // Example: [{"type": "IPAddress", "value": "8.8.8.8"}, {"type": "Hostname", "value": "dns.google"}] + // + // +kubebuilder:validation:MinItems=1 + // +kubebuilder:validation:MaxItems=16 + Addresses []DNSResolverAddress `json:"addresses"` +} + +// DNSResolverAddress specifies the address type and value for a DNS resolver address. +type DNSResolverAddress struct { + // Type specifies the type of address. + Type DNSResolverAddressType `json:"type"` + + // Value specifies the address value. + // When Type is "IPAddress", this must be a valid IPv4 or IPv6 address. + // When Type is "Hostname", this must be a valid hostname. + // + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=253 + Value string `json:"value"` +} + +// DNSResolverAddressType specifies the type of DNS resolver address. +// +kubebuilder:validation:Enum=IPAddress;Hostname +type DNSResolverAddressType string + +const ( + // DNSResolverIPAddressType specifies that the address is an IP address. + DNSResolverIPAddressType DNSResolverAddressType = "IPAddress" + + // DNSResolverHostnameType specifies that the address is a hostname. + DNSResolverHostnameType DNSResolverAddressType = "Hostname" +) + // NginxPlusAllowAddress specifies the address type and value for an NginxPlus allow address. type NginxPlusAllowAddress struct { // Type specifies the type of address. diff --git a/apis/v1alpha2/zz_generated.deepcopy.go b/apis/v1alpha2/zz_generated.deepcopy.go index b12a4f0478..4ae3fecd4c 100644 --- a/apis/v1alpha2/zz_generated.deepcopy.go +++ b/apis/v1alpha2/zz_generated.deepcopy.go @@ -107,6 +107,56 @@ func (in *ContainerSpec) DeepCopy() *ContainerSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DNSResolver) DeepCopyInto(out *DNSResolver) { + *out = *in + if in.Timeout != nil { + in, out := &in.Timeout, &out.Timeout + *out = new(v1alpha1.Duration) + **out = **in + } + if in.CacheTTL != nil { + in, out := &in.CacheTTL, &out.CacheTTL + *out = new(v1alpha1.Duration) + **out = **in + } + if in.DisableIPv6 != nil { + in, out := &in.DisableIPv6, &out.DisableIPv6 + *out = new(bool) + **out = **in + } + if in.Addresses != nil { + in, out := &in.Addresses, &out.Addresses + *out = make([]DNSResolverAddress, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSResolver. +func (in *DNSResolver) DeepCopy() *DNSResolver { + if in == nil { + return nil + } + out := new(DNSResolver) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *DNSResolverAddress) DeepCopyInto(out *DNSResolverAddress) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DNSResolverAddress. +func (in *DNSResolverAddress) DeepCopy() *DNSResolverAddress { + if in == nil { + return nil + } + out := new(DNSResolverAddress) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DaemonSetSpec) DeepCopyInto(out *DaemonSetSpec) { *out = *in @@ -436,6 +486,11 @@ func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = new(int32) **out = **in } + if in.DNSResolver != nil { + in, out := &in.DNSResolver, &out.DNSResolver + *out = new(DNSResolver) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. diff --git a/charts/nginx-gateway-fabric/values.schema.json b/charts/nginx-gateway-fabric/values.schema.json index 7a2b269134..2c2b2e0eb4 100644 --- a/charts/nginx-gateway-fabric/values.schema.json +++ b/charts/nginx-gateway-fabric/values.schema.json @@ -126,6 +126,62 @@ "required": [], "type": "boolean" }, + "dnsResolver": { + "description": "DNSResolver specifies the DNS resolver configuration for external name resolution. This enables support for routing to ExternalName Services.", + "properties": { + "addresses": { + "description": "List of DNS server addresses. Each address specifies a type and value.", + "items": { + "properties": { + "type": { + "description": "Type specifies the type of address.", + "enum": [ + "IPAddress", + "Hostname" + ], + "required": [], + "type": "string" + }, + "value": { + "description": "Value specifies the address value.", + "maxItems": 253, + "minItems": 1, + "required": [], + "type": "string" + } + }, + "required": [ + "type", + "value" + ], + "type": "object" + }, + "maxItems": 16, + "minItems": 1, + "required": [], + "type": "array" + }, + "cacheTTL": { + "description": "CacheTTL specifies how long to cache DNS responses.", + "pattern": "^\\d+[smhd]?$", + "required": [], + "type": "string" + }, + "disableIPv6": { + "description": "DisableIPv6 disables DisableIPv6 lookups. If not specified, or set to false, IPv6 lookups will be enabled.", + "required": [], + "type": "boolean" + }, + "timeout": { + "description": "Timeout specifies the timeout for name resolution.", + "pattern": "^\\d+[smhd]?$", + "required": [], + "type": "string" + } + }, + "required": [], + "type": "object" + }, "ipFamily": { "description": "IPFamily specifies the IP family to be used by the NGINX.", "enum": [ diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 79d259a26d..ca5dcbd440 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -462,6 +462,43 @@ nginx: # minimum: 1 # maximum: 65535 # description: The number of worker connections for NGINX. Default is 1024. + # dnsResolver: + # type: object + # description: DNSResolver specifies the DNS resolver configuration for external name resolution. This enables support for routing to ExternalName Services. + # properties: + # addresses: + # type: array + # description: List of DNS server addresses. Each address specifies a type and value. + # items: + # type: object + # properties: + # type: + # type: string + # enum: + # - IPAddress + # - Hostname + # description: Type specifies the type of address. + # value: + # type: string + # minItems: 1 + # maxItems: 253 + # description: Value specifies the address value. + # required: + # - type + # - value + # minItems: 1 + # maxItems: 16 + # timeout: + # type: string + # description: Timeout specifies the timeout for name resolution. + # pattern: ^\d+[smhd]?$ + # cacheTTL: + # type: string + # description: CacheTTL specifies how long to cache DNS responses. + # pattern: ^\d+[smhd]?$ + # disableIPv6: + # type: boolean + # description: DisableIPv6 disables DisableIPv6 lookups. If not specified, or set to false, IPv6 lookups will be enabled. # @schema # -- The configuration for the data plane that is contained in the NginxProxy resource. This is applied globally to all Gateways # managed by this instance of NGINX Gateway Fabric. diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index a6a47b493e..588e4922d8 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -65,6 +65,57 @@ spec: introduces security risks as described in Gateway API GEP-3567. If not specified, defaults to false (validation enabled). type: boolean + dnsResolver: + description: |- + DNSResolver specifies the DNS resolver configuration for external name resolution. + This enables support for routing to ExternalName Services. + properties: + addresses: + description: |- + Addresses specifies the list of DNS server addresses. + Each address can be an IP address or hostname. + Example: [{"type": "IPAddress", "value": "8.8.8.8"}, {"type": "Hostname", "value": "dns.google"}] + items: + description: DNSResolverAddress specifies the address type and + value for a DNS resolver address. + properties: + type: + description: Type specifies the type of address. + enum: + - IPAddress + - Hostname + type: string + value: + description: |- + Value specifies the address value. + When Type is "IPAddress", this must be a valid IPv4 or IPv6 address. + When Type is "Hostname", this must be a valid hostname. + maxLength: 253 + minLength: 1 + type: string + required: + - type + - value + type: object + maxItems: 16 + minItems: 1 + type: array + cacheTTL: + description: CacheTTL specifies how long to cache DNS responses. + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ + type: string + disableIPv6: + description: |- + DisableIPv6 disables IPv6 lookups. + If not specified, or set to false, IPv6 lookups will be enabled. + type: boolean + timeout: + description: Timeout specifies the timeout for name resolution. + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ + type: string + required: + - addresses + type: object ipFamily: default: dual description: |- diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 9c2299d24b..c4f7b5942a 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -650,6 +650,57 @@ spec: introduces security risks as described in Gateway API GEP-3567. If not specified, defaults to false (validation enabled). type: boolean + dnsResolver: + description: |- + DNSResolver specifies the DNS resolver configuration for external name resolution. + This enables support for routing to ExternalName Services. + properties: + addresses: + description: |- + Addresses specifies the list of DNS server addresses. + Each address can be an IP address or hostname. + Example: [{"type": "IPAddress", "value": "8.8.8.8"}, {"type": "Hostname", "value": "dns.google"}] + items: + description: DNSResolverAddress specifies the address type and + value for a DNS resolver address. + properties: + type: + description: Type specifies the type of address. + enum: + - IPAddress + - Hostname + type: string + value: + description: |- + Value specifies the address value. + When Type is "IPAddress", this must be a valid IPv4 or IPv6 address. + When Type is "Hostname", this must be a valid hostname. + maxLength: 253 + minLength: 1 + type: string + required: + - type + - value + type: object + maxItems: 16 + minItems: 1 + type: array + cacheTTL: + description: CacheTTL specifies how long to cache DNS responses. + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ + type: string + disableIPv6: + description: |- + DisableIPv6 disables IPv6 lookups. + If not specified, or set to false, IPv6 lookups will be enabled. + type: boolean + timeout: + description: Timeout specifies the timeout for name resolution. + pattern: ^[0-9]{1,4}(ms|s|m|h)?$ + type: string + required: + - addresses + type: object ipFamily: default: dual description: |- diff --git a/examples/externalname-service/README.md b/examples/externalname-service/README.md new file mode 100644 index 0000000000..59e5aae979 --- /dev/null +++ b/examples/externalname-service/README.md @@ -0,0 +1,108 @@ +# ExternalName Services Example + +This example demonstrates how to use NGINX Gateway Fabric to route traffic to external services using [Kubernetes ExternalName Services](https://kubernetes.io/docs/concepts/services-networking/service/#externalname). This enables you to proxy traffic to external APIs or services that are not running in your cluster. + +## Overview + +In this example, we will: + +1. Deploy NGINX Gateway Fabric with DNS resolver configuration +2. Create an ExternalName service pointing to an external API +3. Configure HTTP and TLS routes to route traffic to this external service +4. Test both HTTP and TLS routing functionality + +## Running the Example + +## 1. Deploy NGINX Gateway Fabric + +1. Follow the [installation instructions](https://docs.nginx.com/nginx-gateway-fabric/install/) to deploy NGINX Gateway Fabric. + +## 2. Deploy the Gateway with DNS Resolver + +Create the Gateway and NginxProxy configuration that enables DNS resolution for ExternalName services: + +```shell +kubectl apply -f gateway.yaml +``` + +This creates: + +- A Gateway with HTTP and TLS listeners +- An NginxProxy resource with DNS resolver configuration + +## 3. Deploy ExternalName Services + +Create the ExternalName service that points to external APIs: + +```shell +kubectl apply -f external-service.yaml +``` + +This creates an ExternalName service which points to `httpbin.org` with both HTTP and HTTPS ports + +## 4. Configure Routing + +Create the HTTPRoute and TLSRoute that route traffic to the external service: + +```shell +kubectl apply -f route.yaml +``` + +This creates: + +- An HTTPRoute that routes HTTP traffic to the httpbin service +- A TLSRoute that routes TLS traffic to the httpbin service + +## 5. Test the Configuration + +Wait for the Gateway to be ready: + +```shell +kubectl wait --for=condition=Programmed gateway/external-gateway --timeout=60s +``` + +Save the public IP address and ports of the NGINX Service into shell variables: + +```text +GW_IP=XXX.YYY.ZZZ.III +GW_PORT_HTTP= +GW_PORT_HTTPS= +``` + +Test the httpbin service via HTTP: + +```shell +curl --resolve httpbin.example.com:$GW_PORT_HTTP:$GW_IP http://httpbin.example.com:$GW_PORT_HTTP/get +``` + +Test the httpbin service via TLS passthrough: + +```shell +curl -k --resolve httpbin.example.com:$GW_PORT_HTTPS:$GW_IP https://httpbin.example.com:$GW_PORT_HTTPS/get +``` + +You should see a JSON response from httpbin.org showing request details e.g. + +```json +{ + "args": {}, + "headers": { + "Accept": "*/*", + "Host": "httpbin.example.com", + "User-Agent": "curl/8.7.1", + "X-Amzn-Trace-Id": "Root=1-68a49086-1e1dabb51155e05c1ebc1f63" + }, + "origin": "xxx.xxx.xxx.xx", + "url": "https://httpbin.example.com/get" +} +``` + +## Cleanup + +Remove all resources created in this example: + +```shell +kubectl delete -f route.yaml +kubectl delete -f external-service.yaml +kubectl delete -f gateway.yaml +``` diff --git a/examples/externalname-service/external-service.yaml b/examples/externalname-service/external-service.yaml new file mode 100644 index 0000000000..379e569251 --- /dev/null +++ b/examples/externalname-service/external-service.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +kind: Service +metadata: + name: httpbin +spec: + type: ExternalName + externalName: httpbin.org + ports: + - port: 80 + targetPort: 80 + protocol: TCP + name: http + - port: 443 + targetPort: 443 + protocol: TCP + name: https diff --git a/examples/externalname-service/gateway.yaml b/examples/externalname-service/gateway.yaml new file mode 100644 index 0000000000..28f93d3406 --- /dev/null +++ b/examples/externalname-service/gateway.yaml @@ -0,0 +1,42 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: Gateway +metadata: + name: external-gateway +spec: + gatewayClassName: nginx + infrastructure: + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: external-nginx-config + listeners: + - name: http + port: 80 + protocol: HTTP + allowedRoutes: + namespaces: + from: All + - name: tls + port: 443 + protocol: TLS + tls: + mode: Passthrough + allowedRoutes: + namespaces: + from: All + +--- +apiVersion: gateway.nginx.org/v1alpha2 +kind: NginxProxy +metadata: + name: external-nginx-config +spec: + dnsResolver: + addresses: + - type: IPAddress + value: "8.8.8.8" + - type: IPAddress + value: "1.1.1.1" + timeout: "30s" + cacheTTL: "60s" + disableIPv6: false diff --git a/examples/externalname-service/route.yaml b/examples/externalname-service/route.yaml new file mode 100644 index 0000000000..774a44bd47 --- /dev/null +++ b/examples/externalname-service/route.yaml @@ -0,0 +1,32 @@ +apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: httpbin-route +spec: + parentRefs: + - name: external-gateway + hostnames: + - httpbin.example.com + rules: + - matches: + - path: + type: PathPrefix + value: / + backendRefs: + - name: httpbin + port: 80 + +--- +apiVersion: gateway.networking.k8s.io/v1alpha2 +kind: TLSRoute +metadata: + name: httpbin-tls-route +spec: + parentRefs: + - name: external-gateway + hostnames: + - httpbin.example.com + rules: + - backendRefs: + - name: httpbin + port: 443 diff --git a/internal/controller/nginx/agent/agent.go b/internal/controller/nginx/agent/agent.go index 6945515d18..bdcc9efa9e 100644 --- a/internal/controller/nginx/agent/agent.go +++ b/internal/controller/nginx/agent/agent.go @@ -121,6 +121,10 @@ func (n *NginxUpdaterImpl) UpdateUpstreamServers( var applied bool actions := make([]*pb.NGINXPlusAction, 0, len(conf.Upstreams)+len(conf.StreamUpstreams)) for _, upstream := range conf.Upstreams { + // Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors + if upstreamHasResolveServers(upstream) { + continue + } action := &pb.NGINXPlusAction{ Action: &pb.NGINXPlusAction_UpdateHttpUpstreamServers{ UpdateHttpUpstreamServers: buildHTTPUpstreamServers(upstream), @@ -130,6 +134,10 @@ func (n *NginxUpdaterImpl) UpdateUpstreamServers( } for _, upstream := range conf.StreamUpstreams { + // Skip upstreams that have resolve servers to avoid "UpstreamServerImmutable" errors + if upstreamHasResolveServers(upstream) { + continue + } action := &pb.NGINXPlusAction{ Action: &pb.NGINXPlusAction_UpdateStreamServers{ UpdateStreamServers: buildStreamUpstreamServers(upstream), @@ -243,6 +251,16 @@ func (n *NginxUpdaterImpl) sendRequest( return applied, nil } +// upstreamHasResolveServers checks if an upstream contains servers that require DNS resolution. +func upstreamHasResolveServers(upstream dataplane.Upstream) bool { + for _, endpoint := range upstream.Endpoints { + if endpoint.Resolve { + return true + } + } + return false +} + func getPortAndIPFormat(ep resolver.Endpoint) (string, string) { var port string diff --git a/internal/controller/nginx/agent/agent_test.go b/internal/controller/nginx/agent/agent_test.go index f67dfbc8a9..5351fbb944 100644 --- a/internal/controller/nginx/agent/agent_test.go +++ b/internal/controller/nginx/agent/agent_test.go @@ -300,6 +300,16 @@ func TestUpdateUpstreamServers_NoChange(t *testing.T) { }, }, }, + { + Name: "resolve-upstream", + Endpoints: []resolver.Endpoint{ + { + Address: "external.example.com", + Port: 80, + Resolve: true, + }, + }, + }, }, StreamUpstreams: []dataplane.Upstream{ { @@ -310,6 +320,15 @@ func TestUpdateUpstreamServers_NoChange(t *testing.T) { }, }, }, + { + Name: "resolve-stream-upstream", + Endpoints: []resolver.Endpoint{ + { + Address: "external.example.com", + Resolve: true, + }, + }, + }, }, } @@ -345,7 +364,8 @@ func TestUpdateUpstreamServers_NoChange(t *testing.T) { } deployment.SetNGINXPlusActions(initialActions) - // Call UpdateUpstreamServers with the same configuration + // Call UpdateUpstreamServers with the same configuration and external name only + // Upstream servers pointing to external endpoints with resolve should not update via API updater.UpdateUpstreamServers(deployment, conf) // Verify that no new actions were sent diff --git a/internal/controller/nginx/config/base_http_config.go b/internal/controller/nginx/config/base_http_config.go index bdbb1ee29b..4bce95a0bc 100644 --- a/internal/controller/nginx/config/base_http_config.go +++ b/internal/controller/nginx/config/base_http_config.go @@ -11,6 +11,7 @@ import ( var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTPTemplateText)) type httpConfig struct { + DNSResolver *dataplane.DNSResolverConfig Includes []shared.Include NginxReadinessProbePort int32 IPFamily shared.IPFamily @@ -25,6 +26,7 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult { Includes: includes, NginxReadinessProbePort: conf.BaseHTTPConfig.NginxReadinessProbePort, IPFamily: getIPFamily(conf.BaseHTTPConfig), + DNSResolver: conf.BaseHTTPConfig.DNSResolver, } results := make([]executeResult, 0, len(includes)+1) diff --git a/internal/controller/nginx/config/base_http_config_template.go b/internal/controller/nginx/config/base_http_config_template.go index 27adb7360e..e50a83a65f 100644 --- a/internal/controller/nginx/config/base_http_config_template.go +++ b/internal/controller/nginx/config/base_http_config_template.go @@ -1,8 +1,17 @@ package config +//nolint:lll const baseHTTPTemplateText = ` {{- if .HTTP2 }}http2 on;{{ end }} +{{- if .DNSResolver }} +# DNS resolver configuration for ExternalName services +resolver{{ range $addr := .DNSResolver.Addresses }} {{ $addr }}{{ end }}{{ if .DNSResolver.Valid }} valid={{ .DNSResolver.Valid }}{{ end }}{{ if .DNSResolver.DisableIPv6 }} ipv6=off{{ end }}; +{{- if .DNSResolver.Timeout }} +resolver_timeout {{ .DNSResolver.Timeout }}; +{{- end }} +{{- end }} + # Set $gw_api_compliant_host variable to the value of $http_host unless $http_host is empty, then set it to the value # of $host. We prefer $http_host because it contains the original value of the host header, which is required by the # Gateway API. However, in an HTTP/1.0 request, it's possible that $http_host can be empty. In this case, we will use diff --git a/internal/controller/nginx/config/base_http_config_test.go b/internal/controller/nginx/config/base_http_config_test.go index deb58def53..fe7ad3a58d 100644 --- a/internal/controller/nginx/config/base_http_config_test.go +++ b/internal/controller/nginx/config/base_http_config_test.go @@ -210,3 +210,77 @@ func TestExecuteBaseHttp_NginxReadinessProbePort(t *testing.T) { }) } } + +func TestExecuteBaseHttp_DNSResolver(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + expectedConfig string + conf dataplane.Configuration + }{ + { + name: "DNS resolver with all options", + conf: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + DNSResolver: &dataplane.DNSResolverConfig{ + Addresses: []string{"8.8.8.8", "8.8.4.4"}, + Timeout: "10s", + Valid: "60s", + DisableIPv6: true, + }, + }, + }, + expectedConfig: "resolver 8.8.8.8 8.8.4.4 valid=60s ipv6=off;\nresolver_timeout 10s;", + }, + { + name: "DNS resolver with single address and IPv6 enabled", + conf: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + DNSResolver: &dataplane.DNSResolverConfig{ + Addresses: []string{"8.8.8.8"}, + DisableIPv6: false, + }, + }, + }, + expectedConfig: "resolver 8.8.8.8;", + }, + { + name: "no DNS resolver", + conf: dataplane.Configuration{ + BaseHTTPConfig: dataplane.BaseHTTPConfig{ + DNSResolver: nil, + }, + }, + expectedConfig: "", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + res := executeBaseHTTPConfig(test.conf) + g.Expect(res).To(HaveLen(1)) + + httpConfig := string(res[0].data) + + if test.expectedConfig != "" { + // Check that the resolver directive is present + g.Expect(httpConfig).To(ContainSubstring(test.expectedConfig)) + // Check that the comment is present + g.Expect(httpConfig).To(ContainSubstring("# DNS resolver configuration for ExternalName services")) + } else { + // Check that no resolver directive is present + g.Expect(httpConfig).ToNot(ContainSubstring("resolver")) + g.Expect(httpConfig).ToNot(ContainSubstring("# DNS resolver configuration for ExternalName services")) + } + + // Verify that standard config elements are still present + g.Expect(httpConfig).To(ContainSubstring("map $http_host $gw_api_compliant_host {")) + g.Expect(httpConfig).To(ContainSubstring("map $http_upgrade $connection_upgrade {")) + g.Expect(httpConfig).To(ContainSubstring("map $request_uri $request_uri_path {")) + }) + } +} diff --git a/internal/controller/nginx/config/http/config.go b/internal/controller/nginx/config/http/config.go index 29c7fb6373..3a76ab30b4 100644 --- a/internal/controller/nginx/config/http/config.go +++ b/internal/controller/nginx/config/http/config.go @@ -105,6 +105,7 @@ type UpstreamKeepAlive struct { // UpstreamServer holds all configuration for an HTTP upstream server. type UpstreamServer struct { Address string + Resolve bool } // SplitClient holds all configuration for an HTTP split client. diff --git a/internal/controller/nginx/config/servers_template.go b/internal/controller/nginx/config/servers_template.go index ac7e4ae5bf..224e189a6e 100644 --- a/internal/controller/nginx/config/servers_template.go +++ b/internal/controller/nginx/config/servers_template.go @@ -3,6 +3,7 @@ package config const serversTemplateText = ` js_preload_object matches from /etc/nginx/conf.d/matches.json; + {{- range $s := .Servers -}} {{ if $s.IsDefaultSSL -}} server { diff --git a/internal/controller/nginx/config/stream/config.go b/internal/controller/nginx/config/stream/config.go index 21a42c505d..ca554e18f4 100644 --- a/internal/controller/nginx/config/stream/config.go +++ b/internal/controller/nginx/config/stream/config.go @@ -1,6 +1,9 @@ package stream -import "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" +import ( + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/nginx/config/shared" + "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" +) // Server holds all configuration for a stream server. type Server struct { @@ -24,11 +27,13 @@ type Upstream struct { // UpstreamServer holds all configuration for a stream upstream server. type UpstreamServer struct { Address string + Resolve bool } // ServerConfig holds configuration for a stream server and IP family to be used by NGINX. type ServerConfig struct { - Servers []Server - IPFamily shared.IPFamily - Plus bool + DNSResolver *dataplane.DNSResolverConfig + Servers []Server + IPFamily shared.IPFamily + Plus bool } diff --git a/internal/controller/nginx/config/stream_servers.go b/internal/controller/nginx/config/stream_servers.go index 52c32e19f6..cbac92cc7a 100644 --- a/internal/controller/nginx/config/stream_servers.go +++ b/internal/controller/nginx/config/stream_servers.go @@ -16,9 +16,10 @@ func (g GeneratorImpl) executeStreamServers(conf dataplane.Configuration) []exec streamServers := createStreamServers(conf) streamServerConfig := stream.ServerConfig{ - Servers: streamServers, - IPFamily: getIPFamily(conf.BaseHTTPConfig), - Plus: g.plus, + Servers: streamServers, + IPFamily: getIPFamily(conf.BaseHTTPConfig), + Plus: g.plus, + DNSResolver: conf.BaseStreamConfig.DNSResolver, } streamServerResult := executeResult{ diff --git a/internal/controller/nginx/config/stream_servers_template.go b/internal/controller/nginx/config/stream_servers_template.go index 75868bc0de..9ad96e6300 100644 --- a/internal/controller/nginx/config/stream_servers_template.go +++ b/internal/controller/nginx/config/stream_servers_template.go @@ -1,6 +1,15 @@ package config +//nolint:lll const streamServersTemplateText = ` +{{- if .DNSResolver }} +# DNS resolver configuration for ExternalName services +resolver{{ range $addr := .DNSResolver.Addresses }} {{ $addr }}{{ end }}{{ if .DNSResolver.Valid }} valid={{ .DNSResolver.Valid }}{{ end }}{{ if .DNSResolver.DisableIPv6 }} ipv6=off{{ end }}; +{{- if .DNSResolver.Timeout }} +resolver_timeout {{ .DNSResolver.Timeout }}; +{{- end }} +{{- end }} + {{- range $s := .Servers }} server { {{- if or ($.IPFamily.IPv4) ($s.IsSocket) }} diff --git a/internal/controller/nginx/config/stream_servers_test.go b/internal/controller/nginx/config/stream_servers_test.go index a195cc2007..e603fe1fed 100644 --- a/internal/controller/nginx/config/stream_servers_test.go +++ b/internal/controller/nginx/config/stream_servers_test.go @@ -411,3 +411,86 @@ func TestCreateStreamServersWithNone(t *testing.T) { g.Expect(streamServers).To(BeNil()) } + +func TestExecuteStreamServersWithResolver(t *testing.T) { + t.Parallel() + tests := []struct { + name string + expectedConfig string + conf dataplane.Configuration + }{ + { + name: "stream servers with DNS resolver", + conf: dataplane.Configuration{ + BaseStreamConfig: dataplane.BaseStreamConfig{ + DNSResolver: &dataplane.DNSResolverConfig{ + Addresses: []string{"8.8.8.8", "8.8.4.4"}, + Timeout: "10s", + Valid: "60s", + DisableIPv6: true, + }, + }, + }, + expectedConfig: ` +# DNS resolver configuration for ExternalName services +resolver 8.8.8.8 8.8.4.4 valid=60s ipv6=off; +resolver_timeout 10s; + +server { + listen unix:/var/run/nginx/connection-closed-server.sock; + return ""; +} +`, + }, + { + name: "stream servers without DNS resolver", + conf: dataplane.Configuration{ + BaseStreamConfig: dataplane.BaseStreamConfig{ + DNSResolver: nil, + }, + }, + expectedConfig: ` + +server { + listen unix:/var/run/nginx/connection-closed-server.sock; + return ""; +} +`, + }, + { + name: "stream servers with DNS resolver IPv6 enabled", + conf: dataplane.Configuration{ + BaseStreamConfig: dataplane.BaseStreamConfig{ + DNSResolver: &dataplane.DNSResolverConfig{ + Addresses: []string{"2001:4860:4860::8888"}, + Timeout: "5s", + Valid: "30s", + DisableIPv6: false, + }, + }, + }, + expectedConfig: ` +# DNS resolver configuration for ExternalName services +resolver 2001:4860:4860::8888 valid=30s; +resolver_timeout 5s; + +server { + listen unix:/var/run/nginx/connection-closed-server.sock; + return ""; +} +`, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + generator := GeneratorImpl{} + results := generator.executeStreamServers(test.conf) + + g.Expect(results).To(HaveLen(1)) + g.Expect(string(results[0].data)).To(Equal(test.expectedConfig)) + }) + } +} diff --git a/internal/controller/nginx/config/upstreams.go b/internal/controller/nginx/config/upstreams.go index 5a792606bb..4b7e6bd16f 100644 --- a/internal/controller/nginx/config/upstreams.go +++ b/internal/controller/nginx/config/upstreams.go @@ -96,7 +96,11 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre zoneSize := ossZoneSizeStream if g.plus { zoneSize = plusZoneSizeStream - stateFile = fmt.Sprintf("%s/%s.conf", stateDir, up.Name) + // Only set state file if the upstream doesn't have resolve servers + // Upstreams with resolve servers can't be managed via NGINX Plus API + if !upstreamHasResolveServers(up) { + stateFile = fmt.Sprintf("%s/%s.conf", stateDir, up.Name) + } } upstreamServers := make([]stream.UpstreamServer, len(up.Endpoints)) @@ -107,6 +111,7 @@ func (g GeneratorImpl) createStreamUpstream(up dataplane.Upstream) stream.Upstre } upstreamServers[idx] = stream.UpstreamServer{ Address: fmt.Sprintf(format, ep.Address, ep.Port), + Resolve: ep.Resolve, } } @@ -144,7 +149,11 @@ func (g GeneratorImpl) createUpstream( zoneSize := ossZoneSize if g.plus { zoneSize = plusZoneSize - stateFile = fmt.Sprintf("%s/%s.conf", stateDir, up.Name) + // Only set state file if the upstream doesn't have resolve servers + // Upstreams with resolve servers can't be managed via NGINX Plus API + if !upstreamHasResolveServers(up) { + stateFile = fmt.Sprintf("%s/%s.conf", stateDir, up.Name) + } } if upstreamPolicySettings.ZoneSize != "" { @@ -172,6 +181,7 @@ func (g GeneratorImpl) createUpstream( } upstreamServers[idx] = http.UpstreamServer{ Address: fmt.Sprintf(format, ep.Address, ep.Port), + Resolve: ep.Resolve, } } @@ -195,3 +205,13 @@ func createInvalidBackendRefUpstream() http.Upstream { }, } } + +// upstreamHasResolveServers checks if an upstream contains servers that require DNS resolution. +func upstreamHasResolveServers(upstream dataplane.Upstream) bool { + for _, endpoint := range upstream.Endpoints { + if endpoint.Resolve { + return true + } + } + return false +} diff --git a/internal/controller/nginx/config/upstreams_template.go b/internal/controller/nginx/config/upstreams_template.go index ad60ce7d42..15e9b0c1fc 100644 --- a/internal/controller/nginx/config/upstreams_template.go +++ b/internal/controller/nginx/config/upstreams_template.go @@ -19,7 +19,7 @@ upstream {{ $u.Name }} { state {{ $u.StateFile }}; {{- else }} {{ range $server := $u.Servers }} - server {{ $server.Address }}; + server {{ $server.Address }}{{ if $server.Resolve }} resolve{{ end }}; {{- end }} {{- end }} {{ if $u.KeepAlive.Connections -}} @@ -49,7 +49,7 @@ upstream {{ $u.Name }} { state {{ $u.StateFile }}; {{- else }} {{ range $server := $u.Servers }} - server {{ $server.Address }}; + server {{ $server.Address }}{{ if $server.Resolve }} resolve{{ end }}; {{- end }} {{- end }} } diff --git a/internal/controller/nginx/config/upstreams_test.go b/internal/controller/nginx/config/upstreams_test.go index 2dd94cb4ea..c13904d25d 100644 --- a/internal/controller/nginx/config/upstreams_test.go +++ b/internal/controller/nginx/config/upstreams_test.go @@ -527,6 +527,67 @@ func TestCreateUpstream(t *testing.T) { }, msg: "upstreamSettingsPolicy with only keep alive settings", }, + { + stateUpstream: dataplane.Upstream{ + Name: "external-name-service", + Endpoints: []resolver.Endpoint{ + { + Address: "example.com", + Port: 80, + Resolve: true, + }, + }, + }, + expectedUpstream: http.Upstream{ + Name: "external-name-service", + ZoneSize: ossZoneSize, + Servers: []http.UpstreamServer{ + { + Address: "example.com:80", + Resolve: true, + }, + }, + }, + msg: "ExternalName service with DNS name", + }, + { + stateUpstream: dataplane.Upstream{ + Name: "mixed-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.1", + Port: 80, + }, + { + Address: "example.com", + Port: 443, + Resolve: true, + }, + { + Address: "fd00:10:244:1::7", + Port: 80, + IPv6: true, + }, + }, + }, + expectedUpstream: http.Upstream{ + Name: "mixed-endpoints", + ZoneSize: ossZoneSize, + Servers: []http.UpstreamServer{ + { + Address: "10.0.0.1:80", + }, + { + Address: "example.com:443", + Resolve: true, + }, + { + Address: "[fd00:10:244:1::7]:80", + }, + }, + }, + msg: "mixed IP addresses and DNS names", + }, } for _, test := range tests { @@ -723,43 +784,118 @@ func TestCreateStreamUpstreams(t *testing.T) { func TestCreateStreamUpstream(t *testing.T) { t.Parallel() gen := GeneratorImpl{} - up := dataplane.Upstream{ - Name: "multiple-endpoints", - Endpoints: []resolver.Endpoint{ - { - Address: "10.0.0.1", - Port: 80, - }, - { - Address: "10.0.0.2", - Port: 80, + + tests := []struct { + msg string + stateUpstream dataplane.Upstream + expectedUpstream stream.Upstream + }{ + { + stateUpstream: dataplane.Upstream{ + Name: "multiple-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "10.0.0.1", + Port: 80, + }, + { + Address: "10.0.0.2", + Port: 80, + }, + { + Address: "10.0.0.3", + Port: 80, + }, + }, }, - { - Address: "10.0.0.3", - Port: 80, + expectedUpstream: stream.Upstream{ + Name: "multiple-endpoints", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "10.0.0.1:80", + }, + { + Address: "10.0.0.2:80", + }, + { + Address: "10.0.0.3:80", + }, + }, }, + msg: "multiple IP endpoints", }, - } - - expectedUpstream := stream.Upstream{ - Name: "multiple-endpoints", - ZoneSize: ossZoneSize, - Servers: []stream.UpstreamServer{ - { - Address: "10.0.0.1:80", + { + stateUpstream: dataplane.Upstream{ + Name: "external-name-service", + Endpoints: []resolver.Endpoint{ + { + Address: "backend.example.com", + Port: 443, + Resolve: true, + }, + }, }, - { - Address: "10.0.0.2:80", + expectedUpstream: stream.Upstream{ + Name: "external-name-service", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "backend.example.com:443", + Resolve: true, + }, + }, }, - { - Address: "10.0.0.3:80", + msg: "ExternalName service with DNS name", + }, + { + stateUpstream: dataplane.Upstream{ + Name: "mixed-endpoints", + Endpoints: []resolver.Endpoint{ + { + Address: "192.168.1.10", + Port: 8080, + }, + { + Address: "api.example.com", + Port: 443, + Resolve: true, + }, + { + Address: "2001:db8::1", + Port: 9000, + IPv6: true, + }, + }, + }, + expectedUpstream: stream.Upstream{ + Name: "mixed-endpoints", + ZoneSize: ossZoneSize, + Servers: []stream.UpstreamServer{ + { + Address: "192.168.1.10:8080", + }, + { + Address: "api.example.com:443", + Resolve: true, + }, + { + Address: "[2001:db8::1]:9000", + }, + }, }, + msg: "mixed IP addresses and DNS names", }, } - g := NewWithT(t) - result := gen.createStreamUpstream(up) - g.Expect(result).To(Equal(expectedUpstream)) + for _, test := range tests { + t.Run(test.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + result := gen.createStreamUpstream(test.stateUpstream) + g.Expect(result).To(Equal(test.expectedUpstream)) + }) + } } func TestCreateStreamUpstreamPlus(t *testing.T) { diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 94f5f0819a..9b93aa7fc0 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -52,6 +52,7 @@ func BuildConfiguration( gatewaySnippetsFilters := gateway.GetReferencedSnippetsFilters(g.Routes, g.SnippetsFilters) baseHTTPConfig := buildBaseHTTPConfig(gateway, gatewaySnippetsFilters) + baseStreamConfig := buildBaseStreamConfig(gateway) httpServers, sslServers := buildServers(gateway) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) @@ -79,6 +80,7 @@ func BuildConfiguration( logger, gateway, serviceResolver, + g.ReferencedServices, baseHTTPConfig.IPFamily), BackendGroups: backendGroups, SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, gateway.Listeners), @@ -88,6 +90,7 @@ func BuildConfiguration( ), Telemetry: buildTelemetry(g, gateway), BaseHTTPConfig: baseHTTPConfig, + BaseStreamConfig: baseStreamConfig, Logging: buildLogging(gateway), NginxPlus: nginxPlus, MainSnippets: buildSnippetsForContext(gatewaySnippetsFilters, ngfAPIv1alpha1.NginxContextMain), @@ -174,6 +177,7 @@ func buildStreamUpstreams( logger logr.Logger, gateway *graph.Gateway, serviceResolver resolver.ServiceResolver, + referencedServices map[types.NamespacedName]*graph.ReferencedService, ipFamily IPFamilyType, ) []Upstream { // There can be duplicate upstreams if multiple routes reference the same upstream. @@ -211,11 +215,12 @@ func buildStreamUpstreams( allowedAddressType := getAllowedAddressType(ipFamily) - eps, err := serviceResolver.Resolve( + eps, err := resolveUpstreamEndpoints( ctx, logger, - br.SvcNsName, - br.ServicePort, + br, + serviceResolver, + referencedServices, allowedAddressType, ) if err != nil { @@ -783,7 +788,14 @@ func buildUpstream( var errMsg string - eps, err := svcResolver.Resolve(ctx, logger, br.SvcNsName, br.ServicePort, allowedAddressType) + eps, err := resolveUpstreamEndpoints( + ctx, + logger, + br, + svcResolver, + referencedServices, + allowedAddressType, + ) if err != nil { errMsg = err.Error() logger.V(1).Info("failed to resolve endpoints, endpoints may not be ready", "error", errMsg, "service", br.SvcNsName) @@ -1044,6 +1056,24 @@ func buildBaseHTTPConfig( } } + baseConfig.DNSResolver = buildDNSResolverConfig(np.DNSResolver) + + return baseConfig +} + +// buildBaseStreamConfig generates the base stream context config that should be applied to all stream servers. +func buildBaseStreamConfig(gateway *graph.Gateway) BaseStreamConfig { + baseConfig := BaseStreamConfig{} + + // safe to access EffectiveNginxProxy since we only call this function when the Gateway is not nil. + np := gateway.EffectiveNginxProxy + if np == nil { + return baseConfig + } + + // Add DNS resolver configuration for ExternalName services in stream context + baseConfig.DNSResolver = buildDNSResolverConfig(np.DNSResolver) + return baseConfig } @@ -1215,3 +1245,65 @@ func GetDefaultConfiguration(g *graph.Graph, gateway *graph.Gateway) Configurati WorkerConnections: buildWorkerConnections(gateway), } } + +// buildDNSResolverConfig builds a DNSResolverConfig from an NginxProxy DNSResolver configuration. +func buildDNSResolverConfig(dnsResolver *ngfAPIv1alpha2.DNSResolver) *DNSResolverConfig { + if dnsResolver == nil { + return nil + } + + config := &DNSResolverConfig{ + Addresses: convertDNSResolverAddresses(dnsResolver.Addresses), + } + + if dnsResolver.Timeout != nil { + config.Timeout = string(*dnsResolver.Timeout) + } + + if dnsResolver.CacheTTL != nil { + config.Valid = string(*dnsResolver.CacheTTL) + } + + if dnsResolver.DisableIPv6 != nil { + config.DisableIPv6 = *dnsResolver.DisableIPv6 + } + + return config +} + +// resolveUpstreamEndpoints handles service resolution for both regular and ExternalName services. +func resolveUpstreamEndpoints( + ctx context.Context, + logger logr.Logger, + br graph.BackendRef, + svcResolver resolver.ServiceResolver, + referencedServices map[types.NamespacedName]*graph.ReferencedService, + allowedAddressType []discoveryV1.AddressType, +) ([]resolver.Endpoint, error) { + // Check if this is an ExternalName service + if graphSvc, exists := referencedServices[br.SvcNsName]; exists && graphSvc.IsExternalName { + // For ExternalName services, create an endpoint directly with the external name + endpoint := resolver.Endpoint{ + Address: graphSvc.ExternalName, + Port: br.ServicePort.Port, + IPv6: false, // DNS names are neither IPv4 nor IPv6 + Resolve: true, // ExternalName services require DNS resolution + } + + logger.V(1).Info("resolved ExternalName service", + "service", br.SvcNsName, + "externalName", graphSvc.ExternalName, + "port", br.ServicePort.Port) + + return []resolver.Endpoint{endpoint}, nil + } + + // For regular services, use the existing Resolve method + return svcResolver.Resolve( + ctx, + logger, + br.SvcNsName, + br.ServicePort, + allowedAddressType, + ) +} diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 75fefc982a..8e6007873e 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -4271,6 +4271,7 @@ func TestBuildStreamUpstreams(t *testing.T) { secureApp4Key := getL4RouteKey("secure-app4") secureApp5Key := getL4RouteKey("secure-app5") secureApp6Key := getL4RouteKey("secure-app6") + externalAppKey := getL4RouteKey("external-app") gateway := &graph.Gateway{ Source: &v1.Gateway{ @@ -4376,6 +4377,25 @@ func TestBuildStreamUpstreams(t *testing.T) { }, }, }, + externalAppKey: { + Valid: true, + Spec: graph.L4RouteSpec{ + Hostnames: []v1.Hostname{"external.example.com"}, + BackendRef: graph.BackendRef{ + Valid: true, + SvcNsName: externalAppKey.NamespacedName, + ServicePort: apiv1.ServicePort{ + Name: "https", + Protocol: "TCP", + Port: 443, + TargetPort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 443, + }, + }, + }, + }, + }, }, }, }, @@ -4399,7 +4419,24 @@ func TestBuildStreamUpstreams(t *testing.T) { return fakeEndpoints, nil } - streamUpstreams := buildStreamUpstreams(t.Context(), logr.Discard(), gateway, &fakeResolver, Dual) + // Add an ExternalName service for testing DNS resolution + externalNameService := &graph.ReferencedService{ + IsExternalName: true, + ExternalName: "external.example.com", + } + + referencedServices := map[types.NamespacedName]*graph.ReferencedService{ + {Namespace: "default", Name: "external-app"}: externalNameService, + } + + streamUpstreams := buildStreamUpstreams( + t.Context(), + logr.Discard(), + gateway, + &fakeResolver, + referencedServices, + Dual, + ) expectedStreamUpstreams := []Upstream{ { @@ -4410,6 +4447,12 @@ func TestBuildStreamUpstreams(t *testing.T) { Name: "default_secure-app5_8443", Endpoints: fakeEndpoints, }, + { + Name: "default_external-app_443", + Endpoints: []resolver.Endpoint{ + {Address: "external.example.com", Port: 443, Resolve: true}, + }, + }, } g := NewWithT(t) @@ -4996,8 +5039,8 @@ func TestBuildWorkerConnections(t *testing.T) { func TestBuildBaseHTTPConfig_ReadinessProbe(t *testing.T) { t.Parallel() test := []struct { - msg string gateway *graph.Gateway + msg string expected BaseHTTPConfig }{ { @@ -5099,3 +5142,65 @@ func TestBuildBaseHTTPConfig_ReadinessProbe(t *testing.T) { }) } } + +func TestBuildDNSResolverConfig(t *testing.T) { + t.Parallel() + + addr := []ngfAPIv1alpha2.DNSResolverAddress{ + { + Type: ngfAPIv1alpha2.DNSResolverIPAddressType, + Value: "8.8.8.8", + }, + { + Type: ngfAPIv1alpha2.DNSResolverHostnameType, + Value: "dns.google", + }, + } + + tests := []struct { + dnsResolver *ngfAPIv1alpha2.DNSResolver + expected *DNSResolverConfig + name string + }{ + { + name: "nil DNS resolver", + dnsResolver: nil, + expected: nil, + }, + { + name: "DNS resolver with all options", + dnsResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: addr, + Timeout: helpers.GetPointer(ngfAPIv1alpha1.Duration("10s")), + CacheTTL: helpers.GetPointer(ngfAPIv1alpha1.Duration("60s")), + DisableIPv6: helpers.GetPointer(true), + }, + expected: &DNSResolverConfig{ + Addresses: []string{"8.8.8.8", "dns.google"}, + Timeout: "10s", + Valid: "60s", + DisableIPv6: true, + }, + }, + { + name: "DNS resolver with minimal configuration", + dnsResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: addr, + }, + expected: &DNSResolverConfig{ + Addresses: []string{"8.8.8.8", "dns.google"}, + DisableIPv6: false, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + result := buildDNSResolverConfig(test.dnsResolver) + g.Expect(result).To(Equal(test.expected)) + }) + } +} diff --git a/internal/controller/state/dataplane/convert.go b/internal/controller/state/dataplane/convert.go index eb0a10c7da..97a9b27c10 100644 --- a/internal/controller/state/dataplane/convert.go +++ b/internal/controller/state/dataplane/convert.go @@ -8,6 +8,7 @@ import ( v1 "sigs.k8s.io/gateway-api/apis/v1" ngfAPI "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha1" + ngfAPIv1alpha2 "github.com/nginx/nginx-gateway-fabric/v2/apis/v1alpha2" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/graph" "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/mirror" "github.com/nginx/nginx-gateway-fabric/v2/internal/framework/helpers" @@ -187,3 +188,16 @@ func convertSnippetsFilter(filter *graph.SnippetsFilter) SnippetsFilter { return result } + +func convertDNSResolverAddresses(addresses []ngfAPIv1alpha2.DNSResolverAddress) []string { + if len(addresses) == 0 { + return nil + } + + result := make([]string, 0, len(addresses)) + for _, addr := range addresses { + result = append(result, addr.Value) + } + + return result +} diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index ac8e263050..08e7e0867b 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -23,35 +23,37 @@ const ( // Configuration is an intermediate representation of dataplane configuration. type Configuration struct { - // SSLKeyPairs holds all unique SSLKeyPairs. - SSLKeyPairs map[SSLKeyPairID]SSLKeyPair + // AuxiliarySecrets contains additional secret data, like certificates/keys/tokens that are not related to + // Gateway API resources. + AuxiliarySecrets map[graph.SecretFileType][]byte // CertBundles holds all unique Certificate Bundles. CertBundles map[CertBundleID]CertBundle - // HTTPServers holds all HTTPServers. - HTTPServers []VirtualServer - // SSLServers holds all SSLServers. - SSLServers []VirtualServer - // TLSPassthroughServers hold all TLSPassthroughServers - TLSPassthroughServers []Layer4VirtualServer - // Upstreams holds all unique http Upstreams. - Upstreams []Upstream + // BaseStreamConfig holds the configuration options at the stream context. + BaseStreamConfig BaseStreamConfig + // SSLKeyPairs holds all unique SSLKeyPairs. + SSLKeyPairs map[SSLKeyPairID]SSLKeyPair // DeploymentContext contains metadata about NGF and the cluster. DeploymentContext DeploymentContext - // AuxiliarySecrets contains additional secret data, like certificates/keys/tokens that are not related to - // Gateway API resources. - AuxiliarySecrets map[graph.SecretFileType][]byte + // Logging defines logging related settings for NGINX. + Logging Logging // StreamUpstreams holds all unique stream Upstreams StreamUpstreams []Upstream + // TLSPassthroughServers hold all TLSPassthroughServers + TLSPassthroughServers []Layer4VirtualServer // BackendGroups holds all unique BackendGroups. BackendGroups []BackendGroup // MainSnippets holds all the snippets that apply to the main context. MainSnippets []Snippet - // Telemetry holds the Otel configuration. - Telemetry Telemetry - // Logging defines logging related settings for NGINX. - Logging Logging + // Upstreams holds all unique http Upstreams. + Upstreams []Upstream // NginxPlus specifies NGINX Plus additional settings. NginxPlus NginxPlus + // SSLServers holds all SSLServers. + SSLServers []VirtualServer + // HTTPServers holds all HTTPServers. + HTTPServers []VirtualServer + // Telemetry holds the Otel configuration. + Telemetry Telemetry // BaseHTTPConfig holds the configuration options at the http context. BaseHTTPConfig BaseHTTPConfig // WorkerConnections specifies the maximum number of simultaneous connections that can be opened by a worker process. @@ -366,6 +368,8 @@ type SpanAttribute struct { // BaseHTTPConfig holds the configuration options at the http context. type BaseHTTPConfig struct { + // DNSResolver defines the DNS resolver configuration for NGINX. + DNSResolver *DNSResolverConfig // IPFamily specifies the IP family for all servers. IPFamily IPFamilyType // Snippets contain the snippets that apply to the http context. @@ -380,6 +384,12 @@ type BaseHTTPConfig struct { DisableSNIHostValidation bool } +// BaseStreamConfig holds the configuration options at the stream context. +type BaseStreamConfig struct { + // DNSResolver specifies the DNS resolver configuration for ExternalName services. + DNSResolver *DNSResolverConfig +} + // Snippet is a snippet of configuration. type Snippet struct { // Name is the name of the snippet. @@ -398,6 +408,18 @@ type RewriteClientIPSettings struct { IPRecursive bool } +// DNSResolverConfig defines the DNS resolver configuration for NGINX. +type DNSResolverConfig struct { + // Timeout specifies the timeout for name resolution. + Timeout string + // Valid specifies how long to cache DNS responses. + Valid string + // Addresses specifies the list of DNS server addresses. + Addresses []string + // DisableIPv6 specifies whether to disable DisableIPv6 lookups. + DisableIPv6 bool +} + // RewriteIPModeType specifies the mode for rewriting the client IP. type RewriteIPModeType string diff --git a/internal/controller/state/graph/backend_refs.go b/internal/controller/state/graph/backend_refs.go index 8ae81bcfee..997df80c75 100644 --- a/internal/controller/state/graph/backend_refs.go +++ b/internal/controller/state/graph/backend_refs.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "slices" + "strings" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" @@ -181,6 +182,29 @@ func createBackendRef( var conds []conditions.Condition invalidForGateways := make(map[types.NamespacedName]conditions.Condition) + + // Check if this is an ExternalName service and validate DNS resolver configuration + svc, svcExists := services[svcNsName] + if svcExists && svc.Spec.Type == v1.ServiceTypeExternalName { + invalidForGateways = checkExternalNameValidForGateways(route.ParentRefs, invalidForGateways) + + // Check if externalName field is empty or whitespace-only + if strings.TrimSpace(svc.Spec.ExternalName) == "" { + backendRef := BackendRef{ + SvcNsName: svcNsName, + ServicePort: svcPort, + Weight: weight, + Valid: false, + IsMirrorBackend: ref.MirrorBackendIdx != nil, + InvalidForGateways: invalidForGateways, + } + + return backendRef, append(conds, conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service has empty or invalid externalName field", + )) + } + } + for _, parentRef := range route.ParentRefs { if err := verifyIPFamily(parentRef.Gateway.EffectiveNginxProxy, svcIPFamily); err != nil { invalidForGateways[parentRef.Gateway.NamespacedName] = conditions.NewRouteInvalidIPFamily(err.Error()) @@ -374,6 +398,20 @@ func verifyIPFamily(npCfg *EffectiveNginxProxy, svcIPFamily []v1.IPFamily) error return nil } +func checkExternalNameValidForGateways( + parentRefs []ParentRef, + invalidForGateways map[types.NamespacedName]conditions.Condition, +) map[types.NamespacedName]conditions.Condition { + for _, parentRef := range parentRefs { + if parentRef.Gateway.EffectiveNginxProxy == nil || parentRef.Gateway.EffectiveNginxProxy.DNSResolver == nil { + invalidForGateways[parentRef.Gateway.NamespacedName] = conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service requires DNS resolver configuration in Gateway's NginxProxy", + ) + } + } + return invalidForGateways +} + func validateRouteBackendRef( ref RouteBackendRef, routeNs string, diff --git a/internal/controller/state/graph/backend_refs_test.go b/internal/controller/state/graph/backend_refs_test.go index b71f997c5d..0d3e74a1dd 100644 --- a/internal/controller/state/graph/backend_refs_test.go +++ b/internal/controller/state/graph/backend_refs_test.go @@ -3,6 +3,7 @@ package graph import ( "errors" "testing" + "time" . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" @@ -979,6 +980,58 @@ func TestCreateBackend(t *testing.T) { svc1 := createService("service1") svc2 := createService("service2") svc3 := createService("service3") + + // Create an ExternalName service for testing DNS resolver validation + externalSvc := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "external-service", + Namespace: "test", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeExternalName, + ExternalName: "example.com", + Ports: []v1.ServicePort{ + { + Port: 80, + }, + }, + }, + } + + // Create an ExternalName service with empty externalName field for testing validation + externalSvcEmpty := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "external-service-empty", + Namespace: "test", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeExternalName, + ExternalName: "", // Empty external name + Ports: []v1.ServicePort{ + { + Port: 80, + }, + }, + }, + } + + // Create an ExternalName service with whitespace-only externalName field for testing validation + externalSvcWhitespace := &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "external-service-whitespace", + Namespace: "test", + }, + Spec: v1.ServiceSpec{ + Type: v1.ServiceTypeExternalName, + ExternalName: " \t\n ", // Whitespace-only external name + Ports: []v1.ServicePort{ + { + Port: 80, + }, + }, + }, + } + svc1NamespacedName := types.NamespacedName{Namespace: "test", Name: "service1"} svc2NamespacedName := types.NamespacedName{Namespace: "test", Name: "service2"} svc3NamespacedName := types.NamespacedName{Namespace: "test", Name: "service3"} @@ -1204,12 +1257,136 @@ func TestCreateBackend(t *testing.T) { }, name: "invalid policy", }, + { + ref: gatewayv1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef { + backend.Name = "external-service" + return backend + }), + }, + expectedBackend: BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "test", Name: "external-service"}, + ServicePort: v1.ServicePort{Port: 80}, + Weight: 5, + Valid: true, + InvalidForGateways: map[types.NamespacedName]conditions.Condition{ + {Namespace: "test", Name: "gateway"}: conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service requires DNS resolver configuration in Gateway's NginxProxy", + ), + }, + }, + expectedServicePortReference: "test_external-service_80", + expectedConditions: nil, + name: "ExternalName service without DNS resolver", + }, + { + ref: gatewayv1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef { + backend.Name = "external-service" + return backend + }), + }, + nginxProxySpec: &EffectiveNginxProxy{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "8.8.8.8"}, + }, + }, + }, + expectedBackend: BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "test", Name: "external-service"}, + ServicePort: v1.ServicePort{Port: 80}, + Weight: 5, + Valid: true, + InvalidForGateways: map[types.NamespacedName]conditions.Condition{}, + }, + expectedServicePortReference: "test_external-service_80", + expectedConditions: nil, + name: "ExternalName service with DNS resolver", + }, + { + ref: gatewayv1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef { + backend.Name = "external-service" + return backend + }), + }, + expectedBackend: BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "test", Name: "external-service"}, + ServicePort: v1.ServicePort{Port: 80}, + Weight: 5, + Valid: true, + InvalidForGateways: map[types.NamespacedName]conditions.Condition{ + {Namespace: "test", Name: "gateway2"}: conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service requires DNS resolver configuration in Gateway's NginxProxy", + ), + }, + }, + expectedServicePortReference: "test_external-service_80", + expectedConditions: nil, + name: "ExternalName service with multiple gateways - mixed DNS resolver config", + }, + { + ref: gatewayv1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef { + backend.Name = "external-service-empty" + return backend + }), + }, + expectedBackend: BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "test", Name: "external-service-empty"}, + ServicePort: v1.ServicePort{Port: 80}, + Weight: 5, + Valid: false, + InvalidForGateways: map[types.NamespacedName]conditions.Condition{ + {Namespace: "test", Name: "gateway"}: conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service requires DNS resolver configuration in Gateway's NginxProxy", + ), + }, + }, + expectedServicePortReference: "", + expectedConditions: []conditions.Condition{ + conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service has empty or invalid externalName field", + ), + }, + name: "ExternalName service with empty externalName field", + }, + { + ref: gatewayv1.HTTPBackendRef{ + BackendRef: getModifiedRef(func(backend gatewayv1.BackendRef) gatewayv1.BackendRef { + backend.Name = "external-service-whitespace" + return backend + }), + }, + expectedBackend: BackendRef{ + SvcNsName: types.NamespacedName{Namespace: "test", Name: "external-service-whitespace"}, + ServicePort: v1.ServicePort{Port: 80}, + Weight: 5, + Valid: false, + InvalidForGateways: map[types.NamespacedName]conditions.Condition{ + {Namespace: "test", Name: "gateway"}: conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service requires DNS resolver configuration in Gateway's NginxProxy", + ), + }, + }, + expectedServicePortReference: "", + expectedConditions: []conditions.Condition{ + conditions.NewRouteBackendRefUnsupportedValue( + "ExternalName service has empty or invalid externalName field", + ), + }, + name: "ExternalName service with whitespace-only externalName field", + }, } services := map[types.NamespacedName]*v1.Service{ - client.ObjectKeyFromObject(svc1): svc1, - client.ObjectKeyFromObject(svc2): svc2, - client.ObjectKeyFromObject(svc3): svc3, + client.ObjectKeyFromObject(svc1): svc1, + client.ObjectKeyFromObject(svc2): svc2, + client.ObjectKeyFromObject(svc3): svc3, + client.ObjectKeyFromObject(externalSvc): externalSvc, + client.ObjectKeyFromObject(externalSvcEmpty): externalSvcEmpty, + client.ObjectKeyFromObject(externalSvcWhitespace): externalSvcWhitespace, } policies := map[types.NamespacedName]*BackendTLSPolicy{ client.ObjectKeyFromObject(btp.Source): &btp, @@ -1249,6 +1426,27 @@ func TestCreateBackend(t *testing.T) { }, } + // Special case: for the multiple gateways test, add a second gateway + if test.name == "ExternalName service with multiple gateways - mixed DNS resolver config" { + route.ParentRefs = append(route.ParentRefs, ParentRef{ + Gateway: &ParentRefGateway{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "gateway2", + }, + EffectiveNginxProxy: nil, // No DNS resolver + }, + }) + // For this test, the first gateway should have DNS resolver + route.ParentRefs[0].Gateway.EffectiveNginxProxy = &EffectiveNginxProxy{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "8.8.8.8"}, + }, + }, + } + } + backend, conds := createBackendRef( rbr, route, @@ -1441,8 +1639,8 @@ func TestValidateBackendTLSPolicyMatchingAllBackends(t *testing.T) { func TestFindBackendTLSPolicyForService(t *testing.T) { t.Parallel() - oldCreationTimestamp := metav1.Now() - newCreationTimestamp := metav1.Now() + oldCreationTimestamp := metav1.NewTime(time.Now().Add(-time.Hour)) + newCreationTimestamp := metav1.NewTime(time.Now()) getBtp := func(name string, timestamp metav1.Time) *BackendTLSPolicy { return &BackendTLSPolicy{ Valid: true, diff --git a/internal/controller/state/graph/graph.go b/internal/controller/state/graph/graph.go index b833d38da4..e556c798ba 100644 --- a/internal/controller/state/graph/graph.go +++ b/internal/controller/state/graph/graph.go @@ -268,7 +268,7 @@ func BuildGraph( referencedNamespaces := buildReferencedNamespaces(state.Namespaces, gws) - referencedServices := buildReferencedServices(routes, l4routes, gws) + referencedServices := buildReferencedServices(routes, l4routes, gws, state.Services) addGatewaysForBackendTLSPolicies(processedBackendTLSPolicies, referencedServices, controllerName, gws, logger) diff --git a/internal/controller/state/graph/nginxproxy.go b/internal/controller/state/graph/nginxproxy.go index 1e4afbc9dd..752918e6a9 100644 --- a/internal/controller/state/graph/nginxproxy.go +++ b/internal/controller/state/graph/nginxproxy.go @@ -272,6 +272,8 @@ func validateNginxProxy( allErrs = append(allErrs, validateLogging(npCfg)...) + allErrs = append(allErrs, validateDNSResolver(validator, npCfg)...) + allErrs = append(allErrs, validateRewriteClientIP(npCfg)...) allErrs = append(allErrs, validateNginxPlus(npCfg)...) @@ -316,6 +318,72 @@ func validateLogging(npCfg *ngfAPIv1alpha2.NginxProxy) field.ErrorList { return allErrs } +func validateDNSResolver( + validator validation.GenericValidator, + npCfg *ngfAPIv1alpha2.NginxProxy, +) field.ErrorList { + if npCfg.Spec.DNSResolver == nil { + return nil + } + + var allErrs field.ErrorList + dnsResolver := npCfg.Spec.DNSResolver + dnsResolverPath := field.NewPath("spec", "dnsResolver") + + if dnsResolver.Timeout != nil { + if err := validator.ValidateNginxDuration(string(*dnsResolver.Timeout)); err != nil { + allErrs = append(allErrs, field.Invalid( + dnsResolverPath.Child("timeout"), + *dnsResolver.Timeout, + err.Error(), + )) + } + } + + if dnsResolver.CacheTTL != nil { + if err := validator.ValidateNginxDuration(string(*dnsResolver.CacheTTL)); err != nil { + allErrs = append(allErrs, field.Invalid( + dnsResolverPath.Child("valid"), + *dnsResolver.CacheTTL, + err.Error(), + )) + } + } + + addressesPath := dnsResolverPath.Child("addresses") + for i, addr := range dnsResolver.Addresses { + addrPath := addressesPath.Index(i) + + if addr.Type == ngfAPIv1alpha2.DNSResolverIPAddressType { + if errs := k8svalidation.IsValidIP(addrPath.Child("value"), addr.Value); len(errs) > 0 { + allErrs = append(allErrs, field.Invalid( + addrPath.Child("value"), + addr.Value, + "must be a valid IP address", + )) + } + } + + if addr.Type == ngfAPIv1alpha2.DNSResolverHostnameType { + if errs := k8svalidation.IsDNS1123Subdomain(addr.Value); len(errs) > 0 { + for _, e := range errs { + allErrs = append(allErrs, field.Invalid( + addrPath.Child("value"), + addr.Value, + e, + )) + } + } + } + } + + if len(dnsResolver.Addresses) == 0 { + allErrs = append(allErrs, field.Required(addressesPath, "addresses field is required")) + } + + return allErrs +} + func validateRewriteClientIP(npCfg *ngfAPIv1alpha2.NginxProxy) field.ErrorList { var allErrs field.ErrorList spec := field.NewPath("spec") diff --git a/internal/controller/state/graph/nginxproxy_test.go b/internal/controller/state/graph/nginxproxy_test.go index d868b9960d..7cbdc05042 100644 --- a/internal/controller/state/graph/nginxproxy_test.go +++ b/internal/controller/state/graph/nginxproxy_test.go @@ -906,6 +906,188 @@ func TestValidateNginxProxy(t *testing.T) { } } +func TestValidateDNSResolver(t *testing.T) { + t.Parallel() + tests := []struct { + np *ngfAPIv1alpha2.NginxProxy + validator *validationfakes.FakeGenericValidator + name string + errorString string + expectErrCount int + }{ + { + name: "valid DNS resolver with IPv4 addresses", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "8.8.8.8"}, + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "1.1.1.1"}, + }, + Timeout: helpers.GetPointer[ngfAPIv1alpha1.Duration]("30s"), + CacheTTL: helpers.GetPointer[ngfAPIv1alpha1.Duration]("60s"), + }, + }, + }, + expectErrCount: 0, + }, + { + name: "valid DNS resolver with hostnames", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverHostnameType, Value: "dns.google"}, + {Type: ngfAPIv1alpha2.DNSResolverHostnameType, Value: "one.one.one.one"}, + }, + Timeout: helpers.GetPointer[ngfAPIv1alpha1.Duration]("5s"), + CacheTTL: helpers.GetPointer[ngfAPIv1alpha1.Duration]("30s"), + }, + }, + }, + expectErrCount: 0, + }, + { + name: "valid DNS resolver with IPv6 addresses", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "2001:4860:4860::8888"}, + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "2606:4700:4700::1111"}, + }, + }, + }, + }, + expectErrCount: 0, + }, + { + name: "valid DNS resolver with mixed address types", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "8.8.8.8"}, + {Type: ngfAPIv1alpha2.DNSResolverHostnameType, Value: "dns.google"}, + }, + }, + }, + }, + expectErrCount: 0, + }, + { + name: "invalid DNS resolver timeout duration", + validator: createInvalidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "8.8.8.8"}, + }, + Timeout: helpers.GetPointer[ngfAPIv1alpha1.Duration]("invalid-duration"), + }, + }, + }, + errorString: "spec.dnsResolver.timeout", + expectErrCount: 1, + }, + { + name: "invalid DNS resolver - both duration fields invalid", + validator: createInvalidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "8.8.8.8"}, + }, + Timeout: helpers.GetPointer[ngfAPIv1alpha1.Duration]("invalid"), + CacheTTL: helpers.GetPointer[ngfAPIv1alpha1.Duration]("invalid"), + }, + }, + }, + errorString: "spec.dnsResolver", + expectErrCount: 2, + }, + { + name: "invalid IP address", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "256.256.256.256"}, + }, + }, + }, + }, + errorString: "spec.dnsResolver.addresses[0].value", + expectErrCount: 1, + }, + { + name: "invalid hostname", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverHostnameType, Value: "invalid..hostname"}, + }, + }, + }, + }, + errorString: "spec.dnsResolver.addresses[0].value", + expectErrCount: 1, + }, + { + name: "multiple invalid DNS resolver addresses", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{ + {Type: ngfAPIv1alpha2.DNSResolverHostnameType, Value: "invalid..hostname"}, + {Type: ngfAPIv1alpha2.DNSResolverIPAddressType, Value: "999.999.999.999"}, + {Type: ngfAPIv1alpha2.DNSResolverHostnameType, Value: ""}, + }, + }, + }, + }, + errorString: "spec.dnsResolver.addresses", + expectErrCount: 3, + }, + { + name: "empty addresses array", + validator: createValidValidator(), + np: &ngfAPIv1alpha2.NginxProxy{ + Spec: ngfAPIv1alpha2.NginxProxySpec{ + DNSResolver: &ngfAPIv1alpha2.DNSResolver{ + Addresses: []ngfAPIv1alpha2.DNSResolverAddress{}, + }, + }, + }, + errorString: "spec.dnsResolver.addresses", + expectErrCount: 1, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + allErrs := validateNginxProxy(test.validator, test.np) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + if len(allErrs) > 0 { + g.Expect(allErrs.ToAggregate().Error()).To(ContainSubstring(test.errorString)) + } + }) + } +} + func TestValidateRewriteClientIP(t *testing.T) { t.Parallel() tests := []struct { diff --git a/internal/controller/state/graph/service.go b/internal/controller/state/graph/service.go index 7a41b07132..d43ecacfd8 100644 --- a/internal/controller/state/graph/service.go +++ b/internal/controller/state/graph/service.go @@ -1,6 +1,7 @@ package graph import ( + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -11,93 +12,152 @@ import ( type ReferencedService struct { // GatewayNsNames are all the Gateways that this Service indirectly attaches to through a Route. GatewayNsNames map[types.NamespacedName]struct{} + // ExternalName holds the external service name for ExternalName type services. + ExternalName string // Policies is a list of NGF Policies that target this Service. Policies []*Policy + // IsExternalName indicates whether this Service is of type ExternalName. + IsExternalName bool } func buildReferencedServices( l7routes map[RouteKey]*L7Route, l4Routes map[L4RouteKey]*L4Route, gws map[types.NamespacedName]*Gateway, + services map[types.NamespacedName]*v1.Service, ) map[types.NamespacedName]*ReferencedService { referencedServices := make(map[types.NamespacedName]*ReferencedService) + for gwNsName, gw := range gws { if gw == nil { continue } - belongsToGw := func(refs []ParentRef) bool { - for _, ref := range refs { - if ref.Gateway.NamespacedName == client.ObjectKeyFromObject(gw.Source) { - return true - } - } - return false - } + processL7RoutesForGateway(l7routes, gw, gwNsName, referencedServices, services) - // routes all have populated ParentRefs from when they were created. - // - // Get all the service names referenced from all the l7 and l4 routes. - for _, route := range l7routes { - if !route.Valid || !belongsToGw(route.ParentRefs) { - continue - } + processL4RoutesForGateway(l4Routes, gw, gwNsName, referencedServices, services) + } - // Processes both valid and invalid BackendRefs as invalid ones still have referenced services - // we may want to track. - addServicesAndGatewayForL7Routes(route.Spec.Rules, gwNsName, referencedServices) - } + if len(referencedServices) == 0 { + return nil + } - for _, route := range l4Routes { - if !route.Valid || !belongsToGw(route.ParentRefs) { - continue - } + return referencedServices +} - addServicesAndGatewayForL4Routes(route, gwNsName, referencedServices) +// processL7RoutesForGateway processes all L7 routes that belong to the given gateway. +func processL7RoutesForGateway( + l7routes map[RouteKey]*L7Route, + gw *Gateway, + gwNsName types.NamespacedName, + referencedServices map[types.NamespacedName]*ReferencedService, + services map[types.NamespacedName]*v1.Service, +) { + gwKey := client.ObjectKeyFromObject(gw.Source) + + for _, route := range l7routes { + if !route.Valid || !routeBelongsToGateway(route.ParentRefs, gwKey) { + continue } + + // Process both valid and invalid BackendRefs as invalid ones still have referenced services + // we may want to track. + addServicesFromL7RouteRules(route.Spec.Rules, gwNsName, referencedServices, services) } +} - if len(referencedServices) == 0 { - return nil +// processL4RoutesForGateway processes all L4 routes that belong to the given gateway. +func processL4RoutesForGateway( + l4Routes map[L4RouteKey]*L4Route, + gw *Gateway, + gwNsName types.NamespacedName, + referencedServices map[types.NamespacedName]*ReferencedService, + services map[types.NamespacedName]*v1.Service, +) { + gwKey := client.ObjectKeyFromObject(gw.Source) + + for _, route := range l4Routes { + if !route.Valid || !routeBelongsToGateway(route.ParentRefs, gwKey) { + continue + } + + addServiceFromL4Route(route, gwNsName, referencedServices, services) } +} - return referencedServices +// routeBelongsToGateway checks if a route belongs to the specified gateway. +func routeBelongsToGateway(refs []ParentRef, gwKey types.NamespacedName) bool { + for _, ref := range refs { + if ref.Gateway.NamespacedName == gwKey { + return true + } + } + return false } -func addServicesAndGatewayForL4Routes( +// addServiceFromL4Route adds a service from an L4 route to the referenced services map. +func addServiceFromL4Route( route *L4Route, gwNsName types.NamespacedName, referencedServices map[types.NamespacedName]*ReferencedService, + services map[types.NamespacedName]*v1.Service, ) { - nsname := route.Spec.BackendRef.SvcNsName - if nsname != (types.NamespacedName{}) { - if _, ok := referencedServices[nsname]; !ok { - referencedServices[nsname] = &ReferencedService{ - Policies: nil, - GatewayNsNames: make(map[types.NamespacedName]struct{}), - } - } - referencedServices[nsname].GatewayNsNames[gwNsName] = struct{}{} + svcNsName := route.Spec.BackendRef.SvcNsName + if svcNsName == (types.NamespacedName{}) { + return } + + ensureReferencedService(svcNsName, referencedServices, services) + referencedServices[svcNsName].GatewayNsNames[gwNsName] = struct{}{} } -func addServicesAndGatewayForL7Routes( +// addServicesFromL7RouteRules adds services from L7 route rules to the referenced services map. +func addServicesFromL7RouteRules( routeRules []RouteRule, gwNsName types.NamespacedName, referencedServices map[types.NamespacedName]*ReferencedService, + services map[types.NamespacedName]*v1.Service, ) { for _, rule := range routeRules { for _, ref := range rule.BackendRefs { - if ref.SvcNsName != (types.NamespacedName{}) { - if _, ok := referencedServices[ref.SvcNsName]; !ok { - referencedServices[ref.SvcNsName] = &ReferencedService{ - Policies: nil, - GatewayNsNames: make(map[types.NamespacedName]struct{}), - } - } - - referencedServices[ref.SvcNsName].GatewayNsNames[gwNsName] = struct{}{} + if ref.SvcNsName == (types.NamespacedName{}) { + continue } + + ensureReferencedService(ref.SvcNsName, referencedServices, services) + referencedServices[ref.SvcNsName].GatewayNsNames[gwNsName] = struct{}{} } } } + +// ensureReferencedService ensures a ReferencedService exists in the map for the given service. +func ensureReferencedService( + svcNsName types.NamespacedName, + referencedServices map[types.NamespacedName]*ReferencedService, + services map[types.NamespacedName]*v1.Service, +) { + if _, exists := referencedServices[svcNsName]; exists { + return + } + + isExternal, externalName := getServiceExternalNameInfo(svcNsName, services) + referencedServices[svcNsName] = &ReferencedService{ + Policies: nil, + GatewayNsNames: make(map[types.NamespacedName]struct{}), + IsExternalName: isExternal, + ExternalName: externalName, + } +} + +// getServiceExternalNameInfo returns whether a service is an ExternalName service and its external name. +func getServiceExternalNameInfo( + svcNsName types.NamespacedName, + services map[types.NamespacedName]*v1.Service, +) (isExternal bool, externalName string) { + svc, exists := services[svcNsName] + if !exists || svc.Spec.Type != v1.ServiceTypeExternalName { + return false, "" + } + + return true, svc.Spec.ExternalName +} diff --git a/internal/controller/state/graph/service_test.go b/internal/controller/state/graph/service_test.go index e0ef7180ce..4a54cae9a4 100644 --- a/internal/controller/state/graph/service_test.go +++ b/internal/controller/state/graph/service_test.go @@ -4,6 +4,7 @@ import ( "testing" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" v1 "sigs.k8s.io/gateway-api/apis/v1" @@ -162,11 +163,13 @@ func TestBuildReferencedServices(t *testing.T) { l4Routes map[L4RouteKey]*L4Route exp map[types.NamespacedName]*ReferencedService gws map[types.NamespacedName]*Gateway + services map[types.NamespacedName]*corev1.Service name string }{ { - name: "normal routes", - gws: gw, + name: "normal routes", + gws: gw, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute, }, @@ -179,18 +182,23 @@ func TestBuildReferencedServices(t *testing.T) { {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, {Namespace: "tlsroute-ns", Name: "service"}: { GatewayNsNames: map[types.NamespacedName]struct{}{ {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, }, }, { - name: "l7 route with two services in one Rule", // l4 routes don't support multiple services right now - gws: gw, + name: "l7 route with two services in one Rule", // l4 routes don't support multiple services right now + gws: gw, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "two-svc-one-rule"}}: validRouteTwoServicesOneRule, }, @@ -200,18 +208,23 @@ func TestBuildReferencedServices(t *testing.T) { {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, {Namespace: "service-ns2", Name: "service2"}: { GatewayNsNames: map[types.NamespacedName]struct{}{ {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, }, }, { - name: "route with one service per rule", // l4 routes don't support multiple rules right now - gws: gw, + name: "route with one service per rule", // l4 routes don't support multiple rules right now + gws: gw, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules, }, @@ -221,18 +234,23 @@ func TestBuildReferencedServices(t *testing.T) { {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, {Namespace: "service-ns2", Name: "service2"}: { GatewayNsNames: map[types.NamespacedName]struct{}{ {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, }, }, { - name: "multiple valid routes with same services", - gws: gw, + name: "multiple valid routes with same services", + gws: gw, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "one-svc-per-rule"}}: validRouteTwoServicesTwoRules, {NamespacedName: types.NamespacedName{Name: "two-svc-one-rule"}}: validRouteTwoServicesOneRule, @@ -248,30 +266,39 @@ func TestBuildReferencedServices(t *testing.T) { {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, {Namespace: "service-ns2", Name: "service2"}: { GatewayNsNames: map[types.NamespacedName]struct{}{ {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, {Namespace: "tlsroute-ns", Name: "service"}: { GatewayNsNames: map[types.NamespacedName]struct{}{ {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, {Namespace: "tlsroute-ns", Name: "service2"}: { GatewayNsNames: map[types.NamespacedName]struct{}{ {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, }, }, { - name: "invalid routes", - gws: gw, + name: "invalid routes", + gws: gw, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "invalid-route"}}: invalidRoute, }, @@ -281,8 +308,9 @@ func TestBuildReferencedServices(t *testing.T) { exp: nil, }, { - name: "combination of valid and invalid routes", - gws: gw, + name: "combination of valid and invalid routes", + gws: gw, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute, {NamespacedName: types.NamespacedName{Name: "invalid-route"}}: invalidRoute, @@ -297,18 +325,23 @@ func TestBuildReferencedServices(t *testing.T) { {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, {Namespace: "tlsroute-ns", Name: "service"}: { GatewayNsNames: map[types.NamespacedName]struct{}{ {Namespace: "test", Name: "gwNsname"}: {}, {Namespace: "test", Name: "gw2Nsname"}: {}, }, + IsExternalName: false, + ExternalName: "", }, }, }, { - name: "valid route no service nsname", - gws: gw, + name: "valid route no service nsname", + gws: gw, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "no-service-nsname"}}: validRouteNoServiceNsName, }, @@ -322,6 +355,7 @@ func TestBuildReferencedServices(t *testing.T) { gws: map[types.NamespacedName]*Gateway{ gwNsName: nil, }, + services: map[types.NamespacedName]*corev1.Service{}, l7Routes: map[RouteKey]*L7Route{ {NamespacedName: types.NamespacedName{Name: "no-service-nsname"}}: validRouteNoServiceNsName, }, @@ -330,6 +364,55 @@ func TestBuildReferencedServices(t *testing.T) { }, exp: nil, }, + { + name: "external name services", + gws: gw, + services: map[types.NamespacedName]*corev1.Service{ + {Namespace: "banana-ns", Name: "service"}: { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "banana-ns", + Name: "service", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeExternalName, + ExternalName: "api.example.com", + }, + }, + {Namespace: "tlsroute-ns", Name: "service"}: { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tlsroute-ns", + Name: "service", + }, + Spec: corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + }, + }, + }, + l7Routes: map[RouteKey]*L7Route{ + {NamespacedName: types.NamespacedName{Name: "normal-route"}}: normalRoute, + }, + l4Routes: map[L4RouteKey]*L4Route{ + {NamespacedName: types.NamespacedName{Name: "normal-l4-route"}}: normalL4Route, + }, + exp: map[types.NamespacedName]*ReferencedService{ + {Namespace: "banana-ns", Name: "service"}: { + GatewayNsNames: map[types.NamespacedName]struct{}{ + {Namespace: "test", Name: "gwNsname"}: {}, + {Namespace: "test", Name: "gw2Nsname"}: {}, + }, + IsExternalName: true, + ExternalName: "api.example.com", + }, + {Namespace: "tlsroute-ns", Name: "service"}: { + GatewayNsNames: map[types.NamespacedName]struct{}{ + {Namespace: "test", Name: "gwNsname"}: {}, + {Namespace: "test", Name: "gw2Nsname"}: {}, + }, + IsExternalName: false, + ExternalName: "", + }, + }, + }, } for _, test := range tests { @@ -337,7 +420,7 @@ func TestBuildReferencedServices(t *testing.T) { t.Parallel() g := NewWithT(t) - g.Expect(buildReferencedServices(test.l7Routes, test.l4Routes, test.gws)).To(Equal(test.exp)) + g.Expect(buildReferencedServices(test.l7Routes, test.l4Routes, test.gws, test.services)).To(Equal(test.exp)) }) } } diff --git a/internal/controller/state/resolver/resolver.go b/internal/controller/state/resolver/resolver.go index de5ec4e6d4..253b254db0 100644 --- a/internal/controller/state/resolver/resolver.go +++ b/internal/controller/state/resolver/resolver.go @@ -32,12 +32,14 @@ type ServiceResolver interface { // Endpoint is the internal representation of a Kubernetes endpoint. type Endpoint struct { - // Address is the IP address of the endpoint. + // Address is the IP address or DNS name of the endpoint. Address string // Port is the port of the endpoint. Port int32 // IPv6 is true if the endpoint is an IPv6 address. IPv6 bool + // Resolve is true if the address is a DNS name that needs to be resolved (e.g., for ExternalName services). + Resolve bool } // ServiceResolverImpl implements ServiceResolver.