Use of OpenAPI for function definitions

[tsurdilo]

Signed-off-by: Tihomir Surdilovic tsurdilo@redhat.com

Many thanks for submitting your Pull Request ❤️!

Please specify parts this PR updates:

• Specification
• Schema
• Examples
• Usecases
• Extensions
• Roadmap
• Use Cases
• Community
• TCK
• Other

What this PR does / why we need it:
It is needed: Our function definitions are currently not portable. We cannot keep having a custom descriptor for function/service invocations.
What is done: We enforce the use of OpenAPI specification for function/service invocation. The invoked operations from services those services must have an OpenAPI description (json/yaml) and use the operationId parameter to uniquely identify each of their exposed operations.

The function definition now becomes for example:

{  
   "name": "HelloWorldFunction",
   "operation": "https://hellworldservice.api.com/api.json#helloWorld"
}

where https://hellworldservice.api.com/api.json is the URI to the service OpenAPI descriptor file and helloWorld is the unique operationId in such descriptor

It is mentioned that for users that simply cannot use OpenAPI to describe their services to be invoked can still use the function definition metadata parameter, but noted that such workflow models using it cannot be considered portable across multiple runtimes / cloud/container platforms.

[ricardozanini]

+1000 to this!

[ricardozanini]

A couple comments only :)

[ricardozanini]

Why not keep this in the operation field? To me it makes sense the image being the function operation definition. Also we could make the operation a required field since it brings consistency to the function definition, IMO.

[tsurdilo]

with the changes the operation param description say it has the format "URIToOpenApiDesc#operationId".
using it here maybe would imply that users can use it to put anything they want in there. Thats why its not a required field to allow users that simply do not wish to use openapi to be able to use metadata to define their own types of defs.

[ricardozanini]

I think it makes sense to have the operation as a required attribute.

[tsurdilo]

i think its not required so that users that just want to use metadata do not have to define it. if it makes more sense to enforce it (require) thats fine.

What about the "type" param. what did we end up deciding in meeting how to proceed ? Add it back in and make it default to "openapi" or something? I cant remember :)

[ricardozanini]

Yeah, the type would make more sense. So the type + resource fields would be required. If type is not informed, we would assume openapi and the resource a link to the meta information?

[jorgenj]

I was under the impression that this was the conclusion (add 'type' back, but default it to 'openapi'), but I don't think it was clearly stated in the meeting that this was the conclusion. I'm personally in support of this, as I think it brings in first-class support for openapi and encourages it's use but doesn't block use-cases for invoking other types of 'functions' that aren't natively RESTful.

On a related note, is this field extensible to support other API/RPC spec types in the future? (Not specifically looking for WSDL/SOAP support, but perhaps consider that as an example).

[tsurdilo]

ok will add "type" back and put some documentation around it, specifically that runtime implementors need to know that it is a completely not-portable parameter.
as far as extensibility I think can deal with http(restful) service now as that includes the majority of cases and if need to incorporate other protocols find a good standard/specification that we can use . wdyt?

[jorgenj]

as far as extensibility I think can deal with http(restful) service now as that includes the majority of cases and if need to incorporate other protocols find a good standard/specification that we can use . wdyt?

Totally agreed on not adding any other protocol/spec right now.

It's probably best not to over-think this 'someday maybe we'll add another protocol/spec' hypothetical (so feel free to ignore the question), just curious your thoughts on this since you've no doubt spent more time looking at this than I have. =)

If some fancy new protocol/spec comes along in the future though, does the single attribute named 'operation' make sense to re-use for that? Taking WSDL as an example, can a method described there be similarly identified by a single URI like OAS operation, and will it make sense to refer to it as 'operation'?

[tsurdilo]

I thought of it similarly to what we are doing with event definitions and the CloudEvents specification. There we use parameters named "source" and "type" which directly map to equally named CE context attributes.

Since we use the OpenAPI "operationId" as the unique identifier of the operation of the service to be invoked I thought this would fit with what we are trying to do, but if a different parameter name would be more suited we can definitely change to it. What would you propose?

[jorgenj]

Not knowing what potential future spec/standard we might allow for in the future, I agree that it's very reasonable to go with 'operation' since it makes a clear connection to OpenAPI concepts, which is what the spec supports now. Also, I don't think it's a stretch to use 'operation' parameter to reference some other type of spec that might come along, even if the language for that type of spec calls it a 'method' or a 'function', instead of an 'operation'.

On the other hand, I don't think it's unreasonable to keep the 'resource' attribute name, but mandate that when the 'type=openapi' that the resource URI must be a valid URI pointing to an operation in an accessible open-api spec file (either network or locally accessible to the runtime). I think this comes at the cost of extra ambiguity, but it does seem to leave things more open on the types of 'function' that this workflow spec might support.

