diff --git a/internal/controller/state/conditions/conditions.go b/internal/controller/state/conditions/conditions.go index 5098f0522d..75972be68d 100644 --- a/internal/controller/state/conditions/conditions.go +++ b/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, + } +} diff --git a/internal/controller/state/graph/policies.go b/internal/controller/state/graph/policies.go index dbbcd65258..d29b0c0191 100644 --- a/internal/controller/state/graph/policies.go +++ b/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 } diff --git a/internal/controller/state/graph/policies_test.go b/internal/controller/state/graph/policies_test.go index f950b8bde5..03412131cb 100644 --- a/internal/controller/state/graph/policies_test.go +++ b/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,6 +1812,10 @@ 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, @@ -1818,6 +1823,7 @@ func TestAddPolicyAffectedStatusOnTargetRefs(t *testing.T) { {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 }