From 2aa0aec83f93a97f999bb06f39d32364588cef58 Mon Sep 17 00:00:00 2001 From: Sunny Gupta Date: Fri, 24 Jul 2026 07:29:55 +0000 Subject: [PATCH 1/2] Trim whitespace when splitting security-group annotations The Governance Overview Standards/Categories/Controls cards, and the policy drill-down sidebar behind them, group policies by splitting the policy.open-cluster-management.io/{standards,categories,controls} annotation on ",". String.split(',') leaves a leading space on every token after the first (e.g. "A, B".split(',') -> ["A", " B"]), so the same logical value produces a second, distinct row whenever it isn't first in a comma-separated list on some policies but is standalone or first-in-list on others. Trim each token after splitting in both useSecurityGroupViolations (Overview.tsx) and the matching filter in SecurityGroupPolicySummarySidebar.tsx so aggregation and drill-down filtering key off the same normalized value regardless of annotation formatting. Reported in Red Hat case 04500068. Signed-off-by: Sunny Gupta --- .../Governance/overview/Overview.test.tsx | 73 +++++++++++++++++++ .../routes/Governance/overview/Overview.tsx | 2 +- .../SecurityGroupPolicySummarySidebar.tsx | 2 +- 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/frontend/src/routes/Governance/overview/Overview.test.tsx b/frontend/src/routes/Governance/overview/Overview.test.tsx index b147acba180..67e5c331aac 100644 --- a/frontend/src/routes/Governance/overview/Overview.test.tsx +++ b/frontend/src/routes/Governance/overview/Overview.test.tsx @@ -16,6 +16,7 @@ import { import GovernanceOverview from './Overview' import userEvent from '@testing-library/user-event' import { defaultContext, PluginDataContext } from '../../../lib/PluginDataContext' +import { Policy, PolicyApiVersion, PolicyKind } from '../../../resources' describe('Overview Page', () => { beforeEach(async () => nockIgnoreApiPaths()) @@ -141,4 +142,76 @@ describe('Overview Page', () => { userEvent.click(screen.getByText(/show 85 more/i)) expect(queryByText(/show 85 more/i)).not.toBeInTheDocument() }) + + test('Should aggregate Standards card by trimmed annotation value, not raw comma-split token', async () => { + // Regression test: a standard listed anywhere but first in a comma-separated + // policy.open-cluster-management.io/standards annotation must not produce a + // separate row on the Standards card just because of the leading space left + // behind by String.split(','). + const policyWithStandardFirstInList: Policy = { + apiVersion: PolicyApiVersion, + kind: PolicyKind, + metadata: { + name: 'policy-standards-first', + namespace: 'test', + uid: 'standards-test-uid-1', + annotations: { + 'policy.open-cluster-management.io/standards': 'NIST SP 800-53, PCI-DSS 4.0', + }, + }, + spec: { + disabled: false, + 'policy-templates': [], + remediationAction: 'inform', + }, + status: { + compliant: 'Compliant', + }, + } + const policyWithStandardLastInList: Policy = { + apiVersion: PolicyApiVersion, + kind: PolicyKind, + metadata: { + name: 'policy-standards-last', + namespace: 'test', + uid: 'standards-test-uid-2', + annotations: { + 'policy.open-cluster-management.io/standards': 'CIS OpenShift Benchmark, PCI-DSS 4.0, NIST SP 800-53', + }, + }, + spec: { + disabled: false, + 'policy-templates': [], + remediationAction: 'inform', + }, + status: { + compliant: 'Compliant', + }, + } + + const pluginData = { + ...defaultContext, + loadStarted: true, + loadCompleted: true, + } + render( + + { + snapshot.set(policiesState, [policyWithStandardFirstInList, policyWithStandardLastInList]) + snapshot.set(managedClustersState, mockManagedClusters) + }} + > + + + + + + ) + + // Before the fix this rendered two separate rows/spans for the same logical + // standard (one from the untrimmed " NIST SP 800-53" split token). + expect(screen.getAllByText('NIST SP 800-53').length).toBe(1) + expect(screen.getAllByText('PCI-DSS 4.0').length).toBe(1) + }) }) diff --git a/frontend/src/routes/Governance/overview/Overview.tsx b/frontend/src/routes/Governance/overview/Overview.tsx index 2743598cc1f..d39f2e15e7c 100644 --- a/frontend/src/routes/Governance/overview/Overview.tsx +++ b/frontend/src/routes/Governance/overview/Overview.tsx @@ -100,7 +100,7 @@ function useSecurityGroupViolations(group: string, policies: Policy[]) { if (policy.spec.disabled) continue const annotation = policy.metadata.annotations?.[`policy.open-cluster-management.io/${group}`] if (!annotation) continue - const names = annotation.split(',') + const names = annotation.split(',').map((name) => name.trim()) for (const name of names) { let v = clusterViolations[name] if (!v) { diff --git a/frontend/src/routes/Governance/overview/SecurityGroupPolicySummarySidebar.tsx b/frontend/src/routes/Governance/overview/SecurityGroupPolicySummarySidebar.tsx index 872b82cadd4..e2d54a2511f 100644 --- a/frontend/src/routes/Governance/overview/SecurityGroupPolicySummarySidebar.tsx +++ b/frontend/src/routes/Governance/overview/SecurityGroupPolicySummarySidebar.tsx @@ -36,7 +36,7 @@ export function SecurityGroupPolicySummarySidebar(props: { if (!annotation) { return false } - const names = annotation.split(',') + const names = annotation.split(',').map((name) => name.trim()) for (const name of names) { if (name === violation.name && policy.status?.compliant) { return true From 836f5ed5cceccf0d26837d398dc0fa2bff4b621d Mon Sep 17 00:00:00 2001 From: Sunny Gupta Date: Sat, 25 Jul 2026 02:18:05 +0000 Subject: [PATCH 2/2] Add sidebar coverage test for trimmed standards annotation The SecurityGroupPolicySummarySidebar drill-down applies the same comma-split-without-trim fix as useSecurityGroupViolations, but the existing regression test only exercised the Overview card path, leaving the sidebar's fixed line uncovered on new code (SonarCloud flagged 50% coverage on new code, below the 70% gate). Add a test that renders SecurityGroupPolicySummarySidebar directly with a policy whose matching standard is not first in its comma-separated annotation, and asserts the policy appears in the filtered list. Verified this test fails without the sidebar's .trim() fix and passes with it. Signed-off-by: Sunny Gupta --- .../Governance/overview/Overview.test.tsx | 59 ++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/frontend/src/routes/Governance/overview/Overview.test.tsx b/frontend/src/routes/Governance/overview/Overview.test.tsx index 67e5c331aac..6b0d3ea567a 100644 --- a/frontend/src/routes/Governance/overview/Overview.test.tsx +++ b/frontend/src/routes/Governance/overview/Overview.test.tsx @@ -13,7 +13,8 @@ import { mockPolicy, mockPolicyNoStatus, } from '../governance.sharedMocks' -import GovernanceOverview from './Overview' +import GovernanceOverview, { SecurityGroupViolations } from './Overview' +import { SecurityGroupPolicySummarySidebar } from './SecurityGroupPolicySummarySidebar' import userEvent from '@testing-library/user-event' import { defaultContext, PluginDataContext } from '../../../lib/PluginDataContext' import { Policy, PolicyApiVersion, PolicyKind } from '../../../resources' @@ -214,4 +215,60 @@ describe('Overview Page', () => { expect(screen.getAllByText('NIST SP 800-53').length).toBe(1) expect(screen.getAllByText('PCI-DSS 4.0').length).toBe(1) }) + + test('SecurityGroupPolicySummarySidebar should match policies by trimmed annotation value', async () => { + // Regression test for the sidebar drill-down: a policy whose standard is not + // the first item in its comma-separated annotation must still match the + // clicked-on violation, which is keyed by the trimmed value. + const policyWithStandardLastInList: Policy = { + apiVersion: PolicyApiVersion, + kind: PolicyKind, + metadata: { + name: 'policy-standards-last-sidebar', + namespace: 'test', + uid: 'standards-test-uid-3', + annotations: { + 'policy.open-cluster-management.io/standards': 'CIS OpenShift Benchmark, PCI-DSS 4.0, NIST SP 800-53', + }, + }, + spec: { + disabled: false, + 'policy-templates': [], + remediationAction: 'inform', + }, + status: { + compliant: 'Compliant', + }, + } + const violation: SecurityGroupViolations = { + name: 'NIST SP 800-53', + compliant: 1, + noncompliant: 0, + pending: 0, + } + + const pluginData = { + ...defaultContext, + loadStarted: true, + loadCompleted: true, + } + render( + + { + snapshot.set(policiesState, [policyWithStandardLastInList]) + }} + > + + + + + + ) + + // Before the fix, the sidebar's filter compared the untrimmed " NIST SP 800-53" + // split token against violation.name ("NIST SP 800-53") and never matched, + // so the policy would not appear in this list. + expect(await screen.findByText('policy-standards-last-sidebar')).toBeInTheDocument() + }) })