Skip to content

Commit 7f15e41

Browse files
tatarutytataruty
authored
Adding nginx_proxy access_log format ability (#4102)
Problem: As a user of NGF I want to have the ability to configure the log format of NGINX's access and error logs So that I can easily collect logs from NGINX in my logging platform. Solution: Adding accessLog.Disabled and accessLog.Format fields to nginx-proxy CRD, also adding default values for them
1 parent e1e2e73 commit 7f15e41

File tree

13 files changed

+368
-2
lines changed

13 files changed

+368
-2
lines changed

‎apis/v1alpha2/nginxproxy_types.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,12 @@ type NginxLogging struct {
297297
// +optional
298298
// +kubebuilder:default=info
299299
AgentLevel *AgentLogLevel `json:"agentLevel,omitempty"`
300+
301+
// AccessLog defines the access log settings, including format itself and disabling option.
302+
// For now only path /dev/stdout can be used.
303+
//
304+
// +optional
305+
AccessLog *NginxAccessLog `json:"accessLog,omitempty"`
300306
}
301307

302308
// NginxErrorLogLevel type defines the log level of error logs for NGINX.
@@ -352,6 +358,22 @@ const (
352358
AgentLogLevelFatal AgentLogLevel = "fatal"
353359
)
354360

361+
// NginxAccessLog defines the configuration for an NGINX access log.
362+
type NginxAccessLog struct {
363+
// Disable turns off access logging when set to true.
364+
//
365+
// +optional
366+
Disable *bool `json:"disable,omitempty"`
367+
368+
// Format specifies the custom log format string.
369+
// If not specified, NGINX default 'combined' format is used.
370+
// For now only path /dev/stdout can be used.
371+
// See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format
372+
//
373+
// +optional
374+
Format *string `json:"format,omitempty"`
375+
}
376+
355377
// NginxPlus specifies NGINX Plus additional settings. These will only be applied if NGINX Plus is being used.
356378
type NginxPlus struct {
357379
// AllowedAddresses specifies IPAddresses or CIDR blocks to the allow list for accessing the NGINX Plus API.

‎apis/v1alpha2/zz_generated.deepcopy.go‎

Lines changed: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎charts/nginx-gateway-fabric/values.schema.json‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,23 @@
202202
"logging": {
203203
"description": "Logging defines logging related settings for NGINX.",
204204
"properties": {
205+
"accessLog": {
206+
"description": "AccessLog defines the access log settings.",
207+
"properties": {
208+
"disable": {
209+
"description": "Disable turns off access logging when set to true.",
210+
"required": [],
211+
"type": "boolean"
212+
},
213+
"format": {
214+
"description": "Format specifies the custom log format string. If not specified, NGINX default 'combined' format is used.",
215+
"required": [],
216+
"type": "string"
217+
}
218+
},
219+
"required": [],
220+
"type": "object"
221+
},
205222
"agentLevel": {
206223
"enum": [
207224
"debug",

‎charts/nginx-gateway-fabric/values.yaml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,16 @@ nginx:
471471
# - error
472472
# - panic
473473
# - fatal
474+
# accessLog:
475+
# type: object
476+
# description: AccessLog defines the access log settings.
477+
# properties:
478+
# disable:
479+
# type: boolean
480+
# description: Disable turns off access logging when set to true.
481+
# format:
482+
# type: string
483+
# description: Format specifies the custom log format string. If not specified, NGINX default 'combined' format is used.
474484
# nginxPlus:
475485
# type: object
476486
# description: NginxPlus specifies NGINX Plus additional settings.

‎config/crd/bases/gateway.nginx.org_nginxproxies.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8016,6 +8016,23 @@ spec:
80168016
logging:
80178017
description: Logging defines logging related settings for NGINX.
80188018
properties:
8019+
accessLog:
8020+
description: |-
8021+
AccessLog defines the access log settings, including format itself and disabling option.
8022+
For now only path /dev/stdout can be used.
8023+
properties:
8024+
disable:
8025+
description: Disable turns off access logging when set to
8026+
true.
8027+
type: boolean
8028+
format:
8029+
description: |-
8030+
Format specifies the custom log format string.
8031+
If not specified, NGINX default 'combined' format is used.
8032+
For now only path /dev/stdout can be used.
8033+
See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format
8034+
type: string
8035+
type: object
80198036
agentLevel:
80208037
default: info
80218038
description: |-

‎deploy/crds.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8603,6 +8603,23 @@ spec:
86038603
logging:
86048604
description: Logging defines logging related settings for NGINX.
86058605
properties:
8606+
accessLog:
8607+
description: |-
8608+
AccessLog defines the access log settings, including format itself and disabling option.
8609+
For now only path /dev/stdout can be used.
8610+
properties:
8611+
disable:
8612+
description: Disable turns off access logging when set to
8613+
true.
8614+
type: boolean
8615+
format:
8616+
description: |-
8617+
Format specifies the custom log format string.
8618+
If not specified, NGINX default 'combined' format is used.
8619+
For now only path /dev/stdout can be used.
8620+
See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format
8621+
type: string
8622+
type: object
86068623
agentLevel:
86078624
default: info
86088625
description: |-

‎internal/controller/nginx/config/base_http_config.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,15 @@ import (
1010

1111
var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTPTemplateText))
1212

13+
type AccessLog struct {
14+
Format string // User's format string
15+
Path string // Where to write logs (/dev/stdout)
16+
FormatName string // Internal format name (ngf_user_defined_log_format)
17+
Disable bool // User's disable flag
18+
}
1319
type httpConfig struct {
1420
DNSResolver *dataplane.DNSResolverConfig
21+
AccessLog *AccessLog
1522
Includes []shared.Include
1623
NginxReadinessProbePort int32
1724
IPFamily shared.IPFamily
@@ -27,6 +34,7 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult {
2734
NginxReadinessProbePort: conf.BaseHTTPConfig.NginxReadinessProbePort,
2835
IPFamily: getIPFamily(conf.BaseHTTPConfig),
2936
DNSResolver: conf.BaseHTTPConfig.DNSResolver,
37+
AccessLog: buildAccessLog(conf.Logging.AccessLog),
3038
}
3139

3240
results := make([]executeResult, 0, len(includes)+1)
@@ -38,3 +46,19 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult {
3846

3947
return results
4048
}
49+
50+
func buildAccessLog(accessLogConfig *dataplane.AccessLog) *AccessLog {
51+
if accessLogConfig != nil {
52+
accessLog := &AccessLog{
53+
Path: dataplane.DefaultAccessLogPath,
54+
FormatName: dataplane.DefaultLogFormatName,
55+
}
56+
if accessLogConfig.Format != "" {
57+
accessLog.Format = accessLogConfig.Format
58+
}
59+
accessLog.Disable = accessLogConfig.Disable
60+
61+
return accessLog
62+
}
63+
return nil
64+
}

‎internal/controller/nginx/config/base_http_config_template.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,19 @@ server {
4848
}
4949
}
5050
51+
{{- /* Define custom log format */ -}}
52+
{{- /* We use a fixed name for user-defined log format to avoid complexity of passing the name around. */ -}}
53+
{{- if .AccessLog }}
54+
{{- if .AccessLog.Disable }}
55+
access_log off;
56+
{{- else }}
57+
{{- if .AccessLog.Format }}
58+
log_format {{ .AccessLog.FormatName }} '{{ .AccessLog.Format }}';
59+
access_log {{ .AccessLog.Path }} {{ .AccessLog.FormatName }};
60+
{{- end }}
61+
{{- end }}
62+
{{- end }}
63+
5164
{{ range $i := .Includes -}}
5265
include {{ $i.Name }};
5366
{{ end -}}

‎internal/controller/nginx/config/base_http_config_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package config
22

33
import (
4+
"fmt"
45
"sort"
56
"strings"
67
"testing"
@@ -10,6 +11,77 @@ import (
1011
"github.com/nginx/nginx-gateway-fabric/v2/internal/controller/state/dataplane"
1112
)
1213

14+
func TestLoggingSettingsTemplate(t *testing.T) {
15+
t.Parallel()
16+
17+
logFormat := "$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent"
18+
19+
tests := []struct {
20+
name string
21+
accessLog *dataplane.AccessLog
22+
expectedOutputs []string
23+
unexpectedOutputs []string
24+
}{
25+
{
26+
name: "Log format and access log with custom format",
27+
accessLog: &dataplane.AccessLog{Format: logFormat},
28+
expectedOutputs: []string{
29+
fmt.Sprintf("log_format %s '%s'", dataplane.DefaultLogFormatName, logFormat),
30+
fmt.Sprintf("access_log %s %s", dataplane.DefaultAccessLogPath, dataplane.DefaultLogFormatName),
31+
},
32+
},
33+
{
34+
name: "Empty format",
35+
accessLog: &dataplane.AccessLog{Format: ""},
36+
unexpectedOutputs: []string{
37+
fmt.Sprintf("log_format %s '%s'", dataplane.DefaultLogFormatName, logFormat),
38+
fmt.Sprintf("access_log %s %s", dataplane.DefaultAccessLogPath, dataplane.DefaultLogFormatName),
39+
},
40+
},
41+
{
42+
name: "Access log off while format presented",
43+
accessLog: &dataplane.AccessLog{Disable: true, Format: logFormat},
44+
expectedOutputs: []string{
45+
`access_log off;`,
46+
},
47+
unexpectedOutputs: []string{
48+
fmt.Sprintf("access_log off %s", dataplane.DefaultLogFormatName),
49+
},
50+
},
51+
{
52+
name: "Access log off",
53+
accessLog: &dataplane.AccessLog{Disable: true},
54+
expectedOutputs: []string{
55+
`access_log off;`,
56+
},
57+
unexpectedOutputs: []string{
58+
`log_format`,
59+
},
60+
},
61+
}
62+
63+
for _, tt := range tests {
64+
t.Run(tt.name, func(t *testing.T) {
65+
t.Parallel()
66+
g := NewWithT(t)
67+
68+
conf := dataplane.Configuration{
69+
Logging: dataplane.Logging{AccessLog: tt.accessLog},
70+
}
71+
72+
res := executeBaseHTTPConfig(conf)
73+
g.Expect(res).To(HaveLen(1))
74+
httpConfig := string(res[0].data)
75+
for _, expectedOutput := range tt.expectedOutputs {
76+
g.Expect(httpConfig).To(ContainSubstring(expectedOutput))
77+
}
78+
for _, unexpectedOutput := range tt.unexpectedOutputs {
79+
g.Expect(httpConfig).ToNot(ContainSubstring(unexpectedOutput))
80+
}
81+
})
82+
}
83+
}
84+
1385
func TestExecuteBaseHttp_HTTP2(t *testing.T) {
1486
t.Parallel()
1587
confOn := dataplane.Configuration{

‎internal/controller/state/dataplane/configuration.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ const (
2828
defaultErrorLogLevel = "info"
2929
DefaultWorkerConnections = int32(1024)
3030
DefaultNginxReadinessProbePort = int32(8081)
31+
// DefaultLogFormatName is used when user provides custom access_log format.
32+
DefaultLogFormatName = "ngf_user_defined_log_format"
33+
// DefaultAccessLogPath is the default path for the access log.
34+
DefaultAccessLogPath = "/dev/stdout"
3135
)
3236

3337
// BuildConfiguration builds the Configuration from the Graph.
@@ -1206,6 +1210,8 @@ func convertAddresses(addresses []ngfAPIv1alpha2.RewriteClientIPAddress) []strin
12061210
return trustedAddresses
12071211
}
12081212

1213+
// buildLogging converts the API logging spec (currently singular LogFormat / AccessLog fields
1214+
// in v1alpha2) into internal representation used by templates.
12091215
func buildLogging(gateway *graph.Gateway) Logging {
12101216
logSettings := Logging{ErrorLevel: defaultErrorLogLevel}
12111217

@@ -1218,11 +1224,33 @@ func buildLogging(gateway *graph.Gateway) Logging {
12181224
if ngfProxy.Logging.ErrorLevel != nil {
12191225
logSettings.ErrorLevel = string(*ngfProxy.Logging.ErrorLevel)
12201226
}
1227+
1228+
srcLogSettings := ngfProxy.Logging
1229+
1230+
if accessLog := buildAccessLog(srcLogSettings); accessLog != nil {
1231+
logSettings.AccessLog = accessLog
1232+
}
12211233
}
12221234

12231235
return logSettings
12241236
}
12251237

1238+
func buildAccessLog(srcLogSettings *ngfAPIv1alpha2.NginxLogging) *AccessLog {
1239+
if srcLogSettings.AccessLog != nil {
1240+
if srcLogSettings.AccessLog.Disable != nil && *srcLogSettings.AccessLog.Disable {
1241+
return &AccessLog{Disable: true}
1242+
}
1243+
1244+
if srcLogSettings.AccessLog.Format != nil && *srcLogSettings.AccessLog.Format != "" {
1245+
return &AccessLog{
1246+
Format: *srcLogSettings.AccessLog.Format,
1247+
}
1248+
}
1249+
}
1250+
1251+
return nil
1252+
}
1253+
12261254
func buildWorkerConnections(gateway *graph.Gateway) int32 {
12271255
if gateway == nil || gateway.EffectiveNginxProxy == nil {
12281256
return DefaultWorkerConnections

0 commit comments

Comments
 (0)