Implement cascading ModRules through execution tiers

[thedatabaseme]

Hello,

we have the following situation. We want to replace all possible Docker Hub image specifications (like docker.io/image:tag, image:tag, docker.io/library/image:tag ...) in our Pod manifests with our private image registry. We have done some modrules for that like this:

apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-container-library-and-tag
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all containers, with an image formatted like this: "nginx"
  # and mutate the image to:                                          "myregistry.com/library/nginx:latest" 
  patch:
    - op: replace
      select: '$.spec.containers[?(@.image =~ "^([^./:]+)$")].image'
      path: '/spec/containers/#0/image'
      value: '{{ regexReplaceAll "^([^./:]+)$" .SelectedItem "myregistry.com/library/${1}:latest" }}'
---
apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-init-container-library-and-tag
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all initContainers, with an image formatted like this: "nginx"
  # and mutate the image to:                                              "myregistry.com/library/nginx:latest" 
  patch:
    - op: replace
      select: '$.spec.initContainers[?(@.image =~ "^([^.|/]+)(:)(.+)$")].image'
      path: '/spec/initContainers/#0/image'
      value: '{{ regexReplaceAll "^([^.|/]+)(:)(.+)$" .SelectedItem "myregistry.com/library/${1}:${3}" }}'
---
apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-container-library
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all containers, with an image formatted like this: "nginx:latest"
  # and mutate the image to:                                          "myregistry.com/library/nginx:latest" 
  patch:
    - op: replace
      select: '$.spec.containers[?(@.image =~ "^([^.|/]+)(:)(.+)$")].image'
      path: '/spec/containers/#0/image'
      value: '{{ regexReplaceAll "^([^.|/]+)(:)(.+)$" .SelectedItem "myregistry.com/library/${1}:${3}" }}'
---
apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-init-container-library
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all initContainers, with an image formatted like this: "nginx:latest"
  # and mutate the image to:                                              "myregistry.com/library/nginx:latest" 
  patch:
    - op: replace
      select: '$.spec.initContainers[?(@.image =~ "^([^.|/]+)(:)(.+)$")].image'
      path: '/spec/initContainers/#0/image'
      value: '{{ regexReplaceAll "^([^.|/]+)(:)(.+)$" .SelectedItem "myregistry.com/library/${1}:${3}" }}'
---
apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-container
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all containers, with an image formatted like this: "repo/dhcp:latest"
  # and mutate the image to:                                          "myregistry.com/repo/dhcp:latest" 
  patch:
    - op: replace
      select: '$.spec.containers[?(@.image =~ "^([^.]+)(/)(.+)(:)(.+)$")].image'
      path: '/spec/containers/#0/image'
      value: '{{ regexReplaceAll "^([^.]+)(/)(.+)(:)(.+)$" .SelectedItem "myregistry.com/${1}/${3}:${5}" }}'
---
apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-init-container
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all initContainers, with an image formatted like this: "repo/dhcp:latest"
  # and mutate the image to:                                              "myregistry.com/repo/dhcp:latest" 
  patch:
    - op: replace
      select: '$.spec.initContainers[?(@.image =~ "^([^.]+)(/)(.+)(:)(.+)$")].image'
      path: '/spec/initContainers/#0/image'
      value: '{{ regexReplaceAll "^([^.]+)(/)(.+)(:)(.+)$" .SelectedItem "myregistry.com/${1}/${3}:${5}" }}'
---
apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-full-container
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all containers, with an image formatted like this: "docker.io/repo/dhcp:latest"
  # and mutate the image to:                                          "myregistry.com/repo/dhcp:latest" 
  patch:
    - op: replace
      select: '$.spec.containers[?(@.image =~ "^(docker.io)(/)(.+)(:)(.+)$")].image'
      path: '/spec/containers/#0/image'
      value: '{{ regexReplaceAll "^(docker.io)(/)(.+)(:)(.+)$" .SelectedItem "myregistry.com/${3}:${5}" }}'