I worry that I'm overthinking this, bike-shedding on a name here, so know that I think 'operation' is a fine choice. =)

[singhkays]

Few thoughts on this

1. From a user experience perspective, it seems weird to ask the developer to create and upload Swagger spec for the following reasons:

• what is the place where the developer should upload the spec?
• do they have access to the said place?
• If this is the same place where the workflow or function runs,
  • do they have to make the spec publicly accessible?
  • if the spec is publicly accessible, does this open up the workflow or function host to DDoS attacks?
• If this is not the same place where the workflow runs, now they have taken a dependency on a network call and the resource hosting the spec might be down. What is the experience in that case?

functions:
- name: sendRejectionEmailFunction
  operation: http://myapis.org/applicationapi.json#emailRejection

2. How do you specify the resource URI of the function? I think resource parameter was used before this PR

[tsurdilo]

• added type param back with its description

[tsurdilo]

@singhkays

• what is the place where the developer should upload the spec? do they have access to the said place? - This is the responsibility of devels that write the services that we need to orchestrate in most cases. the openapi specification is a big help to them for workflows or any type of tools/users to access and use their services. If for external service that need to be par of the orchestration this does not exist, it is pretty easy to create using the swagger tooling for example.

• If this is the same place where the workflow or function runs, do they have to make the spec publicly accessible? - Not necessarily. The first part of the "operation" param just needs to be in URI format, so it can describe many different locations (classpath, file, etc) how the openapi definition can be accessed during compilation/parsing of workflow definitions.
  Don't have to make it publicly accessible but has to be accessible to your applications that use workflows during the compilation time only.

• if the spec is publicly accessible, does this open up the workflow or function host to DDoS attacks? - I don't think so as the openapi description just describes the service for convenience purposes, and does not give hackers any more info than what they could find out without it. So the attacks have to be cared for regardless if the service defines an openapi description or not.

• If this is not the same place where the workflow runs, now they have taken a dependency on a network call and the resource hosting the spec might be down. What is the experience in that case? - In most if not all cases the reading of the openapi definition only happens at compile/parsing time. Runtimes can use the openapi definition and its many implementations for different languages to read it in and create objects/interfaces/whatever that can then be used during runtime to access this information. So no imo it does not create a runtime dependency on your applications to the service openapi definition.

[jorgenj]

Kay's question had me thinking about something that was nagging at the back of my mind; the 'endpoint' of the api server

Looking a bit more into the OpenApi, here's a couple things that I think might need to be taken into account:

1. OA3 has a 'servers' attribute, the example they give is for 'prod' vs. 'sandbox' endpoints. How will the runtime know which 'server' to use?
2. OA3 also supports 'server templating', which allows variables to be injected into the server name, port, schema and/or base-path to compute the actual endpoint a caller will use.
3. OA3 even allows over-riding the 'server' url per path or even for each individual operation.
4. OA2 is less flexible, but it does allow to specify multiple protocols that a caller may use (ie; give the option of http or https)

I'm curious what others think about how to deal with the more complex situations where OA2/3 allow for many potential server values. How would the workflow implementation know which 'server' value to use to make the api call?

[ricardozanini]

I'd say that could be a metadata information and it's about to the implementation to decide how to handle these values. Maybe reading from a configuration context for the runtime. I wouldn't over think this too much, since it seems more an implementation detail.

I believe that's not to us to "import" their definitions like adding a "server" option in the spec.

This is just what comes to my mind right now, I'll give myself more time to properly think about this.

[tsurdilo]

These questions are valid but imo are more on the runtime impl side rather than workflow markup.

Regarding How would the workflow implementation know which 'server' value to use to make the api call? - this is exactly why openapi is a good fit here. Workflow modellers should not be concerned about low-level details of the actual call. Runtimes can/should implement different strategies for this, e.g. picking the "test" server if the workflow is executed in test environment, possibly doing round-robin picking if server loads are a concern, simply picking the first one in the list is also an option in some cases...but its not a workflow model concern :)

[tsurdilo]

@jorgenj @ricardozanini I added "type" back but now im thinking again to remove it - reason being its a completely non-portable string, and we already have "metadata" for that. I rather not add any non-portable parameters unless its really necessary. Would you guys be ok if we go ahead and remove it like it was in original pr? wdyt (sorry) :)

[jorgenj]

@tsurdilo Maybe I don't understand what it means to be 'non-portable'. If the spec calls out that this field can have many potential values, but the following values (for example; 'openapi') are specifically outlined in the spec, then doesn't that make that particular 'type' portable between runtimes which conform to the spec?

