Skip to content
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.

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

[Bug] Issue with remove elements on the existing target resource with a precondition #10224

Closed
2 tasks done
baechul opened this issue May 11, 2024 · 3 comments · Fixed by #10252
Closed
2 tasks done

[Bug] Issue with remove elements on the existing target resource with a precondition #10224

baechul opened this issue May 11, 2024 · 3 comments · Fixed by #10252
Assignees
Labels
bug Something isn't working mutation-existing Related to the mutate existing ability. regression Issues (bugs) which are regressions from an earlier release. release-critical Critical issues which MUST be addressed in the specified milestone. These cannot get bumped.

Comments

@baechul
Copy link

baechul commented May 11, 2024

Kyverno Version

1.12.0

Description

I have this policy that is triggered by the VirtualService (helloworld2) deletion and mutate another VirtualService (helloworld1).
./policy.yaml:

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: foreach-remove-elements
spec:
  background: false
  schemaValidation: false
  rules:
  - name: remove-elements
    match:
      all:
      - resources:
          kinds:
            - VirtualService
          names:
            - helloworld2  
          operations:
            - DELETE
    mutate:
      targets:
      - apiVersion: networking.istio.io/v1alpha3
        kind: VirtualService
        name: helloworld1
        namespace: default
      foreach:
      - list: "target.spec.http"
        order: Descending 
        preconditions:
          all:
          - key: "{{ element.route[0].destination.host }}"
            operator: Equals
            value: helloworld-three
        patchesJson6902: |-
          - op: remove
            path: /spec/http/{{elementIndex}} 

And 2 deployments per each VirtualService:
./deploy1.yaml

# Target Resource (Mutated) 
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: helloworld1
spec:
  http:
  - match:
    - uri:
        exact: /one
    route:
    - destination:
        host: helloworld-one
        port:
          number: 81
  - match:
    - uri:
        exact: /two
    route:
    - destination:
        host: helloworld-two
        port:
          number: 82
  - match:
    - uri:
        exact: /three
    route:
    - destination:
        host: helloworld-three
        port:
          number: 83

./deploy2.yaml

# Trigger Resource
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
  name: helloworld2
spec:
  http:
  - match:
    - uri:
        exact: /any
    route:
    - destination:
        host: helloworld-haha
        port:
          number: 80

Steps to reproduce:
In a cluster with Istio/Kyverno (v1.12.0)

kubectl apply -f policy.yaml
kubectl apply -f deploy1.yaml
kubectl apply -f deploy2.yaml

At this time, target VS (helloworld1) has this spec.http list.

spec:
  http:
  - match:
    - uri:
        exact: /one
    route:
    - destination:
        host: helloworld-one
        port:
          number: 81
  - match:
    - uri:
        exact: /two
    route:
    - destination:
        host: helloworld-two
        port:
          number: 82
  - match:
    - uri:
        exact: /three
    route:
    - destination:
        host: helloworld-three
        port:
          number: 83

They are ordered by destination.host: helloworld-one, helloworld-two and helloworld-three
Now I will trigger the policy by deleting the helloworld2 VirtualService. The policy is supposed to remove the last one, helloworld-three HTTP route rule.

kubectl delete -f deploy2.yaml

The expected destination.host of the target resource is

spec:
  http:
  - match:
    - uri:
        exact: /one
    route:
    - destination:
        host: helloworld-one
        port:
          number: 81
  - match:
    - uri:
        exact: /two
    route:
    - destination:
        host: helloworld-two
        port:
          number: 82

However the output I got is incorrect:

spec:
  http:
  - match:
    - uri:
        exact: /three
    route:
    - destination:
        host: helloworld-three
        port:
          number: 83
  - match:
    - uri:
        exact: /two
    route:
    - destination:
        host: helloworld-two
        port:
          number: 82     

Here is the log from the background controller:

