Adding nginx_proxy access_log format ability

[tataruty]

Proposed changes

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

Testing:
Tested with custom format:

kubectl patch nginxproxy nginx-gateway-proxy-config -n nginx-gateway --type merge -p '{"spec":{"logging":{"errorLevel":"debug","accessLog":{"format":"$remote_addr - [$time_local] \"$request\" $status $body_bytes_sent"}}}}'

result:
config uses /dev/stdout and default format name:

work:

Tested with turning logs OFF:

kubectl patch nginxproxy nginx-gateway-proxy-config -n nginx-gateway --type merge -p '{"spec":{"logging":{"errorLevel":"debug","accessLog":{"disabled":true}}}}'

result:
config has only off without custom log_format:

Closes #1200

Checklist

Before creating a PR, run through this checklist and mark each as complete.

• I have read the CONTRIBUTING doc
• I have added tests that prove my fix is effective or that my feature works
• I have checked that all unit tests pass after adding my changes
• I have updated necessary documentation
• I have rebased my branch onto main
• I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Add ability to configure access log format or turn logging OFF

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.14%. Comparing base (a5a0f72) to head (a0ba554).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4102      +/-   ##
==========================================
+ Coverage   86.10%   86.14%   +0.03%     
==========================================
  Files         131      131              
  Lines       14162    14185      +23     
  Branches       35       35              
==========================================
+ Hits        12194    12219      +25     
+ Misses       1765     1764       -1     
+ Partials      203      202       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ciarams87]

The API definition feels a bit confusing - as we are only supporting a single log format and a single path, we should simplify the API.

The two-level structure (NginxLogFormat + NginxAccessLog) creates indirection:

• Users define a format with a name
• Then reference that name in accessLog
• This can lead to misconfiguration (mismatched names)
• Both LogFormat AND AccessLog must be set for anything to work

I think we can remove the NginxLogFormat type completely and change the Format field in NginxAccessLog to specify the format string directly. We don't need to provide a mechanism for the user to specify the name of the log format, we can define that as an internal string.

Something like:

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.
    // See https://nginx.org/en/docs/http/ngx_http_log_module.html#log_format
    // +optional
    Format *string `json:"format,omitempty"`
}

Later, if we decide to support different path types (e.g. syslog) we can extend this type to add the path field.

Then, if the access log format is set the template renders something like:

log_format ngf_default '<user-format>';
access_log /dev/stdout ngf_default;

And if it's disabled,

access_log off;

Otherwise we can omit the directive, and NGINX will use the default /dev/stdout combined (same behaviour as we have currently)

Does that make sense?

[ciarams87]

I don't think these fields belong in this type.

I'd recommend moving these to the AccessLog:

type AccessLog struct {
    Format     string  // User's format string
    Disabled   bool    // User's disabled flag
    Path       string  // Where to write logs (/dev/stdout)
    FormatName string  // Internal format name (ngf_user_defined_log_format)
}

And populate the constants in buildAccessLog().

[ciarams87]

We still need to remove these fields from this type

[ciarams87]

We still need to remove these fields from this type

[ciarams87]

nit: this comment is not correct

Suggested change

	// in v1alpha2) into internal slice-based representation used by templates.
	// in v1alpha2) into internal representation used by templates.