To ask in a different way, take the 'type' field on any of the different states outlined in the spec. Presumably a spec-conforming implementation would still be conforming to the spec if they were to introduce their own custom state types, but those particular custom state types would be 'non-portable', right? Does that make the whole 'type' attribute for states non-portable, or just those specific custom state types are non-portable, since they're not state types which are defined in the spec?

[tsurdilo]

@jorgenj I think the best way to try to answer your questions is first look at the spec goals in doc: https://github.com/serverlessworkflow/specification/blob/master/specification.md#specification-goals
particularly the second image there.
One of the main things where i believe we are trying to distinguish ourselves from many other workflow markups out there is portability, meaning that the markup we define should not only be declarative, but also structured in a way that users feel comfortable to use it and know that their workflow markup can be executed across multiple runtimes/containers with no or minimal changes.

If you look at the current situation in business automation for example, when you have multiple vendors and all say they are using "proven standards" and are "specification based", the truth is the markup that their tooling generates is far from it. So being based on a "specification" or a "proven standard" really at that point means very little to end users. And to be frank it has really just become a "buzz" or "marketing" slang that companies use to attract customers with very little "meat" behind it.

In reality the markup generated in one engine has zero compliance on another engine. Quite frankly whats happening right now even with widely used workflow/process specifications is that on the markup level not even "hello world" processes are portable across multiple runtimes.

Now you can say yes that runtime implementations of the specifications are "to blame" here, but I believe it really starts with the markup definition itself. The more ambiguity to provide in the definition the more chance there is for runtime implementations to interpret it differently and thus creating such situations for end users (us :) ).

The "type" parameter is a free-form string, meaning that users and runtimes have free will to determine what is to be placed there. This is different from the state types, as that is an enumeration, with limited choices. Now yes of course people can chose to not follow the ones in the enum and create their own, but we do state in the docs that in order to be compliant with the spec, the markup must conform the the specification json schema.

With the function "type" parameter something that is super easy to do on the runtime side ( and I am not proud of it as have done it myself ) is to see this type of scenario:

if("type" == "X") { ... }
if("type" == "Y") { ... } 
...

The "X" and "Y" as in the values of a free-form type parameter are completely governed by the runtime impl, so being "non-portable" means that they might mean something to one runtime but something completely different for another (if something at all).

We have started doing this type of work in the past by eliminating use of expressions in the markup where users can chose their expression language. I think from my experience this is probably the #1 reason as to the portability issue we are currently facing in business automation atm. Instead what we did is say "all expressions are jsonpath", yes a big limitation but on the other hand it is clear for all runtimes what the expression means :). Same for function definitions, its a hard tradeoff to make sometimes, but by enforcing openapi we might be limiting the overall use but we we gain on the portability" aspect.

Users in the end still have the option to place all their "non-portable" parameters in the function defs metadata section and this is fine as it up-front states that.

Sorry for the long post :) We can keep discussing offline (on our slack channel if you wish). WDYT?

[tsurdilo]

needs to update example if #147 goes in first

[tsurdilo]

rebased and updated new example function defs

[ricardozanini]

This is a trade off that we need to decide, as a community, what to do.

I'm on the team to transform the type into a enum like we are doing on states to force implementations to follow the spec and not arbitrary invent their own "functions", since it won't be portable whatsoever.

But this comes with a great responsibility on our side to support most of the function usages out there, just openapi might not gain enough traction.

On the other hand, having the type as a free form attribute can bring more vendors to implement the spec since will be simple to implement AND extend. And then we start to absorb all of them over time in the next versions and freeze the type as enum.

[jorgenj]

@tsurdilo Thanks for explaining your thinking on portability. I can't say that I follow 100% the reasoning on classifying enums/strings as portable or non-portable, but I think I understand your concerns and I agree with the goal.

My concern with removing 'type' is that it seems to 'close' the function definition for extensibility. Let's pretend that I want to introduce an extension for a new type of 'function' that makes remote calls via email (or some other absurd abuse of technology that isn't REST). In that case, the operationId probably doesn't make sense for this new type of 'function'. Instead I might need to specify from/to addresses and maybe an email server. How should that extension spec model this new type of 'function'? If a user mistakenly defines operationId & the email attributes, which type of 'function' is it supposed to be interpreted as?

I would hope that there would be a 'type' attribute, which allows users to clearly indicate exactly which type of 'function' they intended. Basically, if 'type' is removed now, it seems very likely that it'll just need to be added back later.

[jorgenj]

