8000 Fix test-case-selector by dhimanAbhi · Pull Request #12681 · kyverno/kyverno · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix test-case-selector #12681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix test-case-selector #12681

wants to merge 3 commits into from

Conversation

dhimanAbhi
Copy link
Contributor

Explanation

This PR addresses the issue #12456, #10406 by fixing the resource filtering logic inside the test-case-selector flag. According to the 8000 documention, the test-case-selector allows to run a subset of tests based on policy, rule and resource. However, current behavior of the Kyverno's test-case-selector allows filtering the test cases by policy and rule, but fails to apply filtering based on the resource name. As a result, even when a specific resource is provided via the selector, all the resources are tested instead of the mentioned one. This fix ensures that test cases are filtered by resource selector as well.

Related issue

Closes #12456, #10406

Milestone of this PR

What type of PR is this

/kind bug

Proposed Changes

This PR addresses a bug in the test-case-selector functionality of the Kyverno CLI, where the resource filter was previously ineffective. Despite specifying a resource using the --test-case-selector flag (e.g., resource=nginx-demo2), the CLI continued to run tests against all resources instead of filtering by the specified one. This behavior was inconsistent with the intended functionality described in the official documentation. The proposed fix ensures that the resource value is now properly parsed and respected during test selection, enabling users to run targeted tests based on policy, rule, and resource

Before:

$ kubectl-kyverno test .
Loading test  ( kyverno-test.yaml ) ...
  Loading values/variables ...
  Loading policies ...
  Loading resources ...
  Loading exceptions ...
  Applying 1 policy to 3 resources with 0 exceptions ...
  Checking results ...

│────│─────────────────────────│──────────────────────────│──────────────────────────│────────│────────│
│ ID │ POLICY                  │ RULE                     │ RESOURCE                 │ RESULT │ REASON │
│────│─────────────────────────│──────────────────────────│──────────────────────────│────────│────────│
│ 1  │ require-requests-limits │ validate-resources       │ v1/Pod/default/goodpod01 │ Pass   │ Ok     │
│ 2  │ require-requests-limits │ validate-container-names │ v1/Pod/default/badpod01  │ Pass   │ Ok     │
│ 3  │ require-requests-limits │ validate-container-names │ v1/Pod/default/badpod02  │ Pass   │ Ok     │
│────│─────────────────────────│──────────────────────────│──────────────────────────│────────│────────│


Test Summary: 3 tests passed and 0 tests failed

$ kubectl-kyverno test . --test-case-selector "policy=require-requests-limits, rule=validate-container-names, resource=badpod01"
Loading test  ( kyverno-test.yaml ) ...
  Loading values/variables ...
  Loading policies ...
  Loading resources ...
  Loading exceptions ...
  Applying 1 policy to 3 resources with 0 exceptions ...
  Checking results ...

│────│─────────────────────────│──────────────────────────│─────────────────────────│────────│────────│
│ ID │ POLICY                  │ RULE                     │ RESOURCE                │ RESULT │ REASON │
│────│─────────────────────────│──────────────────────────│─────────────────────────│────────│────────│
│ 1  │ require-requests-limits │ validate-container-names │ v1/Pod/default/badpod01 │ Pass   │ Ok     │
│ 2  │ require-requests-limits │ validate-container-names │ v1/Pod/default/badpod02 │ Pass   │ Ok     │
│────│─────────────────────────│──────────────────────────│─────────────────────────│────────│────────│


Test Summary: 2 tests passed and 0 tests failed

After:

$ kubectl-kyverno test .
Loading test  ( kyverno-test.yaml ) ...
  Loading values/variables ...
  Loading policies ...
  Loading resources ...
  Loading exceptions ...
  Applying 1 policy to 3 resources with 0 exceptions ...
  Checking results ...

│────│─────────────────────────│──────────────────────────│──────────────────────────│────────│────────│
│ ID │ POLICY                  │ RULE                     │ RESOURCE                 │ RESULT │ REASON │
│────│─────────────────────────│──────────────────────────│──────────────────────────│────────│────────│
│ 1  │ require-requests-limits │ validate-resources       │ v1/Pod/default/goodpod01 │ Pass   │ Ok     │
│ 2  │ require-requests-limits │ validate-container-names │ v1/Pod/default/badpod01  │ Pass   │ Ok     │
│ 3  │ require-requests-limits │ validate-container-names │ v1/Pod/default/badpod02  │ Pass   │ Ok     │
│────│─────────────────────────│──────────────────────────│──────────────────────────│────────│────────│


Test Summary: 3 tests passed and 0 tests failed


$ kubectl-kyverno test . --test-case-selector "policy=require-requests-limits, rule=validate-container-names, resource=badpod01"
Loading test  ( kyverno-test.yaml ) ...
  Loading values/variables ...
  Loading policies ...
  Loading resources ...
  Loading exceptions ...
  Applying 1 policy to 3 resources with 0 exceptions ...
  Checking results ...

