Substitute REST path or body parameters in Workflow Steps

[dbwiddis]

Description

When provisioning a workflow, additonal parameters can be passed on the REST path or in the body. For example:

POST /_plugins/_flow_framework/workflow/bhPBgI0BQfVbnJKtHwGU/_provision?foo=one&bar=two

or

POST /_plugins/_flow_framework/workflow/bhPBgI0BQfVbnJKtHwGU/_provision
{
  "foo": "one",
  "bar": "two"
}

When creating a workflow with provision=true the parameters are allowed on the path only, since the body already contains a template.

In this case, instances of the parameter in the template values (user_params, user_inputs, or previous_node_inputs) will be replaced by its value, e.g.,

"user_inputs": {
  "parameters": {
    "math": "${{foo}} plus ${{ foo }} is ${{ bar }}"
  }
}

will become

"user_inputs": {
  "parameters": {
    "math": "one plus one is two"
  }
}

Issues Resolved

Part of #496 and #522.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (24bf51a) 71.91% compared to head (098e75f) 72.71%.

Files	Patch %	Lines
...arch/flowframework/workflow/RegisterAgentStep.java	60.00%	1 Missing and 1 partial ⚠️
...h/flowframework/rest/RestCreateWorkflowAction.java	94.73%	0 Missing and 1 partial ⚠️
.../transport/DeprovisionWorkflowTransportAction.java	50.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #525      +/-   ##
============================================
+ Coverage     71.91%   72.71%   +0.79%     
- Complexity      612      639      +27     
============================================
  Files            78       78              
  Lines          3201     3258      +57     
  Branches        246      255       +9     
============================================
+ Hits           2302     2369      +67     
+ Misses          787      777      -10     
  Partials        112      112              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dbwiddis]

@YANG-DB FYI

[owaiskazi19]

Continuing the discussion here

[joshpalis]

Overall looks good to me, just a small comment

[owaiskazi19]

Overall LGTM with few questions/comments

[amitgalitz]

In the description you highlight the changes for provision API and mention when provision=true on create. If we are creating a template without the provision parameter(false) then should we also allow users to have dynamic params?

Additionally just so I understand correctly, technically a user can create this with create API and then when provisioning give the input to the params or give it during provisioning?

{
  "name": "{template.name}",
  "description": "{template.description}",
  "use_case": "MODEL_DEPLOYMENT_API_KEY",
  "version": {
    "template": "1.0.0",
    "compatibility": [
      "2.12.0",
      "3.0.0"
    ]
  },
  "workflows": {
    "provision": {
      "nodes": [
        {
          "id": "create_connector_1",
          "type": "create_connector",
          "user_inputs": {
            "name": "${create_connector_1}",
            "description": "${create_connector_1.description}",
            "version": "1",
            "protocol": "${create_connector_1.protocol}",
            "parameters": {
              "endpoint": "${create_connector_1.endpoint}",
              "model": "${create_connector_1.model}"
            },
            "credential": {
              "key": "${create_connector_1.credential.key}"
            },
            "actions": [
              {
                "action_type": "predict",
                "method": "POST",
                "url": "https://${parameters.endpoint}/v1/chat/completions"
              }
            ]
          }
        },
        {
          "id": "register_remote_model_1",
          "type": "register_remote_model",
          "previous_node_inputs": {
            "create_connector_1": "parameters"
          },
          "user_inputs": {
            "name": "${register_remote_1_model.name}",
            "function_name": "remote",
            "description": "${register_remote_model_1.description}"
          }
        },
         {
            "id": "deploy_model_1",
            "type": "deploy_model",
            "previous_node_inputs": {
              "register_remote_model_1": "model_id"
            }
          }
      ],
      "edges": [
        {
          "source": "create_connector_1",
          "dest": "register_remote_model_1"
        },
         {
            "source": "register_remote_model_1",
            "dest": "deploy_model_1"
          }
      ]
    }
  }
}

[dbwiddis]

If we are creating a template without the provision parameter(false) then should we also allow users to have dynamic params?

No, because the params are only substituted during provisioning, when processing the data in the Workflow Steps. They have no impact when we are not provisioning, as we are simply parsing and storing the template content.

Additionally just so I understand correctly, technically a user can create this with create API and then when provisioning give the input to the params or give it during provisioning?

Yes, classic example is the feature request issue. The template would include a step configuration with something like this (this exact syntax was from observability workflow RFC, our syntax is slightly different).

"steps": [
{
  "name": "add_timestamp_field_to_otel_spans_index",
  "method": "PUT",
  "endpoint": "/${index_name}",
  "body": { ... }
 },
 ....
]

In this case we'd create the workflow with the "endpoint" field having a value of /${{ index_name }}.

When we went to provision the workflow, if we passed ?index_name=foo on the REST path, then the "endpoint" field value would become /foo. The original template would remain unchanged.

Note that in our current code we couldn't provision a second time in this case, we'd have to "deprovision" first and I'll have to eventually address this before 2.13. We could then deprovision (TODO, not require this) and provision again passing ?index_name=bar on the REST path. Same template, but "endpoint" would now be /bar.

[amitgalitz]

Could we also potentially add an ability to reprovision a workflow and give it a new ID for the state index breaking the 1:1 template to state index ratio. Up to customer to change things if there are issues.

Yeah, generally I'm thinking with "fine grained provisioning" we could allow provisioning a "completed" workflow a second time, and do the "partial" deprovisioning by default as part of it.

[owaiskazi19]

LGTM! Thanks for patiently discussing comments