[Issue 6170][functions] add custom property option to functions #6348

nlu90 · 2020-02-17T18:13:44Z

Motivation

Allow users set custom system properties while submitting functions. This can be used to pass credentials via system property.

Modifications

pulsar-admin client tool to accept multiple --custom-property options while creating the function
function's java runtime to append these provided properties.

Verifying this change

Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

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

(or)

This change added tests and can be verified as follows:

(example:)

Added integration tests for end-to-end deployment with large payloads (10MB)
Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

Dependencies (does it add or upgrade a dependency): no
The public API: no
The schema: no
The default values of configurations: no
The wire protocol: no
The rest endpoints: no
The admin cli options: yes
Anything that affects deployment: no

Documentation

Does this pull request introduce a new feature? no
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
If a feature is not applicable for documentation, explain why?
If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

nlu90 · 2020-02-17T18:16:39Z

Now we can submit a function with multiple custom properties as follows:

./bin/pulsar-admin functions create \
--custom-property prop1="hello_world" --custom-property success=1 \
--jar ./pulsar-functions/java-examples/target/pulsar-functions-api-examples.jar \
--classname org.apache.pulsar.functions.api.examples.ExclamationFunction \
--tenant public --namespace default --name exclamation \
--inputs persistent://public/default/my-topic \
--output persistent://public/default/exclamation

nlu90 · 2020-02-18T01:36:23Z

After a discussion with @sijie , the way this options work is as follows now:

./bin/pulsar-admin functions create \
--custom-property prop1=PROP1_VALUE --custom-property prop2=PROP2_VALUE \
--jar ./pulsar-functions/java-examples/target/pulsar-functions-api-examples.jar \
--classname org.apache.pulsar.functions.api.examples.ExclamationFunction \
--tenant public --namespace default --name exclamation \
--inputs persistent://public/default/my-topic \
--output persistent://public/default/exclamation

both PROP1_VALUE and PROP2_VALUE would be replaced with the actual system property values(assume already set by user) when the function process is being started.

KannarFr · 2020-02-18T10:15:30Z

@nlu90 Hi, thanks for the work you did here :).

So PROP2_VALUE must be a system env var? What if it's undefined? Does the system throws an exception on boot?
All these options are defined using pulsar-admin with arguments. Does the creation via YAML function configuration supports these features?

nlu90 · 2020-02-18T18:45:19Z

@KannarFr Thanks for your feedback

I added check for whether the property is set in broker/worker. If it's not set, an IllegalArgumentException will be thrown and the function submission will be failed.
Yes, this change also works with YAML configuration. For my previously example submission command, you can also use the following YAML file as a configuration file to submit:

jar: ./pulsar-functions/java-examples/target/pulsar-functions-api-examples.jar
className: org.apache.pulsar.functions.api.examples.ExclamationFunction
tenant: public
namespace: default
name: exclamation
customProperties: 
  - prop1=PROP1_VALUE
  - prop2=PROP2_VALUE
inputs: 
  - persistent://public/default/my-topic
output: persistent://public/default/exclamation

and the submission command is:

./bin/pulsar-admin functions create --function-config-file ~/func-conf.yaml

sijie · 2020-02-18T19:14:52Z

@nlu90 the change looks very good. although I think it might be worth thinking of the extensibility of this approach. We might be looking into some more generic variable binding approach that allows user dynamically generating the properties or values through variable bindings. Serverless provides a good example of how to load variable bindings through different formats (e.g. environment variables, cli options, and etc).

nlu90 · 2020-02-18T19:32:10Z

@sijie We can create a new issue for the yaml file variable extension feature and add it in a separate PR.

sijie · 2020-02-18T19:35:11Z

@nlu90 sounds good. do you want to create an issue for that?

sijie · 2020-02-18T19:39:59Z

@nlu90 actually looked into the description of #6170, I think what @KannarFr is looking for might be similar to a variable binding approach. Maybe let's ask @KannarFr for some clarifications.

KannarFr · 2020-02-18T20:03:07Z

The goal is to use system env var defined and pass them to functions via the yaml configuration file. Like myProp: $MY_PROP. Le mar. 18 févr. 2020 à 20:40, Sijie Guo <notifications@github.com> a écrit :

…

@nlu90 <https://github.com/nlu90> actually looked into the description of #6170 <#6170>, I think what @KannarFr <https://github.com/KannarFr> is looking for might be similar to a variable binding approach. Maybe let's ask @KannarFr <https://github.com/KannarFr> for some clarifications. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6348?email_source=notifications&email_token=ABZLQJIO756ZVJTA6SNUK6LRDQ2RBA5CNFSM4KWWN7P2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMD7FCY#issuecomment-587723403>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABZLQJIFEPS37EJEDGMK6PLRDQ2RBANCNFSM4KWWN7PQ> .

