Add cli argument to configure NAP log level (#2479)

build/Dockerfile

@@ -219,16 +219,6 @@ RUN --mount=target=/tmp [ -n "${BUILD_OS##*plus*}" ] && exit 0; mkdir -p etc/ngi
 RUN --mount=target=/tmp [ -n "${NAP_MODULES##*waf*}" ] && exit 0; mkdir -p /etc/nginx/waf/nac-policies /etc/nginx/waf/nac-logconfs /etc/nginx/waf/nac-usersigs /var/log/app_protect /opt/app_protect \
 	&& chown -R nginx:0 /etc/app_protect /usr/share/ts /var/log/app_protect/ /opt/app_protect/ /var/log/nginx/ \
 	&& touch /etc/nginx/waf/nac-usersigs/index.conf \
-	&& printf "%s\n" "MODULE = ALL;" "LOG_LEVEL = TS_CRIT;" "FILE = 2;" > /etc/app_protect/bd/logger.cfg \
-	&& printf "%s\n" "[config_set_compiler]" "log_level=fatal" >> /etc/app_protect/tools/asm_logging.conf \
-	&& for v in \
-	asm_config_server \
-	lock_factory \
-	bd_agent \
-	import_export_policy \
-	set_active \
-	; do sed -i "/\[$v/a log_level=fatal" "/etc/app_protect/tools/asm_logging.conf" \
-	; done \
 	&& cp -a /tmp/build/log-default.json /etc/nginx
 
 # run only on nap dos build

cmd/nginx-ingress/main.go

@@ -71,6 +71,9 @@ var (
 
 	appProtect = flag.Bool("enable-app-protect", false, "Enable support for NGINX App Protect. Requires -nginx-plus.")
 
+	appProtectLogLevel = flag.String("app-protect-log-level", appProtectLogLevelDefault,
+		`Sets log level for App Protect. Allowed values: fatal, error, warn, info, debug, trace. Requires -nginx-plus and -enable-app-protect.`)
+
 	appProtectDos = flag.Bool("enable-app-protect-dos", false, "Enable support for NGINX App Protect dos. Requires -nginx-plus.")
 
 	appProtectDosDebug = flag.Bool("app-protect-dos-debug", false, "Enable debugging for App Protect Dos. Requires -nginx-plus and -enable-app-protect-dos.")
@@ -248,6 +251,17 @@ func main() {
 		glog.Fatal("NGINX App Protect support is for NGINX Plus only")
 	}
 
+	if *appProtectLogLevel != appProtectLogLevelDefault && !*appProtect && !*nginxPlus {
+		glog.Fatal("app-protect-log-level support is for NGINX Plus only and App Protect is enable")
+	}
+
+	if *appProtectLogLevel != appProtectLogLevelDefault && *appProtect && *nginxPlus {
+		logLevelValidationError := validateAppProtectLogLevel(*appProtectLogLevel)
+		if logLevelValidationError != nil {
+			glog.Fatalf("Invalid value for app-protect-log-level: %v", *appProtectLogLevel)
+		}
+	}
+
 	if *appProtectDos && !*nginxPlus {
 		glog.Fatal("NGINX App Protect Dos support is for NGINX Plus only")
 	}
@@ -449,7 +463,7 @@ func main() {
 		aPPluginDone = make(chan error, 1)
 		aPAgentDone = make(chan error, 1)
 
-		nginxManager.AppProtectAgentStart(aPAgentDone, *nginxDebug)
+		nginxManager.AppProtectAgentStart(aPAgentDone, *appProtectLogLevel)
 		nginxManager.AppProtectPluginStart(aPPluginDone)
 	}
 
@@ -783,6 +797,23 @@ func validatePort(port int) error {
 	return nil
 }
 
+const appProtectLogLevelDefault = "fatal"
+
+// validateAppProtectLogLevel makes sure a given logLevel is one of the allowed values
+func validateAppProtectLogLevel(logLevel string) error {
+	switch strings.ToLower(logLevel) {
+	case
+		"fatal",
+		"error",
+		"warn",
+		"info",
+		"debug",
+		"trace":
+		return nil
+	}
+	return fmt.Errorf("invalid App Protect log level: %v", logLevel)
+}
+
 // parseNginxStatusAllowCIDRs converts a comma separated CIDR/IP address string into an array of CIDR/IP addresses.
 // It returns an array of the valid CIDR/IP addresses or an error if given an invalid address.
 func parseNginxStatusAllowCIDRs(input string) (cidrs []string, err error) {

cmd/nginx-ingress/main_test.go

@@ -123,3 +123,33 @@ func TestValidateLocation(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateAppProtectLogLevel(t *testing.T) {
+	badLogLevels := []string{
+		"",
+		"critical",
+		"none",
+		"info;",
+	}
+	for _, badLogLevel := range badLogLevels {
+		err := validateAppProtectLogLevel(badLogLevel)
+		if err == nil {
+			t.Errorf("validateAppProtectLogLevel(%v) returned no error when it should have returned an error", badLogLevel)
+		}
+	}
+
+	goodLogLevels := []string{
+		"fatal",
+		"Error",
+		"WARN",
+		"info",
+		"debug",
+		"trace",
+	}
+	for _, goodLogLevel := range goodLogLevels {
+		err := validateAppProtectLogLevel(goodLogLevel)
+		if err != nil {
+			t.Errorf("validateAppProtectLogLevel(%v) returned an error when it should have returned no error: %v", goodLogLevel, err)
+		}
+	}
+}

deployments/helm-chart/templates/controller-daemonset.yaml

@@ -105,6 +105,9 @@ spec:
           - -nginx-plus={{ .Values.controller.nginxplus }}
           - -nginx-reload-timeout={{ .Values.controller.nginxReloadTimeout }}
           - -enable-app-protect={{ .Values.controller.appprotect.enable }}
+{{- if and .Values.controller.appprotect.enable .Values.controller.appprotect.logLevel }}
+          - -app-protect-log-level={{ .Values.controller.appprotect.logLevel }}
+{{ end }}
           - -enable-app-protect-dos={{ .Values.controller.appprotectdos.enable }}
   {{- if .Values.controller.appprotectdos.enable }}
           - -app-protect-dos-debug={{ .Values.controller.appprotectdos.debug }}

deployments/helm-chart/templates/controller-deployment.yaml

@@ -103,6 +103,9 @@ spec:
           - -nginx-plus={{ .Values.controller.nginxplus }}
           - -nginx-reload-timeout={{ .Values.controller.nginxReloadTimeout }}
           - -enable-app-protect={{ .Values.controller.appprotect.enable }}
+{{- if and .Values.controller.appprotect.enable .Values.controller.appprotect.logLevel }}
+          - -app-protect-log-level={{ .Values.controller.appprotect.logLevel }}
+{{ end }}
           - -enable-app-protect-dos={{ .Values.controller.appprotectdos.enable }}
 {{- if .Values.controller.appprotectdos.enable }}
           - -app-protect-dos-debug={{ .Values.controller.appprotectdos.debug }}

deployments/helm-chart/values.yaml

@@ -16,6 +16,8 @@ controller:
   appprotect:
     ## Enable the App Protect module in the Ingress Controller.
     enable: false
+    ## Sets log level for App Protect. Allowed values: fatal, error, warn, info, debug, trace
+    # logLevel: fatal
 
   ## Support for App Protect Dos
   appprotectdos:

docs/content/configuration/global-configuration/command-line-arguments.md

@@ -337,6 +337,17 @@ Requires [-nginx-plus](#cmdoption-nginx-plus).
 
 * If the argument is set, but `nginx-plus` is set to false, the Ingress Controller will fail to start.  
 
+   
+<a name="cmdoption-app-protect-log-level"></a.> 
+
+### -app-protect-log-level `<string>`
+
+Sets log level for App Protect. Allowed values: fatal, error, warn, info, debug, trace.
+
+Requires [-nginx-plus](#cmdoption-nginx-plus) and [-enable-app-protect](#cmdoption-enable-app-protect).
+
+* If the argument is set, but `nginx-plus` and `enable-app-protect` are set to false, the Ingress Controller will fail to start.  
+
 &nbsp;  
 <a name="cmdoption-enable-app-protect-dos"></a> 
 

internal/nginx/fake_manager.go

@@ -151,7 +151,7 @@ func (*FakeManager) SetOpenTracing(_ bool) {
 }
 
 // AppProtectAgentStart is a fake implementation of AppProtectAgentStart
-func (*FakeManager) AppProtectAgentStart(_ chan error, _ bool) {
+func (*FakeManager) AppProtectAgentStart(_ chan error, _ string) {
 	glog.V(3).Infof("Starting FakeAppProtectAgent")
 }
 

internal/nginx/manager.go

@@ -32,16 +32,11 @@ const (
 
 	appProtectPluginStartCmd = "/usr/share/ts/bin/bd-socket-plugin"
 	appProtectAgentStartCmd  = "/opt/app_protect/bin/bd_agent"
+	appProtectLogLevelCmd    = "/opt/app_protect/bin/set_log_level"
 
 	// appPluginParams is the configuration of App-Protect plugin
 	appPluginParams = "tmm_count 4 proc_cpuinfo_cpu_mhz 2000000 total_xml_memory 307200000 total_umu_max_size 3129344 sys_max_account_id 1024 no_static_config"
 
-	// appProtectDebugLogConfigFileContent holds the content of the file to be written when nginx debug is enabled. It will enable NGINX App Protect debug logs
-	appProtectDebugLogConfigFileContent = "MODULE = IO_PLUGIN;\nLOG_LEVEL = TS_INFO | TS_DEBUG;\nFILE = 2;\nMODULE = ECARD_POLICY;\nLOG_LEVEL = TS_INFO | TS_DEBUG;\nFILE = 2;\n"
-
-	// appProtectLogConfigFileName is the location of the NGINX App Protect logging configuration file
-	appProtectLogConfigFileName = "/etc/app_protect/bd/logger.cfg"
-
 	appProtectDosAgentInstallCmd    = "/usr/bin/adminstall"
 	appProtectDosAgentStartCmd      = "/usr/bin/admd -d --standalone"
 	appProtectDosAgentStartDebugCmd = "/usr/bin/admd -d --standalone --log debug"
@@ -81,7 +76,7 @@ type Manager interface {
 	UpdateServersInPlus(upstream string, servers []string, config ServerConfig) error
 	UpdateStreamServersInPlus(upstream string, servers []string) error
 	SetOpenTracing(openTracing bool)
-	AppProtectAgentStart(apaDone chan error, debug bool)
+	AppProtectAgentStart(apaDone chan error, logLevel string)
 	AppProtectAgentQuit()
 	AppProtectPluginStart(appDone chan error)
 	AppProtectPluginQuit()
@@ -461,20 +456,15 @@ func (lm *LocalManager) SetOpenTracing(openTracing bool) {
 }
 
 // AppProtectAgentStart starts the AppProtect agent
-func (lm *LocalManager) AppProtectAgentStart(apaDone chan error, debug bool) {
-	if debug {
-		glog.V(3).Info("Starting AppProtect Agent in debug mode")
-		err := os.Remove(appProtectLogConfigFileName)
-		if err != nil {
-			glog.Fatalf("Failed removing App Protect Log configuration file")
-		}
-		err = createFileAndWrite(appProtectLogConfigFileName, []byte(appProtectDebugLogConfigFileContent))
-		if err != nil {
-			glog.Fatalf("Failed Writing App Protect Log configuration file")
-		}
+func (lm *LocalManager) AppProtectAgentStart(apaDone chan error, logLevel string) {
+	glog.V(3).Info("Setting log level for App Protect - ", logLevel)
+	appProtectLogLevelCmdfull := fmt.Sprintf("%v %v", appProtectLogLevelCmd, logLevel)
+	logLevelCmd := exec.Command("sh", "-c", appProtectLogLevelCmdfull) // #nosec G204
+	if err := logLevelCmd.Run(); err != nil {
+		glog.Fatalf("Failed to set log level for AppProtect: %v", err)
 	}
-	glog.V(3).Info("Starting AppProtect Agent")
 
+	glog.V(3).Info("Starting AppProtect Agent")
 	cmd := exec.Command(appProtectAgentStartCmd)
 	if err := cmd.Start(); err != nil {
 		glog.Fatalf("Failed to start AppProtect Agent: %v", err)