pravega / zookeeper-operator

Kubernetes Operator for Zookeeper
Apache License 2.0
369 stars 205 forks source link

skipACL is hardcoded to "yes" and cannot be overridden via env vars #316

Open thelabdude opened 3 years ago

thelabdude commented 3 years ago

Description

The skipACL property is set to yes in zoo.cfg, see: https://github.com/pravega/zookeeper-operator/blob/b1519aec85c52bd946eaa91b4bcf58e1767210f2/pkg/zk/generators.go#L250

This prevents consumer applications (Solr in my case) from being able to guard sensitive znodes. Does the operator rely on ACLs not being enforced?

I tried overriding the value from zoo.cfg with an env var:

        env:
          - name: SERVER_JVMFLAGS
            value: -Dzookeeper.skipACL=no

This should work because of: https://github.com/apache/zookeeper/blob/release-3.6.1/zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java#L130

And the system prop is getting passed to the Java process correctly:

... -Dzookeeper.skipACL=no -Dcom.sun.management.jmxremote -Dcom.sun.management.jmxremote.local.only=false org.apache.zookeeper.server.quorum.QuorumPeerMain /data/conf/zoo.cfg

However, this doesn't work b/c QuorumPeerConfig#parseProperties runs before ZooKeeperServer's static initializer block gets called. Unfortunately, parseProperties does this for any additional properties in the zoo.cfg:

            } else {
                System.setProperty("zookeeper." + key, value);
            }

See: https://github.com/apache/zookeeper/blob/release-3.6.1/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java#L408

Importance

must-have! This prevents enforcing ACLs established by our application.

Location

https://github.com/pravega/zookeeper-operator/blob/b1519aec85c52bd946eaa91b4bcf58e1767210f2/pkg/zk/generators.go#L250

Suggestions for an improvement

The operator should allow this config property to be changed.

patrickdung commented 1 year ago

I would like to make this option configurable. skipACL=yes is not secure by default.