Apdoslogconf will warning case format is not splunk (#2991)

deployments/common/crds/appprotectdos.f5.com_apdoslogconfs.yaml

@@ -38,13 +38,11 @@ spec:
                         - splunk
                         - arcsight
                         - user-defined
-                      default: splunk
                       type: string
                     format_string:
                       type: string
                     max_message_size:
                       pattern: ^([1-9]|[1-5][0-9]|6[0-4])k$
-                      default: 5k
                       type: string
                   type: object
                 filter:

deployments/helm-chart/crds/appprotectdos.f5.com_apdoslogconfs.yaml

@@ -38,13 +38,11 @@ spec:
                         - splunk
                         - arcsight
                         - user-defined
-                      default: splunk
                       type: string
                     format_string:
                       type: string
                     max_message_size:
                       pattern: ^([1-9]|[1-5][0-9]|6[0-4])k$
-                      default: 5k
                       type: string
                   type: object
                 filter:

docs/content/app-protect-dos/configuration.md

@@ -112,12 +112,9 @@ For example, say you want to log state changing requests for your Ingress resour
 ```json
 {
     "filter": {
-        "request_type": "all"
-    },
-    "content": {
-        "format": "default",
-        "max_request_size": "any",
-        "max_message_size": "64k"
+        "traffic-mitigation-stats": "all",
+        "bad-actors": "top 10",
+        "attack-signatures": "top 10"
     }
 }
 ```
@@ -130,9 +127,6 @@ kind: APDosLogConf
 metadata:
    name: doslogconf
 spec:
-   content:
-      format: splunk
-      max_message_size: 64k
    filter:
       traffic-mitigation-stats: all
       bad-actors: top 10

examples/custom-resources/app-protect-dos/apdos-logconf.yaml

@@ -3,9 +3,6 @@ kind: APDosLogConf
 metadata:
   name: doslogconf
 spec:
-  content:
-    format: splunk
-    max_message_size: 64k
   filter:
     traffic-mitigation-stats: all
     bad-actors: top 10

examples/ingress-resources/app-protect-dos/apdos-logconf.yaml

@@ -3,9 +3,6 @@ kind: APDosLogConf
 metadata:
   name: doslogconf
 spec:
-  content:
-    format: splunk
-    max_message_size: 64k
   filter:
     traffic-mitigation-stats: all
     bad-actors: top 10

internal/k8s/appprotectdos/app_protect_dos_configuration.go

@@ -114,11 +114,14 @@ func (ci *Configuration) AddOrUpdatePolicy(policyObj *unstructured.Unstructured)
 	resNsName := appprotectcommon.GetNsName(policyObj)
 	policy, err := createAppProtectDosPolicyEx(policyObj)
 	ci.dosPolicies[resNsName] = policy
+	op := AddOrUpdate
 	if err != nil {
-		changes = append(changes, Change{Op: Delete, Resource: policy})
+		op = Delete
 		problems = append(problems, Problem{Object: policyObj, Reason: "Rejected", Message: err.Error()})
 	}
 
+	changes = append(changes, Change{Op: op, Resource: policy})
+
 	protectedResources := ci.GetDosProtectedThatReferencedDosPolicy(resNsName)
 	for _, p := range protectedResources {
 		proChanges, proProblems := ci.AddOrUpdateDosProtectedResource(p)
@@ -134,11 +137,14 @@ func (ci *Configuration) AddOrUpdateLogConf(logConfObj *unstructured.Unstructure
 	resNsName := appprotectcommon.GetNsName(logConfObj)
 	logConf, err := createAppProtectDosLogConfEx(logConfObj)
 	ci.dosLogConfs[resNsName] = logConf
+	op := AddOrUpdate
 	if err != nil {
-		changes = append(changes, Change{Op: Delete, Resource: logConf})
+		op = Delete
 		problems = append(problems, Problem{Object: logConfObj, Reason: "Rejected", Message: err.Error()})
 	}
 
+	changes = append(changes, Change{Op: op, Resource: logConf})
+
 	protectedResources := ci.GetDosProtectedThatReferencedDosLogConf(resNsName)
 	for _, p := range protectedResources {
 		proChanges, proProblems := ci.AddOrUpdateDosProtectedResource(p)
@@ -357,14 +363,21 @@ func createAppProtectDosPolicyEx(policyObj *unstructured.Unstructured) (*DosPoli
 }
 
 func createAppProtectDosLogConfEx(dosLogConfObj *unstructured.Unstructured) (*DosLogConfEx, error) {
-	err := validation.ValidateAppProtectDosLogConf(dosLogConfObj)
+	warning, err := validation.ValidateAppProtectDosLogConf(dosLogConfObj)
 	if err != nil {
 		return &DosLogConfEx{
 			Obj:      dosLogConfObj,
 			IsValid:  false,
 			ErrorMsg: fmt.Sprintf("failed to store ApDosLogconf: %v", err),
 		}, err
 	}
+	if warning != "" {
+		return &DosLogConfEx{
+			Obj:      dosLogConfObj,
+			IsValid:  true,
+			ErrorMsg: warning,
+		}, nil
+	}
 	return &DosLogConfEx{
 		Obj:     dosLogConfObj,
 		IsValid: true,

internal/k8s/appprotectdos/app_protect_dos_configuration_test.go

@@ -70,8 +70,7 @@ func TestCreateAppProtectDosLogConfEx(t *testing.T) {
 			logConf: &unstructured.Unstructured{
 				Object: map[string]interface{}{
 					"spec": map[string]interface{}{
-						"content": map[string]interface{}{},
-						"filter":  map[string]interface{}{},
+						"filter": map[string]interface{}{},
 					},
 				},
 			},
@@ -86,10 +85,26 @@ func TestCreateAppProtectDosLogConfEx(t *testing.T) {
 			logConf: &unstructured.Unstructured{
 				Object: map[string]interface{}{
 					"spec": map[string]interface{}{
-						"content": map[string]interface{}{},
+						"content": map[string]interface{}{
+							"format": "splunk",
+						},
+						"filter": map[string]interface{}{},
 					},
 				},
 			},
+			expectedLogConfEx: &DosLogConfEx{
+				IsValid:  true,
+				ErrorMsg: "the Content field is not supported, defaulting to splunk format.",
+			},
+			wantErr: false,
+			msg:     "Valid DosLogConf with warning",
+		},
+		{
+			logConf: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"spec": map[string]interface{}{},
+				},
+			},
 			expectedLogConfEx: &DosLogConfEx{
 				IsValid:  false,
 				ErrorMsg: "failed to store ApDosLogconf: error validating App Protect Dos Log Configuration : required field map[] not found",
@@ -211,6 +226,11 @@ func TestAddOrUpdateDosPolicy(t *testing.T) {
 			},
 		},
 	}
+
+	basicPolicyResource := &DosPolicyEx{
+		Obj:     basicTestPolicy,
+		IsValid: true,
+	}
 	invalidTestPolicy := &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"metadata": map[string]interface{}{
@@ -245,6 +265,10 @@ func TestAddOrUpdateDosPolicy(t *testing.T) {
 		{
 			policy: basicTestPolicy,
 			expectedChanges: []Change{
+				{
+					Resource: basicPolicyResource,
+					Op:       AddOrUpdate,
+				},
 				{
 					Resource: &DosProtectedResourceEx{
 						Obj:     basicResource,
@@ -298,20 +322,21 @@ func TestAddOrUpdateDosLogConf(t *testing.T) {
 				"name":      "testlogconf",
 			},
 			"spec": map[string]interface{}{
-				"content": map[string]interface{}{},
-				"filter":  map[string]interface{}{},
+				"filter": map[string]interface{}{},
 			},
 		},
 	}
+	validLogConfResource := &DosLogConfEx{
+		Obj:     validLogConf,
+		IsValid: true,
+	}
 	invalidLogConf := &unstructured.Unstructured{
 		Object: map[string]interface{}{
 			"metadata": map[string]interface{}{
 				"namespace": "testing",
 				"name":      "invalid-logconf",
 			},
-			"spec": map[string]interface{}{
-				"content": map[string]interface{}{},
-			},
+			"spec": map[string]interface{}{},
 		},
 	}
 	basicResource := &v1beta1.DosProtectedResource{
@@ -345,6 +370,10 @@ func TestAddOrUpdateDosLogConf(t *testing.T) {
 		{
 			logconf: validLogConf,
 			expectedChanges: []Change{
+				{
+					Resource: validLogConfResource,
+					Op:       AddOrUpdate,
+				},
 				{
 					Resource: &DosProtectedResourceEx{
 						Obj:     basicResource,

internal/k8s/controller.go

@@ -1506,6 +1506,19 @@ func (lbc *LoadBalancerController) processAppProtectDosChanges(changes []appprot
 				lbc.updateResourcesStatusAndEvents(resources, warnings, err)
 				msg := fmt.Sprintf("Configuration for %s/%s was added or updated", impl.Obj.Namespace, impl.Obj.Name)
 				lbc.recorder.Event(impl.Obj, api_v1.EventTypeNormal, "AddedOrUpdated", msg)
+			case *appprotectdos.DosPolicyEx:
+				msg := "Configuration was added or updated"
+				lbc.recorder.Event(impl.Obj, api_v1.EventTypeNormal, "AddedOrUpdated", msg)
+			case *appprotectdos.DosLogConfEx:
+				eventType := api_v1.EventTypeNormal
+				eventTitle := "AddedOrUpdated"
+				msg := "Configuration was added or updated"
+				if impl.ErrorMsg != "" {
+					msg += fmt.Sprintf(" ; with warning(s): %s", impl.ErrorMsg)
+					eventTitle = "AddedOrUpdatedWithWarning"
+					eventType = api_v1.EventTypeWarning
+				}
+				lbc.recorder.Event(impl.Obj, eventType, eventTitle, msg)
 			}
 		} else if c.Op == appprotectdos.Delete {
 			switch impl := c.Resource.(type) {

pkg/apis/dos/validation/dos.go

@@ -19,7 +19,6 @@ var appProtectDosPolicyRequiredFields = [][]string{
 }
 
 var appProtectDosLogConfRequiredFields = [][]string{
-	{"spec", "content"},
 	{"spec", "filter"},
 }
 
@@ -90,15 +89,28 @@ func validateResourceReference(ref string) error {
 	return nil
 }
 
+// checkAppProtectDosLogConfContentField check conetent field doesnt appear in dos log
+func checkAppProtectDosLogConfContentField(obj *unstructured.Unstructured) string {
+	_, found, err := unstructured.NestedMap(obj.Object, "spec", "content")
+	if err == nil && found {
+		unstructured.RemoveNestedField(obj.Object, "spec", "content")
+		msg := "the Content field is not supported, defaulting to splunk format."
+		return msg
+	}
+
+	return ""
+}
+
 // ValidateAppProtectDosLogConf validates LogConfiguration resource
-func ValidateAppProtectDosLogConf(logConf *unstructured.Unstructured) error {
+func ValidateAppProtectDosLogConf(logConf *unstructured.Unstructured) (string, error) {
 	lcName := logConf.GetName()
 	err := validation2.ValidateRequiredFields(logConf, appProtectDosLogConfRequiredFields)
 	if err != nil {
-		return fmt.Errorf("error validating App Protect Dos Log Configuration %v: %w", lcName, err)
+		return "", fmt.Errorf("error validating App Protect Dos Log Configuration %v: %w", lcName, err)
 	}
+	warning := checkAppProtectDosLogConfContentField(logConf)
 
-	return nil
+	return warning, nil
 }
 
 var (

pkg/apis/dos/validation/dos_test.go

@@ -240,66 +240,91 @@ func TestValidateAppProtectDosAccessLogDest(t *testing.T) {
 func TestValidateAppProtectDosLogConf(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
-		logConf   *unstructured.Unstructured
-		expectErr bool
-		msg       string
+		logConf    *unstructured.Unstructured
+		expectErr  bool
+		expectWarn bool
+		msg        string
 	}{
 		{
 			logConf: &unstructured.Unstructured{
 				Object: map[string]interface{}{
 					"spec": map[string]interface{}{
-						"content": map[string]interface{}{},
-						"filter":  map[string]interface{}{},
+						"filter": map[string]interface{}{},
 					},
 				},
 			},
-			expectErr: false,
-			msg:       "valid log conf",
+			expectErr:  false,
+			expectWarn: false,
+			msg:        "valid log conf",
 		},
 		{
 			logConf: &unstructured.Unstructured{
 				Object: map[string]interface{}{
-					"spec": map[string]interface{}{
+					"spec": map[string]interface{}{},
+				},
+			},
+			expectErr:  true,
+			expectWarn: false,
+			msg:        "invalid log conf with no filter field",
+		},
+		{
+			logConf: &unstructured.Unstructured{
+				Object: map[string]interface{}{
+					"something": map[string]interface{}{
 						"filter": map[string]interface{}{},
 					},
 				},
 			},
-			expectErr: true,
-			msg:       "invalid log conf with no content field",
+			expectErr:  true,
+			expectWarn: false,
+			msg:        "invalid log conf with no spec field",
 		},
 		{
 			logConf: &unstructured.Unstructured{
 				Object: map[string]interface{}{
 					"spec": map[string]interface{}{
-						"content": map[string]interface{}{},
+						"content": map[string]interface{}{
+							"format": "user-defined",
+						},
+						"filter": map[string]interface{}{},
 					},
 				},
 			},
-			expectErr: true,
-			msg:       "invalid log conf with no filter field",
+			expectErr:  false,
+			expectWarn: true,
+			msg:        "Support only splunk format",
 		},
 		{
 			logConf: &unstructured.Unstructured{
 				Object: map[string]interface{}{
-					"something": map[string]interface{}{
-						"content": map[string]interface{}{},
-						"filter":  map[string]interface{}{},
+					"spec": map[string]interface{}{
+						"filter": map[string]interface{}{},
+						"content": map[string]interface{}{
+							"format": "user-defined",
+						},
 					},
 				},
 			},
-			expectErr: true,
-			msg:       "invalid log conf with no spec field",
+			expectErr:  false,
+			expectWarn: true,
+			msg:        "valid log conf with warning filter field",
 		},
 	}
 
 	for _, test := range tests {
-		err := ValidateAppProtectDosLogConf(test.logConf)
+		warn, err := ValidateAppProtectDosLogConf(test.logConf)
 		if test.expectErr && err == nil {
 			t.Errorf("validateAppProtectDosLogConf() returned no error for the case of %s", test.msg)
 		}
 		if !test.expectErr && err != nil {
 			t.Errorf("validateAppProtectDosLogConf() returned unexpected error %v for the case of %s", err, test.msg)
 		}
+		if test.expectWarn && warn == "" {
+			t.Errorf("validateAppProtectDosLogConf() returned no warning for the case of %s", test.msg)
+		}
+		if !test.expectWarn && warn != "" {
+			t.Errorf("validateAppProtectDosLogConf() returned unexpected warning: %s, for the case of %s", warn, test.msg)
+		}
 	}
 }