cmccabe opened a new pull request, #15584:
URL: https://github.com/apache/kafka/pull/15584
KAFKA-16222 fixed a bug whereby we didn't undo the name mangling used on
client quota entity names stored in ZooKeeper. However, it incorrectly claimed
to fix the handling of default client quota entities. It also failed to
correctly re-mangle when syncronizing the data back to ZooKeeper.
This PR fixes ZkConfigMigrationClient to do the mangling correctly on both
the read and write paths. We do unmangling before invoking the visitors, since
after all it does not make sense to do the same unmangling step in each and
every visitor.
Additionally, this PR fixes a bug causing default entities to be converted
incorrectly. For example, ClientQuotaEntity(user -> null) is stored under the
/config/users/<default> znode in ZooKeeper. In KRaft it appears as a
ClientQuotaRecord with EntityData(entityType=users, entityName=null). Prior to
this PR, this was being converted to a ClientQuotaRecord with
EntityData(entityType=users, entityName=""). That represents a quota on the
user whose name is the empty string (yes, we allow users to name themselves
with the empty string, sadly.)
The confusion appears to have arisen because for TOPIC and BROKER
configurations, the default ConfigResource is indeed the one named with the
empty (not null) string. For example, the default topic configuration resource
is ConfigResource(name="", type=TOPIC). However, things are different for
client quotas. Default client quota entities in KRaft (and also in AdminClient)
are represented by maps with null values. For example, the default User entity
is represented by Map("user" -> null). In retrospect, using a map with null
values was a poor choice; a Map<String, Optional<String>> would have made more
sense. However, this is the way the API currently is and we have to convert
correctly.
There was an additional level of confusion present in KAFKA-16222 where
someone thought that using the ZooKeeper placeholder string "<default>" in the
AdminClient API would yield a default client quota entity. Thise seems to have
been perpetuated by the ConfigEntityName class that was created recently. In
fact, <default> is not part of any public API in Kafka. Accordingly, this PR
also renames ConfigEntityName.DEFAULT to ZooKeeperInternals.DEFAULT_STRING, to
make it clear that the string <default> is just a detail of the ZooKeeper
implementation. It is not used in the Kafka API to indicate defaults.
Hopefully this will avoid confusion in the future.
Finally, the PR also creates KRaftClusterTest.testDefaultClientQuotas to get
extra test coverage of setting default client quotas.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
