KAFKA-21224: Add mirror-level configs

[fvaleri]

After broker configs, this patch completes the ClusterMirrorConfig
class. Note: I removed the final "s" from the class name because all the
other configuration classes end with "Config".

Reviewers: Luke Chen showuon@gmail.com, Omnia Ibrahim o.g.h.ibrahim@gmail.com

[showuon]

Thanks for adding the tests. LGTM, Only one minor comment.

[OmniaGM]

Thanks @fvaleri for the pr, quick comment the pattern and ACL configs (topics.include/exclude, groups.include/exclude, topic.properties.exclude, acls.include/exclude) are defined without a validator, so AbstractConfig would accept any string for them. They're only checked later, in the constructor, when compilePatternList and parseAclRules run.

I believe here when a value is invalid, the exception thrown at that point is not a ConfigException:

• topics.include=foo[ throws PatternSyntaxException, which is a plain RuntimeException.
• acls.include=TOPIC;.*;REED throws IllegalArgumentException from AclOperation.valueOf.

So once the create-mirror path uses this config, a typo wouldn't come back INVALID_CONFIG, or the mirror would be accepted and only break later, once the link starts on the brokers.

Could we add validators to these define(...)? the same way as TopicCreationConfig::REGEX_VALIDATOR in Connect (catching PatternSyntaxException)? Each entry would then be checked whenever the config is parsed, and a bad one would fail with ConfigException(name, value, reason), which maps to INVALID_CONFIG. It would also name the entry that's wrong. same with rest of regex configs, and the ACL ones could have a similar validator that calls MirrorAclRule.parse.

WDYT?

[OmniaGM]

General thought here ... ClusterMirrorConfig now covers two separate config sets depending on the constructor: fromProperties builds a mirror-level config from CONFIG_DEF, and ClusterMirrorConfig(AbstractConfig) builds a broker-level one from BROKER_CONFIG_DEF. All the getters are on the same class, so calling the wrong one compiles fine but fails at runtime. For example, fetchBackoffMs() on a broker-level instance throws Unknown configuration, and topicsIncludePattern() returns null.

Could we split them the way group configs are split? GroupCoordinatorConfig wraps the broker config, and GroupConfig extends AbstractConfig for per-group configs. For example, keep ClusterMirrorConfigs (or a ClusterMirrorCoordinatorConfig) for the broker-level configs and use ClusterMirrorConfig only for the mirror-level ones. The broker-level constructor isn't used in this PR yet, so now is the easiest time to split it. WDYT?

[OmniaGM]

I've finished the first round of review. Before approving, I'd like us to resolve:

• validation for the regex/ACL configs (so invalid values fail with ConfigException)
• the SASL configs: the PLAIN default, the missing configs (like OAUTHBEARER)
• splitting the broker-level and mirror-level configs into separate classes

The rest are small nits/doc fixes, feel free to address them here or in a follow-up.