nlu90 · 2020-02-18T22:16:59Z

Hi @KannarFr, Just want to confirm if the following solution meet your needs (Notice the last line of customRuntimeOptions):

tenant: "test"
namespace: "test-namespace"
name: "example"
className: "org.apache.pulsar.functions.api.examples.ExclamationFunction"
inputs: ["test_src"]
userConfig:
  "PublishTopic": "test_result"
output: "test_result"
autoAck: true
parallelism: 1
customRuntimeOptions: "-DmyProp=${MY_PROP} -Dother_option=other_value"

and when you create function with this yaml file, the variable ${MY_PROP} will be automatically replaced with the actual system property value.

Let me know your thoughts.

KannarFr · 2020-02-18T22:32:52Z

That's it! All seems perfect then. Le mar. 18 févr. 2020 à 23:17, Neng Lu <notifications@github.com> a écrit :

…

Hi @KannarFr <https://github.com/KannarFr>, Just want to confirm if the following solution meet your needs (Notice the last line of customRuntimeOptions): tenant: "test"namespace: "test-namespace"name: "example"className: "org.apache.pulsar.functions.api.examples.ExclamationFunction"inputs: ["test_src"]userConfig: "PublishTopic": "test_result"output: "test_result"autoAck: trueparallelism: 1customRuntimeOptions: "-DmyProp=${MY_PROP}" and when you create function with this yaml file, the variable ${MY_PROP} will be automatically replaced with the actual system property value. Let me know your thoughts. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6348?email_source=notifications&email_token=ABZLQJIUSGWBTOZJTEDKAXTRDRM5ZA5CNFSM4KWWN7P2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMFR7PQ#issuecomment-587931582>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABZLQJLM6ZDOB4UCTQBVIH3RDRM5ZANCNFSM4KWWN7PQ> .

nlu90 · 2020-02-19T04:23:10Z

@sijie @KannarFr Now you can submit a function with the following yaml file. And ${MY_PROP1} and ${MY_PROP2} will be replaced with the predefined environment variables.

jar: ./pulsar-functions/java-examples/target/pulsar-functions-api-examples.jar
className: org.apache.pulsar.functions.api.examples.ExclamationFunction
tenant: public
namespace: default
name: exclamation
inputs:
  - persistent://public/default/my-topic
output: persistent://public/default/exclamation
customRuntimeOptions: "-Dprop1=${MY_PROP1} -Dprop2=${MY_PROP2}"

One more question is currently if we fetch function info with the cli, it will show the actual value of ${MY_PROP1} and ${MY_PROP2}, is this expected? Or should we hide the actual value in the cli result?

KannarFr · 2020-02-19T14:30:40Z

@nlu90 Well considering logs are often exported to external services, I think we must hide them from logs. Thats being said, we can log the env var name like: injecting $PROP as prop foreach prop. WDYT?

nlu90 · 2020-02-19T23:11:28Z

@KannarFr Now no actual variable value will be logged into files.

KannarFr · 2020-02-19T23:12:06Z

@nlu90 Perfect! Thanks for all the work you did here :).

KannarFr · 2020-02-24T13:03:11Z

@sijie can we merge this?

KannarFr · 2020-02-24T13:04:47Z

@nlu90 oh, there few tests issues, can you check them?

sijie · 2020-02-24T19:25:29Z

/pulsarbot run-failure-checks

…perty-to-functions

KannarFr · 2020-03-19T01:26:41Z

What is the status of this PR?

nlu90 · 2020-03-24T19:41:17Z

@KannarFr Sorry for the late reply, I was busy with some other projects.

After a discussion with @sijie , now the changes enable you put the env variable in userConfig and get the actual value via context.getUserConfigValue

an example yaml would be:

jar: ./pulsar-functions/java-examples/target/pulsar-functions-api-examples.jar
className: org.apache.pulsar.functions.api.examples.ExclamationFunction
tenant: public
namespace: default
name: exclamation
inputs:
  - persistent://public/default/my-topic
output: persistent://public/default/exclamation
userConfig:
  "prop1": "$MY_PROP"

and you can access the actual value for prop1 in your function code like this:
context.getUserConfigValue("prop1")

KannarFr · 2020-03-24T20:12:50Z

@nlu90 Thanks for your answer.

So

  "prop1": "$MY_PROP"

reads $MY_PROP from system env vars? Right?

nlu90 · 2020-03-24T22:08:58Z

@nlu90 Thanks for your answer.

So
  "prop1": "$MY_PROP"
reads $MY_PROP from system env vars? Right?

