Skip to content

Commit f768d25

Browse files
tatarutytataruty
committed
remove logFormat from dataplane as well
1 parent 9d28194 commit f768d25

File tree

7 files changed

+74
-116
lines changed

7 files changed

+74
-116
lines changed

‎apis/v1alpha2/nginxproxy_types.go‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -374,13 +374,6 @@ type NginxAccessLog struct {
374374
Format *string `json:"format,omitempty"`
375375
}
376376

377-
const (
378-
// DefaultLogFormatName is used when user provides custom access_log format.
379-
DefaultLogFormatName = "ngf_user_defined_log_format"
380-
// DefaultAccessLogPath is the default path for the access log.
381-
DefaultAccessLogPath = "/dev/stdout"
382-
)
383-
384377
// NginxPlus specifies NGINX Plus additional settings. These will only be applied if NGINX Plus is being used.
385378
type NginxPlus struct {
386379
// AllowedAddresses specifies IPAddresses or CIDR blocks to the allow list for accessing the NGINX Plus API.

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,9 @@ var baseHTTPTemplate = gotemplate.Must(gotemplate.New("baseHttp").Parse(baseHTTP
1212

1313
type httpConfig struct {
1414
DNSResolver *dataplane.DNSResolverConfig
15-
LogFormat *dataplane.LogFormat
1615
AccessLog *dataplane.AccessLog
16+
DefaultAccessLogPath string
17+
DefaultLogFormatName string
1718
Includes []shared.Include
1819
NginxReadinessProbePort int32
1920
IPFamily shared.IPFamily
@@ -29,8 +30,9 @@ func executeBaseHTTPConfig(conf dataplane.Configuration) []executeResult {
2930
NginxReadinessProbePort: conf.BaseHTTPConfig.NginxReadinessProbePort,
3031
IPFamily: getIPFamily(conf.BaseHTTPConfig),
3132
DNSResolver: conf.BaseHTTPConfig.DNSResolver,
32-
LogFormat: conf.Logging.LogFormat,
3333
AccessLog: conf.Logging.AccessLog,
34+
DefaultAccessLogPath: dataplane.DefaultAccessLogPath,
35+
DefaultLogFormatName: dataplane.DefaultLogFormatName,
3436
}
3537

3638
results := make([]executeResult, 0, len(includes)+1)

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,15 @@ server {
4949
}
5050
5151
{{- /* Define custom log format */ -}}
52-
{{- if .LogFormat }}
52+
{{- /* Access log directives for AccessLog. If path is "off" we disable logging. */ -}}
5353
{{- /* We use a fixed name for user-defined log format to avoid complexity of passing the name around. */ -}}
54-
{{- if .LogFormat.Format }}
55-
log_format ngf_user_defined_log_format '{{ .LogFormat.Format }}';
56-
{{- end }}
57-
{{- end }}
58-
59-
{{- /* Access log directives for AccessLog. If path is "off" we disable logging. If Format set, use /dev/stdout with that format. Otherwise use the given path. */ -}}
6054
{{- if .AccessLog }}
61-
{{- if eq .AccessLog.Path "off" }}
55+
{{- if .AccessLog.Disabled }}
6256
access_log off;
6357
{{- else }}
6458
{{- if .AccessLog.Format }}
65-
access_log /dev/stdout ngf_user_defined_log_format;
59+
log_format {{ .DefaultLogFormatName }} '{{ .AccessLog.Format }}';
60+
access_log {{ .DefaultAccessLogPath }} {{ .DefaultLogFormatName }};
6661
{{- end }}
6762
{{- end }}
6863
{{- end }}

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

Lines changed: 19 additions & 27 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"
@@ -13,58 +14,49 @@ import (
1314
func TestLoggingSettingsTemplate(t *testing.T) {
1415
t.Parallel()
1516

17+
logFormat := "$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent"
18+
1619
tests := []struct {
1720
name string
1821
accessLog *dataplane.AccessLog
19-
logFormat *dataplane.LogFormat
2022
expectedOutputs []string
2123
unexpectedOutputs []string
2224
}{
2325
{
24-
name: "Log format and access log with custom path and custom format name",
25-
logFormat: &dataplane.LogFormat{
26-
Name: "custom_format",
27-
Format: "$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent",
28-
},
29-
accessLog: &dataplane.AccessLog{Path: "/path/to/log.gz", Format: "custom_format"},
26+
name: "Log format and access log with custom path and custom format name",
27+
accessLog: &dataplane.AccessLog{Format: logFormat},
3028
expectedOutputs: []string{
31-
`log_format ngf_user_defined_log_format '$remote_addr - [$time_local] "$request" $status $body_bytes_sent';`,
32-
`access_log /dev/stdout ngf_user_defined_log_format;`,
29+
fmt.Sprintf("log_format %s '%s'", dataplane.DefaultLogFormatName, logFormat),
30+
fmt.Sprintf("access_log %s %s", dataplane.DefaultAccessLogPath, dataplane.DefaultLogFormatName),
3331
},
3432
},
3533
{
36-
name: "Empty log format name and format",
37-
logFormat: &dataplane.LogFormat{
38-
Name: "",
39-
Format: "",
40-
},
41-
accessLog: &dataplane.AccessLog{Path: "", Format: ""},
34+
name: "Empty format",
35+
accessLog: &dataplane.AccessLog{Format: ""},
4236
unexpectedOutputs: []string{
43-
`log_format ngf_user_defined_log_format`,
44-
`access_log /dev/stdout`,
37+
fmt.Sprintf("log_format %s '%s'", dataplane.DefaultLogFormatName, logFormat),
38+
fmt.Sprintf("access_log %s %s", dataplane.DefaultAccessLogPath, dataplane.DefaultLogFormatName),
4539
},
4640
},
4741
{
48-
name: "Access log off while format presented",
49-
logFormat: &dataplane.LogFormat{
50-
Name: "ngf_user_defined_log_format",
51-
Format: "$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent",
52-
},
53-
accessLog: &dataplane.AccessLog{Path: "off", Format: "ngf_user_defined_log_format"},
42+
name: "Access log off while format presented",
43+
accessLog: &dataplane.AccessLog{Disabled: true, Format: logFormat},
5444
expectedOutputs: []string{
5545
`access_log off;`,
56-
`log_format ngf_user_defined_log_format`,
5746
},
5847
unexpectedOutputs: []string{
59-
`access_log off ngf_user_defined_log_format`,
48+
fmt.Sprintf("access_log off %s", dataplane.DefaultLogFormatName),
6049
},
6150
},
6251
{
6352
name: "Access log off",
64-
accessLog: &dataplane.AccessLog{Path: "off"},
53+
accessLog: &dataplane.AccessLog{Disabled: true},
6554
expectedOutputs: []string{
6655
`access_log off;`,
6756
},
57+
unexpectedOutputs: []string{
58+
`log_format`,
59+
},
6860
},
6961
}
7062

@@ -74,7 +66,7 @@ func TestLoggingSettingsTemplate(t *testing.T) {
7466
g := NewWithT(t)
7567

7668
conf := dataplane.Configuration{
77-
Logging: dataplane.Logging{AccessLog: tt.accessLog, LogFormat: tt.logFormat},
69+
Logging: dataplane.Logging{AccessLog: tt.accessLog},
7870
}
7971

8072
res := executeBaseHTTPConfig(conf)

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

Lines changed: 11 additions & 27 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.
@@ -1222,49 +1226,29 @@ func buildLogging(gateway *graph.Gateway) Logging {
12221226
}
12231227

12241228
srcLogSettings := ngfProxy.Logging
1225-
logFormat, accessLog := buildAccessLog(srcLogSettings)
12261229

1227-
if logFormat != nil {
1228-
logSettings.LogFormat = logFormat
1229-
}
1230-
1231-
if accessLog != nil {
1230+
if accessLog := buildAccessLog(srcLogSettings); accessLog != nil {
12321231
logSettings.AccessLog = accessLog
12331232
}
12341233
}
12351234

12361235
return logSettings
12371236
}
12381237

1239-
func buildLogFormat(srcLogSettings *ngfAPIv1alpha2.NginxLogging) *LogFormat {
1240-
if srcLogSettings.AccessLog != nil &&
1241-
srcLogSettings.AccessLog.Format != nil &&
1242-
*srcLogSettings.AccessLog.Format != "" {
1243-
return &LogFormat{
1244-
Name: ngfAPIv1alpha2.DefaultLogFormatName,
1245-
Format: *srcLogSettings.AccessLog.Format,
1246-
}
1247-
}
1248-
1249-
return nil
1250-
}
1251-
1252-
func buildAccessLog(srcLogSettings *ngfAPIv1alpha2.NginxLogging) (*LogFormat, *AccessLog) {
1253-
logFormat := buildLogFormat(srcLogSettings)
1238+
func buildAccessLog(srcLogSettings *ngfAPIv1alpha2.NginxLogging) *AccessLog {
12541239
if srcLogSettings.AccessLog != nil {
12551240
if srcLogSettings.AccessLog.Disabled != nil && *srcLogSettings.AccessLog.Disabled {
1256-
return nil, &AccessLog{Path: "off"}
1241+
return &AccessLog{Disabled: true}
12571242
}
12581243

1259-
if logFormat != nil {
1260-
return logFormat, &AccessLog{
1261-
Path: ngfAPIv1alpha2.DefaultAccessLogPath,
1262-
Format: ngfAPIv1alpha2.DefaultLogFormatName,
1244+
if srcLogSettings.AccessLog.Format != nil && *srcLogSettings.AccessLog.Format != "" {
1245+
return &AccessLog{
1246+
Format: *srcLogSettings.AccessLog.Format,
12631247
}
12641248
}
12651249
}
12661250

1267-
return nil, nil
1251+
return nil
12681252
}
12691253

12701254
func buildWorkerConnections(gateway *graph.Gateway) int32 {

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

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4768,6 +4768,9 @@ func TestBuildRewriteIPSettings(t *testing.T) {
47684768

47694769
func TestBuildLogging(t *testing.T) {
47704770
defaultLogging := Logging{ErrorLevel: defaultErrorLogLevel}
4771+
logFormat := `'$remote_addr - $remote_user [$time_local] '
4772+
'"$request" $status $body_bytes_sent '
4773+
'"$http_referer" "$http_user_agent" '`
47714774

47724775
t.Parallel()
47734776
tests := []struct {
@@ -4885,30 +4888,21 @@ func TestBuildLogging(t *testing.T) {
48854888
expLoggingSettings: Logging{ErrorLevel: "emerg"},
48864889
},
48874890
{
4888-
msg: "LogFormat and AccessLog configured",
4891+
msg: "AccessLog configured",
48894892
gw: &graph.Gateway{
48904893
EffectiveNginxProxy: &graph.EffectiveNginxProxy{
48914894
Logging: &ngfAPIv1alpha2.NginxLogging{
48924895
ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo),
48934896
AccessLog: &ngfAPIv1alpha2.NginxAccessLog{
4894-
Format: helpers.GetPointer(`'$remote_addr - $remote_user [$time_local] '
4895-
'"$request" $status $body_bytes_sent '
4896-
'"$http_referer" "$http_user_agent" '`),
4897+
Format: helpers.GetPointer(logFormat),
48974898
},
48984899
},
48994900
},
49004901
},
49014902
expLoggingSettings: Logging{
49024903
ErrorLevel: "info",
4903-
LogFormat: &LogFormat{
4904-
Name: ngfAPIv1alpha2.DefaultLogFormatName,
4905-
Format: `'$remote_addr - $remote_user [$time_local] '
4906-
'"$request" $status $body_bytes_sent '
4907-
'"$http_referer" "$http_user_agent" '`,
4908-
},
49094904
AccessLog: &AccessLog{
4910-
Path: ngfAPIv1alpha2.DefaultAccessLogPath,
4911-
Format: ngfAPIv1alpha2.DefaultLogFormatName,
4905+
Format: logFormat,
49124906
},
49134907
},
49144908
},
@@ -4920,24 +4914,17 @@ func TestBuildLogging(t *testing.T) {
49204914
ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo),
49214915
AccessLog: &ngfAPIv1alpha2.NginxAccessLog{
49224916
Disabled: helpers.GetPointer(false),
4923-
Format: helpers.GetPointer(`'$remote_addr - $remote_user [$time_local] '
4924-
'"$request" $status $body_bytes_sent '
4925-
'"$http_referer" "$http_user_agent" '`),
4917+
Format: helpers.GetPointer(logFormat),
49264918
},
49274919
},
49284920
},
49294921
},
49304922
expLoggingSettings: Logging{
49314923
ErrorLevel: "info",
4932-
LogFormat: &LogFormat{
4933-
Name: ngfAPIv1alpha2.DefaultLogFormatName,
4934-
Format: `'$remote_addr - $remote_user [$time_local] '
4935-
'"$request" $status $body_bytes_sent '
4936-
'"$http_referer" "$http_user_agent" '`,
4937-
},
4924+
49384925
AccessLog: &AccessLog{
4939-
Path: ngfAPIv1alpha2.DefaultAccessLogPath,
4940-
Format: ngfAPIv1alpha2.DefaultLogFormatName,
4926+
Disabled: false,
4927+
Format: logFormat,
49414928
},
49424929
},
49434930
},
@@ -4955,7 +4942,6 @@ func TestBuildLogging(t *testing.T) {
49554942
},
49564943
expLoggingSettings: Logging{
49574944
ErrorLevel: "info",
4958-
LogFormat: nil,
49594945
AccessLog: nil,
49604946
},
49614947
},
@@ -4967,18 +4953,34 @@ func TestBuildLogging(t *testing.T) {
49674953
ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo),
49684954
AccessLog: &ngfAPIv1alpha2.NginxAccessLog{
49694955
Disabled: helpers.GetPointer(true),
4970-
Format: helpers.GetPointer(`'$remote_addr - $remote_user [$time_local] '
4971-
'"$request" $status $body_bytes_sent '
4972-
'"$http_referer" "$http_user_agent" '`),
4956+
Format: helpers.GetPointer(logFormat),
4957+
},
4958+
},
4959+
},
4960+
},
4961+
expLoggingSettings: Logging{
4962+
ErrorLevel: "info",
4963+
AccessLog: &AccessLog{
4964+
Disabled: true,
4965+
},
4966+
},
4967+
},
4968+
{
4969+
msg: "AccessLog OFF",
4970+
gw: &graph.Gateway{
4971+
EffectiveNginxProxy: &graph.EffectiveNginxProxy{
4972+
Logging: &ngfAPIv1alpha2.NginxLogging{
4973+
ErrorLevel: helpers.GetPointer(ngfAPIv1alpha2.NginxLogLevelInfo),
4974+
AccessLog: &ngfAPIv1alpha2.NginxAccessLog{
4975+
Disabled: helpers.GetPointer(true),
49734976
},
49744977
},
49754978
},
49764979
},
49774980
expLoggingSettings: Logging{
49784981
ErrorLevel: "info",
4979-
LogFormat: nil,
49804982
AccessLog: &AccessLog{
4981-
Path: "off",
4983+
Disabled: true,
49824984
},
49834985
},
49844986
},

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

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -474,10 +474,8 @@ type Ratio struct {
474474

475475
// Logging defines logging related settings for NGINX.
476476
type Logging struct {
477-
// AccessLog defines the configuration for the NGINX access log. For now only path /dev/stdout is used.
477+
// AccessLog defines the configuration for the NGINX access log.
478478
AccessLog *AccessLog
479-
// LogFormat represents a custom log format.
480-
LogFormat *LogFormat
481479
// ErrorLevel defines the error log level.
482480
ErrorLevel string
483481
}
@@ -501,18 +499,10 @@ type DeploymentContext struct {
501499
Integration string `json:"integration"`
502500
}
503501

504-
// LogFormat represents a custom log format.
505-
type LogFormat struct {
506-
// Name is the name of the log format.
507-
Name string
508-
// Format is the format string.
509-
Format string
510-
}
511-
512-
// AccessLog defines the configuration for an NGINX access log. For now only path /dev/stdout is used.
502+
// AccessLog defines the configuration for an NGINX access log.
513503
type AccessLog struct {
514-
// Path is the path of the access log. Currently supported only /dev/stdout, or "off" to disable logging.
515-
Path string
516-
// Format is the log_format name
504+
// Format is the access log format template.
517505
Format string
506+
// Disabled specifies whether the access log is disabled.
507+
Disabled bool
518508
}

0 commit comments

Comments
 (0)