Add status to WAF Policy (#3612)

Add status to WAF Policy

Author: salonichf5

internal/controller/state/conditions/conditions.go

@@ -10,25 +10,8 @@ import (
 	ngfAPI "github.com/nginx/nginx-gateway-fabric/apis/v1alpha1"
 )
 
+// Conditions and Reasons for Route resources.
 const (
-	// GatewayClassReasonGatewayClassConflict indicates there are multiple GatewayClass resources
-	// that reference this controller, and we ignored the resource in question and picked the
-	// GatewayClass that is referenced in the command-line argument.
-	// This reason is used with GatewayClassConditionAccepted (false).
-	GatewayClassReasonGatewayClassConflict v1.GatewayClassConditionReason = "GatewayClassConflict"
-
-	// GatewayClassMessageGatewayClassConflict is a message that describes GatewayClassReasonGatewayClassConflict.
-	GatewayClassMessageGatewayClassConflict = "The resource is ignored due to a conflicting GatewayClass resource"
-
-	// ListenerReasonUnsupportedValue is used with the "Accepted" condition when a value of a field in a Listener
-	// is invalid or not supported.
-	ListenerReasonUnsupportedValue v1.ListenerConditionReason = "UnsupportedValue"
-
-	// ListenerMessageFailedNginxReload is a message used with ListenerConditionProgrammed (false)
-	// when nginx fails to reload.
-	ListenerMessageFailedNginxReload = "The Listener is not programmed due to a failure to " +
-		"reload nginx with the configuration"
-
 	// RouteReasonBackendRefUnsupportedValue is used with the "ResolvedRefs" condition when one of the
 	// Route rules has a backendRef with an unsupported value.
 	RouteReasonBackendRefUnsupportedValue v1.RouteConditionReason = "UnsupportedValue"
@@ -61,6 +44,48 @@ const (
 	// invalid. Used with ResolvedRefs (false).
 	RouteReasonInvalidFilter v1.RouteConditionReason = "InvalidFilter"
 
+	// RouteMessageFailedNginxReload is a message used with RouteReasonGatewayNotProgrammed
+	// when nginx fails to reload.
+	RouteMessageFailedNginxReload = GatewayMessageFailedNginxReload + ". NGINX may still be configured " +
+		"for this Route. However, future updates to this resource will not be configured until the Gateway " +
+		"is programmed again"
+)
+
+// Conditions and Reasons for GatewayClass, Gateway and Listener resources.
+const (
+	// GatewayClassReasonGatewayClassConflict indicates there are multiple GatewayClass resources
+	// that reference this controller, and we ignored the resource in question and picked the
+	// GatewayClass that is referenced in the command-line argument.
+	// This reason is used with GatewayClassConditionAccepted (false).
+	GatewayClassReasonGatewayClassConflict v1.GatewayClassConditionReason = "GatewayClassConflict"
+
+	// GatewayClassMessageGatewayClassConflict is a message that describes GatewayClassReasonGatewayClassConflict.
+	GatewayClassMessageGatewayClassConflict = "The resource is ignored due to a conflicting GatewayClass resource"
+
+	// ListenerReasonUnsupportedValue is used with the "Accepted" condition when a value of a field in a Listener
+	// is invalid or not supported.
+	ListenerReasonUnsupportedValue v1.ListenerConditionReason = "UnsupportedValue"
+
+	// ListenerMessageFailedNginxReload is a message used with ListenerConditionProgrammed (false)
+	// when nginx fails to reload.
+	ListenerMessageFailedNginxReload = "The Listener is not programmed due to a failure to " +
+		"reload nginx with the configuration"
+
+	// GatewayResolvedRefs condition indicates whether the controller was able to resolve the
+	// parametersRef on the Gateway.
+	GatewayResolvedRefs v1.GatewayConditionType = "ResolvedRefs"
+
+	// GatewayReasonResolvedRefs is used with the "GatewayResolvedRefs" condition when the condition is true.
+	GatewayReasonResolvedRefs v1.GatewayConditionReason = "ResolvedRefs"
+
+	// GatewayReasonParamsRefNotFound is used with the "GatewayResolvedRefs" condition when the
+	// parametersRef resource does not exist.
+	GatewayReasonParamsRefNotFound v1.GatewayConditionReason = "ParametersRefNotFound"
+
+	// GatewayReasonParamsRefInvalid is used with the "GatewayResolvedRefs" condition when the
+	// parametersRef resource is invalid.
+	GatewayReasonParamsRefInvalid v1.GatewayConditionReason = "ParametersRefInvalid"
+
 	// GatewayReasonUnsupportedValue is used with GatewayConditionAccepted (false) when a value of a field in a Gateway
 	// is invalid or not supported.
 	GatewayReasonUnsupportedValue v1.GatewayConditionReason = "UnsupportedValue"
@@ -70,12 +95,6 @@ const (
 	GatewayMessageFailedNginxReload = "The Gateway is not programmed due to a failure to " +
 		"reload nginx with the configuration"
 
-	// RouteMessageFailedNginxReload is a message used with RouteReasonGatewayNotProgrammed
-	// when nginx fails to reload.
-	RouteMessageFailedNginxReload = GatewayMessageFailedNginxReload + ". NGINX may still be configured " +
-		"for this Route. However, future updates to this resource will not be configured until the Gateway " +
-		"is programmed again"
-
 	// GatewayClassResolvedRefs condition indicates whether the controller was able to resolve the
 	// parametersRef on the GatewayClass.
 	GatewayClassResolvedRefs v1.GatewayClassConditionType = "ResolvedRefs"
@@ -90,7 +109,10 @@ const (
 	// GatewayClassReasonParamsRefInvalid is used with the "GatewayClassResolvedRefs" condition when the
 	// parametersRef resource is invalid.
 	GatewayClassReasonParamsRefInvalid v1.GatewayClassConditionReason = "ParametersRefInvalid"
+)
 
+// Conditions and Reasons for Policy resources.
+const (
 	// PolicyReasonNginxProxyConfigNotSet is used with the "PolicyAccepted" condition when the
 	// NginxProxy resource is missing or invalid.
 	PolicyReasonNginxProxyConfigNotSet v1alpha2.PolicyConditionReason = "NginxProxyConfigNotSet"
@@ -107,6 +129,22 @@ const (
 	// has an overlapping hostname:port/path combination with another Route.
 	PolicyReasonTargetConflict v1alpha2.PolicyConditionReason = "TargetConflict"
 
+	// PolicyAffectedReason is used with the "PolicyAffected" condition when a
+	// ObservabilityPolicy or ClientSettingsPolicy is applied to Gateways or Routes.
+	PolicyAffectedReason v1alpha2.PolicyConditionReason = "PolicyAffected"
+
+	// WAFPolicyFetchError is used with the "WAFPolicyFetchError" condition when a
+	// WAFPolicy or LogProfileBundle cannot be fetched from the specified file location.
+	WAFPolicyFetchError v1alpha2.PolicyConditionReason = "FetchError"
+
+	// WAFPolicyMessageSourceInvalid is a message used with the "PolicyInvalid" condition
+	// when the WAF policy source is invalid or incomplete.
+	WAFPolicyMessageSourceInvalid = "The WAF policy source is invalid or incomplete."
+
+	// WAFSecurityLogMessageSourceInvalid is a message used with the "PolicyInvalid" condition
+	// when the WAFSecurityLog source is invalid or incomplete.
+	WAFSecurityLogMessageSourceInvalid = "The WAFSecurityLog source is invalid or incomplete."
+
 	// ClientSettingsPolicyAffected is used with the "PolicyAffected" condition when a
 	// ClientSettingsPolicy is applied to a Gateway, HTTPRoute, or GRPCRoute.
 	ClientSettingsPolicyAffected v1alpha2.PolicyConditionType = "ClientSettingsPolicyAffected"
@@ -115,24 +153,9 @@ const (
 	// ObservabilityPolicy is applied to a HTTPRoute, or GRPCRoute.
 	ObservabilityPolicyAffected v1alpha2.PolicyConditionType = "ObservabilityPolicyAffected"
 
-	// PolicyAffectedReason is used with the "PolicyAffected" condition when a
-	// ObservabilityPolicy or ClientSettingsPolicy is applied to Gateways or Routes.
-	PolicyAffectedReason v1alpha2.PolicyConditionReason = "PolicyAffected"
-
-	// GatewayResolvedRefs condition indicates whether the controller was able to resolve the
-	// parametersRef on the Gateway.
-	GatewayResolvedRefs v1.GatewayConditionType = "ResolvedRefs"
-
-	// GatewayReasonResolvedRefs is used with the "GatewayResolvedRefs" condition when the condition is true.
-	GatewayReasonResolvedRefs v1.GatewayConditionReason = "ResolvedRefs"
-
-	// GatewayReasonParamsRefNotFound is used with the "GatewayResolvedRefs" condition when the
-	// parametersRef resource does not exist.
-	GatewayReasonParamsRefNotFound v1.GatewayConditionReason = "ParametersRefNotFound"
-
-	// GatewayReasonParamsRefInvalid is used with the "GatewayResolvedRefs" condition when the
-	// parametersRef resource is invalid.
-	GatewayReasonParamsRefInvalid v1.GatewayConditionReason = "ParametersRefInvalid"
+	// WAFPolicyAffected is used with the "PolicyAffected" condition when an
+	// WAFPolicyAffected is applied to a Gateway, HTTPRoute, or GRPCRoute.
+	WAFPolicyAffected v1alpha2.PolicyConditionType = "WAFPolicyAffected"
 )
 
 // Condition defines a condition to be reported in the status of resources.
@@ -998,3 +1021,24 @@ func NewClientSettingsPolicyAffected() Condition {
 		Message: "ClientSettingsPolicy is applied to the resource",
 	}
 }
+
+// NewWAFPolicyAffected returns a Condition that indicates that a WAFPolicy
+// is applied to the resource.
+func NewWAFPolicyAffected() Condition {
+	return Condition{
+		Type:    string(WAFPolicyAffected),
+		Status:  metav1.ConditionTrue,
+		Reason:  string(PolicyAffectedReason),
+		Message: "WAFPolicy is applied to the resource",
+	}
+}
+
+// NewWAFPolicyFetchError returns a Condition that indicates that there was an error fetching the WAF policy bundle.
+func NewWAFPolicyFetchError(msg string) Condition {
+	return Condition{
+		Type:    string(WAFPolicyFetchError),
+		Status:  metav1.ConditionFalse,
+		Reason:  string(WAFPolicyFetchError),
+		Message: "Failed to fetch the policy bundle due to: " + msg,
+	}
+}

internal/controller/state/graph/policies.go

@@ -532,6 +532,11 @@ func addStatusToTargetRefs(policyKind string, conditionsList *[]conditions.Condi
 			return
 		}
 		*conditionsList = append(*conditionsList, conditions.NewClientSettingsPolicyAffected())
+	case kinds.WAFPolicy:
+		if conditions.HasMatchingCondition(*conditionsList, conditions.NewWAFPolicyAffected()) {
+			return
+		}
+		*conditionsList = append(*conditionsList, conditions.NewWAFPolicyAffected())
 	}
 }
 
@@ -562,7 +567,10 @@ func fetchWAFPolicyBundleData(
 		if wafPolicy.Spec.PolicySource != nil && wafPolicy.Spec.PolicySource.FileLocation != "" {
 			fetcher := createFetcher(buildFetchOptions(wafPolicy.Spec.PolicySource)...)
 			if !fetchAndStoreBundle(wafPolicy.Spec.PolicySource.FileLocation, policy, refPolicyBundles, fetcher) {
-				continue // Policy was marked invalid, skip security logs
+				policy.Conditions = append(policy.Conditions,
+					conditions.NewPolicyInvalid(conditions.WAFPolicyMessageSourceInvalid),
+				)
+				continue
 			}
 		}
 
@@ -573,7 +581,10 @@ func fetchWAFPolicyBundleData(
 
 			fetcher := createFetcher(buildFetchOptions(secLog.LogProfileBundle)...)
 			if !fetchAndStoreBundle(secLog.LogProfileBundle.FileLocation, policy, refPolicyBundles, fetcher) {
-				break // Policy was marked invalid, skip other security logs
+				policy.Conditions = append(policy.Conditions,
+					conditions.NewPolicyInvalid(conditions.WAFSecurityLogMessageSourceInvalid),
+				)
+				break
 			}
 		}
 	}
@@ -596,8 +607,7 @@ func fetchAndStoreBundle(
 	data, err := fetcher.GetRemoteFile(fileLocation)
 	if err != nil {
 		policy.Valid = false
-		// FIXME(ciarams87): Add appropriate condition when available.
-		policy.Conditions = append(policy.Conditions, conditions.NewPolicyInvalid("Error fetching policy: "+err.Error()))
+		policy.Conditions = append(policy.Conditions, conditions.NewWAFPolicyFetchError(err.Error()))
 		return false
 	}
 

internal/controller/state/graph/policies_test.go

@@ -1720,6 +1720,7 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) {
 
 	cspGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "ClientSettingsPolicy"}
 	opGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "ObservabilityPolicy"}
+	wafPolicyGVK := schema.GroupVersionKind{Group: "Group", Version: "Version", Kind: "WAFPolicy"}
 
 	gw1Ref := createTestRef(kinds.Gateway, v1.GroupName, "gw1")
 	gw1TargetRef := createTestPolicyTargetRef(
@@ -1801,7 +1802,7 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) {
 			},
 		},
 		{
-			name: "gateway attached to csp and op policy",
+			name: "gateway attached to csp, op and waf policy",
 			policies: map[PolicyKey]*Policy{
 				createTestPolicyKey(cspGVK, "csp1"): {
 					Source:     createTestPolicy(cspGVK, "csp1", gw2Ref),
@@ -1811,13 +1812,18 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) {
 					Source:     createTestPolicy(opGVK, "observabilityPolicy1", gw2Ref),
 					TargetRefs: []PolicyTargetRef{gw2TargetRef},
 				},
+				createTestPolicyKey(wafPolicyGVK, "wafPolicy1"): {
+					Source:     createTestPolicy(wafPolicyGVK, "wafPolicy1", gw2Ref),
+					TargetRefs: []PolicyTargetRef{gw2TargetRef},
+				},
 			},
 			gws:    createGatewayMap(types.NamespacedName{Namespace: testNs, Name: "gw2"}),
 			routes: nil,
 			expectedConditions: map[types.NamespacedName][]conditions.Condition{
 				{Namespace: testNs, Name: "gw2"}: {
 					conditions.NewClientSettingsPolicyAffected(),
 					conditions.NewObservabilityPolicyAffected(),
+					conditions.NewWAFPolicyAffected(),
 				},
 			},
 		},
@@ -1875,6 +1881,10 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) {
 					Source:     createTestPolicy(opGVK, "observabilityPolicy2", gw3Ref, gr2Ref),
 					TargetRefs: []PolicyTargetRef{gw3TargetRef, gr2TargetRef},
 				},
