-
Notifications
You must be signed in to change notification settings - Fork 141
Adding nginx_proxy access_log format ability #4102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
97c021d
63b15ab
0499b02
2ac80c9
3070740
9d28194
f768d25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Comment on lines
+16
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think these fields belong in this type. I'd recommend moving these to the AccessLog: And populate the constants in buildAccessLog(). |
||
| 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) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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 }}; | ||||||||||
|
Comment on lines
+59
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As per the comment above, this would become
Suggested change
|
||||||||||
| {{- end }} | ||||||||||
| {{- end }} | ||||||||||
| {{- end }} | ||||||||||
|
|
||||||||||
| {{ range $i := .Includes -}} | ||||||||||
| include {{ $i.Name }}; | ||||||||||
| {{ end -}} | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -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", | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
|
||||||
| 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{ | ||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.