You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "avshenuk (via GitHub)" <gi...@apache.org> on 2025/07/25 11:20:14 UTC

[I] New plugin system with pulgin.properties has a critical flaw [pinot]

avshenuk opened a new issue, #16425:
URL: https://github.com/apache/pinot/issues/16425

   According to the new plugin system (and verified from the code), each plugin folder and all its .jar files are isolated and loaded via the whole ClassWorld/Realm loading system, which is great and it works but...
   
   there is one critical flaw, and it's not in the Java code. It's in the Bash scripts loading Pinot.
   
   In `appAssemblerScriptTemplate` we can see that absolutely every single JAR from the plugin folders (regardless whether it uses the old or new loading) is added to the classpath:
   ```
   # Set $PLUGINS_CLASSPATH for plugin jars to be put into classpath.
   # $PLUGINS_DIR and $PLUGINS_INCLUDE are used if $PLUGINS_CLASSPATH is not set.
   # $PLUGINS_DIR is semi-colon separated list of plugin directories, default to '"$BASEDIR"/plugins' if not set.
   # $PLUGINS_INCLUDE is semi-colon separated plugins name, e.g. pinot-avro;pinot-batch-ingestion-standalone. Default is not set, which means load all the plugin jars.
   # If the same plugin is found in multiple directories, it will only be picked up from the first directory it was found. The directories in $PLUGINS_DIR are traversed by their order.
   if [ -z "$PLUGINS_CLASSPATH" ] ; then
   ....
           PLUGIN_JARS=$(find "$DIR" -name \*.jar)
           for PLUGIN_JAR in $PLUGIN_JARS ; do
             if [ -n "$PLUGINS_CLASSPATH" ] ; then
               PLUGINS_CLASSPATH=$PLUGINS_CLASSPATH:$PLUGIN_JAR
             else
               PLUGINS_CLASSPATH=$PLUGIN_JAR
             fi
           done
         fi
   ...
   if [ -n "$PLUGINS_CLASSPATH" ] ; then
     CLASSPATH=$CLASSPATH:$PLUGINS_CLASSPATH
   fi
   ```
   
   So as a result, any dependency you are using: Pinot or not, is going to override whatever Pinot is using internally.
   Already tested this and got a mismatch of classes.
   
   P.S. There is also a small issue in that the Bash script overrides the plugins dir System property:
   ```
   if [ -z "$PLUGINS_CLASSPATH" ] ; then
     if [ -z "$PLUGINS_DIR" ] ; then
       PLUGINS_DIR="$BASEDIR"/plugins
   ...
   if [ -z "$PLUGINS_DIR" ] ; then
     PLUGINS_DIR=$BASEDIR/plugins
   fi
   ALL_JAVA_OPTS="$ALL_JAVA_OPTS -Dplugins.dir=$PLUGINS_DIR"
   ```
   Which means the System property set in e.g. K8S Helm charts is completely ignored. This is what is currently present in the chart
   ```
           env:
             - name: JAVA_OPTS
               value: "{{ .Values.server.jvmOpts }} -Dlog4j2.configurationFile={{ .Values.server.log4j2ConfFile }} -Dplugins.dir={{ .Values.server.pluginsDir }}"
   ```


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] New plugin system with pulgin.properties has a critical flaw [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on issue #16425:
URL: https://github.com/apache/pinot/issues/16425#issuecomment-3133518805

   cc @xiangfu0 @gortiz to take a look


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] New plugin system with plugin.properties has a critical flaw [pinot]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #16425:
URL: https://github.com/apache/pinot/issues/16425#issuecomment-4320903448

   This issue has been closed because it has been inactive for 180 days. If you believe this issue is still relevant, please feel free to reopen it or create a new issue. Thank you!


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] New plugin system with plugin.properties has a critical flaw [pinot]

Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on issue #16425:
URL: https://github.com/apache/pinot/issues/16425#issuecomment-3150137701

   Yes, this is a known issue. We need to modify the bash script to not add the classes to the classpath


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] New plugin system with plugin.properties has a critical flaw [pinot]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #16425:
URL: https://github.com/apache/pinot/issues/16425#issuecomment-4113145923

   This issue has been automatically marked as stale because it has not had any activity in the last 150 days. It will be closed in 30 days if no further activity occurs. If this issue is still relevant, please leave a comment to keep it open. Thank you for your contributions!


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [I] New plugin system with plugin.properties has a critical flaw [pinot]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed issue #16425: New plugin system with plugin.properties has a critical flaw
URL: https://github.com/apache/pinot/issues/16425


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org