+1 (binding) We must have a flag to turn this on/off and disable it by default (this will also be needed for the transition to the new method)
Enrico Il giorno ven 9 dic 2022 alle ore 05:39 Yufei Zhang <affei...@gmail.com> ha scritto: > > Hi Enrico, > > Thank you for your feedback! > > I want to add more context to the issues you mentioned here: > > 1. Migration plan: The proposed changes are backward compatible, it still > allows the current configuration scheme (using command line), only that it > allows an optional alternative way of providing function configs. All the > existing functions and code will not be affected. Also, this change only > affects the `JavaInstanceStarter` (or equivalent in other languages) which > is not exposed to end users. It is used by Pulsar Worker which composes a > start shell command to call the `JavaInstanceRunner`. We don't need to > change anything in the Pulsar Worker as it still can use the old > configuration scheme(using command line). > > > 2. Security concerns: For important security configs, the file will only be > read once when the function instance starts, and later changes to the > config file won't take effect. Secondly, for scenarios that do use config > files, they can choose to generate the config file right before the > function instance launches. I believe it provides a similar security > guarantee as the command line args. For example, if an attacker can change > the mounted ConfigMap in k8s, then it is likely they can modify the start > command as well. > > In the proposed changes we mentioned dynamically reading configs from > files, but that will only be limited to some nonsensitive config entries, > currently, there is only one: `parallelism`. That being said, we need to be > careful in the implementation to make sure adding this proposed change > should not change any existing functionalities and security guarantees. > > Basically, this proposed change is not to replace the existing > configuration scheme, but to add a new option on top of the existing > configuration scheme. > > Hope this helps a bit. Let me know if these address your concerns and thank > you for your valuable feedback again! > > Cheers, > Yufei > > > > On Fri, Dec 9, 2022 at 8:02 AM Yufei Zhang <affei...@gmail.com> wrote: > > > Hi Enrico, > > > > Sure, we can go back to the discussion thread and I'll pause this voting > > thread for now until we address the concerns in the design. > > > > Cheers, > > Yufei > > > > On Fri, Dec 9, 2022 at 6:05 AM Enrico Olivelli <eolive...@gmail.com> > > wrote: > > > >> -1 (binding) > >> sorry I missed the discussion. > >> > >> It is not clear to me about the security model of this change. > >> The command line is not updatable for a process but other users may be > >> able > >> to update the configuration file or access tokens or other security > >> parameters. > >> > >> Also we need to define a migration plan and draw well the upgrade story > >> for > >> an existing cluster with running functions. In k8s it is possible that the > >> function use a different docker image than the function worker. > >> > >> I will follow up on the discussion thread > >> > >> > >> Enrico > >> > >> Il Gio 8 Dic 2022, 05:11 Rui Fu <r...@apache.org> ha scritto: > >> > >> > +1 > >> > > >> > Best, > >> > > >> > Rui Fu > >> > On Dec 8, 2022, 07:48 +0800, Yufei Zhang <affei...@gmail.com>, wrote: > >> > > Hi all, > >> > > > >> > > I'm starting the vote for PIP-225: Pulsar Functions fetch parameters > >> from > >> > > local config file: > >> > > https://github.com/apache/pulsar/issues/18744 > >> > > > >> > > Here is the discussion thread: > >> > > https://lists.apache.org/thread/3m6z7jgn71nzd3ng3x73vsxvd4b1jjcp > >> > > > >> > > The vote will be open for at least 3 days. > >> > > > >> > > Cheers, > >> > > Yufei > >> > > >> > >
