From 59e0764bdac27fddbf621479901dcbb595b28925 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 16 Sep 2024 15:04:36 -0700 Subject: [PATCH 01/20] Add Nginx log level to NginxProxy crd --- apis/v1alpha1/nginxproxy_types.go | 44 ++++++++ apis/v1alpha1/zz_generated.deepcopy.go | 25 +++++ .../bases/gateway.nginx.org_nginxproxies.yaml | 17 +++ deploy/crds.yaml | 17 +++ site/content/reference/api.md | 105 ++++++++++++++++++ 5 files changed, 208 insertions(+) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index acb42e4f5e..fabedddec6 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -59,6 +59,10 @@ type NginxProxySpec struct { // +optional //nolint:lll RewriteClientIP *RewriteClientIP `json:"rewriteClientIP,omitempty"` + // Logging defines logging related settings for NGINX. + // + // +optional + Logging *NginxLogging `json:"logging,omitempty"` // DisableHTTP2 defines if http2 should be disabled for all servers. // Default is false, meaning http2 will be enabled for all servers. // @@ -203,3 +207,43 @@ const ( // kubebuilder:validation:Pattern=`^[\.a-zA-Z0-9:]*(\/([0-9]?[0-9]?[0-9]))$` AddressTypeCIDR AddressType = "cidr" ) + +// NginxLogging defines logging related settings for NGINX. +type NginxLogging struct { + // Level defines the logging level. + // + // +optional + // +kubebuilder:default=info + Level *NginxLogLevel `json:"level,omitempty"` +} + +// NginxLogLevel type defines the logging level of error logs for NGINX. +// +// +kubebuilder:validation:Enum=debug;info;notice;warn;error;crit;alert;emerg +type NginxLogLevel string + +const ( + // NginxLogLevelDebug is the debug level for NGINX logging. + NginxLogLevelDebug NginxLogLevel = "debug" + + // NginxLogLevelInfo is the info level for NGINX logging. + NginxLogLevelInfo NginxLogLevel = "info" + + // NginxLogLevelNotice is the notice level for NGINX logging. + NginxLogLevelNotice NginxLogLevel = "notice" + + // NginxLogLevelWarn is the warn level for NGINX logging. + NginxLogLevelWarn NginxLogLevel = "warn" + + // NginxLogLevelError is the error level for NGINX logging. + NginxLogLevelError NginxLogLevel = "error" + + // NginxLogLevelCrit is the crit level for NGINX logging. + NginxLogLevelCrit NginxLogLevel = "crit" + + // NginxLogLevelAlert is the alert level for NGINX logging. + NginxLogLevelAlert NginxLogLevel = "alert" + + // NginxLogLevelEmerg is the emerg level for NGINX logging. + NginxLogLevelEmerg NginxLogLevel = "emerg" +) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index bffbb7dfdb..c335981928 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -311,6 +311,26 @@ func (in *NginxGatewayStatus) DeepCopy() *NginxGatewayStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *NginxLogging) DeepCopyInto(out *NginxLogging) { + *out = *in + if in.Level != nil { + in, out := &in.Level, &out.Level + *out = new(NginxLogLevel) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxLogging. +func (in *NginxLogging) DeepCopy() *NginxLogging { + if in == nil { + return nil + } + out := new(NginxLogging) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NginxProxy) DeepCopyInto(out *NginxProxy) { *out = *in @@ -387,6 +407,11 @@ func (in *NginxProxySpec) DeepCopyInto(out *NginxProxySpec) { *out = new(RewriteClientIP) (*in).DeepCopyInto(*out) } + if in.Logging != nil { + in, out := &in.Logging, &out.Logging + *out = new(NginxLogging) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NginxProxySpec. diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 0bf4e02b8c..c9a5802d63 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -62,6 +62,23 @@ spec: - ipv4 - ipv6 type: string + logging: + description: Logging defines logging related settings for NGINX. + properties: + level: + default: info + description: Level defines the logging level. + enum: + - debug + - info + - notice + - warn + - error + - crit + - alert + - emerg + type: string + type: object rewriteClientIP: description: RewriteClientIP defines configuration for rewriting the client IP to the original client's IP. diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 1e05fe409d..2761e0743c 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -647,6 +647,23 @@ spec: - ipv4 - ipv6 type: string + logging: + description: Logging defines logging related settings for NGINX. + properties: + level: + default: info + description: Level defines the logging level. + enum: + - debug + - info + - notice + - warn + - error + - crit + - alert + - emerg + type: string + type: object rewriteClientIP: description: RewriteClientIP defines configuration for rewriting the client IP to the original client's IP. diff --git a/site/content/reference/api.md b/site/content/reference/api.md index bc4062b175..8808f521fe 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -343,6 +343,20 @@ RewriteClientIP +logging
+ + +NginxLogging + + + + +(Optional) +

Logging defines logging related settings for NGINX.

+ + + + disableHTTP2
bool @@ -987,6 +1001,83 @@ Logging +

NginxLogLevel +(string alias)

+

+

+(Appears on: +NginxLogging) +

+

+

NginxLogLevel type defines the logging level of error logs for NGINX.

+

+ + + + + + + + + + + + + + + + + + + + + + + + +
ValueDescription

"alert"

NginxLogLevelAlert is the alert level for NGINX logging.

+

"crit"

NginxLogLevelCrit is the crit level for NGINX logging.

+

"debug"

NginxLogLevelDebug is the debug level for NGINX logging.

+

"emerg"

NginxLogLevelEmerg is the emerg level for NGINX logging.

+

"error"

NginxLogLevelError is the error level for NGINX logging.

+

"info"

NginxLogLevelInfo is the info level for NGINX logging.

+

"notice"

NginxLogLevelNotice is the notice level for NGINX logging.

+

"warn"

NginxLogLevelWarn is the warn level for NGINX logging.

+
+

NginxLogging + +

+

+(Appears on: +NginxProxySpec) +

+

+

NginxLogging defines logging related settings for NGINX.

+

+ + + + + + + + + + + + + +
FieldDescription
+level
+ + +NginxLogLevel + + +		 +(Optional) +

Level defines the logging level.

+

NginxProxySpec

@@ -1050,6 +1141,20 @@ RewriteClientIP +logging
+ + +NginxLogging + + + + +(Optional) +

Logging defines logging related settings for NGINX.

+ + + + disableHTTP2
bool From 1547c383f916573222dda7beaadec5060706a552 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 16 Sep 2024 15:11:44 -0700 Subject: [PATCH 02/20] Adjust wording of description --- apis/v1alpha1/nginxproxy_types.go | 20 +++++++++---------- .../bases/gateway.nginx.org_nginxproxies.yaml | 2 +- deploy/crds.yaml | 2 +- site/content/reference/api.md | 20 +++++++++---------- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index fabedddec6..02a818701e 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -210,40 +210,40 @@ const ( // NginxLogging defines logging related settings for NGINX. type NginxLogging struct { - // Level defines the logging level. + // Level defines the error log level. // // +optional // +kubebuilder:default=info Level *NginxLogLevel `json:"level,omitempty"` } -// NginxLogLevel type defines the logging level of error logs for NGINX. +// NginxLogLevel type defines the log level of error logs for NGINX. // // +kubebuilder:validation:Enum=debug;info;notice;warn;error;crit;alert;emerg type NginxLogLevel string const ( - // NginxLogLevelDebug is the debug level for NGINX logging. + // NginxLogLevelDebug is the debug level for NGINX error logs. NginxLogLevelDebug NginxLogLevel = "debug" - // NginxLogLevelInfo is the info level for NGINX logging. + // NginxLogLevelInfo is the info level for NGINX error logs. NginxLogLevelInfo NginxLogLevel = "info" - // NginxLogLevelNotice is the notice level for NGINX logging. + // NginxLogLevelNotice is the notice level for NGINX error logs. NginxLogLevelNotice NginxLogLevel = "notice" - // NginxLogLevelWarn is the warn level for NGINX logging. + // NginxLogLevelWarn is the warn level for NGINX error logs. NginxLogLevelWarn NginxLogLevel = "warn" - // NginxLogLevelError is the error level for NGINX logging. + // NginxLogLevelError is the error level for NGINX error logs. NginxLogLevelError NginxLogLevel = "error" - // NginxLogLevelCrit is the crit level for NGINX logging. + // NginxLogLevelCrit is the crit level for NGINX error logs. NginxLogLevelCrit NginxLogLevel = "crit" - // NginxLogLevelAlert is the alert level for NGINX logging. + // NginxLogLevelAlert is the alert level for NGINX error logs. NginxLogLevelAlert NginxLogLevel = "alert" - // NginxLogLevelEmerg is the emerg level for NGINX logging. + // NginxLogLevelEmerg is the emerg level for NGINX error logs. NginxLogLevelEmerg NginxLogLevel = "emerg" ) diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index c9a5802d63..4a4b32f7a7 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -67,7 +67,7 @@ spec: properties: level: default: info - description: Level defines the logging level. + description: Level defines the error log level. enum: - debug - info diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 2761e0743c..86b6d73b33 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -652,7 +652,7 @@ spec: properties: level: default: info - description: Level defines the logging level. + description: Level defines the error log level. enum: - debug - info diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 8808f521fe..7bc796df6c 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -1009,7 +1009,7 @@ Logging NginxLogging)

-

NginxLogLevel type defines the logging level of error logs for NGINX.

+

NginxLogLevel type defines the log level of error logs for NGINX.

@@ -1019,28 +1019,28 @@ Logging - - - - - - - -

"alert"

NginxLogLevelAlert is the alert level for NGINX logging.

+

NginxLogLevelAlert is the alert level for NGINX error logs.

"crit"

NginxLogLevelCrit is the crit level for NGINX logging.

+

NginxLogLevelCrit is the crit level for NGINX error logs.

"debug"

NginxLogLevelDebug is the debug level for NGINX logging.

+

NginxLogLevelDebug is the debug level for NGINX error logs.

"emerg"

NginxLogLevelEmerg is the emerg level for NGINX logging.

+

NginxLogLevelEmerg is the emerg level for NGINX error logs.

"error"

NginxLogLevelError is the error level for NGINX logging.

+

NginxLogLevelError is the error level for NGINX error logs.

"info"

NginxLogLevelInfo is the info level for NGINX logging.

+

NginxLogLevelInfo is the info level for NGINX error logs.

"notice"

NginxLogLevelNotice is the notice level for NGINX logging.

+

NginxLogLevelNotice is the notice level for NGINX error logs.

"warn"

NginxLogLevelWarn is the warn level for NGINX logging.

+

NginxLogLevelWarn is the warn level for NGINX error logs.
@@ -1073,7 +1073,7 @@ NginxLogLevel (Optional) -

Level defines the logging level.

+

Level defines the error log level.

From eb33ac3919d274a26cbc3bbe2457d359d5521d2f Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 16 Sep 2024 15:39:43 -0700 Subject: [PATCH 03/20] Add feedback for changes to NginxProxy CRD --- apis/v1alpha1/nginxproxy_types.go | 27 +++--- apis/v1alpha1/zz_generated.deepcopy.go | 6 +- .../bases/gateway.nginx.org_nginxproxies.yaml | 8 +- deploy/crds.yaml | 8 +- site/content/reference/api.md | 97 ++++++++++--------- 5 files changed, 80 insertions(+), 66 deletions(-) diff --git a/apis/v1alpha1/nginxproxy_types.go b/apis/v1alpha1/nginxproxy_types.go index 02a818701e..88568e65bb 100644 --- a/apis/v1alpha1/nginxproxy_types.go +++ b/apis/v1alpha1/nginxproxy_types.go @@ -210,40 +210,43 @@ const ( // NginxLogging defines logging related settings for NGINX. type NginxLogging struct { - // Level defines the error log level. + // ErrorLevel defines the error log level. Possible log levels listed in order of increasing severity are + // debug, info, notice, warn, error, crit, alert, and emerg. Setting a certain log level will cause all messages + // of the specified and more severe log levels to be logged. For example, the log level 'error' will cause error, + // crit, alert, and emerg messages to be logged. https://nginx.org/en/docs/ngx_core_module.html#error_log // // +optional // +kubebuilder:default=info - Level *NginxLogLevel `json:"level,omitempty"` + ErrorLevel *NginxErrorLogLevel `json:"errorlevel,omitempty"` } -// NginxLogLevel type defines the log level of error logs for NGINX. +// NginxErrorLogLevel type defines the log level of error logs for NGINX. // // +kubebuilder:validation:Enum=debug;info;notice;warn;error;crit;alert;emerg -type NginxLogLevel string +type NginxErrorLogLevel string const ( // NginxLogLevelDebug is the debug level for NGINX error logs. - NginxLogLevelDebug NginxLogLevel = "debug" + NginxLogLevelDebug NginxErrorLogLevel = "debug" // NginxLogLevelInfo is the info level for NGINX error logs. - NginxLogLevelInfo NginxLogLevel = "info" + NginxLogLevelInfo NginxErrorLogLevel = "info" // NginxLogLevelNotice is the notice level for NGINX error logs. - NginxLogLevelNotice NginxLogLevel = "notice" + NginxLogLevelNotice NginxErrorLogLevel = "notice" // NginxLogLevelWarn is the warn level for NGINX error logs. - NginxLogLevelWarn NginxLogLevel = "warn" + NginxLogLevelWarn NginxErrorLogLevel = "warn" // NginxLogLevelError is the error level for NGINX error logs. - NginxLogLevelError NginxLogLevel = "error" + NginxLogLevelError NginxErrorLogLevel = "error" // NginxLogLevelCrit is the crit level for NGINX error logs. - NginxLogLevelCrit NginxLogLevel = "crit" + NginxLogLevelCrit NginxErrorLogLevel = "crit" // NginxLogLevelAlert is the alert level for NGINX error logs. - NginxLogLevelAlert NginxLogLevel = "alert" + NginxLogLevelAlert NginxErrorLogLevel = "alert" // NginxLogLevelEmerg is the emerg level for NGINX error logs. - NginxLogLevelEmerg NginxLogLevel = "emerg" + NginxLogLevelEmerg NginxErrorLogLevel = "emerg" ) diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index c335981928..b54f857c1a 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -314,9 +314,9 @@ func (in *NginxGatewayStatus) DeepCopy() *NginxGatewayStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NginxLogging) DeepCopyInto(out *NginxLogging) { *out = *in - if in.Level != nil { - in, out := &in.Level, &out.Level - *out = new(NginxLogLevel) + if in.ErrorLevel != nil { + in, out := &in.ErrorLevel, &out.ErrorLevel + *out = new(NginxErrorLogLevel) **out = **in } } diff --git a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml index 4a4b32f7a7..0c133c9ac0 100644 --- a/config/crd/bases/gateway.nginx.org_nginxproxies.yaml +++ b/config/crd/bases/gateway.nginx.org_nginxproxies.yaml @@ -65,9 +65,13 @@ spec: logging: description: Logging defines logging related settings for NGINX. properties: - level: + errorlevel: default: info - description: Level defines the error log level. + description: |- + ErrorLevel defines the error log level. Possible log levels listed in order of increasing severity are + debug, info, notice, warn, error, crit, alert, and emerg. Setting a certain log level will cause all messages + of the specified and more severe log levels to be logged. For example, the log level 'error' will cause error, + crit, alert, and emerg messages to be logged. https://nginx.org/en/docs/ngx_core_module.html#error_log enum: - debug - info diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 86b6d73b33..65d408b474 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -650,9 +650,13 @@ spec: logging: description: Logging defines logging related settings for NGINX. properties: - level: + errorlevel: default: info - description: Level defines the error log level. + description: |- + ErrorLevel defines the error log level. Possible log levels listed in order of increasing severity are + debug, info, notice, warn, error, crit, alert, and emerg. Setting a certain log level will cause all messages + of the specified and more severe log levels to be logged. For example, the log level 'error' will cause error, + crit, alert, and emerg messages to be logged. https://nginx.org/en/docs/ngx_core_module.html#error_log enum: - debug - info diff --git a/site/content/reference/api.md b/site/content/reference/api.md index 7bc796df6c..b8f3c126f1 100644 --- a/site/content/reference/api.md +++ b/site/content/reference/api.md @@ -892,6 +892,49 @@ ControllerLogLevel +

NginxErrorLogLevel +(string alias)

+

+

+(Appears on: +NginxLogging) +

+

+

NginxErrorLogLevel type defines the log level of error logs for NGINX.

+

+ + + + + + + + + + + + + + + + + + + + + + + + +
ValueDescription

"alert"

NginxLogLevelAlert is the alert level for NGINX error logs.

+

"crit"

NginxLogLevelCrit is the crit level for NGINX error logs.

+

"debug"

NginxLogLevelDebug is the debug level for NGINX error logs.

+

"emerg"

NginxLogLevelEmerg is the emerg level for NGINX error logs.

+

"error"

NginxLogLevelError is the error level for NGINX error logs.

+

"info"

NginxLogLevelInfo is the info level for NGINX error logs.

+

"notice"

NginxLogLevelNotice is the notice level for NGINX error logs.

+

"warn"

NginxLogLevelWarn is the warn level for NGINX error logs.

+

NginxGatewayConditionReason (string alias)

@@ -1001,49 +1044,6 @@ Logging -

NginxLogLevel -(string alias)

-

-

-(Appears on: -NginxLogging) -

-

-

NginxLogLevel type defines the log level of error logs for NGINX.

-

- - - - - - - - - - - - - - - - - - - - - - - - -
ValueDescription

"alert"

NginxLogLevelAlert is the alert level for NGINX error logs.

-

"crit"

NginxLogLevelCrit is the crit level for NGINX error logs.

-

"debug"

NginxLogLevelDebug is the debug level for NGINX error logs.

-

"emerg"

NginxLogLevelEmerg is the emerg level for NGINX error logs.

-

"error"

NginxLogLevelError is the error level for NGINX error logs.

-

"info"

NginxLogLevelInfo is the info level for NGINX error logs.

-

"notice"

NginxLogLevelNotice is the notice level for NGINX error logs.

-

"warn"

NginxLogLevelWarn is the warn level for NGINX error logs.

-

NginxLogging

@@ -1064,16 +1064,19 @@ Logging -level
+errorlevel
- -NginxLogLevel + +NginxErrorLogLevel (Optional) -

Level defines the error log level.

+

ErrorLevel defines the error log level. Possible log levels listed in order of increasing severity are +debug, info, notice, warn, error, crit, alert, and emerg. Setting a certain log level will cause all messages +of the specified and more severe log levels to be logged. For example, the log level ‘error’ will cause error, +crit, alert, and emerg messages to be logged. https://nginx.org/en/docs/ngx_core_module.html#error_log

From d0cc5f795c231cc001023f293439f17ee4492830 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 18 Sep 2024 09:52:16 -0700 Subject: [PATCH 04/20] WIP --- .../mode/static/nginx/config/generator.go | 13 +++++++++ .../static/state/dataplane/configuration.go | 15 +++++++++- internal/mode/static/state/dataplane/types.go | 8 +++++ .../mode/static/state/graph/nginxproxy.go | 29 +++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index be2877bf29..7f544d948c 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -191,6 +191,19 @@ func generateConfigVersion(configVersion int) file.File { } } +//func generateMainContextConf(conf dataplane.Configuration) file.File { +// var c []byte +// if conf.Telemetry.Endpoint != "" { +// c = []byte("load_module modules/ngx_otel_module.so;") +// } +// +// return file.File{ +// Content: c, +// Path: loadModulesFile, +// Type: file.TypeRegular, +// } +//} + func generateLoadModulesConf(conf dataplane.Configuration) file.File { var c []byte if conf.Telemetry.Endpoint != "" { diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index eefe5e4bb4..94ebd31c5f 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -15,7 +15,7 @@ import ( ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" - policies "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" ) @@ -50,6 +50,7 @@ func BuildConfiguration( keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners) certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) telemetry := buildTelemetry(g) + logging := buildLogging(g) config := Configuration{ HTTPServers: httpServers, @@ -63,6 +64,7 @@ func BuildConfiguration( CertBundles: certBundles, Telemetry: telemetry, BaseHTTPConfig: baseHTTPConfig, + Logging: logging, } return config @@ -901,3 +903,14 @@ func convertAddresses(addresses []ngfAPI.Address) []string { } return trustedAddresses } + +func buildLogging(g *graph.Graph) Logging { + defaultErrorLogLevel := "info" + + ngfProxy := g.NginxProxy + if ngfProxy != nil && ngfProxy.Source.Spec.Logging != nil { + return Logging{ErrorLevel: string(*ngfProxy.Source.Spec.Logging.ErrorLevel)} + } + + return Logging{ErrorLevel: defaultErrorLogLevel} +} diff --git a/internal/mode/static/state/dataplane/types.go b/internal/mode/static/state/dataplane/types.go index 59110f8cbb..9ebc7a185f 100644 --- a/internal/mode/static/state/dataplane/types.go +++ b/internal/mode/static/state/dataplane/types.go @@ -40,6 +40,8 @@ type Configuration struct { BackendGroups []BackendGroup // Telemetry holds the Otel configuration. Telemetry Telemetry + // Logging defines logging related settings for NGINX. + Logging Logging // BaseHTTPConfig holds the configuration options at the http context. BaseHTTPConfig BaseHTTPConfig // Version represents the version of the generated configuration. @@ -355,3 +357,9 @@ type Ratio struct { // Value is the value of the ratio. Value int32 } + +// Logging defines logging related settings for NGINX. +type Logging struct { + // ErrorLevel defines the error log level. + ErrorLevel string +} diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index cf6dc70990..87faedb8bd 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -1,6 +1,8 @@ package graph import ( + "slices" + "k8s.io/apimachinery/pkg/types" k8svalidation "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" @@ -127,6 +129,33 @@ func validateNginxProxy( npCfg.Spec.IPFamily = helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual) } + if npCfg.Spec.Logging != nil { + logging := npCfg.Spec.Logging + loggingPath := spec.Child("logging") + errLevel := string(*logging.ErrorLevel) + + validLogLevels := []string{ + string(ngfAPI.NginxLogLevelDebug), + string(ngfAPI.NginxLogLevelInfo), + string(ngfAPI.NginxLogLevelNotice), + string(ngfAPI.NginxLogLevelWarn), + string(ngfAPI.NginxLogLevelError), + string(ngfAPI.NginxLogLevelCrit), + string(ngfAPI.NginxLogLevelAlert), + string(ngfAPI.NginxLogLevelEmerg), + } + + if !slices.Contains(validLogLevels, errLevel) { + allErrs = append( + allErrs, + field.NotSupported( + loggingPath, + logging, + validLogLevels, + )) + } + } // TODO: Might need to put an elseif here and fill in values for default logging + allErrs = append(allErrs, validateRewriteClientIP(npCfg)...) return allErrs From d2f385ddbb00047c556e574da0914d565a7ca5ce Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 08:36:38 -0700 Subject: [PATCH 05/20] WIP --- .../templates/deployment.yaml | 10 +++--- internal/mode/static/nginx/conf/nginx.conf | 3 +- .../mode/static/nginx/config/generator.go | 36 ++++++------------- .../mode/static/nginx/config/main_includes.go | 19 ++++++++++ .../nginx/config/main_includes_template.go | 7 ++++ .../static/state/dataplane/configuration.go | 15 ++++---- 6 files changed, 51 insertions(+), 39 deletions(-) create mode 100644 internal/mode/static/nginx/config/main_includes.go create mode 100644 internal/mode/static/nginx/config/main_includes_template.go diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index 6ce6240c29..f99f67e415 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -131,8 +131,8 @@ spec: mountPath: /etc/nginx/conf.d - name: nginx-stream-conf mountPath: /etc/nginx/stream-conf.d - - name: module-includes - mountPath: /etc/nginx/module-includes + - name: main-includes + mountPath: /etc/nginx/main-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -170,8 +170,8 @@ spec: mountPath: /etc/nginx/conf.d - name: nginx-stream-conf mountPath: /etc/nginx/stream-conf.d - - name: module-includes - mountPath: /etc/nginx/module-includes + - name: main-includes + mountPath: /etc/nginx/main-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -206,7 +206,7 @@ spec: emptyDir: {} - name: nginx-stream-conf emptyDir: {} - - name: module-includes + - name: main-includes emptyDir: {} - name: nginx-secrets emptyDir: {} diff --git a/internal/mode/static/nginx/conf/nginx.conf b/internal/mode/static/nginx/conf/nginx.conf index 2cbc09fa3f..5e13fc8dce 100644 --- a/internal/mode/static/nginx/conf/nginx.conf +++ b/internal/mode/static/nginx/conf/nginx.conf @@ -1,10 +1,9 @@ load_module /usr/lib/nginx/modules/ngx_http_js_module.so; -include /etc/nginx/module-includes/*.conf; +include /etc/nginx/main-includes/*.conf; worker_processes auto; pid /var/run/nginx/nginx.pid; -error_log stderr info; events { worker_connections 1024; diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 7f544d948c..912f596731 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -23,8 +23,8 @@ const ( // streamFolder is the folder where NGINX Stream configuration files are stored. streamFolder = configFolder + "/stream-conf.d" - // modulesIncludesFolder is the folder where the included "load_module" file is stored. - modulesIncludesFolder = configFolder + "/module-includes" + // mainIncludesFolder is the folder where NGINX main context configuration files is stored. + mainIncludesFolder = configFolder + "/main-includes" // secretsFolder is the folder where secrets (like TLS certs/keys) are stored. secretsFolder = configFolder + "/secrets" @@ -44,12 +44,12 @@ const ( // httpMatchVarsFile is the path to the http_match pairs configuration file. httpMatchVarsFile = httpFolder + "/matches.json" - // loadModulesFile is the path to the file containing any load_module directives. - loadModulesFile = modulesIncludesFolder + "/load-modules.conf" + // mainIncludesConfigFile is the path to the file containing NGINX configuration in the main context. + mainIncludesConfigFile = mainIncludesFolder + "/main.conf" ) // ConfigFolders is a list of folders where NGINX configuration files are stored. -var ConfigFolders = []string{httpFolder, secretsFolder, includesFolder, modulesIncludesFolder, streamFolder} +var ConfigFolders = []string{httpFolder, secretsFolder, includesFolder, mainIncludesFolder, streamFolder} // Generator generates NGINX configuration files. // This interface is used for testing purposes only. @@ -107,7 +107,7 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File { files = append(files, generateCertBundle(id, bundle)) } - files = append(files, generateLoadModulesConf(conf)) + files = append(files, generateMainContextConf(conf)) return files } @@ -191,28 +191,12 @@ func generateConfigVersion(configVersion int) file.File { } } -//func generateMainContextConf(conf dataplane.Configuration) file.File { -// var c []byte -// if conf.Telemetry.Endpoint != "" { -// c = []byte("load_module modules/ngx_otel_module.so;") -// } -// -// return file.File{ -// Content: c, -// Path: loadModulesFile, -// Type: file.TypeRegular, -// } -//} - -func generateLoadModulesConf(conf dataplane.Configuration) file.File { - var c []byte - if conf.Telemetry.Endpoint != "" { - c = []byte("load_module modules/ngx_otel_module.so;") - } +func generateMainContextConf(conf dataplane.Configuration) file.File { + result := executeMainIncludesConfig(conf) return file.File{ - Content: c, - Path: loadModulesFile, + Content: result.data, + Path: result.dest, Type: file.TypeRegular, } } diff --git a/internal/mode/static/nginx/config/main_includes.go b/internal/mode/static/nginx/config/main_includes.go new file mode 100644 index 0000000000..ac1e05c51e --- /dev/null +++ b/internal/mode/static/nginx/config/main_includes.go @@ -0,0 +1,19 @@ +package config + +import ( + gotemplate "text/template" + + "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +var mainIncludesTemplate = gotemplate.Must(gotemplate.New("mainIncludes").Parse(mainIncludesTemplateText)) + +func executeMainIncludesConfig(conf dataplane.Configuration) executeResult { + result := executeResult{ + dest: mainIncludesConfigFile, + data: helpers.MustExecuteTemplate(mainIncludesTemplate, conf), + } + + return result +} diff --git a/internal/mode/static/nginx/config/main_includes_template.go b/internal/mode/static/nginx/config/main_includes_template.go new file mode 100644 index 0000000000..c444b1efe2 --- /dev/null +++ b/internal/mode/static/nginx/config/main_includes_template.go @@ -0,0 +1,7 @@ +package config + +const mainIncludesTemplateText = ` +{{- if .Telemetry.Endpoint }}load_module modules/ngx_otel_module.so;{{ end -}} + +error_log stderr {{ .Logging.ErrorLevel }}; +` diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 94ebd31c5f..80048b1dbb 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -32,12 +32,8 @@ func BuildConfiguration( serviceResolver resolver.ServiceResolver, configVersion int, ) Configuration { - if g.GatewayClass == nil || !g.GatewayClass.Valid { - return Configuration{Version: configVersion} - } - - if g.Gateway == nil { - return Configuration{Version: configVersion} + if g.GatewayClass == nil || !g.GatewayClass.Valid || g.Gateway == nil { + return getDefaultConfiguration(configVersion) } baseHTTPConfig := buildBaseHTTPConfig(g) @@ -914,3 +910,10 @@ func buildLogging(g *graph.Graph) Logging { return Logging{ErrorLevel: defaultErrorLogLevel} } + +func getDefaultConfiguration(configVersion int) Configuration { + return Configuration{ + Version: configVersion, + Logging: Logging{ErrorLevel: "info"}, + } +} From e0a3ad39f9ad4451cdedaa6af12c147c84d7bf91 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 11:36:55 -0700 Subject: [PATCH 06/20] Refactor unit tests --- internal/mode/static/handler_test.go | 30 ++++++++++++------- .../static/nginx/config/generator_test.go | 10 +++++-- .../static/state/dataplane/configuration.go | 4 +-- .../mode/static/state/graph/nginxproxy.go | 15 ++++++++-- 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/internal/mode/static/handler_test.go b/internal/mode/static/handler_test.go index 93ad4e1b61..2b4ee9b537 100644 --- a/internal/mode/static/handler_test.go +++ b/internal/mode/static/handler_test.go @@ -155,11 +155,11 @@ var _ = Describe("eventHandler", func() { handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - dcfg := &dataplane.Configuration{Version: 1} + dcfg := dataplane.GetDefaultConfiguration(1) checkUpsertEventExpectations(e) - expectReconfig(*dcfg, fakeCfgFiles) - Expect(helpers.Diff(handler.GetLatestConfiguration(), dcfg)).To(BeEmpty()) + expectReconfig(dcfg, fakeCfgFiles) + Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) }) It("should process Delete", func() { @@ -171,11 +171,11 @@ var _ = Describe("eventHandler", func() { handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - dcfg := &dataplane.Configuration{Version: 1} + dcfg := dataplane.GetDefaultConfiguration(1) checkDeleteEventExpectations(e) - expectReconfig(*dcfg, fakeCfgFiles) - Expect(helpers.Diff(handler.GetLatestConfiguration(), dcfg)).To(BeEmpty()) + expectReconfig(dcfg, fakeCfgFiles) + Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) }) }) @@ -194,7 +194,9 @@ var _ = Describe("eventHandler", func() { checkDeleteEventExpectations(deleteEvent) handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - Expect(helpers.Diff(handler.GetLatestConfiguration(), &dataplane.Configuration{Version: 2})).To(BeEmpty()) + + dcfg := dataplane.GetDefaultConfiguration(2) + Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) }) }) }) @@ -517,7 +519,9 @@ var _ = Describe("eventHandler", func() { fakeNginxRuntimeMgr.IsPlusReturns(true) handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - Expect(helpers.Diff(handler.GetLatestConfiguration(), &dataplane.Configuration{Version: 1})).To(BeEmpty()) + + dcfg := dataplane.GetDefaultConfiguration(1) + Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) Expect(fakeGenerator.GenerateCallCount()).To(Equal(1)) Expect(fakeNginxFileMgr.ReplaceFilesCallCount()).To(Equal(1)) @@ -528,7 +532,9 @@ var _ = Describe("eventHandler", func() { When("not running NGINX Plus", func() { It("should not call the NGINX Plus API", func() { handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - Expect(helpers.Diff(handler.GetLatestConfiguration(), &dataplane.Configuration{Version: 1})).To(BeEmpty()) + + dcfg := dataplane.GetDefaultConfiguration(1) + Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) Expect(fakeGenerator.GenerateCallCount()).To(Equal(1)) Expect(fakeNginxFileMgr.ReplaceFilesCallCount()).To(Equal(1)) @@ -623,7 +629,8 @@ var _ = Describe("eventHandler", func() { Expect(handler.cfg.nginxConfiguredOnStartChecker.readyCheck(nil)).ToNot(Succeed()) handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - Expect(helpers.Diff(handler.GetLatestConfiguration(), &dataplane.Configuration{Version: 1})).To(BeEmpty()) + dcfg := dataplane.GetDefaultConfiguration(1) + Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) Expect(readyChannel).To(BeClosed()) @@ -670,7 +677,8 @@ var _ = Describe("eventHandler", func() { handler.HandleEventBatch(context.Background(), ctlrZap.New(), batch) - Expect(helpers.Diff(handler.GetLatestConfiguration(), &dataplane.Configuration{Version: 2})).To(BeEmpty()) + dcfg := dataplane.GetDefaultConfiguration(2) + Expect(helpers.Diff(handler.GetLatestConfiguration(), &dcfg)).To(BeEmpty()) Expect(readyChannel).To(BeClosed()) diff --git a/internal/mode/static/nginx/config/generator_test.go b/internal/mode/static/nginx/config/generator_test.go index 4940d72452..ed901af39f 100644 --- a/internal/mode/static/nginx/config/generator_test.go +++ b/internal/mode/static/nginx/config/generator_test.go @@ -90,6 +90,9 @@ func TestGenerate(t *testing.T) { BatchSize: 512, BatchCount: 4, }, + Logging: dataplane.Logging{ + ErrorLevel: "debug", + }, BaseHTTPConfig: dataplane.BaseHTTPConfig{ HTTP2: true, }, @@ -135,8 +138,11 @@ func TestGenerate(t *testing.T) { expString := "{}" g.Expect(string(files[2].Content)).To(Equal(expString)) - g.Expect(files[3].Path).To(Equal("/etc/nginx/module-includes/load-modules.conf")) - g.Expect(files[3].Content).To(Equal([]byte("load_module modules/ngx_otel_module.so;"))) + g.Expect(files[3].Path).To(Equal("/etc/nginx/main-includes/main.conf")) + + mainCfg := string(files[3].Content) + g.Expect(mainCfg).To(ContainSubstring("load_module modules/ngx_otel_module.so;")) + g.Expect(mainCfg).To(ContainSubstring("error_log stderr debug;")) g.Expect(files[4].Path).To(Equal("/etc/nginx/secrets/test-certbundle.crt")) certBundle := string(files[4].Content) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 80048b1dbb..f26968c2ae 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -33,7 +33,7 @@ func BuildConfiguration( configVersion int, ) Configuration { if g.GatewayClass == nil || !g.GatewayClass.Valid || g.Gateway == nil { - return getDefaultConfiguration(configVersion) + return GetDefaultConfiguration(configVersion) } baseHTTPConfig := buildBaseHTTPConfig(g) @@ -911,7 +911,7 @@ func buildLogging(g *graph.Graph) Logging { return Logging{ErrorLevel: defaultErrorLogLevel} } -func getDefaultConfiguration(configVersion int) Configuration { +func GetDefaultConfiguration(configVersion int) Configuration { return Configuration{ Version: configVersion, Logging: Logging{ErrorLevel: "info"}, diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 87faedb8bd..6647f7527c 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -129,6 +129,17 @@ func validateNginxProxy( npCfg.Spec.IPFamily = helpers.GetPointer[ngfAPI.IPFamilyType](ngfAPI.Dual) } + allErrs = append(allErrs, validateLogging(npCfg)...) + + allErrs = append(allErrs, validateRewriteClientIP(npCfg)...) + + return allErrs +} + +func validateLogging(npCfg *ngfAPI.NginxProxy) field.ErrorList { + var allErrs field.ErrorList + spec := field.NewPath("spec") + if npCfg.Spec.Logging != nil { logging := npCfg.Spec.Logging loggingPath := spec.Child("logging") @@ -154,9 +165,7 @@ func validateNginxProxy( validLogLevels, )) } - } // TODO: Might need to put an elseif here and fill in values for default logging - - allErrs = append(allErrs, validateRewriteClientIP(npCfg)...) + } return allErrs } From c923e2fd9dab414a3e7b8d42dbbc2ea7fb89632e Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 11:38:51 -0700 Subject: [PATCH 07/20] Add generated manifests from change to charts --- config/tests/static-deployment.yaml | 10 +++++----- deploy/aws-nlb/deploy.yaml | 10 +++++----- deploy/azure/deploy.yaml | 10 +++++----- deploy/default/deploy.yaml | 10 +++++----- deploy/experimental-nginx-plus/deploy.yaml | 10 +++++----- deploy/experimental/deploy.yaml | 10 +++++----- deploy/nginx-plus/deploy.yaml | 10 +++++----- deploy/nodeport/deploy.yaml | 10 +++++----- deploy/openshift/deploy.yaml | 10 +++++----- 9 files changed, 45 insertions(+), 45 deletions(-) diff --git a/config/tests/static-deployment.yaml b/config/tests/static-deployment.yaml index bb2fb62765..a2105152b4 100644 --- a/config/tests/static-deployment.yaml +++ b/config/tests/static-deployment.yaml @@ -74,8 +74,8 @@ spec: mountPath: /etc/nginx/conf.d - name: nginx-stream-conf mountPath: /etc/nginx/stream-conf.d - - name: module-includes - mountPath: /etc/nginx/module-includes + - name: main-includes + mountPath: /etc/nginx/main-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -106,8 +106,8 @@ spec: mountPath: /etc/nginx/conf.d - name: nginx-stream-conf mountPath: /etc/nginx/stream-conf.d - - name: module-includes - mountPath: /etc/nginx/module-includes + - name: main-includes + mountPath: /etc/nginx/main-includes - name: nginx-secrets mountPath: /etc/nginx/secrets - name: nginx-run @@ -127,7 +127,7 @@ spec: emptyDir: {} - name: nginx-stream-conf emptyDir: {} - - name: module-includes + - name: main-includes emptyDir: {} - name: nginx-secrets emptyDir: {} diff --git a/deploy/aws-nlb/deploy.yaml b/deploy/aws-nlb/deploy.yaml index 49b29bf988..5fbe28a833 100644 --- a/deploy/aws-nlb/deploy.yaml +++ b/deploy/aws-nlb/deploy.yaml @@ -248,8 +248,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -280,8 +280,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -302,7 +302,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} diff --git a/deploy/azure/deploy.yaml b/deploy/azure/deploy.yaml index 968c1a2926..c9886ff1d0 100644 --- a/deploy/azure/deploy.yaml +++ b/deploy/azure/deploy.yaml @@ -245,8 +245,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -277,8 +277,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -301,7 +301,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} diff --git a/deploy/default/deploy.yaml b/deploy/default/deploy.yaml index 6245a2bbc7..a0f4adf0ce 100644 --- a/deploy/default/deploy.yaml +++ b/deploy/default/deploy.yaml @@ -245,8 +245,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -277,8 +277,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -299,7 +299,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} diff --git a/deploy/experimental-nginx-plus/deploy.yaml b/deploy/experimental-nginx-plus/deploy.yaml index ed9c748e1a..0d04cf0b58 100644 --- a/deploy/experimental-nginx-plus/deploy.yaml +++ b/deploy/experimental-nginx-plus/deploy.yaml @@ -260,8 +260,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -292,8 +292,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -314,7 +314,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} diff --git a/deploy/experimental/deploy.yaml b/deploy/experimental/deploy.yaml index 28cc7b6d19..0f9227ce5a 100644 --- a/deploy/experimental/deploy.yaml +++ b/deploy/experimental/deploy.yaml @@ -251,8 +251,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -283,8 +283,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -305,7 +305,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} diff --git a/deploy/nginx-plus/deploy.yaml b/deploy/nginx-plus/deploy.yaml index 76249e80c2..82f6ac0173 100644 --- a/deploy/nginx-plus/deploy.yaml +++ b/deploy/nginx-plus/deploy.yaml @@ -256,8 +256,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -288,8 +288,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -310,7 +310,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} diff --git a/deploy/nodeport/deploy.yaml b/deploy/nodeport/deploy.yaml index db81fdf259..1deb34bf6c 100644 --- a/deploy/nodeport/deploy.yaml +++ b/deploy/nodeport/deploy.yaml @@ -245,8 +245,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -277,8 +277,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -299,7 +299,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} diff --git a/deploy/openshift/deploy.yaml b/deploy/openshift/deploy.yaml index cb78ce0f39..fa081d5cb1 100644 --- a/deploy/openshift/deploy.yaml +++ b/deploy/openshift/deploy.yaml @@ -253,8 +253,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -285,8 +285,8 @@ spec: name: nginx-conf - mountPath: /etc/nginx/stream-conf.d name: nginx-stream-conf - - mountPath: /etc/nginx/module-includes - name: module-includes + - mountPath: /etc/nginx/main-includes + name: main-includes - mountPath: /etc/nginx/secrets name: nginx-secrets - mountPath: /var/run/nginx @@ -307,7 +307,7 @@ spec: - emptyDir: {} name: nginx-stream-conf - emptyDir: {} - name: module-includes + name: main-includes - emptyDir: {} name: nginx-secrets - emptyDir: {} From edd501d6fb4b92c31d076b662f94c4e66eaa65cc Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 11:40:34 -0700 Subject: [PATCH 08/20] Make change to nginx plus conf file --- internal/mode/static/nginx/conf/nginx-plus.conf | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/mode/static/nginx/conf/nginx-plus.conf b/internal/mode/static/nginx/conf/nginx-plus.conf index 23d97717c9..77526ac8ad 100644 --- a/internal/mode/static/nginx/conf/nginx-plus.conf +++ b/internal/mode/static/nginx/conf/nginx-plus.conf @@ -1,10 +1,9 @@ load_module /usr/lib/nginx/modules/ngx_http_js_module.so; -include /etc/nginx/module-includes/*.conf; +include /etc/nginx/main-includes/*.conf; worker_processes auto; pid /var/run/nginx/nginx.pid; -error_log stderr info; events { worker_connections 1024; From 4a5c1fcdfdc9698521dee2e8a55e4d7c1ece74ce Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 12:11:55 -0700 Subject: [PATCH 09/20] Add unit test for main_includes --- .../static/nginx/config/main_includes_test.go | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 internal/mode/static/nginx/config/main_includes_test.go diff --git a/internal/mode/static/nginx/config/main_includes_test.go b/internal/mode/static/nginx/config/main_includes_test.go new file mode 100644 index 0000000000..6fec3b5c6f --- /dev/null +++ b/internal/mode/static/nginx/config/main_includes_test.go @@ -0,0 +1,70 @@ +package config + +import ( + "strings" + "testing" + + . "github.com/onsi/gomega" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" +) + +func TestExecuteMainIncludesConfig(t *testing.T) { + // Configuration.Logging will always be set, so no need to test if it is missing + t.Parallel() + + completeConfiguration := dataplane.Configuration{ + Telemetry: dataplane.Telemetry{ + Endpoint: "1.2.3.4:123", + ServiceName: "ngf:gw-ns:gw-name:my-name", + Interval: "5s", + BatchSize: 512, + BatchCount: 4, + }, + Logging: dataplane.Logging{ + ErrorLevel: "info", + }, + } + + missingTelemetryEndpoint := dataplane.Configuration{ + Logging: dataplane.Logging{ + ErrorLevel: "info", + }, + } + + tests := []struct { + name string + conf dataplane.Configuration + expTelemetryEndpointCount int + }{ + { + name: "complete configuration", + conf: completeConfiguration, + expTelemetryEndpointCount: 1, + }, + { + name: "missing telemetry endpoint", + conf: missingTelemetryEndpoint, + expTelemetryEndpointCount: 0, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + res := executeMainIncludesConfig(test.conf) + + g.Expect(strings.Count( + string(res.data), + "load_module modules/ngx_otel_module.so;"), + ).To(Equal(test.expTelemetryEndpointCount)) + + g.Expect(strings.Count( + string(res.data), + "error_log stderr "+test.conf.Logging.ErrorLevel+";", + )).To(Equal(1)) + }) + } +} From c678cff20d8c42a573cb76685d532add5a98d91c Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 13:00:40 -0700 Subject: [PATCH 10/20] Move small comment --- internal/mode/static/nginx/config/main_includes_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/nginx/config/main_includes_test.go b/internal/mode/static/nginx/config/main_includes_test.go index 6fec3b5c6f..076fb4550d 100644 --- a/internal/mode/static/nginx/config/main_includes_test.go +++ b/internal/mode/static/nginx/config/main_includes_test.go @@ -10,7 +10,6 @@ import ( ) func TestExecuteMainIncludesConfig(t *testing.T) { - // Configuration.Logging will always be set, so no need to test if it is missing t.Parallel() completeConfiguration := dataplane.Configuration{ @@ -32,6 +31,7 @@ func TestExecuteMainIncludesConfig(t *testing.T) { }, } + // Configuration.Logging will always be set, so no need to test if it is missing tests := []struct { name string conf dataplane.Configuration From 955dd0835647bcd5efc41d6504c47655fdddbadc Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 14:00:28 -0700 Subject: [PATCH 11/20] Add configuration tests --- .../static/state/dataplane/configuration.go | 7 +- .../state/dataplane/configuration_test.go | 190 +++++++++++++++++- 2 files changed, 191 insertions(+), 6 deletions(-) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index f26968c2ae..6711d42f27 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -902,13 +902,16 @@ func convertAddresses(addresses []ngfAPI.Address) []string { func buildLogging(g *graph.Graph) Logging { defaultErrorLogLevel := "info" + logSettings := Logging{ErrorLevel: defaultErrorLogLevel} ngfProxy := g.NginxProxy if ngfProxy != nil && ngfProxy.Source.Spec.Logging != nil { - return Logging{ErrorLevel: string(*ngfProxy.Source.Spec.Logging.ErrorLevel)} + if ngfProxy.Source.Spec.Logging.ErrorLevel != nil { + logSettings.ErrorLevel = string(*ngfProxy.Source.Spec.Logging.ErrorLevel) + } } - return Logging{ErrorLevel: defaultErrorLogLevel} + return logSettings } func GetDefaultConfiguration(configVersion int) Configuration { diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 07b6d40946..937f14d8ba 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -22,7 +22,7 @@ import ( ngfAPI "github.com/nginxinc/nginx-gateway-fabric/apis/v1alpha1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies" - policiesfakes "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/policiesfakes" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config/policies/policiesfakes" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/resolver/resolverfakes" @@ -61,6 +61,9 @@ func getExpectedConfiguration() Configuration { }, }, CertBundles: map[CertBundleID]CertBundle{}, + Logging: Logging{ + ErrorLevel: "info", + }, } } @@ -1493,7 +1496,7 @@ func TestBuildConfiguration(t *testing.T) { } return g }), - expConf: Configuration{}, + expConf: Configuration{Logging: Logging{ErrorLevel: "info"}}, msg: "invalid gatewayclass", }, { @@ -1512,7 +1515,7 @@ func TestBuildConfiguration(t *testing.T) { } return g }), - expConf: Configuration{}, + expConf: Configuration{Logging: Logging{ErrorLevel: "info"}}, msg: "missing gatewayclass", }, { @@ -1520,7 +1523,7 @@ func TestBuildConfiguration(t *testing.T) { g.Gateway = nil return g }), - expConf: Configuration{}, + expConf: Configuration{Logging: Logging{ErrorLevel: "info"}}, msg: "missing gateway", }, { @@ -2228,6 +2231,36 @@ func TestBuildConfiguration(t *testing.T) { }), msg: "NginxProxy with rewriteClientIP details set", }, + { + graph: getModifiedGraph(func(g *graph.Graph) *graph.Graph { + g.Gateway.Source.ObjectMeta = metav1.ObjectMeta{ + Name: "gw", + Namespace: "ns", + } + g.Gateway.Listeners = append(g.Gateway.Listeners, &graph.Listener{ + Name: "listener-80-1", + Source: listener80, + Valid: true, + Routes: map[graph.RouteKey]*graph.L7Route{}, + }) + g.NginxProxy = &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelDebug)}, + }, + }, + } + return g + }), + expConf: getModifiedExpectedConfiguration(func(conf Configuration) Configuration { + conf.SSLServers = []VirtualServer{} + conf.SSLKeyPairs = map[SSLKeyPairID]SSLKeyPair{} + conf.Logging = Logging{ErrorLevel: "debug"} + return conf + }), + msg: "NginxProxy with error log level set to debug", + }, } for _, test := range tests { @@ -2252,6 +2285,7 @@ func TestBuildConfiguration(t *testing.T) { g.Expect(result.CertBundles).To(Equal(test.expConf.CertBundles)) g.Expect(result.Telemetry).To(Equal(test.expConf.Telemetry)) g.Expect(result.BaseHTTPConfig).To(Equal(test.expConf.BaseHTTPConfig)) + g.Expect(result.Logging).To(Equal(test.expConf.Logging)) }) } } @@ -3744,3 +3778,151 @@ func TestBuildRewriteIPSettings(t *testing.T) { }) } } + +func TestBuildLogging(t *testing.T) { + t.Parallel() + tests := []struct { + msg string + g *graph.Graph + expLoggingSettings Logging + }{ + { + msg: "NginxProxy is nil", + g: &graph.Graph{}, + expLoggingSettings: Logging{ErrorLevel: "info"}, + }, + { + msg: "NginxProxy does not specify log level", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{}, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "info"}, + }, + { + msg: "NginxProxy log level set to debug", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelDebug)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "debug"}, + }, + { + msg: "NginxProxy log level set to info", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelInfo)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "info"}, + }, + { + msg: "NginxProxy log level set to notice", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelNotice)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "notice"}, + }, + { + msg: "NginxProxy log level set to warn", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelWarn)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "warn"}, + }, + { + msg: "NginxProxy log level set to error", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelError)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "error"}, + }, + { + msg: "NginxProxy log level set to crit", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelCrit)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "crit"}, + }, + { + msg: "NginxProxy log level set to alert", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelAlert)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "alert"}, + }, + { + msg: "NginxProxy log level set to emerg", + g: &graph.Graph{ + NginxProxy: &graph.NginxProxy{ + Valid: true, + Source: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelEmerg)}, + }, + }, + }, + }, + expLoggingSettings: Logging{ErrorLevel: "emerg"}, + }, + } + + for _, tc := range tests { + t.Run(tc.msg, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + logging := buildLogging(tc.g) + g.Expect(logging).To(Equal(tc.expLoggingSettings)) + }) + } +} From 832acf986a8349b9b274a68429c38b6beb9f858c Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 19 Sep 2024 14:45:10 -0700 Subject: [PATCH 12/20] Add nginxproxy tests --- .../mode/static/state/graph/nginxproxy.go | 43 +++--- .../static/state/graph/nginxproxy_test.go | 145 ++++++++++++++++++ 2 files changed, 168 insertions(+), 20 deletions(-) diff --git a/internal/mode/static/state/graph/nginxproxy.go b/internal/mode/static/state/graph/nginxproxy.go index 6647f7527c..1ec2fb2be5 100644 --- a/internal/mode/static/state/graph/nginxproxy.go +++ b/internal/mode/static/state/graph/nginxproxy.go @@ -143,27 +143,30 @@ func validateLogging(npCfg *ngfAPI.NginxProxy) field.ErrorList { if npCfg.Spec.Logging != nil { logging := npCfg.Spec.Logging loggingPath := spec.Child("logging") - errLevel := string(*logging.ErrorLevel) - - validLogLevels := []string{ - string(ngfAPI.NginxLogLevelDebug), - string(ngfAPI.NginxLogLevelInfo), - string(ngfAPI.NginxLogLevelNotice), - string(ngfAPI.NginxLogLevelWarn), - string(ngfAPI.NginxLogLevelError), - string(ngfAPI.NginxLogLevelCrit), - string(ngfAPI.NginxLogLevelAlert), - string(ngfAPI.NginxLogLevelEmerg), - } - if !slices.Contains(validLogLevels, errLevel) { - allErrs = append( - allErrs, - field.NotSupported( - loggingPath, - logging, - validLogLevels, - )) + if logging.ErrorLevel != nil { + errLevel := string(*logging.ErrorLevel) + + validLogLevels := []string{ + string(ngfAPI.NginxLogLevelDebug), + string(ngfAPI.NginxLogLevelInfo), + string(ngfAPI.NginxLogLevelNotice), + string(ngfAPI.NginxLogLevelWarn), + string(ngfAPI.NginxLogLevelError), + string(ngfAPI.NginxLogLevelCrit), + string(ngfAPI.NginxLogLevelAlert), + string(ngfAPI.NginxLogLevelEmerg), + } + + if !slices.Contains(validLogLevels, errLevel) { + allErrs = append( + allErrs, + field.NotSupported( + loggingPath.Child("errorlevel"), + logging.ErrorLevel, + validLogLevels, + )) + } } } diff --git a/internal/mode/static/state/graph/nginxproxy_test.go b/internal/mode/static/state/graph/nginxproxy_test.go index 6d547a1cb8..5a907e1f16 100644 --- a/internal/mode/static/state/graph/nginxproxy_test.go +++ b/internal/mode/static/state/graph/nginxproxy_test.go @@ -562,3 +562,148 @@ func TestValidateRewriteClientIP(t *testing.T) { }) } } + +func TestValidateLogging(t *testing.T) { + t.Parallel() + invalidLogLevel := ngfAPI.NginxErrorLogLevel("invalid-log-level") + + tests := []struct { + np *ngfAPI.NginxProxy + name string + errorString string + expectErrCount int + }{ + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelDebug), + }, + }, + }, + name: "valid debug log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelInfo), + }, + }, + }, + name: "valid info log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelNotice), + }, + }, + }, + name: "valid notice log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelWarn), + }, + }, + }, + name: "valid warn log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelError), + }, + }, + }, + name: "valid error log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelCrit), + }, + }, + }, + name: "valid crit log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelAlert), + }, + }, + }, + name: "valid alert log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: helpers.GetPointer(ngfAPI.NginxLogLevelEmerg), + }, + }, + }, + name: "valid emerg log level", + errorString: "", + expectErrCount: 0, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{ + ErrorLevel: &invalidLogLevel, + }, + }, + }, + name: "invalid log level", + errorString: "spec.logging.errorlevel: Unsupported value: \"invalid-log-level\": supported values:" + + " \"debug\", \"info\", \"notice\", \"warn\", \"error\", \"crit\", \"alert\", \"emerg\"", + expectErrCount: 1, + }, + { + np: &ngfAPI.NginxProxy{ + Spec: ngfAPI.NginxProxySpec{ + Logging: &ngfAPI.NginxLogging{}, + }, + }, + name: "empty log level", + errorString: "", + expectErrCount: 0, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + g := NewWithT(t) + + allErrs := validateLogging(test.np) + g.Expect(allErrs).To(HaveLen(test.expectErrCount)) + if len(allErrs) > 0 { + g.Expect(allErrs.ToAggregate().Error()).To(Equal(test.errorString)) + } + }) + } +} From 3be58d9d14d520420c3b2b6b30bbd352dc866bcb Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Fri, 20 Sep 2024 10:26:38 -0700 Subject: [PATCH 13/20] Refactor generator to run all executeFuncs --- .../mode/static/nginx/config/generator.go | 31 +++---------------- .../mode/static/nginx/config/main_includes.go | 4 +-- .../static/nginx/config/main_includes_test.go | 6 ++-- internal/mode/static/nginx/config/version.go | 10 ++++-- .../mode/static/nginx/config/version_test.go | 8 ++++- 5 files changed, 25 insertions(+), 34 deletions(-) diff --git a/internal/mode/static/nginx/config/generator.go b/internal/mode/static/nginx/config/generator.go index 912f596731..a1216a1c3c 100644 --- a/internal/mode/static/nginx/config/generator.go +++ b/internal/mode/static/nginx/config/generator.go @@ -99,16 +99,12 @@ func (g GeneratorImpl) Generate(conf dataplane.Configuration) []file.File { observability.NewGenerator(conf.Telemetry), ) - files = append(files, g.generateHTTPConfig(conf, policyGenerator)...) - - files = append(files, generateConfigVersion(conf.Version)) + files = append(files, g.runExecuteFuncs(conf, policyGenerator)...) for id, bundle := range conf.CertBundles { files = append(files, generateCertBundle(id, bundle)) } - files = append(files, generateMainContextConf(conf)) - return files } @@ -141,7 +137,7 @@ func generateCertBundleFileName(id dataplane.CertBundleID) string { return filepath.Join(secretsFolder, string(id)+".crt") } -func (g GeneratorImpl) generateHTTPConfig( +func (g GeneratorImpl) runExecuteFuncs( conf dataplane.Configuration, generator policies.Generator, ) []file.File { @@ -177,26 +173,7 @@ func (g GeneratorImpl) getExecuteFuncs(generator policies.Generator) []executeFu g.executeStreamServers, g.executeStreamUpstreams, executeStreamMaps, - } -} - -// generateConfigVersion writes the config version file. -func generateConfigVersion(configVersion int) file.File { - c := executeVersion(configVersion) - - return file.File{ - Content: c, - Path: configVersionFile, - Type: file.TypeRegular, - } -} - -func generateMainContextConf(conf dataplane.Configuration) file.File { - result := executeMainIncludesConfig(conf) - - return file.File{ - Content: result.data, - Path: result.dest, - Type: file.TypeRegular, + executeVersion, + executeMainIncludesConfig, } } diff --git a/internal/mode/static/nginx/config/main_includes.go b/internal/mode/static/nginx/config/main_includes.go index ac1e05c51e..04e3675dfc 100644 --- a/internal/mode/static/nginx/config/main_includes.go +++ b/internal/mode/static/nginx/config/main_includes.go @@ -9,11 +9,11 @@ import ( var mainIncludesTemplate = gotemplate.Must(gotemplate.New("mainIncludes").Parse(mainIncludesTemplateText)) -func executeMainIncludesConfig(conf dataplane.Configuration) executeResult { +func executeMainIncludesConfig(conf dataplane.Configuration) []executeResult { result := executeResult{ dest: mainIncludesConfigFile, data: helpers.MustExecuteTemplate(mainIncludesTemplate, conf), } - return result + return []executeResult{result} } diff --git a/internal/mode/static/nginx/config/main_includes_test.go b/internal/mode/static/nginx/config/main_includes_test.go index 076fb4550d..b5f26f91fe 100644 --- a/internal/mode/static/nginx/config/main_includes_test.go +++ b/internal/mode/static/nginx/config/main_includes_test.go @@ -56,13 +56,15 @@ func TestExecuteMainIncludesConfig(t *testing.T) { res := executeMainIncludesConfig(test.conf) + g.Expect(res).To(HaveLen(1)) + g.Expect(strings.Count( - string(res.data), + string(res[0].data), "load_module modules/ngx_otel_module.so;"), ).To(Equal(test.expTelemetryEndpointCount)) g.Expect(strings.Count( - string(res.data), + string(res[0].data), "error_log stderr "+test.conf.Logging.ErrorLevel+";", )).To(Equal(1)) }) diff --git a/internal/mode/static/nginx/config/version.go b/internal/mode/static/nginx/config/version.go index 5baa7f24b8..6e29f36056 100644 --- a/internal/mode/static/nginx/config/version.go +++ b/internal/mode/static/nginx/config/version.go @@ -4,10 +4,16 @@ import ( gotemplate "text/template" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) var versionTemplate = gotemplate.Must(gotemplate.New("version").Parse(versionTemplateText)) -func executeVersion(version int) []byte { - return helpers.MustExecuteTemplate(versionTemplate, version) +func executeVersion(conf dataplane.Configuration) []executeResult { + result := executeResult{ + dest: configVersionFile, + data: helpers.MustExecuteTemplate(versionTemplate, conf.Version), + } + + return []executeResult{result} } diff --git a/internal/mode/static/nginx/config/version_test.go b/internal/mode/static/nginx/config/version_test.go index 176db3dfec..51008e625e 100644 --- a/internal/mode/static/nginx/config/version_test.go +++ b/internal/mode/static/nginx/config/version_test.go @@ -5,6 +5,8 @@ import ( "testing" . "github.com/onsi/gomega" + + "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/dataplane" ) func TestExecuteVersion(t *testing.T) { @@ -14,7 +16,11 @@ func TestExecuteVersion(t *testing.T) { "return 200 42;": 1, } - maps := string(executeVersion(42)) + cfg := dataplane.Configuration{ + Version: 42, + } + + maps := string(executeVersion(cfg)[0].data) for expSubStr, expCount := range expSubStrings { g.Expect(expCount).To(Equal(strings.Count(maps, expSubStr))) } From b3d1833b3d4a1cee0a1a1c55f4629b475a187dcc Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 23 Sep 2024 09:55:04 -0700 Subject: [PATCH 14/20] WIP --- examples/nginxproxy-example/nginxproxy-error-loglevel.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 examples/nginxproxy-example/nginxproxy-error-loglevel.yaml diff --git a/examples/nginxproxy-example/nginxproxy-error-loglevel.yaml b/examples/nginxproxy-example/nginxproxy-error-loglevel.yaml new file mode 100644 index 0000000000..b48600081a --- /dev/null +++ b/examples/nginxproxy-example/nginxproxy-error-loglevel.yaml @@ -0,0 +1,7 @@ +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + name: ngf-proxy-config +spec: + logging: + errorlevel: error From 53c62d553f195da6c1f0fb2fa68d82b4f474261f Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 23 Sep 2024 09:55:15 -0700 Subject: [PATCH 15/20] WIP --- .../how-to/data-plane-configuration.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/site/content/how-to/data-plane-configuration.md b/site/content/how-to/data-plane-configuration.md index 6274246dd7..7978ab4238 100644 --- a/site/content/how-to/data-plane-configuration.md +++ b/site/content/how-to/data-plane-configuration.md @@ -112,3 +112,24 @@ Status: ``` If everything is valid, the `ResolvedRefs` condition should be `True`. Otherwise, you will see an `InvalidParameters` condition in the status. + +## Dynamically Configure the Data Plane Log Level + +You can use the `NginxProxy` resource to dynamically configure the Data Plane Log Level. + +The following command creates a basic `NginxProxy` configuration that sets the log level to `warn` instead of the default value of `info`: + +```yaml +kubectl apply -f - < Date: Tue, 24 Sep 2024 09:30:34 -0700 Subject: [PATCH 16/20] Add functionality to run nginx in debug mode --- build/Dockerfile.nginx | 6 +- build/Dockerfile.nginxplus | 4 +- charts/nginx-gateway-fabric/README.md | 1 + .../templates/deployment.yaml | 7 + charts/nginx-gateway-fabric/values.yaml | 5 + deploy/nginx-debug/deploy.yaml | 355 ++++++++++++++++++ examples/helm/nginx-debug/values.yaml | 8 + 7 files changed, 381 insertions(+), 5 deletions(-) create mode 100644 deploy/nginx-debug/deploy.yaml create mode 100644 examples/helm/nginx-debug/values.yaml diff --git a/build/Dockerfile.nginx b/build/Dockerfile.nginx index 1e375bfe9e..446808c9c0 100644 --- a/build/Dockerfile.nginx +++ b/build/Dockerfile.nginx @@ -7,8 +7,8 @@ ARG BUILD_AGENT RUN apk add --no-cache libcap \ && mkdir -p /var/lib/nginx /usr/lib/nginx/modules \ - && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx \ - && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx \ + && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ + && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && apk del libcap COPY ${NJS_DIR}/httpmatches.js /usr/lib/nginx/modules/njs/httpmatches.js @@ -22,4 +22,4 @@ LABEL org.nginx.ngf.image.build.agent="${BUILD_AGENT}" USER 101:1001 -CMD ["sh", "-c", "rm -rf /var/run/nginx/*.sock && /docker-entrypoint.sh nginx -g 'daemon off;'"] +CMD ["sh", "-c", "rm -rf /var/run/nginx/*.sock && nginx -g 'daemon off;'"] diff --git a/build/Dockerfile.nginxplus b/build/Dockerfile.nginxplus index f1957f1921..d439663e3f 100644 --- a/build/Dockerfile.nginxplus +++ b/build/Dockerfile.nginxplus @@ -20,8 +20,8 @@ RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/apk/cert.pem,mode=0644 \ && printf "%s\n" "https://pkgs.nginx.com/plus/${NGINX_PLUS_VERSION}/alpine/v$(grep -E -o '^[0-9]+\.[0-9]+' /etc/alpine-release)/main" >> /etc/apk/repositories \ && apk add --no-cache nginx-plus nginx-plus-module-njs nginx-plus-module-otel libcap \ && mkdir -p /var/lib/nginx /usr/lib/nginx/modules \ - && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx \ - && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx \ + && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ + && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && apk del libcap \ # forward request and error logs to docker log collector && ln -sf /dev/stdout /var/log/nginx/access.log \ diff --git a/charts/nginx-gateway-fabric/README.md b/charts/nginx-gateway-fabric/README.md index 9ca6cb9073..43ef977a71 100644 --- a/charts/nginx-gateway-fabric/README.md +++ b/charts/nginx-gateway-fabric/README.md @@ -261,6 +261,7 @@ The following table lists the configurable parameters of the NGINX Gateway Fabri | `metrics.port` | Set the port where the Prometheus metrics are exposed. Format: [1024 - 65535] | int | `9113` | | `metrics.secure` | Enable serving metrics via https. By default metrics are served via http. Please note that this endpoint will be secured with a self-signed certificate. | bool | `false` | | `nginx.config` | The configuration for the data plane that is contained in the NginxProxy resource. | object | `{}` | +| `nginx.debug` | Is NGINX run in debug mode. This should be used with setting the NGINX error log level to debug. | bool | `false` | | `nginx.extraVolumeMounts` | extraVolumeMounts are the additional volume mounts for the nginx container. | list | `[]` | | `nginx.image.pullPolicy` | | string | `"Always"` | | `nginx.image.repository` | The NGINX image to use. | string | `"ghcr.io/nginxinc/nginx-gateway-fabric/nginx"` | diff --git a/charts/nginx-gateway-fabric/templates/deployment.yaml b/charts/nginx-gateway-fabric/templates/deployment.yaml index f99f67e415..f9f5071885 100644 --- a/charts/nginx-gateway-fabric/templates/deployment.yaml +++ b/charts/nginx-gateway-fabric/templates/deployment.yaml @@ -183,6 +183,13 @@ spec: {{- with .Values.nginx.extraVolumeMounts -}} {{ toYaml . | nindent 8 }} {{- end }} + {{- if .Values.nginx.debug}} + command: + - "/bin/sh" + args: + - "-c" + - "rm -rf /var/run/nginx/*.sock && nginx-debug -g 'daemon off;'" + {{- end }} terminationGracePeriodSeconds: {{ .Values.terminationGracePeriodSeconds }} {{- if .Values.affinity }} affinity: diff --git a/charts/nginx-gateway-fabric/values.yaml b/charts/nginx-gateway-fabric/values.yaml index b81fb9063d..57cb2a96aa 100644 --- a/charts/nginx-gateway-fabric/values.yaml +++ b/charts/nginx-gateway-fabric/values.yaml @@ -88,6 +88,9 @@ nginx: # -- Is NGINX Plus image being used plus: false + # -- Is NGINX run in debug mode. This should be used with setting the NGINX error log level to debug. + debug: false + # -- The configuration for the data plane that is contained in the NginxProxy resource. config: {} @@ -112,6 +115,8 @@ nginx: # batchCount: 4 # serviceName: "" # spanAttributes: [] + # logging: + # errorlevel: info # Configuration for NGINX Plus usage reporting. usage: diff --git a/deploy/nginx-debug/deploy.yaml b/deploy/nginx-debug/deploy.yaml new file mode 100644 index 0000000000..1673670fdd --- /dev/null +++ b/deploy/nginx-debug/deploy.yaml @@ -0,0 +1,355 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: nginx-gateway +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway +rules: +- apiGroups: + - "" + resources: + - namespaces + - services + - secrets + verbs: + - get + - list + - watch +- apiGroups: + - "" + resources: + - pods + verbs: + - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get +- apiGroups: + - "" + resources: + - nodes + verbs: + - list +- apiGroups: + - "" + resources: + - events + verbs: + - create + - patch +- apiGroups: + - discovery.k8s.io + resources: + - endpointslices + verbs: + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - gatewayclasses + - gateways + - httproutes + - referencegrants + - grpcroutes + verbs: + - list + - watch +- apiGroups: + - gateway.networking.k8s.io + resources: + - httproutes/status + - gateways/status + - gatewayclasses/status + - grpcroutes/status + verbs: + - update +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways + verbs: + - get + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxproxies + - clientsettingspolicies + - observabilitypolicies + verbs: + - list + - watch +- apiGroups: + - gateway.nginx.org + resources: + - nginxgateways/status + - clientsettingspolicies/status + - observabilitypolicies/status + verbs: + - update +- apiGroups: + - coordination.k8s.io + resources: + - leases + verbs: + - create + - get + - update +- apiGroups: + - apiextensions.k8s.io + resources: + - customresourcedefinitions + verbs: + - list + - watch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: nginx-gateway +subjects: +- kind: ServiceAccount + name: nginx-gateway + namespace: nginx-gateway +--- +apiVersion: v1 +kind: Service +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +spec: + externalTrafficPolicy: Local + ports: + - name: http + port: 80 + protocol: TCP + targetPort: 80 + - name: https + port: 443 + protocol: TCP + targetPort: 443 + selector: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + type: LoadBalancer +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway + namespace: nginx-gateway +spec: + replicas: 1 + selector: + matchLabels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + template: + metadata: + annotations: + prometheus.io/port: "9113" + prometheus.io/scrape: "true" + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + spec: + containers: + - args: + - static-mode + - --gateway-ctlr-name=gateway.nginx.org/nginx-gateway-controller + - --gatewayclass=nginx + - --config=nginx-gateway-config + - --service=nginx-gateway + - --metrics-port=9113 + - --health-port=8081 + - --leader-election-lock-name=nginx-gateway-leader-election + env: + - name: POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: POD_NAMESPACE + valueFrom: + fieldRef: + fieldPath: metadata.namespace + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + image: ghcr.io/nginxinc/nginx-gateway-fabric:edge + imagePullPolicy: Always + name: nginx-gateway + ports: + - containerPort: 9113 + name: metrics + - containerPort: 8081 + name: health + readinessProbe: + httpGet: + path: /readyz + port: health + initialDelaySeconds: 3 + periodSeconds: 1 + securityContext: + allowPrivilegeEscalation: false + capabilities: + add: + - KILL + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 102 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf + - mountPath: /etc/nginx/main-includes + name: main-includes + - mountPath: /etc/nginx/secrets + name: nginx-secrets + - mountPath: /var/run/nginx + name: nginx-run + - mountPath: /etc/nginx/includes + name: nginx-includes + - args: + - -c + - rm -rf /var/run/nginx/*.sock && nginx-debug -g 'daemon off;' + command: + - /bin/sh + image: ghcr.io/nginxinc/nginx-gateway-fabric/nginx:edge + imagePullPolicy: Always + name: nginx + ports: + - containerPort: 80 + name: http + - containerPort: 443 + name: https + securityContext: + capabilities: + add: + - NET_BIND_SERVICE + drop: + - ALL + readOnlyRootFilesystem: true + runAsGroup: 1001 + runAsUser: 101 + seccompProfile: + type: RuntimeDefault + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: nginx-conf + - mountPath: /etc/nginx/stream-conf.d + name: nginx-stream-conf + - mountPath: /etc/nginx/main-includes + name: main-includes + - mountPath: /etc/nginx/secrets + name: nginx-secrets + - mountPath: /var/run/nginx + name: nginx-run + - mountPath: /var/cache/nginx + name: nginx-cache + - mountPath: /etc/nginx/includes + name: nginx-includes + securityContext: + fsGroup: 1001 + runAsNonRoot: true + serviceAccountName: nginx-gateway + shareProcessNamespace: true + terminationGracePeriodSeconds: 30 + volumes: + - emptyDir: {} + name: nginx-conf + - emptyDir: {} + name: nginx-stream-conf + - emptyDir: {} + name: main-includes + - emptyDir: {} + name: nginx-secrets + - emptyDir: {} + name: nginx-run + - emptyDir: {} + name: nginx-cache + - emptyDir: {} + name: nginx-includes +--- +apiVersion: gateway.networking.k8s.io/v1 +kind: GatewayClass +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx +spec: + controllerName: gateway.nginx.org/nginx-gateway-controller + parametersRef: + group: gateway.nginx.org + kind: NginxProxy + name: nginx-gateway-proxy-config +--- +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxGateway +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway-config + namespace: nginx-gateway +spec: + logging: + level: info +--- +apiVersion: gateway.nginx.org/v1alpha1 +kind: NginxProxy +metadata: + labels: + app.kubernetes.io/instance: nginx-gateway + app.kubernetes.io/name: nginx-gateway + app.kubernetes.io/version: edge + name: nginx-gateway-proxy-config +spec: + logging: + errorlevel: debug diff --git a/examples/helm/nginx-debug/values.yaml b/examples/helm/nginx-debug/values.yaml new file mode 100644 index 0000000000..b8086e8275 --- /dev/null +++ b/examples/helm/nginx-debug/values.yaml @@ -0,0 +1,8 @@ +nginxGateway: + name: nginx-gateway + +nginx: + debug: true + config: + logging: + errorlevel: debug From 84f8b545520e467562ca336224dee0cb97794b21 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 24 Sep 2024 10:58:01 -0700 Subject: [PATCH 17/20] Add documentation on running NGINX in debug mode --- .../how-to/data-plane-configuration.md | 31 +++++++++++++++++-- .../installation/installing-ngf/manifests.md | 10 ++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/site/content/how-to/data-plane-configuration.md b/site/content/how-to/data-plane-configuration.md index 7978ab4238..d06a83615a 100644 --- a/site/content/how-to/data-plane-configuration.md +++ b/site/content/how-to/data-plane-configuration.md @@ -131,5 +131,32 @@ spec: EOF ``` -After following the same instructions shown in Manually Creating The Configuration of attaching the NginxProxy to the GatewayClass, -the log level of the data plane should be updated to `warn`. +After attaching the NginxProxy to the GatewayClass, the log level of the data plane will be updated to `warn`. + +To view the full list of supported log levels, see the `NginxProxy spec` in the [API reference]({{< relref "reference/api.md" >}}) + +{{< note >}}For `debug` logging to work, NGINX needs to be built with `--with-debug` or "in debug mode". For more information visit the official NGINX documentation +["A debugging log"](https://nginx.org/en/docs/debugging_log.html). NGINX Gateway Fabric can easily +be [run with NGINX in debug mode](#run-nginx-gateway-fabric-with-nginx-in-debug-mode) upon startup through the addition +of a few arguments. {{}} + +## Run NGINX Gateway Fabric with NGINX in debug mode + +To run NGINX Gateway Fabric with NGINX in debug mode, follow the [installation document]({{< relref "installation/installing-ngf" >}}) with these additional steps: + +Using Helm: Set `nginx.debug` to true. + +Using Kubernetes Manifests: Under the `nginx` container of the deployment manifest, add `-c` and `rm -rf /var/run/nginx/*.sock && nginx-debug -g 'daemon off;'` +as arguments and add `/bin/sh` as the command. The deployment manifest should look something like this: + +```text +... +- args: + - -c + - rm -rf /var/run/nginx/*.sock && nginx-debug -g 'daemon off;' + command: + - /bin/sh +... +``` + +To view a full example, look at our generated [deployment manifest](https://github.com/nginxinc/nginx-gateway-fabric/blob/main/deploy/nginx-debug/deploy.yaml). diff --git a/site/content/installation/installing-ngf/manifests.md b/site/content/installation/installing-ngf/manifests.md index 8ed98732ed..e27d00c0b9 100644 --- a/site/content/installation/installing-ngf/manifests.md +++ b/site/content/installation/installing-ngf/manifests.md @@ -138,6 +138,16 @@ kubectl apply -f https://raw.githubusercontent.com/nginxinc/nginx-gateway-fabric {{% /tab %}} +{{%tab name="NGINX in debug mode"%}} + +Deploys NGINX Gateway Fabric with NGINX OSS in debug mode. + +```shell +kubectl apply -f https://raw.githubusercontent.com/nginxinc/nginx-gateway-fabric/v1.4.0/deploy/nginx-debug/deploy.yaml +``` + +{{% /tab %}} + {{}} ### 4. Verify the Deployment From 49b4bdc68eb8388f1644228ffb25ca1d53bd7220 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 24 Sep 2024 15:34:05 -0700 Subject: [PATCH 18/20] Add setcap command for both nginx and nginx-debug binaries --- build/Dockerfile.nginx | 2 ++ build/Dockerfile.nginxplus | 2 ++ 2 files changed, 4 insertions(+) diff --git a/build/Dockerfile.nginx b/build/Dockerfile.nginx index 446808c9c0..c8fafa3071 100644 --- a/build/Dockerfile.nginx +++ b/build/Dockerfile.nginx @@ -7,6 +7,8 @@ ARG BUILD_AGENT RUN apk add --no-cache libcap \ && mkdir -p /var/lib/nginx /usr/lib/nginx/modules \ + && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx \ + && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx \ && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && apk del libcap diff --git a/build/Dockerfile.nginxplus b/build/Dockerfile.nginxplus index d439663e3f..b87d86186e 100644 --- a/build/Dockerfile.nginxplus +++ b/build/Dockerfile.nginxplus @@ -20,6 +20,8 @@ RUN --mount=type=secret,id=nginx-repo.crt,dst=/etc/apk/cert.pem,mode=0644 \ && printf "%s\n" "https://pkgs.nginx.com/plus/${NGINX_PLUS_VERSION}/alpine/v$(grep -E -o '^[0-9]+\.[0-9]+' /etc/alpine-release)/main" >> /etc/apk/repositories \ && apk add --no-cache nginx-plus nginx-plus-module-njs nginx-plus-module-otel libcap \ && mkdir -p /var/lib/nginx /usr/lib/nginx/modules \ + && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx \ + && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx \ && setcap 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && setcap -v 'cap_net_bind_service=+ep' /usr/sbin/nginx-debug \ && apk del libcap \ From fb6518451b0bab0ddf847838d79a703dab1458a2 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 24 Sep 2024 15:41:56 -0700 Subject: [PATCH 19/20] Add small changes to data plane configuration document --- site/content/how-to/data-plane-configuration.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/content/how-to/data-plane-configuration.md b/site/content/how-to/data-plane-configuration.md index d06a83615a..b3597e133e 100644 --- a/site/content/how-to/data-plane-configuration.md +++ b/site/content/how-to/data-plane-configuration.md @@ -9,7 +9,7 @@ Learn how to dynamically update the NGINX Gateway Fabric global data plane confi ## Overview -NGINX Gateway Fabric can dynamically update the global data plane configuration without restarting. The data plane configuration is a global configuration for NGINX that has options that are not available using the standard Gateway API resources. This includes such things as setting an OpenTelemetry collector config, disabling http2, or changing the IP family. +NGINX Gateway Fabric can dynamically update the global data plane configuration without restarting. The data plane configuration is a global configuration for NGINX that has options that are not available using the standard Gateway API resources. This includes such things as setting an OpenTelemetry collector config, disabling http2, changing the IP family, or setting the NGINX error log level. The data plane configuration is stored in the NginxProxy custom resource, which is a cluster-scoped resource that is attached to the `nginx` GatewayClass. @@ -113,7 +113,7 @@ Status: If everything is valid, the `ResolvedRefs` condition should be `True`. Otherwise, you will see an `InvalidParameters` condition in the status. -## Dynamically Configure the Data Plane Log Level +## Configure the Data Plane Log Level You can use the `NginxProxy` resource to dynamically configure the Data Plane Log Level. From 9b6c854d64e60948143a455a7d481000c45bc0a8 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 24 Sep 2024 15:49:52 -0700 Subject: [PATCH 20/20] Add small refactoring to configuration --- .../static/state/dataplane/configuration.go | 27 +++++++------------ .../state/dataplane/configuration_test.go | 4 +-- 2 files changed, 12 insertions(+), 19 deletions(-) diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index 6711d42f27..bfcad2cb95 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -21,8 +21,9 @@ import ( ) const ( - wildcardHostname = "~^" - alpineSSLRootCAPath = "/etc/ssl/cert.pem" + wildcardHostname = "~^" + alpineSSLRootCAPath = "/etc/ssl/cert.pem" + defaultErrorLogLevel = "info" ) // BuildConfiguration builds the Configuration from the Graph. @@ -38,29 +39,22 @@ func BuildConfiguration( baseHTTPConfig := buildBaseHTTPConfig(g) - upstreams := buildUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily) httpServers, sslServers := buildServers(g) - passthroughServers := buildPassthroughServers(g) - streamUpstreams := buildStreamUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily) backendGroups := buildBackendGroups(append(httpServers, sslServers...)) - keyPairs := buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners) - certBundles := buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups) - telemetry := buildTelemetry(g) - logging := buildLogging(g) config := Configuration{ HTTPServers: httpServers, SSLServers: sslServers, - TLSPassthroughServers: passthroughServers, - Upstreams: upstreams, - StreamUpstreams: streamUpstreams, + TLSPassthroughServers: buildPassthroughServers(g), + Upstreams: buildUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily), + StreamUpstreams: buildStreamUpstreams(ctx, g.Gateway.Listeners, serviceResolver, baseHTTPConfig.IPFamily), BackendGroups: backendGroups, - SSLKeyPairs: keyPairs, + SSLKeyPairs: buildSSLKeyPairs(g.ReferencedSecrets, g.Gateway.Listeners), Version: configVersion, - CertBundles: certBundles, - Telemetry: telemetry, + CertBundles: buildCertBundles(g.ReferencedCaCertConfigMaps, backendGroups), + Telemetry: buildTelemetry(g), BaseHTTPConfig: baseHTTPConfig, - Logging: logging, + Logging: buildLogging(g), } return config @@ -901,7 +895,6 @@ func convertAddresses(addresses []ngfAPI.Address) []string { } func buildLogging(g *graph.Graph) Logging { - defaultErrorLogLevel := "info" logSettings := Logging{ErrorLevel: defaultErrorLogLevel} ngfProxy := g.NginxProxy diff --git a/internal/mode/static/state/dataplane/configuration_test.go b/internal/mode/static/state/dataplane/configuration_test.go index 937f14d8ba..58bc4be64a 100644 --- a/internal/mode/static/state/dataplane/configuration_test.go +++ b/internal/mode/static/state/dataplane/configuration_test.go @@ -3921,8 +3921,8 @@ func TestBuildLogging(t *testing.T) { t.Run(tc.msg, func(t *testing.T) { t.Parallel() g := NewWithT(t) - logging := buildLogging(tc.g) - g.Expect(logging).To(Equal(tc.expLoggingSettings)) + + g.Expect(buildLogging(tc.g)).To(Equal(tc.expLoggingSettings)) }) } }