---
apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-io-full-init-container
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'
  # this will find all initContainers, with an image formatted like this: "docker.io/repo/dhcp:latest"
  # and mutate the image to:                                              "myregistry.com/repo/dhcp:latest" 
  patch:
    - op: replace
      select: '$.spec.initContainers[?(@.image =~ "^(docker.io)(/)(.+)(:)(.+)$")].image'
      path: '/spec/initContainers/#0/image'
      value: '{{ regexReplaceAll "^(docker.io)(/)(.+)(:)(.+)$" .SelectedItem "myregistry.com/${3}:${5}" }}'

Because we need regcreds for the private registry, we then created a second mutation rule, that adds the regcred secret as imagePullSecret.

apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: docker-remote-jfrog-io-image-pull-secrets
  namespace: kubemod-system
spec:
  type: Patch
  targetNamespaceRegex: .*
  match:
    - select: '$.kind'
      matchValue: 'Pod'

    - select: '$.spec.containers[*].image'
      matchRegex: '^myregistry\.com/.*$'

    - select: '$.spec.imagePullSecrets[*].name'
      matchValue: 'regcred'
      negate: true

  patch:
    - op: add
      path: '/spec/imagePullSecrets/-1'
      value: |-
        name: regcred

For testing we use this deployment for example:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment
  namespace: mytest
  labels:
    app: nginx
spec:
  replicas: 3
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      initContainers:
      - name: initnginx
        image: docker.io/nginx:1.14.2
        imagePullPolicy: Always
        ports:
        - containerPort: 80
      containers:
      - name: nginx
        image: docker.io/library/nginx:1.14.2
        imagePullPolicy: Always
        ports:
        - containerPort: 80
      - name: nginxtest
        image: nginx
        imagePullPolicy: Always
        ports:
        - containerPort: 80

What we now experience is the following.

• The replacement of the image reference in the Pod work as expected. This applies for all combinations and for all containers and initContainers.
• The rule for adding the regcred as imagePullSecret does not match and is not triggered.

We found out, that all rules seem to check always the initial Pod specification. Cause if we change matchRegex: '^myregistry\.com/.*$' to matchRegex: '^docker.io/.*$', the rule is triggered and does what it should. This makes me a bit confused cause I assumed, that changing the image specification of a Pod will lead to a status update which will lead to the regcred rule to get triggered. We worked around this issue by removing the select part in the regcred rule entirely. This now adds regcreds to all Pods regardless if needed or not.

So I have the following questions or proposals:

1. Is my understanding correct, that all rules will check the initial resource specification and not getting triggered after mutation by another rule happened?
2. If so, would this not be a reasonable feature to have something like cascading rules? For example. Kubemod is getting triggered and checks the resource if a modrule needs to be applied, then a second modrule will get triggered and the already mutated resource will be checked by this rule and so on. After all cascading rules are finished, the final mutated resource definition is handed back to Kubernetes and gets applied. I know that you don't have an influence in the order when admission webhooks will be called, but this will be "inside" Kubemod in my understanding and therefore "self-contained".
3. Do you have any idea how this specific issue could be solved otherwise?

I hope my explanation is comprehensible. If you have any questions, don't hesitate to ask.

Kind regards
Philip

[vassilvk]

Hi @thedatabaseme,

I understand.
My original design of KubeMod's processing included a "keep-executing-all-rules-until-all-of-them-stop-modding" logic, similar to the one you are suggesting.

This turned out to be less than practical for two reasons - performance and re-entrance.

KubeMod is quite a promiscuous mutating webhook - by default it subscribes to be notified about Create/Update events for a pretty wide range of K8S resources.

