Skip to content

Commit

Permalink
Make access_log in http context configurable
Browse files Browse the repository at this point in the history 
Closes #5625
  • Loading branch information
@hafe
hafe committed Jul 29, 2024
1 parent 4590c8c commit e2fa62b
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 34 deletions.
1 change: 1 addition & 0 deletions docs/content/configuration/global-configuration/configmap-resource.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ For more information, view the [VirtualServer and VirtualServerRoute resources](
|ConfigMap Key | Description | Default | Example |
| ---| ---| ---| --- |
|*error-log-level* | Sets the global [error log level](https://nginx.org/en/docs/ngx_core_module.html#error_log) for NGINX. | *notice* | |
|*access-log* | Sets the directive [access log](https://nginx.org/en/docs/http/ngx_http_log_module.html#access_log). Only syslog is allowed. | ``/dev/stdout main`` | ``syslog:server=localhost:514`` |
|*access-log-off* | Disables the [access log](https://nginx.org/en/docs/http/ngx_http_log_module.html#access_log). | *False* | |
|*default-server-access-log-off* | Disables the [access log](https://nginx.org/en/docs/http/ngx_http_log_module.html#access_log) for the default server. If access log is disabled globally (*access-log-off: "True"*), then the default server access log is always disabled. | *False* | |
|*log-format* | Sets the custom [log format](https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format) for HTTP and HTTPS traffic. For convenience, it is possible to define the log format across multiple lines (each line separated by *\n*). In that case, the Ingress Controller will replace every *\n* character with a space character. All *'* characters must be escaped. | See the [template file](https://github.com/nginxinc/kubernetes-ingress/blob/v{{< nic-version >}}/internal/configs/version1/nginx.tmpl) for the access log. | [Custom Log Format](https://github.com/nginxinc/kubernetes-ingress/tree/v{{< nic-version >}}/examples/shared-examples/custom-log-format). |
Expand Down
3 changes: 2 additions & 1 deletion internal/configs/config_params.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type ConfigParams struct {
Keepalive int
LBMethod string
LocationSnippets []string
MainAccessLogOff bool
MainAccessLog string
MainErrorLogLevel string
MainHTTPSnippets []string
MainKeepaliveRequests int64
Expand Down Expand Up @@ -188,6 +188,7 @@ func NewDefaultConfigParams(isPlus bool) *ConfigParams {
ProxySendTimeout: "60s",
ClientMaxBodySize: "1m",
SSLRedirect: true,
MainAccessLog: "/dev/stdout main",
MainServerNamesHashBucketSize: "256",
MainServerNamesHashMaxSize: "1024",
MainMapHashBucketSize: "256",
Expand Down
14 changes: 12 additions & 2 deletions internal/configs/configmaps.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,21 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA
cfgParams.MainErrorLogLevel = errorLogLevel
}

if accessLog, exists := cfgm.Data["access-log"]; exists {
if !strings.HasPrefix(accessLog, "syslog:") {
glog.Errorf("Configmap %s/%s: Invalid value for key access-log: %q", cfgm.GetNamespace(), cfgm.GetName(), accessLog)
} else {
cfgParams.MainAccessLog = accessLog
}
}

if accessLogOff, exists, err := GetMapKeyAsBool(cfgm.Data, "access-log-off", cfgm); exists {
if err != nil {
glog.Error(err)
} else {
cfgParams.MainAccessLogOff = accessLogOff
if accessLogOff {
cfgParams.MainAccessLog = "off"
}
}
}

Expand Down Expand Up @@ -514,7 +524,7 @@ func ParseConfigMap(cfgm *v1.ConfigMap, nginxPlus bool, hasAppProtect bool, hasA
// GenerateNginxMainConfig generates MainConfig.
func GenerateNginxMainConfig(staticCfgParams *StaticConfigParams, config *ConfigParams) *version1.MainConfig {
nginxCfg := &version1.MainConfig{
AccessLogOff: config.MainAccessLogOff,
AccessLog: config.MainAccessLog,
DefaultServerAccessLogOff: config.DefaultServerAccessLogOff,
DefaultServerReturn: config.DefaultServerReturn,
DisableIPV6: staticCfgParams.DisableIPV6,
Expand Down
79 changes: 79 additions & 0 deletions internal/configs/configmaps_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,82 @@ func TestParseConfigMapWithoutTLSPassthroughProxyProtocol(t *testing.T) {
})
}
}

func TestParseConfigMapAccessLog(t *testing.T) {
t.Parallel()
tests := []struct {
accessLog string
accessLogOff string
want string
msg string
}{
{
accessLogOff: "False",
accessLog: "syslog:server=localhost:514",
want: "syslog:server=localhost:514",
msg: "Non default access_log",
},
{
accessLogOff: "False",
accessLog: "/tmp/nginx main",
want: "/dev/stdout main",
msg: "access_log to file is not allowed",
},
{
accessLogOff: "True",
accessLog: "/dev/stdout main",
want: "off",
msg: "Disabled access_log",
},
}
nginxPlus := true
hasAppProtect := false
hasAppProtectDos := false
hasTLSPassthrough := false
for _, test := range tests {
t.Run(test.msg, func(t *testing.T) {
cm := &v1.ConfigMap{
Data: map[string]string{
"access-log": test.accessLog,
"access-log-off": test.accessLogOff,
},
}
result := ParseConfigMap(cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
if result.MainAccessLog != test.want {
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
}
})
}
}

func TestParseConfigMapAccessLogDefault(t *testing.T) {
t.Parallel()
tests := []struct {
accessLog string
accessLogOff string
want string
msg string
}{
{
want: "/dev/stdout main",
msg: "Default access_log",
},
}
nginxPlus := true
hasAppProtect := false
hasAppProtectDos := false
hasTLSPassthrough := false
for _, test := range tests {
t.Run(test.msg, func(t *testing.T) {
cm := &v1.ConfigMap{
Data: map[string]string{
"access-log-off": "False",
},
}
result := ParseConfigMap(cm, nginxPlus, hasAppProtect, hasAppProtectDos, hasTLSPassthrough)
if result.MainAccessLog != test.want {
t.Errorf("want %q, got %q", test.want, result.MainAccessLog)
}
})
}
}
53 changes: 32 additions & 21 deletions internal/configs/version1/__snapshots__/template_test.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;

sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -164,7 +164,8 @@ http {
' "$http_referer" "$http_user_agent"'
;
app_protect_dos_arb_fqdn arb.test.server.com;
access_log /dev/stdout main;
access_log ;
app_protect_failure_mode_action pass;
app_protect_compressed_requests_action pass;
app_protect_cookie_seed ABCDEFGHIJKLMNOP;
Expand Down Expand Up @@ -306,7 +307,8 @@ http {
'' $sent_http_grpc_status;
}
app_protect_enforcer_address enforcer.svc.local;
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -1711,7 +1713,8 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -1848,7 +1851,8 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -1985,7 +1989,8 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -2122,7 +2127,8 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -2259,7 +2265,8 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -2422,7 +2429,8 @@ http {
' "$http_referer" "$http_user_agent"'
;
app_protect_dos_arb_fqdn arb.test.server.com;
access_log /dev/stdout main;
access_log ;
app_protect_failure_mode_action pass;
app_protect_compressed_requests_action pass;
app_protect_cookie_seed ABCDEFGHIJKLMNOP;
Expand Down Expand Up @@ -2569,7 +2577,8 @@ http {
'outcome=$app_protect_dos_outcome, reason=$app_protect_dos_outcome_reason, '
'ip_tls=$remote_addr:$app_protect_dos_tls_fp, ';
app_protect_dos_arb_fqdn arb.test.server.com;
access_log /dev/stdout main;
access_log ;
app_protect_failure_mode_action pass;
app_protect_compressed_requests_action pass;
app_protect_cookie_seed ABCDEFGHIJKLMNOP;
Expand Down Expand Up @@ -2722,7 +2731,8 @@ http {
' "$http_referer" "$http_user_agent"'
;
app_protect_dos_arb_fqdn arb.test.server.com;
access_log /dev/stdout main;
access_log ;
app_protect_failure_mode_action pass;
app_protect_compressed_requests_action pass;
app_protect_cookie_seed ABCDEFGHIJKLMNOP;
Expand Down Expand Up @@ -2863,7 +2873,8 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3017,7 +3028,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3138,7 +3149,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3259,7 +3270,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3380,7 +3391,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3501,7 +3512,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3641,7 +3652,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3762,7 +3773,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -3884,7 +3895,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down Expand Up @@ -4005,7 +4016,7 @@ http {
default $upstream_trailer_grpc_status;
'' $sent_http_grpc_status;
}
access_log /dev/stdout main;
access_log ;
sendfile on;
#tcp_nopush on;
Expand Down
1 change: 1 addition & 0 deletions internal/configs/version1/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ type Location struct {

// MainConfig describe the main NGINX configuration file.
type MainConfig struct {
AccessLog string
AccessLogOff bool
DefaultServerAccessLogOff bool
DefaultServerReturn string
Expand Down
6 changes: 1 addition & 5 deletions internal/configs/version1/nginx-plus.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,7 @@ http {
app_protect_enforcer_address {{ .AppProtectV5EnforcerAddr }};
{{- end}}

{{- if .AccessLogOff}}
access_log off;
{{- else}}
access_log /dev/stdout main;
{{- end}}
access_log {{.AccessLog}};

{{- if .LatencyMetrics}}
log_format response_time '{"upstreamAddress":"$upstream_addr", "upstreamResponseTime":"$upstream_response_time", "proxyHost":"$proxy_host", "upstreamStatus": "$upstream_status"}';
Expand Down
6 changes: 1 addition & 5 deletions internal/configs/version1/nginx.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,7 @@ http {
default "{{ .StaticSSLPath }}";
}
{{- end }}
{{- if .AccessLogOff}}
access_log off;
{{- else}}
access_log /dev/stdout main;
{{- end}}
access_log {{.AccessLog}};

{{- if .LatencyMetrics}}
log_format response_time '{"upstreamAddress":"$upstream_addr", "upstreamResponseTime":"$upstream_response_time", "proxyHost":"$proxy_host", "upstreamStatus": "$upstream_status"}';
Expand Down

0 comments on commit e2fa62b

Please sign in to comment.