│────│─────────────────────────│──────────────────────────│─────────────────────────│────────│────────│
│ ID │ POLICY                  │ RULE                     │ RESOURCE                │ RESULT │ REASON │
│────│─────────────────────────│──────────────────────────│─────────────────────────│────────│────────│
│ 1  │ require-requests-limits │ validate-container-names │ v1/Pod/default/badpod01 │ Pass   │ Ok     │
│────│─────────────────────────│──────────────────────────│─────────────────────────│────────│────────│


Test Summary: 1 tests passed and 0 tests failed

Proof Manifests

require-pod-requests-limits.yaml

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: require-requests-limits
  annotations:
    policies.kyverno.io/title: Require Limits and Requests
    policies.kyverno.io/category: Best Practices, EKS Best Practices
    policies.kyverno.io/severity: medium
    policies.kyverno.io/subject: Pod
    policies.kyverno.io/minversion: 1.6.0
    policies.kyverno.io/description: >-
      As application workloads share cluster resources, it is important to limit resources
      requested and consumed by each Pod. It is recommended to require resource requests and
      limits per Pod, especially for memory and CPU. If a Namespace level request or limit is specified,
      defaults will automatically be applied to each Pod based on the LimitRange configuration.
      This policy validates that all containers have something specified for memory and CPU
      requests and memory limits.
spec:
  validationFailureAction: Audit
  background: true
  rules:
  - name: validate-resources
    match:
      any:
      - resources:
          kinds:
          - Pod
    validate:
      message: "CPU and memory resource requests and memory limits are required for containers."
      pattern:
        spec:
          containers:
          - resources:
              requests:
                memory: "?*"
                cpu: "?*"
  - name: validate-container-names
    match:
      any:
      - resources:
          kinds:
          - Pod
    validate:
      message: "Container names must be prefixed with 'myapp-'."
      pattern:
        spec:
          containers:
          - name: myapp-*

resource.yaml

apiVersion: v1
kind: Pod
metadata:
  name: goodpod01
  labels:
    app: myapp
spec:
  containers:
  - name: nginx
    image: nginx
    resources:
      requests:
        memory: "256Mi"
        cpu: "0.5"
      limits:
        memory: "256Mi"
---
apiVersion: v1
kind: Pod
metadata:
  name: badpod01
  labels:
    app: myapp
spec:
  containers:
  - name: nginx
    image: nginx
---
apiVersion: v1
kind: Pod
metadata:
  name: badpod02
  labels:
    app: myapp
spec:
  containers:
  - name: nginx
    image: nginx
    resources:
      requests:

kyverno-test.yaml

apiVersion: cli.kyverno.io/v1alpha1
kind: Test
metadata:
  name: require-requests-limits
policies:
- ./require-pod-requests-limits.yaml
resources:
- ./resource.yaml
results:
- kind: Pod
  policy: require-requests-limits
  resources:
  - goodpod01
  result: pass
  rule: validate-resources
- kind: Pod
  policy: require-requests-limits
  resources:  
  - badpod01
  - badpod02
  result: fail
  rule: validate-container-names

Proof Manifests

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.

Further Comments

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 9, 2025
@realshuting
Copy link
Member

@dhimanAbhi - can you please sign off your commits?

@dhimanAbhi dhimanAbhi force-pushed the fix_test-case-selector branch from 31c461e to a885d2a Compare April 24, 2025 06:07
@dhimanAbhi
fix test-case-selector
1fea2bf 
Signed-off-by: Abhishek Dhiman <abhi2002dhiman@gmail.com>
@dhimanAbhi
fix test-case-selector
6d23bcd 
Signed-off-by: abhi2002dhi <abhi2002dhiman@gmail.com>
Signed-off-by: Abhishek Dhiman <abhi2002dhiman@gmail.com>
@dhimanAbhi
add unit test
033c07b 
Signed-off-by: Abhishek Dhiman <abhi2002dhiman@gmail.com>
@dhimanAbhi dhimanAbhi force-pushed the fix_test-case-selector branch from a885d2a to 033c07b Compare April 25, 2025 09:21
@codecov Codecov
Copy link
codecov bot commented Apr 25, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 72.72727% with 6 lines in your changes missing coverage. Please review.

Project coverage is 14.62%. Comparing base (b87ff5d) to head (033c07b).
Report is 74 commits behind head on main.

Files with missing lines Patch % Lines
cmd/cli/kubectl-kyverno/commands/test/command.go 25.00% 3 Missing ⚠️
cmd/cli/kubectl-kyverno/test/filter/filter.go 83.33% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main   #12681   +/-   ##
=======================================
  Coverage   14.61%   14.62%           
=======================================
  Files         932      932           
  Lines      102895   102913   +18     
=======================================
+ Hits        15037    15047   +10     
- Misses      86118    86124    +6     
- Partials     1740     1742    +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dhimanAbhi
Copy link
Contributor Author

@realshuting As per our discussion from our last community meeting, I have added unit tests as well and signed my commits. Please Review.

Thanks!

@realshuting realshuting self-requested a review May 7, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@realshuting realshuting Awaiting requested review from realshuting

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] [CLI] test-case-selector option does not work with resource
2 participants
@dhimanAbhi @realshuting
0