support non-string types in trigger parameter sources

[gavinmead]

Is your feature request related to a problem? Please describe. If a destination field is not a string (ex: k8s Deployment replicas), then k8s trigger operations will fail.

The issue seems to be that https://github.com/argoproj/argo-events/blob/master/sensors/triggers/params.go#L236 getValueByKey(...) returns a string

Example Use a service account that can create Deployments

apiVersion: argoproj.io/v1alpha1
kind: Sensor
metadata:
  name: webhook
spec:
  template:
    serviceAccountName: k8s-sa
  dependencies:
    - name: test-dep
      eventSourceName: webhook
      eventName: example
  triggers:
    - template:
        name: webhook-workflow-trigger
        k8s:
          group: apps
          version: v1
          resource: deployments
          operation: create
          source:
            resource:
              apiVersion: apps/v1
              kind: Deployment
              metadata:
                name: test-deployment
                labels:
                  app: whalesay
              spec:
                replicas: 1
                selector:
                  matchLabels:
                    app: whalesay
                template:
                    containers:
                    - name: cowsay
                      image: docker/whalesay:latest
                      command: [cowsay]
                      args: ["THIS_WILL_BE_REPLACED"]
          parameters:
            - src:
                dependencyName: test-dep
                dataKey: body.message
              dest: spec.template.containers.0.args.0
            - src:
                dependencyName: test-dep
                dataKey: body.replicas
              dest: spec.replicas

send the following event

curl -d '{"message":"this is my first webhook", "replicas": 2}' -H "Content-Type: application/json" -X POST http://localhost:12000/example

You'll see the following error in the sensor logs

{"level":"info","ts":1622402963.8126473,"logger":"argo-events.sensor","caller":"standard-k8s/standar-k8s.go:171","msg":"creating the object...","sensorName":"webhook","triggerName":"webhook-workflow-trigger","triggerType":"Kubernetes"}
{"level":"error","ts":1622402963.8473272,"logger":"argo-events.sensor","caller":"sensors/listener.go:271","msg":"failed to execute a trigger","sensorName":"webhook","error":"failed to execute trigger: timed out waiting for the condition: Deployment in version \"v1\" cannot be handled as a Deployment: v1.Deployment.Spec: v1.DeploymentSpec.Replicas: readUint32: unexpected character: �, error found in #10 byte of ...|eplicas\":\"2\",\"select|..., bigger context ...|t\",\"namespace\":\"argo-events\"},\"spec\":{\"replicas\":\"2\",\"selector\":{\"matchLabels\":{\"app\":\"whalesay\"}},\"|...","errorVerbose":"timed out waiting for the condition: Deployment in version \"v1\" cannot be handled as a Deployment: v1.Deployment.Spec: v1.DeploymentSpec.Replicas: readUint32: unexpected character: �, error found in #10 byte of ...|eplicas\":\"2\",\"select|..., bigger context ...|t\",\"namespace\":\"argo-events\"},\"spec\":{\"replicas\":\"2\",\"selector\":{\"matchLabels\":{\"app\":\"whalesay\"}},\"|...\nfailed to execute trigger\ngithub.com/argoproj/argo-events/sensors.(*SensorContext).triggerOne\n\t/home/runner/work/argo-events/argo-events/sensors/listener.go:328\ngithub.com/argoproj/argo-events/sensors.(*SensorContext).triggerActions\n\t/home/runner/work/argo-events/argo-events/sensors/listener.go:269\ngithub.com/argoproj/argo-events/sensors.(*SensorContext).listenEvents.func1.3\n\t/home/runner/work/argo-events/argo-events/sensors/listener.go:181\nruntime.goexit\n\t/opt/hostedtoolcache/go/1.14.15/x64/src/runtime/asm_amd64.s:1373","triggerName":"webhook-workflow-trigger","triggeredBy":["test-dep"],"triggeredByEvents":["31396262316664352d333237632d346232632d396434362d383730396335386662383038"],"stacktrace":"github.com/argoproj/argo-events/sensors.(*SensorContext).triggerActions\n\t/home/runner/work/argo-events/argo-events/sensors/listener.go:271\ngithub.com/argoproj/argo-events/sensors.(*SensorContext).listenEvents.func1.3\n\t/home/runner/work/argo-events/argo-events/sensors/listener.go:181"}

Describe the solution you'd like Add an optional dataType field (defaulting to string) to the src to help resolve the parameters to a particular type supported by gjson.

            - src:
                dependencyName: test-dep
                dataKey: body.replicas
                dataType: int
              dest: spec.replicas

---

Message from the maintainers:

If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

[VaibhavPage]

Will add it in v1.5

[bdegeeter]

I have a similar use case where replacing an array and object is needed for a more generic trigger template.

with a message body like this. I'd like to replace and array or object in a k8s template.

'{"parameters": { "foo":"bar" }, "args": ["one", "two", "three"]}'

k8s:
  group: ""
  version: v1
  resource: pods
  operation: create
  source:
    resource:
      apiVersion: v1
      kind: Pod
      metadata:
        generateName: hello-world-
      spec:
        containers:
          - name: hello-container
            args:
              - "hello-world"
            command:
              - cowsay
            image: "docker/whalesay:latest"
  parameters:
    - src:
        dependencyName: test-dep
        dataKey: body.args
      dest: spec.containers.0.args

I'm curious about the complexity of a solution that would support more complex data structures in templates.

[jackivanov]

Any updates on this?

[AalokAhluwalia]

@whynowy I've been taking a look at this issue this week and started making changes. A couple questions I was hoping you'd have some thoughts on:

1. Would it be helpful instead of having a field in the sensor like dataType to use a boolean field useRawDataValue? When set to true, the data value will be used without converting to string. When set to false, and by default, the string representation will be used (no change from existing behavior). If we used a dataKey field that took a string, we'd have to write code to do the type checking, casting + input validation. It would something like this:

  - src:
      dependencyName: test-dep
      dataKey: body.replicas
      useRawDataValue : <true> | <false>   # Possible new boolean field
      dataType: number | json | boolean | string (default)  # Other possible new string field
    dest: spec.replicas

2. Should there be any changes to the type handling for the parameter default value type as part of this issue?
3. Does this look like a complete list of types that should be supported after the change:

• string
• number (floats, ints)
• boolean
• json (objects, lists)

4. It looks like a week ago, the team merged https://github.com/argoproj/argo-events/pull/2273 to fix json serialization. Does that mean the scope of this issue is just numbers and booleans?

[whynowy]

@AalokAhluwalia - You are right, only numbers and booleans have problems right now, but I more prefer your first idea, to have a useRawDataValue. If there's such a field with a value true, my understanding is there's no need to specify dataType, alway using raw type should work for numbers and booleans, right?

[AalokAhluwalia]

That's what I thought too. The boolean field does seem simpler and will work. One problem with specifying the type in the yaml is the case when there's a mismatch between event type and yaml type.

[reinvantveer]

:raised_hands: