You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/10/23 20:20:51 UTC
[GitHub] [pulsar] flowchartsman opened a new pull request #8365: [Go functions] Provide pre-start access to secrets and conf
flowchartsman opened a new pull request #8365:
URL: https://github.com/apache/pulsar/pull/8365
### Motivation
The pulsar go functions SDK has no access to user config outside of the function context, which is only accessible from inside the function handler itself (i.e. once processing has started). This is a somewhat awkward access method when it comes to user configuration values which might be used to initialize a persistent connection to a database or other service, since it would require a largely-superfluous check for the initialization and related configuration values in every run of the function, when it's only required once at startup. In addition, secrets are passed into the function context, but are completely inaccessible both inside and outside of the function, so support for these has been added to.
### Modifications
Two global methods have been provided to access the user config map and the secrets map, and transport of the secrets from the proto into the function context have been added.
### Verifying this change
This change is predicated on the changes (#8132) made towards solving #8216, and will need to be tested in conjunction with them, however a very similar example should suffice:
```go
package main
import (
"context"
"fmt"
log "github.com/apache/pulsar/pulsar-function-go/logutil"
"github.com/apache/pulsar/pulsar-function-go/pf"
)
func contextFunc(ctx context.Context) {
if fc, ok := pf.FromContext(ctx); ok {
log.Infof("function ID is:%s, ", fc.GetFuncID())
log.Infof("function version is:%s\n", fc.GetFuncVersion())
for k := range fc.GetUserConfMap() {
log.Infof("Config %q -> \"%v\"", k, fc.GetUserConfValue(k))
}
for k := range fc.GetSecretsMap() {
log.Infof("Secret %q -> \"%v\"", k, fc.GetSecretsValue(k))
}
}
}
func main() {
conf := pf.GetUserConfig()
secrets := pf.GetSecrets()
fmt.Println("***************************")
fmt.Printf("got conf: %#v\n", conf)
fmt.Printf("got secrets: %#v\n", secrets)
fmt.Println("***************************")
pf.Start(contextFunc)
}
```
### 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): (yes / no)
- The public API: (yes)
- Two new global methods are added to the Pulsar Functions for Go SDK, `GetUserConfig()` and `GetSecrets()`, as well as two new methods to `FunctionsContext`: `GetSecretsMap() map[string]interface{}` and `GetSecretsValue(key string) interface{}`
- The schema: (no)
- The default values of configurations: (yes / no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (don't know)
#### Public API
This adds two new global methods to the Pulsar Functions for Go SDK, in addition to two new methods on
### Documentation
- Does this pull request introduce a new feature? (yes)
- If yes, how is the feature documented? (not applicable)
- If a feature is not applicable for documentation, explain why?
- godoc will suffice
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on a change in pull request #8365: [Go functions] Provide pre-start access to secrets and conf
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #8365:
URL: https://github.com/apache/pulsar/pull/8365#discussion_r512854054
##########
File path: pulsar-function-go/pf/context.go
##########
@@ -119,6 +118,15 @@ func (c *FunctionContext) GetUserConfMap() map[string]interface{} {
return c.userConfigs
}
+func (c *FunctionContext) GetSecretsMap() map[string]interface{} {
Review comment:
I don't think it is a good idea to pass `secrets` in plain text. In Java & Python functions, we have introduced a notion called a secrets provider. The secrets provider is responsible for retrieving secrets for functions.
I'd suggest moving the secrets change to a separate pull request. Because it requires a fair amount of work to get a good implementation with a secrets provider.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on pull request #8365: [Go functions] Provide pre-start access to secrets and conf
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #8365:
URL: https://github.com/apache/pulsar/pull/8365#issuecomment-715665873
Thanks @flowchartsman, Can you provide a unit test case for this change and add docs for this interface?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] flowchartsman closed pull request #8365: [Go functions] Provide pre-start access to secrets and conf
Posted by GitBox <gi...@apache.org>.
flowchartsman closed pull request #8365:
URL: https://github.com/apache/pulsar/pull/8365
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] flowchartsman commented on pull request #8365: [Go functions] Provide pre-start access to secrets and conf
Posted by GitBox <gi...@apache.org>.
flowchartsman commented on pull request #8365:
URL: https://github.com/apache/pulsar/pull/8365#issuecomment-723254352
I am closing this PR in favor of #8467, which is more narrow in scope and applies only to the user config.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] wolfstudy commented on a change in pull request #8365: [Go functions] Provide pre-start access to secrets and conf
Posted by GitBox <gi...@apache.org>.
wolfstudy commented on a change in pull request #8365:
URL: https://github.com/apache/pulsar/pull/8365#discussion_r516380304
##########
File path: pulsar-function-go/pf/context.go
##########
@@ -119,6 +118,15 @@ func (c *FunctionContext) GetUserConfMap() map[string]interface{} {
return c.userConfigs
}
+func (c *FunctionContext) GetSecretsMap() map[string]interface{} {
Review comment:
@flowchartsman As we discussed, we can separate this pr and provide the secretsProvider interface to allow users to customize their own encryption methods. By default, we can provide an implementation of reading secrets from environment variables.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] flowchartsman commented on a change in pull request #8365: [Go functions] Provide pre-start access to secrets and conf
Posted by GitBox <gi...@apache.org>.
flowchartsman commented on a change in pull request #8365:
URL: https://github.com/apache/pulsar/pull/8365#discussion_r515972686
##########
File path: pulsar-function-go/pf/context.go
##########
@@ -119,6 +118,15 @@ func (c *FunctionContext) GetUserConfMap() map[string]interface{} {
return c.userConfigs
}
+func (c *FunctionContext) GetSecretsMap() map[string]interface{} {
Review comment:
So, according to the setup code I looked at, the secrets are already getting sent into the function context, just not actually referenced anywhere. Is the concern that the existing method doesn't allow for a pluggable `SecretsProvider` interface? If so, that makes sense and I'll amend the PR and we can address it later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org