ACM-34249: Add Kyverno CEL-based discovered policy types#6331
Open
Randy424 wants to merge 3 commits into
Open
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Randy424 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughCentralize Kyverno API-group detection (kyverno.io + policies.kyverno.io) and apply it across discovery, grouping, rendering, filtering, fetching, and tests. ChangesKyverno API Group Expansion
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/routes/Governance/discovered/details/DiscoveredByCluster.tsx (1)
66-69: 💤 Low valueConsider using
isLegacyKyvernoApiGrouphelper for consistency.For consistency with the rest of the PR's centralized API-group checks, line 67 could use
isLegacyKyvernoApiGroup(apiGroup)instead of the hardcodedapiGroup === 'kyverno.io'.♻️ Proposed refactor for consistency
- } else if ( - (apiGroup === 'kyverno.io' && policyKind === 'Policy') || - (apiGroup === 'policies.kyverno.io' && policyKind.startsWith('Namespaced')) - ) { + } else if ( + (isLegacyKyvernoApiGroup(apiGroup) && policyKind === 'Policy') || + (apiGroup === 'policies.kyverno.io' && policyKind.startsWith('Namespaced')) + ) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/Governance/discovered/details/DiscoveredByCluster.tsx` around lines 66 - 69, Replace the hardcoded legacy Kyverno API-group check in DiscoveredByCluster's conditional ((apiGroup === 'kyverno.io' && policyKind === 'Policy') ...) with the centralized helper isLegacyKyvernoApiGroup(apiGroup); update the condition to use isLegacyKyvernoApiGroup(apiGroup) && policyKind === 'Policy' (keeping the other polices.kyverno.io check intact), and add an import for isLegacyKyvernoApiGroup at the top of the file if it's not already imported so the DiscoveredByCluster component uses the shared API-group helper consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/routes/Governance/discovered/details/DiscoveredByCluster.tsx`:
- Around line 66-69: Replace the hardcoded legacy Kyverno API-group check in
DiscoveredByCluster's conditional ((apiGroup === 'kyverno.io' && policyKind ===
'Policy') ...) with the centralized helper isLegacyKyvernoApiGroup(apiGroup);
update the condition to use isLegacyKyvernoApiGroup(apiGroup) && policyKind ===
'Policy' (keeping the other polices.kyverno.io check intact), and add an import
for isLegacyKyvernoApiGroup at the top of the file if it's not already imported
so the DiscoveredByCluster component uses the shared API-group helper
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a591d4b6-c6d7-4114-8ce5-1720075ffbf8
📒 Files selected for processing (11)
frontend/src/routes/Governance/common/util.test.tsxfrontend/src/routes/Governance/common/util.tsxfrontend/src/routes/Governance/discovered/DiscoveredPolicies.test.tsxfrontend/src/routes/Governance/discovered/DiscoveredPolicies.tsxfrontend/src/routes/Governance/discovered/details/DiscoveredByCluster.tsxfrontend/src/routes/Governance/discovered/details/DiscoveredResources.tsxfrontend/src/routes/Governance/discovered/details/common.test.tsxfrontend/src/routes/Governance/discovered/details/common.tsxfrontend/src/routes/Governance/discovered/grouping.test.tsfrontend/src/routes/Governance/discovered/grouping.tsfrontend/src/routes/Governance/discovered/useFetchPolicies.tsx
Add support for new policies.kyverno.io/v1 API group types (ValidatingPolicy, MutatingPolicy, GeneratingPolicy, ImageValidatingPolicy and their Namespaced variants) alongside existing kyverno.io/v1 types. Legacy types display a deprecation label. Includes unit tests covering new helper functions, grouping logic, violation summary, and response action filters. Signed-off-by: Randy Bruno Piverger <[email protected]> Co-Authored-By: Claude Opus 4.6 <[email protected]>
Randy424
force-pushed
the
ACM-34249/kyverno-discovered-policies
branch
from
8170a3d to
074fccd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/routes/Governance/discovered/details/common.test.tsx (1)
257-280: ⚡ Quick winReplace
anycasts in the new response-action filter tests.Lines 257, 268, and 280 use
any, which bypasses strict typing in this frontend test path.Proposed fix
- const denyOption = filter.options.find((o: any) => o.value === 'Deny') + const denyOption = filter.options.find((o) => o.value === 'Deny') - const item = { + const item: Pick<DiscoveredPolicyItem, 'apigroup' | 'responseAction'> = { apigroup: 'policies.kyverno.io', responseAction: 'Deny', - } as any + } - const item = { + const item: Pick<DiscoveredPolicyItem, 'apigroup' | 'responseAction'> = { apigroup: 'kyverno.io', responseAction: 'Audit', - } as any + }As per coding guidelines, “Avoid
anytype — useunknownif the type is truly unknown.”#!/bin/bash rg -nP '\bas\s+any\b|:\s*any\b' frontend/src/routes/Governance/discovered/details/common.test.tsx🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/routes/Governance/discovered/details/common.test.tsx` around lines 257 - 280, Replace the unsafe "as any" casts in the tests by giving the test items a concrete type or using unknown then narrowing: declare a small interface/type (e.g., ResponseActionItem { apigroup: string; responseAction: string }) and type the test item constants with it (const item: ResponseActionItem = { ... }) and ensure calls to filter.tableFilterFn receive that typed item; alternatively, if the real shape is unknown, cast with "as unknown as ResponseActionItem" rather than "as any". Update occurrences around denyOption/test blocks and the tests that call getResponseActionFilter and tableFilterFn to use the new typed item instead of "as any".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/routes/Governance/discovered/details/common.tsx`:
- Around line 38-43: The code treats namespaced Kyverno policies via
isNamespacedKyverno and calls addComplianceToKyvernoPolicyViolations(policy,
compliance, kyvernoPolicyViolations) then continues before the later
disabled-policy check, so disabled policies get aggregated; fix by performing
the disabled check before handling namespaced Kyverno entries (or include the
disabled condition in the isNamespacedKyverno branch) so that policies with
disabled truthy flag are skipped and not passed to
addComplianceToKyvernoPolicyViolations; update the control flow around
isNamespacedKyverno, addComplianceToKyvernoPolicyViolations, and the existing
disabled check to ensure disabled policies are filtered out first.
---
Nitpick comments:
In `@frontend/src/routes/Governance/discovered/details/common.test.tsx`:
- Around line 257-280: Replace the unsafe "as any" casts in the tests by giving
the test items a concrete type or using unknown then narrowing: declare a small
interface/type (e.g., ResponseActionItem { apigroup: string; responseAction:
string }) and type the test item constants with it (const item:
ResponseActionItem = { ... }) and ensure calls to filter.tableFilterFn receive
that typed item; alternatively, if the real shape is unknown, cast with "as
unknown as ResponseActionItem" rather than "as any". Update occurrences around
denyOption/test blocks and the tests that call getResponseActionFilter and
tableFilterFn to use the new typed item instead of "as any".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 89f10099-b089-458e-8b94-badbb06a7b2d
📒 Files selected for processing (11)
frontend/src/routes/Governance/common/util.test.tsxfrontend/src/routes/Governance/common/util.tsxfrontend/src/routes/Governance/discovered/DiscoveredPolicies.test.tsxfrontend/src/routes/Governance/discovered/DiscoveredPolicies.tsxfrontend/src/routes/Governance/discovered/details/DiscoveredByCluster.tsxfrontend/src/routes/Governance/discovered/details/DiscoveredResources.tsxfrontend/src/routes/Governance/discovered/details/common.test.tsxfrontend/src/routes/Governance/discovered/details/common.tsxfrontend/src/routes/Governance/discovered/grouping.test.tsfrontend/src/routes/Governance/discovered/grouping.tsfrontend/src/routes/Governance/discovered/useFetchPolicies.tsx
✅ Files skipped from review due to trivial changes (1)
- frontend/src/routes/Governance/discovered/DiscoveredPolicies.test.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- frontend/src/routes/Governance/discovered/details/DiscoveredResources.tsx
- frontend/src/routes/Governance/common/util.tsx
- frontend/src/routes/Governance/discovered/useFetchPolicies.tsx
- frontend/src/routes/Governance/discovered/grouping.test.ts
- frontend/src/routes/Governance/common/util.test.tsx
- frontend/src/routes/Governance/discovered/details/DiscoveredByCluster.tsx
- frontend/src/routes/Governance/discovered/DiscoveredPolicies.tsx
- frontend/src/routes/Governance/discovered/grouping.ts
…ummary Move the disabled-policy check before the isNamespacedKyverno branch so that disabled policies are skipped instead of being passed to addComplianceToKyvernoPolicyViolations. Signed-off-by: Randy Bruno Piverger <[email protected]> Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
Author
|
Holding temporarily. Will complete manual test plan and will supply some screenshot to the PR thread before opening up for review. Thanks. |
Replace inlined Split/Label/ExclamationTriangleIcon markup with the existing DeprecatedTitle component from Applications, removing redundant PatternFly imports. Signed-off-by: Randy Bruno Piverger <[email protected]> Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds support for the 8 new Kyverno CEL-based policy types (
policies.kyverno.io/v1) to the Discovered Policies page, alongside the existing legacy types (kyverno.io/v1). Legacy types now display a deprecation label.Changes:
ValidatingPolicy,MutatingPolicy,GeneratingPolicy,ImageValidatingPolicy, and theirNamespacedvariantsExclamationTriangleIcon) on legacykyverno.iopolicy namesKyverno Denyoption for new types usingvalidationActions[]cluster:nameisKyvernoApiGroup(),isLegacyKyvernoApiGroup();getEngineString()handlespolicies.kyverno.ioapiGroup === 'kyverno.io'checks updated toisKyvernoApiGroup()for both API groupsDesign doc: ACM-DDR-074
Test plan
kyverno.ioClusterPolicy/Policy rows show orange "Deprecated" labelpolicies.kyverno.iotypes do NOT show deprecation labeltotalViolations)Kyverno Denyresponse action filter works for new typesSigned-off-by: Randy Bruno Piverger [email protected]
Summary by CodeRabbit
New Features
UI/UX Improvements
Tests
Screenshots:
New Kyverno types appear in Discovered Policy Table, ClusterPolicy and Policy types now render alongside "Deprecated" tag
Table filter supports new types:
<img width="936" height="621" alt="Screenshot 2026-06-12 at 3 53 31 PM" src="https://github.com/user-attachments/assets/354d14df-12
Discovered Policies > Details > Clusters displays namespace for namespaced Kyverno.
e1-42fc-875c-dc8a1a92050c" />
Discovered Policies > Details > Related resources supports "Reason" column for new Kyverno types
Discovered Policies > Details > Clusters new Kyverno filter values for "Response action"
