Environment Variables are not effective

[spareslant]

Hi,
In my test environment, I am trying to run Pinot with some environmental variables set already.

I think according to this code, environment variables should work: https://github.com/apache/pinot/blob/release-0.12.1/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java#L82

Following steps were followed:

export PINOT_CONTROLLER_ADMIN_ACCESS_CONTROL_PRINCIPALS=admin
export PINOT_CONTROLLER_ADMIN_ACCESS_CONTROL_PRINCIPALS_ADMIN_PASSWORD=admin123
export PINOT_CONTROLLER_ADMIN_ACCESS_CONTROL_FACTORY_CLASS="org.apache.pinot.controller.api.access.BasicAuthAccessControlFactory"

./conf/pinot-controller.conf file contents:

pinot.service.role=CONTROLLER
pinot.cluster.name=pinot-quickstart
pinot.zk.server=localhost:2191
pinot.set.instance.id.to.hostname=true
controller.vip.host=worker1.virtual.machine
controller.host=worker1.virtual.machine
controller.data.dir=/tmp/pinot/data/controller
controller.tls.keystore.path=/root/apache-pinot-0.12.0-bin/certs/pinot-keystore
controller.tls.keystore.password=pinot123
controller.tls.client.auth=false
controller.access.protocols=https
controller.access.protocols.https.port=10000
controller.broker.protocol=https
controller.vip.protocol=https
controller.vip.port=10000

Start Zookeeper command:

bin/pinot-admin.sh StartZookeeper kPort 2191

Start controller command:

bin/pinot-admin.sh StartController -configFileName ./conf/pinot-controller.conf

Expected Outcome:

Controller starts successfully. And I am able to browse its UI at https://localhost:10000/ . But environment variables are also set and it should ask me username and password. But it never gives that username/password screen.

More Info:

If I use following three lines in ./conf/pinot-controller.conf then username password screen appears fine.

controller.admin.access.control.principals=admin
controller.admin.access.control.principals.admin.password=admin123
controller.admin.access.control.factory.class=org.apache.pinot.controller.api.access.BasicAuthAccessControlFactory

Leaving controller.admin.access.control.factory.class option without principals options in conf file does not work. It gives exception while starting controller. (Principals are set in environment variables.)

Additional Info:

Pinot version: apache-pinot-0.12.0

java version:

openjdk version "11.0.3" 2019-04-16
OpenJDK Runtime Environment 18.9 (build 11.0.3+7)
OpenJDK 64-Bit Server VM 18.9 (build 11.0.3+7, mixed mode, sharing)

[Jackie-Jiang]

cc @xiangfu0

I can see 2 problems by reading PinotConfiguration:

1. Seems we are not using the constructor with environment variables
2. We keep only the variable with PINOT_ prefix, so there is no way to add key without pinot.

[xiangfu0]

I see, to me I feel we should add pinot. prefix to all the controller configs, we can prefer configs starting with pinot.controller.* to controller.*

[t0mpere]

@Jackie-Jiang @xiangfu0 Did someone work on this? Can I help? Are there any concerns in adding ENV support and pinot. prefix?
This is pretty critical to have a safe (and practical) deployment on k8s.

[t0mpere]

@xiangfu0 @Jackie-Jiang I would love to get your input on this. What would be the best way to deal with this?

• Prefix pinot. to all config keys. (Not backward compatible)
• Looking for different prefixes in env variables (PINOT_, CONTROLLER_ etc). Seems quite messy, there's a lot of them.
• Load all env

Do you see a fourth option?

[xiangfu0]

I think env var: PINOT_CONTROLLER_ADMIN_ACCESS_CONTROL_PRINCIPALS should be map to pinot.controller.admin.access.control.principals in the config file. But the config is actually controller.admin.access.control.principals, so that the env vars do not take effective.

See: https://github.com/apache/pinot/blob/master/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java

@t0mpere can you add a test to ensure the configs passed along with env variables are override as expected?

If the solution here is generic, then we can later on generalize the configs across all the pinot components.

[Jackie-Jiang]

Fixed with #12307