ResourceInterpreterCustomization: support multiple per Kind and operation

[a7i]

Currently, the logic prevents having two ResourceInterpreterCustomization for the same target Kind.

Take the scenario below:

1. One ResourceInterpreterCustomization that ignores tls.crt from secrets
2. Another ResourceInterpreterCustomization that ignores service-account-data information

Note that they're both operation retention and target.Kind: Secret

---
apiVersion: config.karmada.io/v1alpha1
kind: ResourceInterpreterCustomization
metadata:
  name: retain-duplicate-secret-tls-data
spec:
  customizations:
    retention:
      luaScript:
        function Retain(desiredObj, observedObj)
          if desiredObj.data ~= nil then
            if desiredObj.data["tls.crt"] ~= nil then
              desiredObj.data = observedObj.data
            end
          end
          return desiredObj
        end
  target:
    apiVersion: v1
    kind: Secret
---
apiVersion: config.karmada.io/v1alpha1
kind: ResourceInterpreterCustomization
metadata:
  name: retain-service-account-token
spec:
  customizations:
    retention:
      luaScript: |
        function Retain(desiredObj, observedObj)
          if desiredObj.type == "kubernetes.io/service-account-token" then
            desiredObj.data = observedObj.data
          end
          return desiredObj
        end
  target:
    apiVersion: v1
    kind: Secret

This results in error:

resourceinterpretercustomization.config.karmada.io/retain-duplicate-secret-tls-data serverside-applied
Error from server (Forbidden): admission webhook "resourceinterpretercustomization.karmada.io" denied the request: conflicting with InterpreterOperation(Retain) of existing ResourceInterpreterCustomization(retain-duplicate-secret-tls-data)

What would you like to be added:

Ability to support multiple ResourceInterpreterCustomization for same kind and operation.

Why is this needed: This allows for smaller lua scripts per ResourceInterpreterCustomization

[a7i]

Code path that enforces one per kind/operation: https://github.com/karmada-io/karmada/blob/master/pkg/webhook/resourceinterpretercustomization/helper.go#L29C67-L46

[XiShanYongYe-Chang]

If the same kind supports multiple ResourceInterpreterCustomization configurations, the execution sequence may affect the final result. What do you think about this?

Invite @chaunceyjiang to have a look.

[grosser]

We could start with "sort ResourceInterpreterCustomization by name", but something like priority could eventually be added. Worst case we could also do "these will execute in a random order make sure your ResourceInterpreterCustomization do not conflict."

[chaunceyjiang]

If the same kind supports multiple ResourceInterpreterCustomization configurations, the execution sequence may affect the final result.

Yes, the reason we introduce this constraint is that if multiple operations perform mutually exclusive modifications to the same resource, the resulting state may be unpredictable.

[yike21]

---
apiVersion: config.karmada.io/v1alpha1
kind: ResourceInterpreterCustomization
metadata:
  name: retain-data
spec:
  customizations:
    retention:
      luaScript: |
        function Retain(desiredObj, observedObj)
          # A || B , priority A > B
          # rule A
          if desiredObj.data ~= nil then
            if desiredObj.data["tls.crt"] ~= nil then
              desiredObj.data = observedObj.data
              return desiredObj
            end
          end
          # rule B
          if desiredObj.type == "kubernetes.io/service-account-token" then
            desiredObj.data = observedObj.data
          end
          return desiredObj
        end
  target:
    apiVersion: v1
    kind: Secret

An optional way to do this is to make one single ResourceInterpreterCustomization more complex to support multiple operations. Or perhaps introduce a configuration policy to manage the ResourceInterpreterCustomization.

[XiShanYongYe-Chang]

An optional way to do this is to make one single ResourceInterpreterCustomization more complex to support multiple operations. Or perhaps introduce a configuration policy to manage the ResourceInterpreterCustomization.

Hi @yike21 Does Lua support this?

[XiShanYongYe-Chang]

We could start with "sort ResourceInterpreterCustomization by name",

The problem may still exist. For example, if resource a and resource c exist and resource b is inserted, the effect of resource c may overwrite that of resource b.

but something like priority could eventually be added.

Introducing priority maybe possible, but will the management of priority bring new complexity to users? For example, user b does not know what ResourceInterpreterCustomization resources user a has created.

Worst case we could also do "these will execute in a random order make sure your ResourceInterpreterCustomization do not conflict."

Wouldn't random solve the problem of the effect being overwritten?

[XiShanYongYe-Chang]

Wouldn't random solve the problem of the effect being overwritten?

Can you elaborate on this benefit, or what's delicious in addition to this benefit.

[yike21]

Hi @yike21 Does Lua support this?

We can write some logic in lua to make the operation return early (this is not flexible), for example:

luaScript: |
        # A || B || C, priority A > B > C, only math one
        function Retain(desiredObj, observedObj)
          # rule A
          if desiredObj.data ~= nil then       ## match rule A
            if desiredObj.data["tls.crt"] ~= nil then
              desiredObj.data = observedObj.data
              return desiredObj                ## just return 
            end
          end

          # rule B
          if desiredObj.type == "kubernetes.io/service-account-token" then ## match rule B
            desiredObj.data = observedObj.data
            return desiredObj                                              ## just return                               
          end

          # rule C
          if desiredObj.type match rule C then              ## match rule C
            desiredObj.data = observedObj.data
            return desiredObj                               ## just return
          end

          return desiredObj  ## return here
        end

And Lua doesn't support configuration policy.

[XiShanYongYe-Chang]

Ok @yike21 Thanks

I understand that this is actually a requirement for users. After all, lua scripts are written by users.

[a7i]

That is what we ended up doing (putting everything in one and using conditional statement), but as mentioned in the Issue, it would be nice to split it up to separate the logic.

[XiShanYongYe-Chang]

Ask @RainbowMango to help take a look.

[RainbowMango]

Before talking about how to, I want to know why to do so.

What's the pain point for now? Do not want to write and maintain a large configuration?

[a7i]

What's the pain point for now?

In the example given in the Issue, we have to add bunch of conditional statements to switch based on secret type to retain respective fields.

Do not want to write and maintain a large configuration?

That's exactly correct.