walterddr commented on code in PR #8314:
URL: https://github.com/apache/pinot/pull/8314#discussion_r863225039
##########
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerConf.java:
##########
@@ -240,6 +242,8 @@ private static long getRandomInitialDelayInSeconds() {
private static final int DEFAULT_JERSEY_ADMIN_PORT = 21000;
private static final String DEFAULT_ACCESS_CONTROL_FACTORY_CLASS =
"org.apache.pinot.controller.api.access.AllowAllAccessFactory";
+ private static final String DEFAULT_ACCESS_CONTROL_USERNAME = "admin";
+ private static final String DEFAULT_ACCESS_CONTROL_PASSWORD = "admin";
Review Comment:
next: if not configured we should simply throw exception if a secure auth
enabled cluster - i don't think we should allow plain text default
##########
pinot-broker/src/main/java/org/apache/pinot/broker/broker/AccessControlFactory.java:
##########
@@ -28,11 +30,24 @@ public abstract class AccessControlFactory {
public static final Logger LOGGER =
LoggerFactory.getLogger(AccessControlFactory.class);
public static final String ACCESS_CONTROL_CLASS_CONFIG = "class";
- public abstract void init(PinotConfiguration confguration);
+ public void init(PinotConfiguration configuration) {
+ };
+
+ /**
+ * Extend original init method inorder to support Zookeeper
BasicAuthAccessControlFactory
Review Comment:
+1 either use PinotHelixResourceManager or ZkHelixPropertyStore since you
used PinotHelixResourceManager in the init method of
`ZkBasicAuthAccessControlFactory`
but can be address in next release
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
