[
https://issues.apache.org/jira/browse/ARTEMIS-4709?focusedWorklogId=912667&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-912667
]
ASF GitHub Bot logged work on ARTEMIS-4709:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 02/Apr/24 14:29
Start Date: 02/Apr/24 14:29
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4871:
URL: https://github.com/apache/activemq-artemis/pull/4871#discussion_r1547968121
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/impl/ConnectionPeriodicExpiryPlugin.java:
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.core.server.plugin.impl;
+
+import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+
+import org.apache.activemq.artemis.api.core.ActiveMQDisconnectedException;
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyAcceptor;
+import
org.apache.activemq.artemis.core.remoting.impl.netty.NettyServerConnection;
+import org.apache.activemq.artemis.core.remoting.server.RemotingService;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerBasePlugin;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import org.apache.activemq.artemis.spi.core.remoting.Acceptor;
+import org.apache.activemq.artemis.utils.RandomUtil;
+
+public class ConnectionPeriodicExpiryPlugin implements
ActiveMQServerBasePlugin {
+
+ private String name;
+ private long periodSeconds;
+ private int accuracyWindowSeconds;
+ private String acceptorMatchRegex;
+
+ private ScheduledExecutorService executor;
+ private RemotingService remotingService;
+ private Pattern matchPattern;
+ private ScheduledFuture<?> task;
+
+ public ConnectionPeriodicExpiryPlugin() {
+ periodSeconds = TimeUnit.MINUTES.toSeconds(15);
+ accuracyWindowSeconds = 30;
+ acceptorMatchRegex = ""; // no match
+ }
+
+ @Override
+ public void registered(ActiveMQServer server) {
+ executor = server.getScheduledPool();
+ remotingService = server.getRemotingService();
+ matchPattern = Pattern.compile(acceptorMatchRegex);
+
+ task = executor.scheduleWithFixedDelay(() -> {
+
+ final long currentTime = System.currentTimeMillis();
+ for (Acceptor acceptor : remotingService.getAcceptors().values()) {
+ if (matchPattern.matcher(acceptor.getName()).matches()) {
+ if (acceptor instanceof NettyAcceptor) {
+ NettyAcceptor nettyAcceptor = (NettyAcceptor) acceptor;
+
+ for (NettyServerConnection nettyServerConnection :
nettyAcceptor.getConnections().values()) {
+ RemotingConnection remotingConnection =
remotingService.getConnection(nettyServerConnection.getID());
+ if (currentTime > remotingConnection.getCreationTime() +
periodSeconds ) {
Review Comment:
Seems like it needs a validity check, connection could have already gone
away by other means while this checking was happening, so perhaps it could NPE
here.
Related...as is, if this task ever throws for any reason (e.g above
potential NPE) the expiry process will then simply stop working as the task
wont be rescheduled.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/impl/ConnectionPeriodicExpiryPlugin.java:
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
Review Comment:
Shouldnt have the \<p\> elements and the URL should be indented. As in the
other files.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/impl/ConnectionPeriodicExpiryPlugin.java:
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.core.server.plugin.impl;
+
+import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.regex.Pattern;
+
+import org.apache.activemq.artemis.api.core.ActiveMQDisconnectedException;
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyAcceptor;
+import
org.apache.activemq.artemis.core.remoting.impl.netty.NettyServerConnection;
+import org.apache.activemq.artemis.core.remoting.server.RemotingService;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerBasePlugin;
+import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
+import org.apache.activemq.artemis.spi.core.remoting.Acceptor;
+import org.apache.activemq.artemis.utils.RandomUtil;
+
+public class ConnectionPeriodicExpiryPlugin implements
ActiveMQServerBasePlugin {
+
+ private String name;
+ private long periodSeconds;
+ private int accuracyWindowSeconds;
+ private String acceptorMatchRegex;
+
+ private ScheduledExecutorService executor;
+ private RemotingService remotingService;
+ private Pattern matchPattern;
+ private ScheduledFuture<?> task;
+
+ public ConnectionPeriodicExpiryPlugin() {
+ periodSeconds = TimeUnit.MINUTES.toSeconds(15);
+ accuracyWindowSeconds = 30;
+ acceptorMatchRegex = ""; // no match
+ }
+
+ @Override
+ public void registered(ActiveMQServer server) {
+ executor = server.getScheduledPool();
+ remotingService = server.getRemotingService();
+ matchPattern = Pattern.compile(acceptorMatchRegex);
+
+ task = executor.scheduleWithFixedDelay(() -> {
+
+ final long currentTime = System.currentTimeMillis();
+ for (Acceptor acceptor : remotingService.getAcceptors().values()) {
+ if (matchPattern.matcher(acceptor.getName()).matches()) {
+ if (acceptor instanceof NettyAcceptor) {
+ NettyAcceptor nettyAcceptor = (NettyAcceptor) acceptor;
+
+ for (NettyServerConnection nettyServerConnection :
nettyAcceptor.getConnections().values()) {
+ RemotingConnection remotingConnection =
remotingService.getConnection(nettyServerConnection.getID());
+ if (currentTime > remotingConnection.getCreationTime() +
periodSeconds ) {
+ executor.schedule(() -> {
+
remotingService.removeConnection(remotingConnection.getID());
+ remotingConnection.fail(new
ActiveMQDisconnectedException("terminated by session expiry plugin"));
+ }, RandomUtil.randomMax(accuracyWindowSeconds),
TimeUnit.SECONDS);
+ }
+ }
+ }
+ }
+ }
+ }, accuracyWindowSeconds, accuracyWindowSeconds, TimeUnit.SECONDS);
+ }
+
+ @Override
+ public void unregistered(ActiveMQServer server) {
+ task.cancel(true);
+ }
+
+ @Override
+ public void init(Map<String, String> properties) {
+ name = properties.getOrDefault("name", name);
+ periodSeconds = Long.parseLong(properties.getOrDefault("periodSeconds",
Long.toString(periodSeconds)));
+ accuracyWindowSeconds =
Integer.parseInt(properties.getOrDefault("accuracyWindowSeconds",
Long.toString(accuracyWindowSeconds)));
+ if (accuracyWindowSeconds <= 0) {
+ throw new IllegalArgumentException("accuracyWindowSeconds must be >
0");
+ }
+ acceptorMatchRegex = properties.getOrDefault("acceptorMatchRegex",
acceptorMatchRegex);
Review Comment:
Should it even be allowed to configure it with an empty regex, as it seems
this would?
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/impl/ConnectionPeriodicExpiryPlugin.java:
##########
@@ -0,0 +1,130 @@
+/**
Review Comment:
Shouldnt be a javadoc
##########
docs/user-manual/broker-plugins.adoc:
##########
@@ -178,3 +178,30 @@ In the example below `ROLE_PROPERTY` is set to
`permissions` when that property
</broker-plugin>
</broker-plugins>
----
+
+== Using the ConnectionPeriodicExpiryPlugin
+
+The `ConnectionPeriodicExpiryPlugin` will implement a global expiry (and
disconnect) for connections that live longer than `periodSeconds` on a matching
acceptor basis.
+
+This plugin can be useful when credential rotation or credential validation
must be enforced at regular intervals as authentication will be enforced on
reconnect.
+
+The plugin requires the configuration of the `acceptorMatchRegex` to determine
the acceptors to monitor. It is typical to separate client acceptors and
federation or cluster acceptors such that only client connections will be
subject to periodic expiry. The `acceptorMatchRegex` must be configured to
match the name of the acceptor(s) whose connections will be subject to periodic
expiry.
+
+|===
+| Property | Property Description | Default Value
+
+|`acceptorMatchRegex`|the regular expression used to match against the names
of acceptors to monitor | ""
+|`periodSeconds`|the max duration or period, in seconds, that a connection can
last | 15 minutes (as seconds)
Review Comment:
the default would be more obvious if offered in the way it is to actually be
used, e.g: "900 (i.e 15 minutes)"
Issue Time Tracking
-------------------
Worklog Id: (was: 912667)
Time Spent: 20m (was: 10m)
> Add a plugin to provide periodic expiry of connections on a per acceptor basis
> ------------------------------------------------------------------------------
>
> Key: ARTEMIS-4709
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4709
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Components: Broker
> Affects Versions: 2.33.0
> Reporter: Gary Tully
> Assignee: Gary Tully
> Priority: Major
> Fix For: 2.34.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> When credential rotation needs to be enforced, active connections need to be
> terminated on some timeline to ensure credentials are reevaluated. There are
> management apis that can be used but these require some intervention.
> In addition to enforce some SLA around duration of connections, having an
> easy way to limit connections to a given maximum period can be helpful.
> A plugin that will be applied on an per acceptor basis, that can be used to
> disconnect connections that have lived for some period can provide a nice
> building block for these use cases.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