This puts KubeMod's resource processing logic on the hot path of a lot of Kubernetes operations. I have medium sized Helm charts whose installations lead to hundreds of objects being passed through KubeMod per second.
If KubeMod performs multiple passes over its full list of ModRules for every object passed through it, this will choke up deployments, lead to missed webhook response deadlines, etc. Scaling up the number of KubeMod replicas can help, but processing is expensive and having to do it multiple times for the same object will represent a major performance hit.

The second issue is more subtle. Contradicting ModRules can cancel each other and lead to infinite loops (flapping changes). And the kicker is that the more ModRules you have in your system, the harder it is to track which combination of ModRules leads to infinitely flapping changes. In addition, any attempt to resolve this with a circuit breaker (say no more than 8 passes) is just a band-aid based on an arbitrary number.

I think there may be a better solution to your issue.
Currently, when multiple ModRules match an object, they are executed against that object in an arbitrary order.
Each ModRule is executed against the result of the previously executed ModRule, but matching is performed before ModRule execution.

I think that introducing some sense of order in the rule execution may be just what you need.
For example, we can add an optional ModRule attribute executionPriorityGroup, and match and execute ModRules in groups of priority in ascending order (where priority group 0 is higher than priority group 1). Default executionPriorityGroup would be 0.
KubeMod will guarantee that ModRules in a higher priority group will be matched and executed before ModRules in lower priority groups, but ModRules in the same priority group will be matched and executed in an indeterminate order.

This way we can avoid both slow multi-pass execution, as well as endless flapping with contradicting rules.

Thoughts?

[thedatabaseme]

Having the possibility to give an order or priority to the modrules would be a good solution.

[vassilvk]

Ok, I'll plan on implementing and shipping this with the next version.

[orbatschow]

One suggestion from my side:

ArgoCD has something similar, called sync waves, which is done via an annotation and sets everything by default to sync wave "0", and you can configure the resources to be either executed before everything else (-1,-2,-3...) or after everything else (1,2,3...).

Personally I found this to be quite a nice mechanism as it is extremely flexible and easy to use from a users perspective.

Example:

metadata:
  annotations:
    argocd.argoproj.io/sync-wave: "5"

This could be translated to:

metadata:
  annotations:
    api.kubemod.io/sync-wave: "5"

While i quite like this approach, I'm not a huge fan to pinpoint a behavioral propertoy to metadata information, so my final suggestion would be to to something like this:

apiVersion: api.kubemod.io/v1beta1
kind: ModRule
metadata:
  name: my-modrule
spec:
  type: Patch
  syncWave: 5

  match:
    # Match deployments ...
    - select: '$.kind'
      matchValue: Deployment

    # ... with label app = nginx ...
    - select: '$.metadata.labels.app'
      matchValue: 'nginx'

    # ... and at least one container whose image matches nginx:1.14.* ...
    - select: '$.spec.template.spec.containers[*].image'
      matchRegex: 'nginx:1\.14\..*'

    # ... but has no explicit runAsNonRoot security context.
    # Note: "negate: true" flips the meaning of the match.
    - select: '$.spec.template.spec.securityContext.runAsNonRoot == true'
      negate: true

  patch:
    # Add custom annotation.
    - op: add
      path: /metadata/annotations/my-annotation
      value: whatever

    # Enforce non-root securityContext and make nginx run as user 101.
    - op: add
      path: /spec/template/spec/securityContext
      value: |-
        fsGroup: 101
        runAsGroup: 101
        runAsUser: 101
        runAsNonRoot: true

[vassilvk]

Hi @orbatschow,

Right, this is exactly how it will work - a new integer property on the ModRule CRD (not an annotation), with default value of 0 and match + execution of groups of ModRules ordered in ascending order by that value. The user will be able to set the value of this property to negative or positive numbers as they see fit.
I’m not sure the name syncWave is appropriate as there is no syncing involved. Also, don't like the name priorityExecutionGroup as it is confusing - not immediately clear which value indicates higher priority.
I am planning to name the property executionTier.

[vassilvk]

Hi @thedatabaseme, @orbatschow,

Please note that this has been released with kubemod 0.16.0.