Yes.

KannarFr · 2020-03-24T22:37:26Z

Awesome!

Anonymitaet · 2020-06-29T07:06:51Z

Docs have been added here.
Will copy to Pulsar later.

…he#6348) Fixes apache#6170 ### Motivation Allow users set custom system properties while submitting functions. This can be used to pass credentials via system property. ### Modifications - pulsar-admin client tool to accept multiple `--custom-property` options while creating the function - function's java runtime to append these provided properties.

add custom property option to functions

7efe1d2

nlu90 requested review from srkukarni, jerrypeng and sijie February 17, 2020 18:16

nlu90 added 2 commits February 17, 2020 11:44

simplify adding logic

aae6054

replace actual property value when starting functions

0fe5867

check if the property value is set

f5a9d8e

sijie assigned nlu90 Feb 18, 2020

sijie added the area/function label Feb 18, 2020

sijie added this to the 2.6.0 milestone Feb 18, 2020

enable environment variable substitution in customRuntimeOptions

e8c01a6

append custom option to function start cmd

1502c9d

substitute environment variable only when starting the function

de69133

sijie added the doc-required Your PR changes impact docs and you will update later. label Feb 24, 2020

sijie approved these changes Feb 24, 2020

View reviewed changes

Merge remote-tracking branch 'apache/master' into neng/add-custom-pro…

96e3be3

…perty-to-functions

use userConfig for system env passing

2d88fa6

sijie approved these changes Mar 25, 2020

View reviewed changes

sijie merged commit 00463ad into apache:master Mar 26, 2020

sijie deleted the neng/add-custom-property-to-functions branch March 26, 2020 07:02

sijie mentioned this pull request May 20, 2020

[discussion] Pulsar release 2.6.0 #5819

Closed

Anonymitaet removed the doc-required Your PR changes impact docs and you will update later. label Jun 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Issue 6170][functions] add custom property option to functions #6348

[Issue 6170][functions] add custom property option to functions #6348

nlu90 commented Feb 17, 2020

nlu90 commented Feb 17, 2020

nlu90 commented Feb 18, 2020 •

edited

Loading

KannarFr commented Feb 18, 2020

nlu90 commented Feb 18, 2020 •

edited

Loading

sijie commented Feb 18, 2020

nlu90 commented Feb 18, 2020

sijie commented Feb 18, 2020

sijie commented Feb 18, 2020

KannarFr commented Feb 18, 2020 via email

nlu90 commented Feb 18, 2020 •

edited

Loading

KannarFr commented Feb 18, 2020 via email

nlu90 commented Feb 19, 2020 •

edited

Loading

KannarFr commented Feb 19, 2020

nlu90 commented Feb 19, 2020

KannarFr commented Feb 19, 2020

KannarFr commented Feb 24, 2020

KannarFr commented Feb 24, 2020

sijie commented Feb 24, 2020

KannarFr commented Mar 19, 2020

nlu90 commented Mar 24, 2020

KannarFr commented Mar 24, 2020

nlu90 commented Mar 24, 2020

KannarFr commented Mar 24, 2020

Anonymitaet commented Jun 29, 2020

[Issue 6170][functions] add custom property option to functions #6348

[Issue 6170][functions] add custom property option to functions #6348

Conversation

nlu90 commented Feb 17, 2020

Motivation

Modifications

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

nlu90 commented Feb 17, 2020

nlu90 commented Feb 18, 2020 • edited Loading

KannarFr commented Feb 18, 2020

nlu90 commented Feb 18, 2020 • edited Loading

sijie commented Feb 18, 2020

nlu90 commented Feb 18, 2020

sijie commented Feb 18, 2020

sijie commented Feb 18, 2020

KannarFr commented Feb 18, 2020 via email

nlu90 commented Feb 18, 2020 • edited Loading

KannarFr commented Feb 18, 2020 via email

nlu90 commented Feb 19, 2020 • edited Loading

KannarFr commented Feb 19, 2020

nlu90 commented Feb 19, 2020

KannarFr commented Feb 19, 2020

KannarFr commented Feb 24, 2020

KannarFr commented Feb 24, 2020

sijie commented Feb 24, 2020

KannarFr commented Mar 19, 2020

nlu90 commented Mar 24, 2020

KannarFr commented Mar 24, 2020

nlu90 commented Mar 24, 2020

KannarFr commented Mar 24, 2020

Anonymitaet commented Jun 29, 2020

nlu90 commented Feb 18, 2020 •

edited

Loading

nlu90 commented Feb 18, 2020 •

edited

Loading

nlu90 commented Feb 18, 2020 •

edited

Loading

nlu90 commented Feb 19, 2020 •

edited

Loading