diff --git a/apis/v1alpha2/nginxproxy_types.go b/apis/v1alpha2/nginxproxy_types.go index c5d9dfcef1..6f9e63066f 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 { + // Disabled turns off access logging when set to true. + // + // +optional + Disabled *bool `json:"disabled,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..46037a8c11 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.Disabled != nil { + in, out := &in.Disabled, &out.Disabled + *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/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 89de827bd4..07b7911901 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: + disabled: + description: Disabled 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 a96a495103..046ff44bf3 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: + disabled: + description: Disabled 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..536843cb39 100644 --- a/internal/controller/nginx/config/base_http_config.go +++ b/internal/controller/nginx/config/base_http_config.go @@ -12,6 +12,9 @@ var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTP type httpConfig struct { DNSResolver *dataplane.DNSResolverConfig + AccessLog *dataplane.AccessLog + DefaultAccessLogPath string + DefaultLogFormatName string Includes []shared.Include NginxReadinessProbePort int32 IPFamily shared.IPFamily @@ -27,6 +30,9 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult { NginxReadinessProbePort: conf.BaseHTTPConfig.NginxReadinessProbePort, IPFamily: getIPFamily(conf.BaseHTTPConfig), DNSResolver: conf.BaseHTTPConfig.DNSResolver, + AccessLog: conf.Logging.AccessLog, + DefaultAccessLogPath: dataplane.DefaultAccessLogPath, + DefaultLogFormatName: dataplane.DefaultLogFormatName, } 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 e50a83a65f..9bea301a89 100644 --- a/internal/controller/nginx/config/base_http_config_template.go +++ b/internal/controller/nginx/config/base_http_config_template.go @@ -48,6 +48,20 @@ server { } } +{{- /* Define custom log format */ -}} +{{- /* Access log directives for AccessLog. If path is "off" we disable logging. */ -}} +{{- /* We use a fixed name for user-defined log format to avoid complexity of passing the name around. */ -}} +{{- if .AccessLog }} + {{- if .AccessLog.Disabled }} +access_log off; + {{- else }} + {{- if .AccessLog.Format }} +log_format {{ .DefaultLogFormatName }} '{{ .AccessLog.Format }}'; +access_log {{ .DefaultAccessLogPath }} {{ .DefaultLogFormatName }}; + {{- 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..76dee76c5e 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 path and custom format name", + 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{Disabled: 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{Disabled: 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/provisioner/objects.go b/internal/controller/provisioner/objects.go index 8ad2e1b72b..4f40eab705 100644 --- a/internal/controller/provisioner/objects.go +++ b/internal/controller/provisioner/objects.go @@ -406,11 +406,18 @@ func (p *NginxProvisioner) buildNginxConfigMaps( workerConnections = *nProxyCfg.WorkerConnections } + // Add LogFormats and AccessLogs to mainFields + accessLog := addAccessLogsToNginxConfig(logging) + mainFields := map[string]interface{}{ "ErrorLevel": logLevel, "WorkerConnections": workerConnections, } + if accessLog != nil { + mainFields["AccessLog"] = accessLog + } + // Create events ConfigMap data using template eventsFields := map[string]interface{}{ "WorkerConnections": workerConnections, @@ -1436,3 +1443,19 @@ func DetermineNginxImageName( return fmt.Sprintf("%s:%s", image, tag), pullPolicy } + +func addAccessLogsToNginxConfig(logging *ngfAPIv1alpha2.NginxLogging) *ngfAPIv1alpha2.NginxAccessLog { + accessLog := &ngfAPIv1alpha2.NginxAccessLog{} + if logging == nil { + return accessLog + } + + if logging.AccessLog != nil { + accessLog = &ngfAPIv1alpha2.NginxAccessLog{ + Disabled: logging.AccessLog.Disabled, + Format: logging.AccessLog.Format, + } + } + + return accessLog +} diff --git a/internal/controller/state/dataplane/configuration.go b/internal/controller/state/dataplane/configuration.go index 6615fd7036..5805919fd3 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 slice-based 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.Disabled != nil && *srcLogSettings.AccessLog.Disabled { + return &AccessLog{Disabled: 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..4f79958156 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 Disabled = false", + gw: &graph.Gateway{ + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Disabled: helpers.GetPointer(false), + Format: helpers.GetPointer(logFormat), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + + AccessLog: &AccessLog{ + Disabled: 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{ + Disabled: 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{ + Disabled: helpers.GetPointer(true), + Format: helpers.GetPointer(logFormat), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + AccessLog: &AccessLog{ + Disabled: true, + }, + }, + }, + { + msg: "AccessLog OFF", + gw: &graph.Gateway{ + EffectiveNginxProxy: &graph.EffectiveNginxProxy{ + Logging: &ngfAPIv1alpha2.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo), + AccessLog: &ngfAPIv1alpha2.NginxAccessLog{ + Disabled: helpers.GetPointer(true), + }, + }, + }, + }, + expLoggingSettings: Logging{ + ErrorLevel: "info", + AccessLog: &AccessLog{ + Disabled: true, + }, + }, + }, } for _, tc := range tests { diff --git a/internal/controller/state/dataplane/types.go b/internal/controller/state/dataplane/types.go index 987c021c7f..c974a606b0 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 + // Disabled specifies whether the access log is disabled. + Disabled 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), },