Fix boolean parameters for download command

[jiangpengcheng]

Fixes #388

Motivation

Jcommander doesn't accept additional parameter for bool, https://jcommander.org/#_boolean

Modifications

use env when TLS related parameters are true

Verifying this change

• Make sure that the change passes the CI checks.

• This change is already covered by existing tests, such as (please describe tests).

Documentation

Check the box below.

Need to update docs?

• doc-required

  (If you need help on updating docs, create a doc issue)

• no-need-doc

  bug fix

• doc

  (If this PR contains doc changes)

[freeznet]

could you please check https://github.com/apache/pulsar/blob/master/docker/pulsar/scripts/apply-config-from-env.py, which is contained in each runner image by default (but java and go runner image do not have python installed yet), and it should be useful to overwrite the tls / auth configs in conf/client.conf from ENV, and can be used to simplify the pulsar-admin command here.

[jiangpengcheng]

looks great, so we need to install python in the base-runner?

[freeznet]

@jiangpengcheng I think it is acceptable, but it requires some work to keep the backward compatibility.

[jiangpengcheng]

for compatibility, we may need to use shell to check whether python is installed on the runtime image which makes the command more complicated, any other advice for compatibility?

[jiangpengcheng]

@freeznet
looks like we can use env > conf/client.conf to do similar thing, but some config names we used in configs and secrets need to be transformed when writing into the config file:

webServiceURL -> webServiceUrl
brokerServiceURL -> brokerServiceUrl
clientAuthenticationPlugin -> authPlugin
clientAuthenticationParameters -> authParams
tlsHostnameVerificationEnable -> tlsEnableHostnameVerification

for this transformation, I think we can inform users to update their configs&secrets before upgrading function-mesh, HDYT?

[freeznet]

@jiangpengcheng env > conf/client.conf needs extra commands as well right?
What do you think if we separate the package download part as an initContainer? will it solve the backward compatibility issue?

[jiangpengcheng]

sounds perfect! but it will be a big change(needs to add an init container and volume if download path is specified?), and we still need to assemble download commands in the init container

[freeznet]

Correct, the file path can be extracted via .spec.java.jar / .spec.python.py / .spec.golang.go.

[jiangpengcheng]

hm, during development, I found that .spec.java.jar can be any value, like a raw filename without specifying dir, like pulsar-functions-api-examples.jar, then in the VolumeMounts for downloader, we need to use /pulsar(the workdir) as the MountPath? which will override everything under the workdir

[jiangpengcheng]

if use init containers, we need to do follow things:

1. publish a docker image for downloader, or use existing images like streamnative/pulsar-functions-java-runner ?
2. create an init container if the download path is specified
3. create an emptyDir volume for sharing the downloaded file if the download path is specified
4. create a volumeMount on the runner container to share the downloaded file
5. new GitHub actions to test when the download path is specified(we are missing that, right?)

@freeznet please check

[freeznet]

@jiangpengcheng lgtm, add some thoughts below:

• make a new image, one idea is a pulsar-admin ready-to-use toolkit image, but please be aware of tls / auth parameters
• the runner container should check file existence

Lets make this task an Epic and break-down into small tasks first.

[jiangpengcheng]

suppressed by #411