[Bug] [CLI] kyverno test are applying previous mutation rules to subsequent test cases causing failures

[mvaalexp]

Kyverno CLI Version

1.9.2

Description

If you have 2 rules in the same policy that start with the same string, it seems to process the policy twice if both are specfied in the same kyverno-test.yaml file.

I am unsure if this is supposed to be a feature or some oversight, but it was very hard to debug. Seems like if you are specifying the rule to be tested, other rules should not be applied.

Steps to reproduce

name: policy-test
policies:
  - my-policy.yaml
resources:
  - resource.yaml
results:
  - policy: my-policy
    rule: foo-rule
    resource: resource-1
    patchedResource: patched-resource-1.yaml
    kind: Pod
    result: pass
  - policy: my-policy
    rule: foo-rule-bar # same prefix as foo-rule
    resource: resource-1
    kind: Pod
    result: skip

The second rule will fail because for some reason since foo-rule-bar starts with foo-rule it also applies the first result rule to resource-2. If you remove the first result rule, it passes.

Expected behavior

Rule names in prior results should not affect the results of later results if the prior rule is a substring of the current rule.

Screenshots

No response

Kyverno logs

No response

Slack discussion

No response

Troubleshooting

• I have read and followed the troubleshooting guide.
• I have searched other issues in this repository and mine is not recorded.

[welcome]

Thanks for opening your first issue here! Be sure to follow the issue template!

[mvaalexp]

If it helps, the exact names I used were:
add-nodeselector-lifecycle-on-demand
add-nodeselector-lifecycle-on-demand-append-hard

[mvaalexp]

Actually, I can't pin this down, seems like changing the name is not the problem, it just reordered the way the results were applied. What actually seems to be happening is mutations on prior results are getting applied, and it is affecting subsequent rules when using the same resource.

So in my example above, since we have a pass -> skip

  - policy: my-policy
    rule: foo-rule
    resource: resource-1
    patchedResource: patched-resource-1.yaml
    kind: Pod
    result: pass
  - policy: my-policy
    rule: foo-rule-bar # same prefix as foo-rule
    resource: resource-1
    kind: Pod
    result: skip

The first one seem to mutate the second one which is expecting a skip. If the first rule is commented out, the skip works as intended.

[mvaalexp]

Happen to run across this in the logs:

 EngineMutate "msg"="processing mutate rule" "applyRules"="All" 

I think the test may be applying rules (sorted by alphabetical from the results) instead of just the specific rule specified in the test.

[mvaalexp]

Alright, pretty sure I have narrowed it down now:
The rules are filtered out by name if they exist in the result set:
https://github.com/kyverno/kyverno/blob/main/cmd/cli/kubectl-kyverno/test/test_command.go#L816-L846

This means that adding a rule anywhere in the result list now gets applied to all resources in the result list. This is why when I comment one out, it starts to pass and no longer gets applied.

I can see it in the logs:

applying 1 policy to 3 resources..

https://github.com/kyverno/kyverno/blob/main/cmd/cli/kubectl-kyverno/test/test_command.go#L872

I am unsure why we would specify rule here if it doesn't apply exclusively to the test case.
This seems like a bug, seems like only the rule specified should be applied to the resource.

[chipzoller]

This seems like an obvious bug...

[mvaalexp]

I don't mind taking a stab at fixing this, but I need help understanding what is actually supposed to happen.
There are a few things I would like to fix:

1. it seems to default to a single file name, so if you want to run something like this: https://github.com/kyverno/kyverno/tree/main/test/cli/test-unit/selectResourcesForCheck you have to run it 4 times instead of just once, or you have to create 4 separate directories.

• Proposal, allow file prefix instead (file-prefix=kyverno-test)

2. You can supply multiple policies, but then the rule gets specified in the results

• Proposal, move rule from result and have policies accept rules

name: dummy-policy
policies:
- file: dummy-policy.yaml
  rules: 
  - require-image-tag
resources:
- resource-duplicates.yaml

results:
- policy: dummy-policy
  resource: myapp-pod1
  kind: Pod
  result: pass
- policy: dummy-policy
  resource: myapp-pod2
  kind: Pod
  result: pass

This would be a breaking change though, but makes more sense than what is currently happening.

[eddycharly]

@mvaalexp can you provide the manifests to reproduce the issue ?

I think it's fixed now the cli was refactored but i would like to confirm and add a test before closing the issue.

[mvaalexp]

Give me a few hours, I found my old test case for this and was able to recreate but it has proprietary stuff in it, let me attempt to remove it.

[eddycharly]

@mvaalexp sure, awesome ! thanks

[mvaalexp]

I have attached the test case.
cli-issue.zip

[eddycharly]

I guess you expect both tests to pass ?

[eddycharly]

@mvaalexp there are a couple of issues with your test.

1. why do you expect this to be skipped (second test case) ?

  - policy: karpenter-annotations-to-nodeselector
    rule: hard-nodeselector-lifecycle-on-demand
    resource: soft-pod-antiaffinity-1-copy
    kind: Pod
    result: skip

2. we don't record the state of a resource at the rule level so when providing a patchedResource it's actually compared to what came out of the entire policy, not a specific rule.

[eddycharly]

With the considerations above the cli is now fixed and it will be available in 1.11.

[mvaalexp]

When ran individually, they both succeed, when ran together, it fails.

I am not at my desk at the moment, so will need to refresh my memory, the point is test 1 is altering the state of test 2 (which it shouldn't). If you comment out

# nodeselector-lifecycle-on-demand
  - policy: karpenter-annotations-to-nodeselector
    rule: nodeselector-lifecycle-on-demand
    resource: soft-pod-antiaffinity-1
    patchedResource: patchSoftPodAntiaffinity1OnDemand.yaml
    kind: Pod
    result: pass

from the results, it does indeed skip and pass the test

[eddycharly]

We changed the cli, rules are not removed anymore.

Now the CLI evaluates all policies you provide against all resources and we use that to check the test results. This is closer to what happens in real world and is easier to reason about.

So now, the second test case is not skipped anymore, because of the condition:

          - key: "{{ request.object.spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution || '' }}"
            operator: NotEquals
            value: ''

request.object.spec.affinity.nodeAffinity.requiredDuringSchedulingIgnoredDuringExecution was set in the previous rule.

[eddycharly]

@mvaalexp see #8363 for the fix PR (it includes a new test suite based on yours).

[mvaalexp]

Now the CLI evaluates all policies you provide against all resources and we use that to check the test results. This is closer to what happens in real world and is easier to reason about.

By all policies, do you mean all rules in a policy (I agree this makes more sense)?

If so, then shouldn't the rule field be removed entirely?

  - policy: karpenter-annotations-to-nodeselector
    rule: hard-nodeselector-lifecycle-on-demand # <-- shouldn't this be removed?
    resource: soft-pod-antiaffinity-1-copy
    kind: Pod
    result: pass

[eddycharly]

By all policies, do you mean all rules in a policy (I agree this makes more sense)?

Yes, we record the mutated resource per policy, not per rule, hence patchedResource cannot be bound to a specific rule.

If so, then shouldn't the rule field be removed entirely?

That's a good question. Either something like this, or we can record the patched resource at the rule level, not sure what is best.