2024-05-11T22:52:48Z	INFO	engine.mutate	mutation/common.go:107	mutate.foreach.preconditions not met	{"policy.name": "foreach-remove-elements", "policy.namespace": "", "policy.apply": "All", "new.kind": "VirtualService", "new.namespace": "default", "new.name": "helloworld2", "old.kind": "VirtualService", "old.namespace": "default", "old.name": "helloworld2", "rule.name": "remove-elements", "elementIndex": 1, "message": ""}
2024-05-11T22:52:48Z	INFO	engine.mutate	mutation/common.go:107	mutate.foreach.preconditions not met	{"policy.name": "foreach-remove-elements", "policy.namespace": "", "policy.apply": "All", "new.kind": "VirtualService", "new.namespace": "default", "new.name": "helloworld2", "old.kind": "VirtualService", "old.namespace": "default", "old.name": "helloworld2", "rule.name": "remove-elements", "elementIndex": 0, "message": ""}
2024-05-11T22:52:48Z	INFO	engine.mutate	mutation/common.go:49	mutateResp.PatchedResource	{"policy.name": "foreach-remove-elements", "policy.namespace": "", "policy.apply": "All", "new.kind": "VirtualService", "new.namespace": "default", "new.name": "helloworld2", "old.kind": "VirtualService", "old.namespace": "default", "old.name": "helloworld2", "rule.name": "remove-elements", "resource": {"Object":{"apiVersion":"networking.istio.io/v1alpha3","kind":"VirtualService","metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"networking.istio.io/v1alpha3\",\"kind\":\"VirtualService\",\"metadata\":{\"annotations\":{},\"name\":\"helloworld1\",\"namespace\":\"default\"},\"spec\":{\"http\":[{\"match\":[{\"uri\":{\"exact\":\"/one\"}}],\"route\":[{\"destination\":{\"host\":\"helloworld-one\",\"port\":{\"number\":81}}}]},{\"match\":[{\"uri\":{\"exact\":\"/two\"}}],\"route\":[{\"destination\":{\"host\":\"helloworld-two\",\"port\":{\"number\":82}}}]},{\"match\":[{\"uri\":{\"exact\":\"/three\"}}],\"route\":[{\"destination\":{\"host\":\"helloworld-three\",\"port\":{\"number\":83}}}]}]}}\n"},"creationTimestamp":"2024-05-11T22:52:15Z","generation":1,"managedFields":[{"apiVersion":"networking.istio.io/v1alpha3","fieldsType":"FieldsV1","fieldsV1":{"f:metadata":{"f:annotations":{".":{},"f:kubectl.kubernetes.io/last-applied-configuration":{}}},"f:spec":{".":{},"f:http":{}}},"manager":"kubectl-client-side-apply","operation":"Update","time":"2024-05-11T22:52:15Z"}],"name":"helloworld1","namespace":"default","resourceVersion":"127236","uid":"aafe8f8b-3230-4869-9bfa-3ff8de46afc6"},"spec":{"http":[{"match":[{"uri":{"exact":"/three"}}],"route":[{"destination":{"host":"helloworld-three","port":{"number":83}}}]},{"match":[{"uri":{"exact":"/two"}}],"route":[{"destination":{"host":"helloworld-two","port":{"number":82}}}]}]}}}}

There are 2 issues:

  1. A wrong HTTP route rule was removed. The hellowworld-one was removed while hellowworld-three should have been removed. The 1st iteration (elementIndex=2) in the reversed list meet the precondition and it looks elementIndex=2 was applied to the reversed list instead of applied to the original list. The reversed list's index, 2 is the first route rule in the original list hence it looks helloworld-one was mistakenly removed.
  2. The output has the revered order. I thought when Descending is given to foreach's order, it just internally iterates reverse order for patching but the output should in the same order in the given list. In VirtualService, the order is important as it is applied from top to bottom. The order of the list elements after removal should be kept the same before mutated..

Other thing I noticed:
Removing elements in the standard mutation (i.e mutating the trigger resources) worked as expected. The correct one was removed and the un-mutated elements of the list remained in the same order.

Slack discussion

No response

Troubleshooting

  • I have read and followed the documentation AND the troubleshooting guide.
  • I have searched other issues in this repository and mine is not recorded.
@baechul baechul added bug Something isn't working triage Default label assigned to all new issues indicating label curation is needed to fully organize. labels May 11, 2024
Copy link

welcome bot commented May 11, 2024

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

@baechul
Copy link
Author

baechul commented May 15, 2024

I checked both 1.11.4 and 1.11.5. It worked as expected. Hence it must be a regression issue.

@JimBugwadia JimBugwadia removed the triage Default label assigned to all new issues indicating label curation is needed to fully organize. label May 15, 2024
@JimBugwadia JimBugwadia added this to the Kyverno Release 1.12.2 milestone May 15, 2024
@realshuting realshuting added mutation-existing Related to the mutate existing ability. regression Issues (bugs) which are regressions from an earlier release. labels May 16, 2024
@realshuting realshuting added the release-critical Critical issues which MUST be addressed in the specified milestone. These cannot get bumped. label May 16, 2024
@baechul
Copy link
Author

baechul commented May 22, 2024

The fix is verified with a 1.12.2 RC version. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mutation-existing Related to the mutate existing ability. regression Issues (bugs) which are regressions from an earlier release. release-critical Critical issues which MUST be addressed in the specified milestone. These cannot get bumped.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants