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

Nested foreach #5589

Merged
merged 28 commits into from
Dec 12, 2022
Merged

Nested foreach #5589

merged 28 commits into from
Dec 12, 2022

Conversation

JimBugwadia
Copy link
Member

@JimBugwadia JimBugwadia commented Dec 6, 2022

Explanation

Support nested foreach

Related issue

Closes #5451

Milestone of this PR

1.9

What type of PR is this

/kind feature

Proposed Changes

Support nested foreach in validate and mutate rules

Proof Manifests

apiVersion: kyverno.io/v2beta1
kind: ClusterPolicy
metadata:
  name: replace-image-registry
spec:
  background: false
  validationFailureAction: Enforce
  rules:
  - name: replace-dns-suffix
    match:
      any:
      - resources:
          kinds:
            - Ingress
    mutate:
      foreach:
      - list: request.object.spec.rules
        patchesJson6902: |-
          - path: /spec/rules/{{elementIndex}}/host
            op: replace
            value: {{replace('{{element.host}}', '.foo.com', '.newfoo.com', `1`)}}
      - list: request.object.spec.tls
        patchesJson6902: |-
          - path: /spec/tls/{{elementIndex}}/secretName
            op: replace
            value: {{replace('{{element.secretName}}', '.foo.com', '.newfoo.com', `1`)}}         
      - list: request.object.spec.tls
        foreach:
        - list: "element.hosts"
          patchesJson6902: |-
            - path: "/spec/tls/{{elementIndex0}}/hosts/{{elementIndex1}}"
              op: replace
              value: "{{replace_all('{{element}}', '.foo.com', '.newfoo.com')}}"
apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: replace-image-registry
spec:
  background: false
  rules:
    - name: replace-dns-suffix
      match:
        any:
          - resources:
              kinds:
                - Ingress
      validate:
        foreach:
          - list: "request.object.spec.rules"
            foreach:
            - list: "{{element}}.http.paths"
              pattern:
                path: "/"   

Resource:

---
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: tls-example-ingress
spec:
  tls:
  - hosts:
    - https-example.foo.com
    secretName: testsecret-tls
  - hosts:
    - https-example2.foo.com
    secretName: testsecret-tls-2
  rules:
  - host: https-example.foo.com
    http:
      paths:
      - path: "/"
        pathType: Prefix
        backend:
          service:
            name: service1
            port:
              number: 80
  - host: https-example2.foo.com
    http:
      paths:
      - path: "/"
        pathType: Prefix
        backend:
          service:
            name: service2
            port:
              number: 80

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.
    • I have added or changed the documentation myself in an existing PR and the link is:
    • I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@codecov
Copy link

codecov bot commented Dec 6, 2022

Codecov Report

Merging #5589 (f98f258) into main (d36a42b) will increase coverage by 0.06%.
The diff coverage is 51.55%.

@@            Coverage Diff             @@
##             main    #5589      +/-   ##
==========================================
+ Coverage   36.06%   36.13%   +0.06%     
==========================================
  Files         177      178       +1     
  Lines       19848    19900      +52     
==========================================
+ Hits         7159     7191      +32     
- Misses      11896    11916      +20     
  Partials      793      793              
Impacted Files Coverage Δ
api/kyverno/v1/common_types.go 0.00% <0.00%> (ø)
api/kyverno/v1/zz_generated.deepcopy.go 3.14% <0.00%> (-0.03%) ⬇️
pkg/engine/context/context.go 28.39% <0.00%> (-0.72%) ⬇️
pkg/engine/utils.go 70.58% <ø> (+0.27%) ⬆️
pkg/engine/variables/vars.go 54.30% <0.00%> (+3.52%) ⬆️
pkg/utils/api/json.go 0.00% <0.00%> (ø)
pkg/engine/forceMutate.go 35.08% <23.07%> (-8.82%) ⬇️
pkg/engine/mutate/mutation.go 35.52% <33.33%> (ø)
pkg/engine/validation.go 64.23% <66.66%> (+0.37%) ⬆️
pkg/engine/mutation.go 64.36% <72.09%> (+3.87%) ⬆️
... and 6 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pkg/engine/mutation.go Outdated Show resolved Hide resolved
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
JimBugwadia and others added 10 commits December 9, 2022 19:26
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
.vscode/launch.json Outdated Show resolved Hide resolved
api/kyverno/v1/common_types.go Show resolved Hide resolved
api/kyverno/v1/common_types.go Show resolved Hide resolved
api/kyverno/v1/common_types.go Show resolved Hide resolved
pkg/engine/mutation.go Outdated Show resolved Hide resolved
pkg/engine/variables/vars.go Show resolved Hide resolved
pkg/openapi/manager_test.go Outdated Show resolved Hide resolved
@realshuting
Copy link
Member

Logged a doc issue kyverno/website#719.

return patchedResource, nil
}

func applyForeachMutate(name string, foreach []kyvernov1.ForEachMutation, resource unstructured.Unstructured, ctx context.Interface, logger logr.Logger) (patchedResource unstructured.Unstructured, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func applyForeachMutate(name string, foreach []kyvernov1.ForEachMutation, resource unstructured.Unstructured, ctx context.Interface, logger logr.Logger) (patchedResource unstructured.Unstructured, err error) {
func applyForEachMutate(name string, foreach []kyvernov1.ForEachMutation, resource unstructured.Unstructured, ctx context.Interface, logger logr.Logger) (patchedResource unstructured.Unstructured, err error) {

Can we stick with ForEach -> we capitalize it everywhere else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! Will create a new PR for that.

rclient registryclient.Client
nesting int
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is nesting actually used for? It seems to just be in the context - so only in logs? Or am I missing something

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @MarcelMue - the nesting count is used to calculate elementIndex in the context. This way an expression like /spec/tls/{{elementIndex0}}/hosts/{{elementIndex1}} can be resolved.

@JimBugwadia JimBugwadia merged commit 9d3b176 into kyverno:main Dec 12, 2022
@JimBugwadia JimBugwadia deleted the nested_foreach branch December 12, 2022 16:25
MdSahil-oss pushed a commit to MdSahil-oss/kyverno that referenced this pull request Dec 29, 2022
* updated foreach logic and added tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* uncomment tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix vars and unit tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix vars and unit tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix some tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix more tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* format

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* make codegen

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* linter

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* cleanup

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix linter issue

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* revert local launch

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* propagate context

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* uncomment tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix propagation of registry client

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>
Signed-off-by: Md Sahil <Mohdssahil1@gmail.com>
@chipzoller
Copy link
Member

@JimBugwadia it looks like nested loops are getting blocked when used in a validate rule:

Error from server: error when creating "validate.yaml": admission webhook "validate-policy.kyverno.svc" denied the request: path: spec.rules[0].validate..: one of pattern, anyPattern, deny must be specified

Or are these not intended to work in a validate rule? It's being returned when describing the schema:

$ k explain clusterpolicy.spec.rules.validate.foreach
KIND:     ClusterPolicy
VERSION:  kyverno.io/v1

RESOURCE: foreach <[]Object>

DESCRIPTION:
     ForEach applies validate rules to a list of sub-elements by creating a
     context for each entry in the list and looping over it to apply the
     specified logic.

     ForEach applies validate rules to a list of sub-elements by creating a
     context for each entry in the list and looping over it to apply the
     specified logic.

FIELDS:
   anyPattern   <>
     AnyPattern specifies list of validation patterns. At least one of the
     patterns must be satisfied for the validation rule to succeed.

   context      <[]Object>
     Context defines variables and data sources that can be used during rule
     execution.

   deny <Object>
     Deny defines conditions used to pass or fail a validation rule.

   elementScope <boolean>
     ElementScope specifies whether to use the current list element as the scope
     for validation. Defaults to "true" if not specified. When set to "false",
     "request.object" is used as the validation scope within the foreach block
     to allow referencing other elements in the subtree.

   foreach      <>
     Foreach declares a nested foreach iterator

   list <string>
     List specifies a JMESPath expression that results in one or more elements
     to which the validation logic is applied.

   pattern      <>
     Pattern specifies an overlay-style pattern used to check resources.

   preconditions        <>
     AnyAllConditions are used to determine if a policy rule should be applied
     by evaluating a set of conditions. The declaration can contain nested `any`
     or `all` statements. See:
     https://kyverno.io/docs/writing-policies/preconditions/

MdSahil-oss pushed a commit to MdSahil-oss/kyverno that referenced this pull request Jan 11, 2023
* updated foreach logic and added tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* uncomment tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix vars and unit tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix vars and unit tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix some tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix more tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* format

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* make codegen

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* linter

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* cleanup

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix linter issue

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* revert local launch

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* propagate context

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* uncomment tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix propagation of registry client

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>
Signed-off-by: MdSahil-oss <Mohdssahil1@gmail.com>
MdSahil-oss pushed a commit to MdSahil-oss/kyverno that referenced this pull request Jan 11, 2023
* updated foreach logic and added tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* uncomment tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix vars and unit tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix vars and unit tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix some tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix more tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* format

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* make codegen

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* linter

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* cleanup

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix linter issue

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* revert local launch

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* propagate context

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* uncomment tests

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* fix propagation of registry client

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>
Signed-off-by: MdSahil-oss <Mohdssahil1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Nested foreach loops
4 participants