diff --git a/apis/v1alpha2/nginxproxy_types.go b/apis/v1alpha2/nginxproxy_types.go index c5d9dfcef1..bf7fa9c783 100644 --- a/apis/v1alpha2/nginxproxy_types.go +++ b/apis/v1alpha2/nginxproxy_types.go @@ -297,6 +297,12 @@ type NginxLogging struct { // +optional // +kubebuilder:default=info AgentLevel *AgentLogLevel `json:"agentLevel,omitempty"` + + // AccessLog defines the access log settings, including format itself and disabling option. + // For now only path /dev/stdout can be used. + // + // +optional + AccessLog *NginxAccessLog `json:"accessLog,omitempty"` } // NginxErrorLogLevel type defines the log level of error logs for NGINX. @@ -352,6 +358,22 @@ const ( AgentLogLevelFatal AgentLogLevel = "fatal" ) +// NginxAccessLog defines the configuration for an NGINX access log. +type NginxAccessLog struct { + // Disable turns off access logging when set to true. + // + // +optional + Disable *bool `json:"disable,omitempty"` + + // Format specifies the custom log format string. + // If not specified, NGINX default 'combined' format is used. + // For now only path /dev/stdout can be used. + // See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format + // + // +optional + Format *string `json:"format,omitempty"` +} + // NginxPlus specifies NGINX Plus additional settings. These will only be applied if NGINX Plus is being used. type NginxPlus struct { // AllowedAddresses specifies IPAddresses or CIDR blocks to the allow list for accessing the NGINX Plus API. diff --git a/apis/v1alpha2/zz_generated.deepcopy.go b/apis/v1alpha2/zz_generated.deepcopy.go index 7033aabb3a..06f09b2985 100644 --- a/apis/v1alpha2/zz_generated.deepcopy.go +++ b/apis/v1alpha2/zz_generated.deepcopy.go @@ -315,6 +315,31 @@ func (in *Metrics) DeepCopy() *Metrics { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxAccessLog) DeepCopyInto(out *NginxAccessLog) { + *out = *in + if in.Disable != nil { + in, out := &in.Disable, &out.Disable + *out = new(bool) + **out = **in + } + if in.Format != nil { + in, out := &in.Format, &out.Format + *out = new(string) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxAccessLog. +func (in *NginxAccessLog) DeepCopy() *NginxAccessLog { + if in == nil { + return nil + } + out := new(NginxAccessLog) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NginxLogging) DeepCopyInto(out *NginxLogging) { *out = *in @@ -328,6 +353,11 @@ func (in *NginxLogging) DeepCopyInto(out *NginxLogging) { *out = new(AgentLogLevel) **out = **in } + if in.AccessLog != nil { + in, out := &in.AccessLog, &out.AccessLog + *out = new(NginxAccessLog) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxLogging. diff --git a/charts/nginx-gateway-fabric/values.schema.json b/charts/nginx-gateway-fabric/values.schema.json index 50d6d7e987..85967e4913 100644 --- a/charts/nginx-gateway-fabric/values.schema.json +++ b/charts/nginx-gateway-fabric/values.schema.json @@ -202,6 +202,23 @@ "logging": { "description": "Logging defines logging related settings for NGINX.", "properties": { + "accessLog": { + "description": "AccessLog defines the access log settings.", + "properties": { + "disable": { + "description": "Disable turns off access logging when set to true.", + "required": [], + "type": "boolean" + }, + "format": { + "description": "Format specifies the custom log format string. If not specified, NGINX default 'combined' format is used.", + "required": [], + "type": "string" + } + }, + "required": [], + "type": "object" + }, "agentLevel": { "enum": [ "debug", diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index 772a58daa4..9e8e7de21d 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -471,6 +471,16 @@ nginx: # - error # - panic # - fatal + # accessLog: + # type: object + # description: AccessLog defines the access log settings. + # properties: + # disable: + # type: boolean + # description: Disable turns off access logging when set to true. + # format: + # type: string + # description: Format specifies the custom log format string. If not specified, NGINX default 'combined' format is used. # nginxPlus: # type: object # description: NginxPlus specifies NGINX Plus additional settings. diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 89de827bd4..ecc55e418a 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -8016,6 +8016,23 @@ spec: logging: description: Logging defines logging related settings for NGINX. properties: + accessLog: + description: |- + AccessLog defines the access log settings, including format itself and disabling option. + For now only path /dev/stdout can be used. + properties: + disable: + description: Disable turns off access logging when set to + true. + type: boolean + format: + description: |- + Format specifies the custom log format string. + If not specified, NGINX default 'combined' format is used. + For now only path /dev/stdout can be used. + See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format + type: string + type: object agentLevel: default: info description: |- diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 6bef9c9a27..2a526a961f 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -8603,6 +8603,23 @@ spec: logging: description: Logging defines logging related settings for NGINX. properties: + accessLog: + description: |- + AccessLog defines the access log settings, including format itself and disabling option. + For now only path /dev/stdout can be used. + properties: + disable: + description: Disable turns off access logging when set to + true. + type: boolean + format: + description: |- + Format specifies the custom log format string. + If not specified, NGINX default 'combined' format is used. + For now only path /dev/stdout can be used. + See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format + type: string + type: object agentLevel: default: info description: |- diff --git a/internal/controller/nginx/config/base_http_config.go b/internal/controller/nginx/config/base_http_config.go index 4bce95a0bc..0232116849 100644 --- a/internal/controller/nginx/config/base_http_config.go +++ b/internal/controller/nginx/config/base_http_config.go @@ -10,8 +10,15 @@ import ( var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTPTemplateText)) +type AccessLog struct { + Format string // User's format string + Path string // Where to write logs (/dev/stdout) + FormatName string // Internal format name (ngf_user_defined_log_format) + Disable bool // User's disable flag +} type httpConfig struct { DNSResolver *dataplane.DNSResolverConfig + AccessLog *AccessLog Includes []shared.Include NginxReadinessProbePort int32 IPFamily shared.IPFamily @@ -27,6 +34,7 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult { NginxReadinessProbePort: conf.BaseHTTPConfig.NginxReadinessProbePort, IPFamily: getIPFamily(conf.BaseHTTPConfig), DNSResolver: conf.BaseHTTPConfig.DNSResolver, + AccessLog: buildAccessLog(conf.Logging.AccessLog), } results := make([]executeResult, 0, len(includes)+1) @@ -38,3 +46,19 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult { return results } + +func buildAccessLog(accessLogConfig *dataplane.AccessLog) *AccessLog { + if accessLogConfig != nil { + accessLog := &AccessLog{ + Path: dataplane.DefaultAccessLogPath, + FormatName: dataplane.DefaultLogFormatName, + } + if accessLogConfig.Format != "" { + accessLog.Format = accessLogConfig.Format + } + accessLog.Disable = accessLogConfig.Disable + + return accessLog + } + return nil +} diff --git a/internal/controller/nginx/config/base_http_config_template.go b/internal/controller/nginx/config/base_http_config_template.go index e50a83a65f..5f7a3caf4a 100644 --- a/internal/controller/nginx/config/base_http_config_template.go +++ b/internal/controller/nginx/config/base_http_config_template.go @@ -48,6 +48,19 @@ server { } } +{{- /* Define custom log format */ -}} +{{- /* We use a fixed name for user-defined log format to avoid complexity of passing the name around. */ -}} +{{- if .AccessLog }} +{{- if .AccessLog.Disable }} +access_log off; +{{- else }} +{{- if .AccessLog.Format }} +log_format {{ .AccessLog.FormatName }} '{{ .AccessLog.Format }}'; +access_log {{ .AccessLog.Path }} {{ .AccessLog.FormatName }}; +{{- end }} +{{- end }} +{{- end }} + {{ range $i := .Includes -}} include {{ $i.Name }}; {{ end -}} diff --git a/internal/controller/nginx/config/base_http_config_test.go b/internal/controller/nginx/config/base_http_config_test.go index fe7ad3a58d..871643c708 100644 --- a/internal/controller/nginx/config/base_http_config_test.go +++ b/internal/controller/nginx/config/base_http_config_test.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "sort" "strings" "testing" @@ -10,6 +11,77 @@ import ( "github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane" ) +func TestLoggingSettingsTemplate(t *testing.T) { + t.Parallel() + + logFormat := "$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent" + + tests := []struct { + name string + accessLog *dataplane.AccessLog + expectedOutputs []string + unexpectedOutputs []string + }{ + { + name: "Log format and access log with custom format", + accessLog: &dataplane.AccessLog{Format: logFormat}, + expectedOutputs: []string{ + fmt.Sprintf("log_format %s '%s'", dataplane.DefaultLogFormatName, logFormat), + fmt.Sprintf("access_log %s %s", dataplane.DefaultAccessLogPath, dataplane.DefaultLogFormatName), + }, + }, + { + name: "Empty format", + accessLog: &dataplane.AccessLog{Format: ""}, + unexpectedOutputs: []string{ + fmt.Sprintf("log_format %s '%s'", dataplane.DefaultLogFormatName, logFormat), + fmt.Sprintf("access_log %s %s", dataplane.DefaultAccessLogPath, dataplane.DefaultLogFormatName), + }, + }, + { + name: "Access log off while format presented", + accessLog: &dataplane.AccessLog{Disable: true, Format: logFormat}, + expectedOutputs: []string{ + `access_log off;`, + }, + unexpectedOutputs: []string{ + fmt.Sprintf("access_log off %s", dataplane.DefaultLogFormatName), + }, + }, + { + name: "Access log off", + accessLog: &dataplane.AccessLog{Disable: true}, + expectedOutputs: []string{ + `access_log off;`, + }, + unexpectedOutputs: []string{ + `log_format`, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + conf := dataplane.Configuration{ + Logging: dataplane.Logging{AccessLog: tt.accessLog}, + } + + res := executeBaseHTTPConfig(conf) + g.Expect(res).To(HaveLen(1)) + httpConfig := string(res[0].data) + for _, expectedOutput := range tt.expectedOutputs { + g.Expect(httpConfig).To(ContainSubstring(expectedOutput)) + } + for _, unexpectedOutput := range tt.unexpectedOutputs { + g.Expect(httpConfig).ToNot(ContainSubstring(unexpectedOutput)) + } + }) + } +} + func TestExecuteBaseHttp_HTTP2(t *testing.T) { t.Parallel() confOn := dataplane.Configuration{ diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 6615fd7036..ff89473d71 100644 --- a/internal/controller/state/dataplane/configuration.go +++ b/internal/controller/state/dataplane/configuration.go @@ -28,6 +28,10 @@ const ( defaultErrorLogLevel = "info" DefaultWorkerConnections = int32(1024) DefaultNginxReadinessProbePort = int32(8081) + // DefaultLogFormatName is used when user provides custom access_log format. + DefaultLogFormatName = "ngf_user_defined_log_format" + // DefaultAccessLogPath is the default path for the access log. + DefaultAccessLogPath = "/dev/stdout" ) // BuildConfiguration builds the Configuration from the Graph. @@ -1206,6 +1210,8 @@ func convertAddresses(addresses []ngfAPIv1alpha2.RewriteClientIPAddress) []strin return trustedAddresses } +// buildLogging converts the API logging spec (currently singular LogFormat / AccessLog fields +// in v1alpha2) into internal representation used by templates. func buildLogging(gateway *graph.Gateway) Logging { logSettings := Logging{ErrorLevel: defaultErrorLogLevel} @@ -1218,11 +1224,33 @@ func buildLogging(gateway *graph.Gateway) Logging { if ngfProxy.Logging.ErrorLevel != nil { logSettings.ErrorLevel = string(*ngfProxy.Logging.ErrorLevel) } + + srcLogSettings := ngfProxy.Logging + + if accessLog := buildAccessLog(srcLogSettings); accessLog != nil { + logSettings.AccessLog = accessLog + } } return logSettings } +func buildAccessLog(srcLogSettings *ngfAPIv1alpha2.NginxLogging) *AccessLog { + if srcLogSettings.AccessLog != nil { + if srcLogSettings.AccessLog.Disable != nil && *srcLogSettings.AccessLog.Disable { + return &AccessLog{Disable: true} + } + + if srcLogSettings.AccessLog.Format != nil && *srcLogSettings.AccessLog.Format != "" { + return &AccessLog{ + Format: *srcLogSettings.AccessLog.Format, + } + } + } + + return nil +} + func buildWorkerConnections(gateway *graph.Gateway) int32 { if gateway == nil || gateway.EffectiveNginxProxy == nil { return DefaultWorkerConnections diff --git a/internal/controller/state/dataplane/configuration_test.go b/internal/controller/state/dataplane/configuration_test.go index 73f109be7c..9c72160c06 100644 --- a/internal/controller/state/dataplane/configuration_test.go +++ b/internal/controller/state/dataplane/configuration_test.go @@ -4768,12 +4768,15 @@ func TestBuildRewriteIPSettings(t *testing.T) { func TestBuildLogging(t *testing.T) { defaultLogging := Logging{ErrorLevel: defaultErrorLogLevel} + logFormat := `'$remote_addr - $remote_user [$time_local] ' + '"$request" $status $body_bytes_sent ' + '"$http_referer" "$http_user_agent" '` t.Parallel() tests := []struct { - msg string - gw *graph.Gateway expLoggingSettings Logging + gw *graph.Gateway + msg string }{ { msg: "Gateway is nil", @@ -4884,6 +4887,103 @@ func TestBuildLogging(t *testing.T) { }, expLoggingSettings: Logging{ErrorLevel: "emerg"}, }, + { + msg: "AccessLog configured", + gw: &graph.Gateway{ + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Format: helpers.GetPointer(logFormat), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + AccessLog: &AccessLog{ + Format: logFormat, + }, + }, + }, + { + msg: "AccessLog is configured and Disable = false", + gw: &graph.Gateway{ + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Disable: helpers.GetPointer(false), + Format: helpers.GetPointer(logFormat), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + + AccessLog: &AccessLog{ + Disable: false, + Format: logFormat, + }, + }, + }, + { + msg: "Nothing configured if AccessLog Format is missing", + gw: &graph.Gateway{ + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Disable: helpers.GetPointer(false), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + AccessLog: nil, + }, + }, + { + msg: "AccessLog OFF while LogFormat is configured", + gw: &graph.Gateway{ + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Disable: helpers.GetPointer(true), + Format: helpers.GetPointer(logFormat), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + AccessLog: &AccessLog{ + Disable: true, + }, + }, + }, + { + msg: "AccessLog OFF", + gw: &graph.Gateway{ + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Disable: helpers.GetPointer(true), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + AccessLog: &AccessLog{ + Disable: true, + }, + }, + }, } for _, tc := range tests { diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 987c021c7f..a8acb861dd 100644 --- a/internal/controller/state/dataplane/types.go +++ b/internal/controller/state/dataplane/types.go @@ -474,6 +474,8 @@ type Ratio struct { // Logging defines logging related settings for NGINX. type Logging struct { + // AccessLog defines the configuration for the NGINX access log. + AccessLog *AccessLog // ErrorLevel defines the error log level. ErrorLevel string } @@ -496,3 +498,11 @@ type DeploymentContext struct { // Integration is "ngf". Integration string `json:"integration"` } + +// AccessLog defines the configuration for an NGINX access log. +type AccessLog struct { + // Format is the access log format template. + Format string + // Disable specifies whether the access log is disabled. + Disable bool +} diff --git a/internal/controller/state/graph/multiple_gateways_test.go b/internal/controller/state/graph/multiple_gateways_test.go index 0ae796eb38..7c636d7917 100644 --- a/internal/controller/state/graph/multiple_gateways_test.go +++ b/internal/controller/state/graph/multiple_gateways_test.go @@ -224,6 +224,9 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) { Logging: &ngfAPIv1alpha2.NginxLogging{ ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelDebug), AgentLevel: helpers.GetPointer(ngfAPIv1alpha2.AgentLogLevelDebug), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Format: helpers.GetPointer("$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent"), + }, }, }) @@ -335,6 +338,9 @@ func Test_MultipleGateways_WithNginxProxy(t *testing.T) { Logging: &ngfAPIv1alpha2.NginxLogging{ ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelDebug), AgentLevel: helpers.GetPointer(ngfAPIv1alpha2.AgentLogLevelDebug), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Format: helpers.GetPointer("$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent"), + }, }, DisableHTTP2: helpers.GetPointer(true), },