+				createTestPolicyKey(wafPolicyGVK, "wafPolicy1"): {
+					Source:     createTestPolicy(wafPolicyGVK, "wafPolicy1", gw3Ref, hr2Ref),
+					TargetRefs: []PolicyTargetRef{gw3TargetRef, hr2TargetRef},
+				},
 			},
 			gws: createGatewayMap(
 				types.NamespacedName{Namespace: testNs, Name: "gw3"},
@@ -1901,10 +1911,12 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) {
 				{Namespace: testNs, Name: "gw3"}: {
 					conditions.NewClientSettingsPolicyAffected(),
 					conditions.NewObservabilityPolicyAffected(),
+					conditions.NewWAFPolicyAffected(),
 				},
 				{Namespace: testNs, Name: "hr2"}: {
 					conditions.NewObservabilityPolicyAffected(),
 					conditions.NewClientSettingsPolicyAffected(),
+					conditions.NewWAFPolicyAffected(),
 				},
 				{Namespace: testNs, Name: "gr2"}: {
 					conditions.NewObservabilityPolicyAffected(),
@@ -2103,6 +2115,7 @@ func TestFetchPolicyBundleData(t *testing.T) {
 		expectedPolicyState   map[string]bool
 		expectFetchConditions map[string]bool
 		name                  string
+		expectedConds         []conditions.Condition
 		expectedBundleCount   int
 	}{
 		{
@@ -2150,6 +2163,9 @@ func TestFetchPolicyBundleData(t *testing.T) {
 			expectFetchConditions: map[string]bool{
 				"invalid-waf": false,
 			},
+			expectedConds: []conditions.Condition{
+				conditions.NewPolicyInvalid(conditions.WAFPolicyMessageSourceInvalid),
+			},
 		},
 		{
 			name: "WAF policy with empty FileLocation",
@@ -2304,6 +2320,9 @@ func TestFetchPolicyBundleData(t *testing.T) {
 			expectFetchConditions: map[string]bool{
 				"waf-fail": true,
 			},
+			expectedConds: []conditions.Condition{
+				conditions.NewPolicyInvalid(conditions.WAFPolicyMessageSourceInvalid),
+			},
 		},
 		{
 			name: "WAF policy with PolicySource success but SecurityLog failure",
@@ -2338,6 +2357,10 @@ func TestFetchPolicyBundleData(t *testing.T) {
 			expectFetchConditions: map[string]bool{
 				"waf-mixed": true,
 			},
+			expectedConds: []conditions.Condition{
+				conditions.NewPolicyInvalid(conditions.WAFSecurityLogMessageSourceInvalid),
+				conditions.NewWAFPolicyFetchError("network error"),
+			},
 		},
 		{
 			name: "WAF policy with multiple SecurityLog bundles - partial failure",
@@ -2378,6 +2401,10 @@ func TestFetchPolicyBundleData(t *testing.T) {
 			expectFetchConditions: map[string]bool{
 				"waf-multi": true,
 			},
+			expectedConds: []conditions.Condition{
+				conditions.NewPolicyInvalid(conditions.WAFSecurityLogMessageSourceInvalid),
+				conditions.NewWAFPolicyFetchError("network error"),
+			},
 		},
 	}
 
@@ -2420,6 +2447,7 @@ func TestFetchPolicyBundleData(t *testing.T) {
 
 			for policyName, expectedValid := range test.expectedPolicyState {
 				found := false
+				invalidSourceErrMessage := "source is invalid or incomplete."
 				for _, policy := range test.processedPolicies {
 					if policy.Source.GetName() == policyName {
 						found = true
@@ -2429,8 +2457,16 @@ func TestFetchPolicyBundleData(t *testing.T) {
 						if expectFetchConditions, exists := test.expectFetchConditions[policyName]; exists && expectFetchConditions {
 							g.Expect(policy.Conditions).ToNot(BeEmpty(),
 								fmt.Sprintf("Policy %s should have fetch error conditions", policyName))
-							g.Expect(policy.Conditions[0].Reason).To(Equal("Invalid"))
-							g.Expect(policy.Conditions[0].Message).To(ContainSubstring("Error fetching policy:"))
+
+							if len(policy.Conditions) > 1 {
+								g.Expect(policy.Conditions[0].Reason).To(Equal("FetchError"))
+								g.Expect(policy.Conditions[0].Message).To(ContainSubstring("Failed to fetch the policy bundle due to:"))
+								g.Expect(policy.Conditions[1].Reason).To(Equal("Invalid"))
+								g.Expect(policy.Conditions[1].Message).To(ContainSubstring(invalidSourceErrMessage))
+							} else {
+								g.Expect(policy.Conditions[0].Reason).To(Equal("Invalid"))
+								g.Expect(policy.Conditions[0].Message).To(ContainSubstring(invalidSourceErrMessage))
+							}
 						}
 						break
 					}