Just saw Ricardo's comment. I'm in agreement, in that I think 'type' is necessary. Free-form vs. enum, I'm not sure which makes more sense, though I lean towards free-form string with the spec clearly defining the behavior for the case where the value is 'openapi'.

[tsurdilo]

@ricardozanini @jorgenj I understand your guys concerns but I believe that for the use cases you describe there are other better ways of handling this:

1. Add a "description" parameter to function defs - this is better to me than a "type" param if users do want to give the function definition a custom description (and it makes more sense for tooling and indexing etc)
2. If users want to define custom invocation info, again, use the "metadata" field..that's what its for :)

There is absolutely nothing that stops users to do silly stuff like:

   "functions": [
   {
       "name": "Its Friday!",
       "operation": "fridaynight",
       "metadata": {
         "step1": "open fridge",
         "step2": "get beer",
         "step3": "close fridge"
      }
   }
]

If their runtime understands and allows for this / any syntax. The problem here is that
as a specification we cannot prevent this, but it gives us the opportunity to clearly define that this is not portable.

I think we are worrying about possibly 5% usecases where openapi does not fit, but do not see the overall benefits of the 95% of cases where this is actually a huge improvement.

[ricardozanini]

I'm okay removing the type, and go with openapi only. We can test the community adoption and see how it goes. We can always add it back to support a scenario we're not able to see right now. I'm just curious how this examples will be defined:
https://github.com/serverlessworkflow/specification/blob/master/examples/examples-argo.md#hello-world-with-parameters

A "container" function does not have a REST endpoint, by reading it I guess the runtime is reading from the container's standard output.

[tsurdilo]

@ricardozanini talked to Alex about this and he said he would have time to meet with us next Monday to discuss this. So lets wait to see what recommendations he has and go from there. My assumption was that Argo also has support for openapi - https://github.com/argoproj/argo/tree/master/api/openapi-spec but Alex would know this best.

@manuelstein did we get a chance to set something up yet? If not I can send something out tonight.

[tsurdilo]

@ricardozanini @jorgenj I made some additional updates, trying to clearly describe the decision about utilizing standards wherever we can in our markup.
changes:

1. better description
2. make 'operationid' not required
3. show example using metadata (all non-restful style servicess)

I do believe that there are going to be a standards similar to openapi for describing invocations of non-resful services we definitely want to use them as much as possible in the future.

For now, we really need to move this forward as it's blocking our release that we would like to do before KubeCon in November.
So let's have a vote in tomorrows meeting on this if possible and move it forward. We can always keep improving this as always.

[jorgenj]

Suggested change

	The `operation` property is a combination of the function/service OpenAPI definition document URI and the particular service operation that needs to be onvoked, separated by a '#'.
	The `operation` property is a combination of the function/service OpenAPI definition document URI and the particular service operation that needs to be invoked, separated by a '#'.

[jorgenj]

I think this is probably something we can circle back on later (not blocking this PR), but I struggle a little bit with directing runtimes to use Metadata here as it seems a bit inconsistent with description of metadata here

Metadata should not affect workflow execution. Implementations may choose to use metadata information or ignore it.

I think it depends on peoples idea of what it means to affect workflow execution, but to me specifying what-to-invoke/how-to-invoke-it is something I would interpret to be affecting workflow execution.

I can understand the decision to go this way for now, since we don't have a good standard but this seems to compromise the internal consistency of the doc.

Some ideas to address this or maybe side-step the inconsistency:
a) Give some reason here for why we're using metadata even though the overall metadata description seems to say not to do this.
b) Relax the description for metadata so this usage of metadata fits
c) What about using a different property name like 'vendor', 'runtime' or something to convey that the attributes under this property are non-portable, user-beware, etc?

I think for the sake of forward progress we should leave it using 'metadata' for now, but I wanted to throw this out there to get peoples thoughts on how we should be thinking about:
a) how to allow for runtimes to model things that affect execution but don't fit exactly fit with what the spec explicitly supports (like the docker examples)...
b) ...while still making it clear to users what parts of their definition/usage are and are-not portable

[tsurdilo]

Agree, like the ideas. Will think of something using them. Thanks

[jorgenj]

Suggested change

	property in the services OpenAPI definition document which uniquely identifies a specific service operation that needs to be infoked.
	property in the services OpenAPI definition document which uniquely identifies a specific service operation that needs to be invoked.

[tsurdilo]

omg ... guess i need to practice writing "invoked" :)

[tsurdilo]

@jorgenj fixed spelling mistakes - thanks!

[tsurdilo]

In our team meeting today we agreed to push these changes. We will look into possible improvements in the near future.