cshannon commented on code in PR #2539:
URL: https://github.com/apache/activemq/pull/2539#discussion_r4207323191
##########
activemq-broker/src/main/java/org/apache/activemq/broker/TransportConnector.java:
##########
@@ -80,6 +83,15 @@ public class TransportConnector implements Connector,
BrokerServiceAware {
private int maximumConsumersAllowedPerConnection = Integer.MAX_VALUE;
private PublishedAddressPolicy publishedAddressPolicy = new
PublishedAddressPolicy();
private boolean allowLinkStealing = false;
+ // remote address allow/deny lists; see setAllowList / setDenyList
+ private String allowList;
+ private String denyList;
+ private boolean allowDenyValidationEnabled = false;
+ private RemoteAddressValidator remoteAddressValidator;
+ private long allowListCount;
Review Comment:
These counts should probably be abstracted away into a metrics object that
is only optionally created and tracked if a validator is configured so we don't
need to add all the fields.
Really in general I think a lot of the changes here in TransportConnector
could be abstracted away into its own object that TransportConnector then
references (the new fields and the installRemoteAddressValidator method) so the
changes here are more minimal and the logic can be tested in isolation better.
##########
activemq-broker/src/main/java/org/apache/activemq/broker/Connector.java:
##########
@@ -86,6 +86,44 @@ public interface Connector extends Service {
long getMaxConnectionExceededCount();
+ /** @return the configured remote address allow list, comma separated
CIDRs or a file: URI, or null */
+ String getAllowList();
Review Comment:
Just like my comment about TransportConnector, I don't think we should be
adding a method for every single count when they could be combined.
Instead of having methods like "getDenyList()" or
"isAllowDenyValidationEnabled()" separately, they could be combined. Maybe
create a new policy type object or abstraction to manage the configuration that
could be referenced?
Mostly I just think everything here is too tightly coupled to Conenctor and
TransportConnector
--
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]
For further information, visit: https://activemq.apache.org/contact
