From d7f8bf457521b5c27062db1e82561e90d5ce2dcc Mon Sep 17 00:00:00 2001 From: taimurhafeez Date: Tue, 14 Jul 2026 12:54:49 +0100 Subject: [PATCH] Fix false-positive audit_error_alert_exists on empty PrometheusRule spec --- cmd/manager/scap.go | 52 ++++++++++++------- cmd/manager/scap_test.go | 37 +++++++++++-- .../testcontent/cel_content/ssg-ocp4-ds.xml | 4 +- .../deprecated_profile/ssg-ocp4-ds.xml | 4 +- images/testcontent/from/ssg-ocp4-ds.xml | 4 +- images/testcontent/hide_rule/ssg-ocp4-ds.xml | 4 +- .../kubelet_default/ssg-ocp4-ds.xml | 4 +- .../testcontent/kubeletconfig/ssg-ocp4-ds.xml | 4 +- .../new_kubeletconfig/ssg-ocp4-ds.xml | 4 +- images/testcontent/to/ssg-ocp4-ds.xml | 4 +- .../unexistent_resource/ssg-ocp4-ds.xml | 4 +- .../variabletemplate/ssg-ocp4-ds.xml | 4 +- tests/data/prometheusrules_mixed.json | 36 +++++++++++++ tests/data/ssg-ocp4-ds-suppressed.xml | 4 +- 14 files changed, 126 insertions(+), 43 deletions(-) create mode 100644 tests/data/prometheusrules_mixed.json diff --git a/cmd/manager/scap.go b/cmd/manager/scap.go index e002c595c1..0d6a569ee3 100644 --- a/cmd/manager/scap.go +++ b/cmd/manager/scap.go @@ -539,30 +539,46 @@ func fetch(ctx context.Context, streamDispatcher streamerDispatcherFn, rfClients return results, warnings, nil } -func filter(ctx context.Context, rawobj []byte, filter string) ([]byte, error) { - fltr, fltrErr := gojq.Parse(filter) - if fltrErr != nil { - return nil, fmt.Errorf("could not create filter '%s': %w", filter, fltrErr) - } - obj := map[string]interface{}{} - unmarshallErr := json.Unmarshal(rawobj, &obj) - if unmarshallErr != nil { - return nil, fmt.Errorf("Error unmarshalling json: %w", unmarshallErr) +func execFilter(ctx context.Context, obj map[string]interface{}, filterExpr string) (interface{}, gojq.Iter, error) { + fltr, err := gojq.Parse(filterExpr) + if err != nil { + return nil, nil, fmt.Errorf("could not create filter '%s': %w", filterExpr, err) } iter := fltr.RunWithContext(ctx, obj) v, ok := iter.Next() if !ok { - DBG("No result from filter. This is an issue and an error will be returned.") - return nil, fmt.Errorf("couldn't get filtered object") + return nil, nil, fmt.Errorf("couldn't get filtered object") } if err, ok := v.(error); ok { - DBG("Error while filtering: %s", err) - // gojq may return a diverse set of internal errors caused by null values. - // These errors are happen when a piped filter ends up acting on a null value. - if strings.HasSuffix(err.Error(), ": null") { - return nil, fmt.Errorf("Skipping empty filter result from '%s': %w", filter, NullValErr) + return nil, nil, err + } + return v, iter, nil +} + +func filter(ctx context.Context, rawobj []byte, filterExpr string) ([]byte, error) { + obj := map[string]interface{}{} + unmarshallErr := json.Unmarshal(rawobj, &obj) + if unmarshallErr != nil { + return nil, fmt.Errorf("Error unmarshalling json: %w", unmarshallErr) + } + + v, iter, execErr := execFilter(ctx, obj, filterExpr) + if execErr != nil { + DBG("Error while filtering: %s", execErr) + if strings.HasSuffix(execErr.Error(), ": null") { + // Retry with optional iteration to gracefully skip null values + // instead of failing the entire filter. + optFilter := strings.ReplaceAll(filterExpr, "[]", "[]?") + if optFilter != filterExpr { + DBG("Retrying with optional iteration: '%s'", optFilter) + v, iter, execErr = execFilter(ctx, obj, optFilter) + } + if execErr != nil { + return nil, fmt.Errorf("Skipping empty filter result from '%s': %w", filterExpr, NullValErr) + } + } else { + return nil, execErr } - return nil, err } var out []byte @@ -591,7 +607,7 @@ func filter(ctx context.Context, rawobj []byte, filter string) ([]byte, error) { _, isNotEOF := iter.Next() if isNotEOF { DBG("No more results should have come from the filter. This is an issue with the content.") - return out, fmt.Errorf("Skipping extra results from filter '%s': %w", filter, MoreThanOneObjErr) + return out, fmt.Errorf("Skipping extra results from filter '%s': %w", filterExpr, MoreThanOneObjErr) } return out, nil } diff --git a/cmd/manager/scap_test.go b/cmd/manager/scap_test.go index 7103f9edab..9cfbfacc5b 100644 --- a/cmd/manager/scap_test.go +++ b/cmd/manager/scap_test.go @@ -325,9 +325,40 @@ var _ = Describe("Testing filtering", func() { rawmc, readErr = io.ReadAll(nsFile) Expect(readErr).To(BeNil()) }) - It("skips filter piping errors", func() { - _, filterErr := filter(context.TODO(), rawmc, `[.items[] | select(.metadata.name | test("^rendered-worker-[0-9a-z]+$|^rendered-master-[0-9a-z]+$"))] | map(.spec.fips == true)`) - Expect(filterErr).Should(MatchError(NullValErr)) + It("retries with optional iteration on null items", func() { + filteredOut, filterErr := filter(context.TODO(), rawmc, `[.items[] | select(.metadata.name | test("^rendered-worker-[0-9a-z]+$|^rendered-master-[0-9a-z]+$"))] | map(.spec.fips == true)`) + Expect(filterErr).To(BeNil()) + Expect(string(filteredOut)).To(Equal("[]")) + }) + }) + Context("PrometheusRule with empty spec", func() { + var rawpr []byte + BeforeEach(func() { + prFile, err := os.Open("../../tests/data/prometheusrules_mixed.json") + Expect(err).To(BeNil()) + var readErr error + rawpr, readErr = io.ReadAll(prFile) + Expect(readErr).To(BeNil()) + }) + It("retries with optional iteration and preserves valid data", func() { + filteredOut, filterErr := filter(context.TODO(), rawpr, + `[.items[].spec.groups[].rules[].expr]`) + Expect(filterErr).To(BeNil()) + var exprArr []interface{} + unmErr := json.Unmarshal(filteredOut, &exprArr) + Expect(unmErr).To(BeNil()) + Expect(exprArr).To(HaveLen(1)) + Expect(exprArr[0]).To(Equal("apiserver_audit_error_total / apiserver_audit_event_total > 0")) + }) + It("succeeds directly with optional iterator", func() { + filteredOut, filterErr := filter(context.TODO(), rawpr, + `[.items[]?.spec.groups[]?.rules[]?.expr]`) + Expect(filterErr).To(BeNil()) + var exprArr []interface{} + unmErr := json.Unmarshal(filteredOut, &exprArr) + Expect(unmErr).To(BeNil()) + Expect(exprArr).To(HaveLen(1)) + Expect(exprArr[0]).To(Equal("apiserver_audit_error_total / apiserver_audit_event_total > 0")) }) }) }) diff --git a/images/testcontent/cel_content/ssg-ocp4-ds.xml b/images/testcontent/cel_content/ssg-ocp4-ds.xml index b859285fa3..100ddcc335 100644 --- a/images/testcontent/cel_content/ssg-ocp4-ds.xml +++ b/images/testcontent/cel_content/ssg-ocp4-ds.xml @@ -13350,7 +13350,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules#5fd5244e3dcae63319f7e86b918cb8ea6ce1b4124670ccb43750d7a75ca03cb7 file. @@ -39366,7 +39366,7 @@ openvswitch Run the following command: -$ oc get --all-namespaces prometheusrules -o json | jq '[.items[].spec.groups[].rules[].expr]' | grep apiserver_audit +$ oc get --all-namespaces prometheusrules -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' | grep apiserver_audit Make sure that there's a prometheus rule that verifies the apiserver_audit_error_total and apiserver_audit_event_total metrics and will alert based on an error threshold. Is it the case that Audit log errors do not generate an alert? diff --git a/images/testcontent/deprecated_profile/ssg-ocp4-ds.xml b/images/testcontent/deprecated_profile/ssg-ocp4-ds.xml index 5963721c3c..4527ab269e 100644 --- a/images/testcontent/deprecated_profile/ssg-ocp4-ds.xml +++ b/images/testcontent/deprecated_profile/ssg-ocp4-ds.xml @@ -13350,7 +13350,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules#5fd5244e3dcae63319f7e86b918cb8ea6ce1b4124670ccb43750d7a75ca03cb7 file. @@ -39366,7 +39366,7 @@ openvswitch Run the following command: -$ oc get --all-namespaces prometheusrules -o json | jq '[.items[].spec.groups[].rules[].expr]' | grep apiserver_audit +$ oc get --all-namespaces prometheusrules -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' | grep apiserver_audit Make sure that there's a prometheus rule that verifies the apiserver_audit_error_total and apiserver_audit_event_total metrics and will alert based on an error threshold. Is it the case that Audit log errors do not generate an alert? diff --git a/images/testcontent/from/ssg-ocp4-ds.xml b/images/testcontent/from/ssg-ocp4-ds.xml index 7067530e75..d1e188ad9f 100644 --- a/images/testcontent/from/ssg-ocp4-ds.xml +++ b/images/testcontent/from/ssg-ocp4-ds.xml @@ -15697,7 +15697,7 @@ and make sure it outputs a value. Run the following command: -oc get prometheusrules instances -o json | jq '[.items[].spec.groups[].rules[].expr]' +oc get prometheusrules instances -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' The output should return a list of URL entries with https:// or tls:// transport. Is it the case that Audit log errors do not generate an alert? @@ -23385,7 +23385,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#0fd94c224732cf855db0ac2a1e214959fdf670d9066fb8ae5485ec5ab748f464 file. diff --git a/images/testcontent/hide_rule/ssg-ocp4-ds.xml b/images/testcontent/hide_rule/ssg-ocp4-ds.xml index 9146a4cb50..f8e7438d81 100644 --- a/images/testcontent/hide_rule/ssg-ocp4-ds.xml +++ b/images/testcontent/hide_rule/ssg-ocp4-ds.xml @@ -12935,7 +12935,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#72e9ad360bb6bdf4ad9e43217cd0ec9cb90e7c3b08d4fbe0edf087ad899e05a6 file. @@ -40075,7 +40075,7 @@ and make sure it outputs 0. Run the following command: -oc get --all-namespaces prometheusrules -o json | jq '[.items[].spec.groups[].rules[].expr]' | grep apiserver_audit +oc get --all-namespaces prometheusrules -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' | grep apiserver_audit Make sure that there's a prometheus rule that verifies the apiserver_audit_error_total and apiserver_audit_event_total metrics and will alert based on an error threshold. Is it the case that Audit log errors do not generate an alert? diff --git a/images/testcontent/kubelet_default/ssg-ocp4-ds.xml b/images/testcontent/kubelet_default/ssg-ocp4-ds.xml index fecec43e93..3c897df689 100644 --- a/images/testcontent/kubelet_default/ssg-ocp4-ds.xml +++ b/images/testcontent/kubelet_default/ssg-ocp4-ds.xml @@ -13064,7 +13064,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#72e9ad360bb6bdf4ad9e43217cd0ec9cb90e7c3b08d4fbe0edf087ad899e05a6 file. @@ -40387,7 +40387,7 @@ TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 Run the following command: -oc get --all-namespaces prometheusrules -o json | jq '[.items[].spec.groups[].rules[].expr]' | grep apiserver_audit +oc get --all-namespaces prometheusrules -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' | grep apiserver_audit Make sure that there's a prometheus rule that verifies the apiserver_audit_error_total and apiserver_audit_event_total metrics and will alert based on an error threshold. Is it the case that Audit log errors do not generate an alert? diff --git a/images/testcontent/kubeletconfig/ssg-ocp4-ds.xml b/images/testcontent/kubeletconfig/ssg-ocp4-ds.xml index feecbd8c48..036db78afe 100644 --- a/images/testcontent/kubeletconfig/ssg-ocp4-ds.xml +++ b/images/testcontent/kubeletconfig/ssg-ocp4-ds.xml @@ -15657,7 +15657,7 @@ and make sure it outputs a value. Run the following command: -oc get prometheusrules instances -o json | jq '[.items[].spec.groups[].rules[].expr]' +oc get prometheusrules instances -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' The output should return a list of URL entries with https:// or tls:// transport. Is it the case that Audit log errors do not generate an alert? @@ -23297,7 +23297,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#0fd94c224732cf855db0ac2a1e214959fdf670d9066fb8ae5485ec5ab748f464 file. diff --git a/images/testcontent/new_kubeletconfig/ssg-ocp4-ds.xml b/images/testcontent/new_kubeletconfig/ssg-ocp4-ds.xml index c83725a3ce..64e60048a1 100644 --- a/images/testcontent/new_kubeletconfig/ssg-ocp4-ds.xml +++ b/images/testcontent/new_kubeletconfig/ssg-ocp4-ds.xml @@ -10270,7 +10270,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules#5fd5244e3dcae63319f7e86b918cb8ea6ce1b4124670ccb43750d7a75ca03cb7 file. @@ -36063,7 +36063,7 @@ and make sure it outputs 0. Run the following command: -oc get --all-namespaces prometheusrules -o json | jq '[.items[].spec.groups[].rules[].expr]' | grep apiserver_audit +oc get --all-namespaces prometheusrules -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' | grep apiserver_audit Make sure that there's a prometheus rule that verifies the apiserver_audit_error_total and apiserver_audit_event_total metrics and will alert based on an error threshold. Is it the case that Audit log errors do not generate an alert? diff --git a/images/testcontent/to/ssg-ocp4-ds.xml b/images/testcontent/to/ssg-ocp4-ds.xml index 15adb78116..3a5273f291 100644 --- a/images/testcontent/to/ssg-ocp4-ds.xml +++ b/images/testcontent/to/ssg-ocp4-ds.xml @@ -15697,7 +15697,7 @@ and make sure it outputs a value. Run the following command: -oc get prometheusrules instances -o json | jq '[.items[].spec.groups[].rules[].expr]' +oc get prometheusrules instances -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' The output should return a list of URL entries with https:// or tls:// transport. Is it the case that Audit log errors do not generate an alert? @@ -23386,7 +23386,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#0fd94c224732cf855db0ac2a1e214959fdf670d9066fb8ae5485ec5ab748f464 file. diff --git a/images/testcontent/unexistent_resource/ssg-ocp4-ds.xml b/images/testcontent/unexistent_resource/ssg-ocp4-ds.xml index 15adb78116..3a5273f291 100644 --- a/images/testcontent/unexistent_resource/ssg-ocp4-ds.xml +++ b/images/testcontent/unexistent_resource/ssg-ocp4-ds.xml @@ -15697,7 +15697,7 @@ and make sure it outputs a value. Run the following command: -oc get prometheusrules instances -o json | jq '[.items[].spec.groups[].rules[].expr]' +oc get prometheusrules instances -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' The output should return a list of URL entries with https:// or tls:// transport. Is it the case that Audit log errors do not generate an alert? @@ -23386,7 +23386,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#0fd94c224732cf855db0ac2a1e214959fdf670d9066fb8ae5485ec5ab748f464 file. diff --git a/images/testcontent/variabletemplate/ssg-ocp4-ds.xml b/images/testcontent/variabletemplate/ssg-ocp4-ds.xml index d4c8eb5d62..a934cdb0aa 100644 --- a/images/testcontent/variabletemplate/ssg-ocp4-ds.xml +++ b/images/testcontent/variabletemplate/ssg-ocp4-ds.xml @@ -15778,7 +15778,7 @@ and make sure it outputs a value. Run the following command: -oc get prometheusrules instances -o json | jq '[.items[].spec.groups[].rules[].expr]' +oc get prometheusrules instances -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' The output should return a list of URL entries with https:// or tls:// transport. Is it the case that Audit log errors do not generate an alert? @@ -23545,7 +23545,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#0fd94c224732cf855db0ac2a1e214959fdf670d9066fb8ae5485ec5ab748f464 file. diff --git a/tests/data/prometheusrules_mixed.json b/tests/data/prometheusrules_mixed.json new file mode 100644 index 0000000000..67152aa751 --- /dev/null +++ b/tests/data/prometheusrules_mixed.json @@ -0,0 +1,36 @@ +{ + "apiVersion": "monitoring.coreos.com/v1", + "kind": "PrometheusRuleList", + "items": [ + { + "apiVersion": "monitoring.coreos.com/v1", + "kind": "PrometheusRule", + "metadata": { + "name": "audit-errors", + "namespace": "openshift-kube-apiserver" + }, + "spec": { + "groups": [ + { + "name": "apiserver-audit", + "rules": [ + { + "alert": "AuditLogError", + "expr": "apiserver_audit_error_total / apiserver_audit_event_total > 0" + } + ] + } + ] + } + }, + { + "apiVersion": "monitoring.coreos.com/v1", + "kind": "PrometheusRule", + "metadata": { + "name": "empty-rule", + "namespace": "default" + }, + "spec": {} + } + ] +} diff --git a/tests/data/ssg-ocp4-ds-suppressed.xml b/tests/data/ssg-ocp4-ds-suppressed.xml index b02747130d..6c21907b11 100644 --- a/tests/data/ssg-ocp4-ds-suppressed.xml +++ b/tests/data/ssg-ocp4-ds-suppressed.xml @@ -13275,7 +13275,7 @@ For more information, consult the Therefore, you need to use a tool that can query the OCP API, retrieve the following: /apis/monitoring.coreos.com/v1/prometheusrules?limit=500 API endpoint, filter with with the jq utility using the following filter - [.items[].spec.groups[].rules[].expr] + [.items[]?.spec.groups[]?.rules[]?.expr] and persist it to the local /apis/monitoring.coreos.com/v1/prometheusrules?limit=500#72e9ad360bb6bdf4ad9e43217cd0ec9cb90e7c3b08d4fbe0edf087ad899e05a6 file. @@ -41229,7 +41229,7 @@ and make sure it outputs 0. Run the following command: -oc get --all-namespaces prometheusrules -o json | jq '[.items[].spec.groups[].rules[].expr]' | grep apiserver_audit +oc get --all-namespaces prometheusrules -o json | jq '[.items[]?.spec.groups[]?.rules[]?.expr]' | grep apiserver_audit Make sure that there's a prometheus rule that verifies the apiserver_audit_error_total and apiserver_audit_event_total metrics and will alert based on an error threshold. Is it the case that Audit log errors do not generate an alert?