For the subscription I’ll start a new thread as you suggested.
About the NPE I tried again to run the test from my Eclipse IDE (after
rebuilding all the images “mvn clean install -DskipITs -DskipTests -Pdocker”)
and the test environment seems to start correctly.
Which is your output of “docker images | grep kapua”?
Sounds like this one?
docker images | grep kapua
kapua/kapua-job-engine
2.0.0-ARTEMIS-SNAPSHOT 0f4656b1fbda 15 minutes ago 733MB
kapua/kapua-job-engine
2022-04-20 0f4656b1fbda 15 minutes ago 733MB
kapua/kapua-job-engine latest
0f4656b1fbda 15 minutes ago 733MB
kapua/kapua-service-authentication
2.0.0-ARTEMIS-SNAPSHOT 866b2caedd80 15 minutes ago 650MB
kapua/kapua-service-authentication
2022-04-20 866b2caedd80 15 minutes ago 650MB
kapua/kapua-service-authentication latest
866b2caedd80 15 minutes ago 650MB
kapua/kapua-consumer-lifecycle
2.0.0-ARTEMIS-SNAPSHOT 6f35d27645b6 15 minutes ago 656MB
kapua/kapua-consumer-lifecycle
2022-04-20 6f35d27645b6 15 minutes ago 656MB
kapua/kapua-consumer-lifecycle latest
6f35d27645b6 15 minutes ago 656MB
kapua/kapua-consumer-telemetry
2.0.0-ARTEMIS-SNAPSHOT e313f89c0dce 15 minutes ago 675MB
kapua/kapua-consumer-telemetry
2022-04-20 e313f89c0dce 15 minutes ago 675MB
kapua/kapua-consumer-telemetry latest
e313f89c0dce 15 minutes ago 675MB
kapua/kapua-api
2.0.0-ARTEMIS-SNAPSHOT c5cb9cb37941 16 minutes ago 799MB
kapua/kapua-api
2022-04-20 c5cb9cb37941 16 minutes ago 799MB
kapua/kapua-api latest
c5cb9cb37941 16 minutes ago 799MB
kapua/kapua-broker-artemis
2.0.0-ARTEMIS-SNAPSHOT b417ae53f26a 17 minutes ago 795MB
kapua/kapua-broker-artemis
2022-04-20 b417ae53f26a 17 minutes ago 795MB
kapua/kapua-broker-artemis latest
b417ae53f26a 17 minutes ago 795MB
kapua/kapua-events-broker
2.0.0-ARTEMIS-SNAPSHOT c1046e5af987 27 minutes ago 130MB
kapua/kapua-events-broker
2022-04-20 c1046e5af987 27 minutes ago 130MB
kapua/kapua-events-broker latest
c1046e5af987 27 minutes ago 130MB
kapua/kapua-sql
2.0.0-ARTEMIS-SNAPSHOT e1c6753a7cf7 28 minutes ago 587MB
kapua/kapua-sql
2022-04-20 e1c6753a7cf7 28 minutes ago 587MB
kapua/kapua-sql latest
e1c6753a7cf7 28 minutes ago 587MB
kapua/jetty-base
2.0.0-ARTEMIS-SNAPSHOT 3f2edd64f434 28 minutes ago 610MB
kapua/jetty-base
2022-04-20 3f2edd64f434 28 minutes ago 610MB
kapua/jetty-base latest
3f2edd64f434 28 minutes ago 610MB
kapua/java-base
2.0.0-ARTEMIS-SNAPSHOT 6b19bd516e28 28 minutes ago 585MB
kapua/java-base
2022-04-20 6b19bd516e28 28 minutes ago 585MB
kapua/java-base latest
6b19bd516e28 28 minutes ago 585MB
Da: Justin Bertram <[email protected]>
Data: martedì, 19 aprile 2022 18:07
A: [email protected] <[email protected]>
Oggetto: Re: Artemis security plugin doesn't allow to change clientId
Even after running `mvn clean install -DskipITs -DskipTests -Pdocker` I
still get the same error when I run RunDeviceBrokerI9nTest:
ERROR o.e.k.q.i.steps.DockerSteps - Error while starting base docker
environment: Image not found: kapua/kapua-sql:2.0.0-ARTEMIS-SNAPSHOT
As for using docker-compose, I don't know where to find your
"compose.diff." Perhaps you attached it to your email, but I don't believe
the mailing list supports attachments.
In any case, since you can't reproduce the NPE without Kapua then it must
be something in the Kapua code that's causing the unexpected null. I would
like to make the code more safe so that it fails more gracefully instead of
throwing an NPE, but it would still fail which means you would still need
to change the Kapua code that's causing the unexpected null. However,
without a way to reproduce it I can't make any changes.
Please start a new thread or open a Jira regarding the subscription issue.
Justin
On Tue, Apr 12, 2022 at 5:35 AM Modanese, Riccardo
<[email protected]> wrote:
> I wasn’t able to reproduce the NPE with Artemis 2.22 without Kapua code.
>
> Anyway, the stealing link NPE with Kapua plugins could be reproduced also
> with docker-compose (the patch remove the not needed containers)
>
> So (from root Kapua git repo path):
>
> git apply compose.diff
>
> cd deployment/docker/unix/
>
> ./docker-deploy.sh
>
>
>
> once the environment is running you can run the test class (TestMqttClient
> requires only log4j and Paho as external dependencies)
>
>
>
> About the subscription issue I was able to reproduce it also without Kapua
> plugins but since you prefer to focus to NPE I’ll tell you in a second time.
>
>
>
> Riccardo
>
>
>
> *Da: *Modanese, Riccardo <[email protected]>
> *Data: *martedì, 12 aprile 2022 09:12
> *A: *[email protected] <[email protected]>
> *Oggetto: *R: Artemis security plugin doesn't allow to change clientId
>
> Sure, the ITs are using docker images. You can build all the images with
> docker profile:
>
> mvn clean install -DskipITs -DskipTests -Pdocker
>
>
>
> if you change Artemis code base you don’t need to rebuild all.
>
> You can just trigger the assembly/broker-artemis module to rebuild the
> message-broker docker image.
>
> mvn clean install -DskipITs -DskipTests -Pdocker -f
> assembly/broker-artemis/pom.xml
>
>
>
> I’m planning to do some more investigation using Artemis 2.22 without
> Kapua code hoping that could help you. I’ll give you feedback as soon as I
> have one
>
>
>
> Riccardo
>
>
>
> *Da: *Justin Bertram <[email protected]>
> *Data: *lunedì, 11 aprile 2022 19:10
> *A: *[email protected] <[email protected]>
> *Oggetto: *Re: Artemis security plugin doesn't allow to change clientId
>
> I'm struggling to reproduce the NPE. I pulled down Kapua, switched to your
> branch (i.e. upgrade-artemis-2_21), configured Docker, etc., but I get this
> error when I run RunDeviceBrokerI9nTest:
>
> ERROR o.e.k.q.i.steps.DockerSteps - Error while starting base docker
> environment: Image not found: kapua/kapua-sql:2.0.0-ARTEMIS-SNAPSHOT
>
> I assume I missed a step somewhere. If you could outline the steps
> necessary to run the test starting from a completely bare environment I'd
> be grateful. Also, it might be useful for you to try reproducing the
> problem without the Kapua plugins to narrow the problem down a bit.
>
> Regarding the other problems, I'd prefer to focus on one thing at a time if
> possible.
>
>
> Justin
>
> On Mon, Apr 11, 2022 at 10:29 AM Modanese, Riccardo
> <[email protected]> wrote:
>
> > Hi Justin, I created a small test (using Paho client) and I confirm the
> > null pointer while a “regular” stealing link happens (with Kapua security
> > and server plugins configured)
> >
> > ERROR [org.apache.activemq.artemis.core.protocol.mqtt] AMQ834002: Error
> > processing control packet:
> > MqttConnectMessage[fixedHeader=MqttFixedHeader[messageType=CONNECT,
> > isDup=false, qosLevel=AT_MOST_ONCE, isRetain=false, remainingLength=58],
> > variableHeader=MqttConnectVariableHeader[name=MQIsdp, version=3,
> > hasUserName=true, hasPassword=true, isWillRetain=false, isWillFlag=false,
> > isCleanSession=true, keepAliveTimeSeconds=60],
> > payload=MqttConnectPayload[clientIdentifier=test-client-admin,
> > willTopic=null, willMessage=null, userName=kapua-sys, password=[107, 97,
> > 112, 117, 97, 45, 112, 97, 115, 115, 119, 111, 114, 100]]]:
> > java.lang.NullPointerException
> > at
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.validateClientId(MQTTUtil.java:524)
> > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> > at
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:228)
> > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> >
> >
> > I saw another couple of weird behaviors while testing the 2.22.0-SNAPSHOT
> > (openjdk 13).
> > 1) Kapua plugin receives the MQTT client id null during the connect.
> > 2) If I set the clientId value, anyway, no messages are received by the
> > client (on valid subscriptions)
> > The client has the subscriptions granted by the broker (here there is an
> > extract of Kapua log):
> > ### authorizing address: $EDC/kapua-sys/test-client-1/# - check type:
> > CREATE_ADDRESS - clientId: test-client-1 - clientIp: /172.29.0.1:58480 -
> > RESULT: true
> > ### authorizing address:
> >
> $EDC/kapua-sys/test-client-1/#::test-client-1.$EDC/kapua-sys/test-client-1/#
> > - check type: CREATE_DURABLE_QUEUE - clientId: test-client-1 - clientIp:
> /
> > 172.29.0.1:58480 - RESULT: true
> > ### authorizing address:
> >
> $EDC/kapua-sys/test-client-1/#::test-client-1.$EDC/kapua-sys/test-client-1/#
> > - check type: CONSUME - clientId: test-client-1 - clientIp: /
> > 172.29.0.1:58480 - RESULT: true
> > But NO messages are received by the client (with 2.19.0 – openjdk 8
> > worked)
> >
> > With the 2.22.0-SNAPSHOT + your PR the client id is correctly received by
> > the plugin and from the subscriptions I can say the client id
> manipulation
> > seems to be “acquired” by the broker:
> > ### authorizing address: $EDC/kapua-sys/test-client-1/# - check type:
> > CREATE_ADDRESS - clientId: test-client-1 - clientIp: /172.29.0.1:58528 -
> > RESULT: true
> > ### authorizing address:
> >
> $EDC/kapua-sys/test-client-1/#::AQ|test-client-1.$EDC/kapua-sys/test-client-1/#
> > - check type: CREATE_DURABLE_QUEUE - clientId: test-client-1 - clientIp:
> /
> > 172.29.0.1:58528 - RESULT: true
> > ### authorizing address:
> >
> $EDC/kapua-sys/test-client-1/#::AQ|test-client-1.$EDC/kapua-sys/test-client-1/#
> > - check type: CONSUME - clientId: test-client-1 - clientIp: /
> > 172.29.0.1:58528 - RESULT: true
> > (the client id “test-client-1” become “AQ|test-client-1” and it’s what I
> > expect) but, also in this case, NO messages are received by the client on
> > valid subscriptions.
> >
> > If could be helpful we had customized the wildcards to use the MQTT one
> (I
> > didn’t find any documentation about syntax changes on 2.22 so I’m
> assuming
> > is still valid right?)
> > <wildcard-addresses>
> > <routing-enabled>true</routing-enabled>
> > <delimiter>/</delimiter>
> > <any-words>#</any-words>
> > <single-word>+</single-word>
> > </wildcard-addresses>
> >
> > I’m looking forward for your feedback
> > Thanks!
> >
> > Riccardo
> >
> > Da: Modanese, Riccardo <[email protected]>
> > Data: lunedì, 11 aprile 2022 08:58
> > A: [email protected] <[email protected]>
> > Oggetto: R: Artemis security plugin doesn't allow to change clientId
> > May be our plugin can be the cause? Anyway I’m still investigating.
> >
> > Riccardo
> >
> > Da: Justin Bertram <[email protected]>
> > Data: venerdì, 8 aprile 2022 21:58
> > A: [email protected] <[email protected]>
> > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> > That's weird. There's a test in the ActiveMQ Artemis test-suite that
> > exercises this use-case and it's running fine. I'll see if I can set up
> > Kapua locally and reproduce the failure you're seeing.
> >
> >
> > Justin
> >
> > On Fri, Apr 8, 2022 at 10:51 AM Modanese, Riccardo
> > <[email protected]> wrote:
> >
> > > Hi Justin I got a NPE while broker is handling “normal” stealing link:
> > >
> > > 2022-04-08 15:23:37,540 ERROR
> > > [org.apache.activemq.artemis.core.protocol.mqtt] AMQ834002: Error
> > > processing control packet:
> > > MqttConnectMessage[fixedHeader=MqttFixedHeader[messageType=CONNECT,
> > > isDup=false, qosLevel=AT_MOST_ONCE, isRetain=false,
> remainingLength=42],
> > > variableHeader=MqttConnectVariableHeader[name=MQTT, version=4,
> > > hasUserName=true, hasPassword=true, isWillRetain=false,
> isWillFlag=false,
> > > isCleanSession=true, keepAliveTimeSeconds=60],
> > > payload=MqttConnectPayload[clientIdentifier=clientId, willTopic=null,
> > > willMessage=null, userName=acme-2, password=[75, 101, 101, 112, 67, 97,
> > > 108, 109, 49, 50, 51, 46]]]: java.lang.NullPointerException
> > > at
> > >
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTUtil.validateClientId(MQTTUtil.java:524)
> > > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> > > at
> > >
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.handleConnect(MQTTProtocolHandler.java:228)
> > > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> > > at
> > >
> >
> org.apache.activemq.artemis.core.protocol.mqtt.MQTTProtocolHandler.act(MQTTProtocolHandler.java:153)
> > > [artemis-mqtt-protocol-2.22.0-SNAPSHOT.jar:2.22.0-SNAPSHOT]
> > > at
> > > org.apache.activemq.artemis.utils.actors.Actor.doTask(Actor.java:33)
> > > [artemis-commons-2.22.0-SNAPSHOT.jar:]
> > > at
> > >
> >
> org.apache.activemq.artemis.utils.actors.ProcessorBase.executePendingTasks(ProcessorBase.java:65)
> > > [artemis-commons-2.22.0-SNAPSHOT.jar:]
> > > at
> > >
> >
> java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
> > > [java.base:]
> > > at
> > >
> >
> java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
> > > [java.base:]
> > > at
> > >
> >
> org.apache.activemq.artemis.utils.ActiveMQThreadFactory$1.run(ActiveMQThreadFactory.java:118)
> > > [artemis-commons-2.22.0-SNAPSHOT.jar:]
> > >
> > >
> > > if (connection != null) {
> > > MQTTSession existingSession =
> > > session.getProtocolManager().getSessionState(clientId).getSession();
> > > if (session.getVersion() == MQTTVersion.MQTT_5) {
> > >
> > >
> >
> existingSession.getProtocolHandler().sendDisconnect(MQTTReasonCodes.SESSION_TAKEN_OVER);
> > > }
> > > // [MQTT-3.1.4-2] If the client ID represents a client already
> > > connected to the server then the server MUST disconnect the existing
> > client
> > > existingSession.getConnectionManager().disconnect(false);//NPE
> > > }
> > > so the connection is not null but has no session defined from what I
> can
> > > understand.
> > >
> > > If could be helpful I created a new branch on my Kapua fork (
> > > https://github.com/riccardomodanese/kapua/tree/upgrade-artemis-2_21)
> > > linked to Artemis 2.22.0-SNAPSHOT (I built Artemis locally from main
> > branch
> > > + cherry-pick your commit)
> > > The test I run was: RunDeviceBrokerI9nTest (see
> DeviceBrokerI9n.feature)
> > >
> > > Looking forward for your feedback!
> > >
> > > Riccardo
> > >
> > >
> > >
> > >
> > >
> > >
> > > Da: Modanese, Riccardo <[email protected]>
> > > Data: venerdì, 8 aprile 2022 08:42
> > > A: [email protected] <[email protected]>
> > > Oggetto: R: Artemis security plugin doesn't allow to change clientId
> > > Sure, I’ll test asap thanks!
> > > (I’m currently doing my testing on 2.19, I don’t expect conflicts if I
> > > cherry-pick the commit)
> > >
> > > Da: Justin Bertram <[email protected]>
> > > Data: venerdì, 8 aprile 2022 03:05
> > > A: [email protected] <[email protected]>
> > > Oggetto: Re: Artemis security plugin doesn't allow to change clientId
> > > I just sent a PR [1] for this. Riccardo, could you try this out and see
> > if
> > > it works for you?
> > >
> > >
> > > Justin
> > >
> > > [1] https://github.com/apache/activemq-artemis/pull/4021
> > >
> > > On Thu, Apr 7, 2022 at 2:04 PM Justin Bertram <[email protected]>
> > wrote:
> > >
> > > > I went ahead and opened ARTEMIS-3770 [1] for this work.
> > > >
> > > >
> > > > Justin
> > > >
> > > > [1] https://issues.apache.org/jira/browse/ARTEMIS-3770
> > > >
> > > > On Thu, Apr 7, 2022 at 12:35 PM Justin Bertram <[email protected]>
> > > > wrote:
> > > >
> > > >> Technically speaking, an implementation of
> > > >> o.a.a.a.s.c.s.ActiveMQSecurityManager5 like you have *is* allowed to
> > > change
> > > >> the client ID on the o.a.a.a.s.c.p.RemotingConnection it receives
> > (just
> > > as
> > > >> you are doing in your implementation). The problem is that MQTT
> > > >> implementation doesn't use this client ID and instead overwrites it
> > with
> > > >> the value from the MQTT CONNECT packet. However, I think the code
> > could
> > > be
> > > >> refactored fairly easily to accommodate this use-case. Can you open
> a
> > > Jira?
> > > >>
> > > >>
> > > >> Justin
> > > >>
> > > >> On Thu, Apr 7, 2022 at 11:19 AM Modanese, Riccardo
> > > >> <[email protected]> wrote:
> > > >>
> > > >>> Hello,
> > > >>> we are moving a security plugin from ActiveMQ 5.x broker to
> > Artemis
> > > >>> 2.x.
> > > >>> To summarize the use case:
> > > >>> we need to prefix the MQTT client id provided during the
> connect
> > > >>> with the account name (something like account_name|client_id) to
> > allow
> > > >>> devices with the same clientId, but different accounts, to connect
> to
> > > the
> > > >>> broker without triggering the stealing link.
> > > >>>
> > > >>> Doing that with the ActiveMQ was possible. With Artemis
> > SecurityPlugin
> > > >>> any clientId set through the proper RemotingConnection setter has
> no
> > > effect
> > > >>> (
> > > >>>
> > >
> >
> https://github.com/riccardomodanese/kapua/blob/feature-artemisAuthentication/broker/artemis/plugin/src/main/java/org/eclipse/kapua/broker/artemis/plugin/security/SecurityPlugin.java#L140
> > > >>> ).
> > > >>> Also the fully qualified queue name still use the “original”
> clientId
> > > >>> without the account_name prefix
> > > >>>
> > > >>> We received a suggestion to use the interceptor but unfortunately
> the
> > > >>> MQTTConnect is final and has all the fields final so we cannot
> change
> > > the
> > > >>> clientId
> > > >>> We tried, just as experiment, using reflection to change the
> > > >>> accessibility (no security manager) and it seems to work. But,
> > > obviously,
> > > >>> is just an experiment and cannot be used in a real environment.
> > > >>>
> > > >>> The MQTTConnect message is created by MQTTDecoder (
> > > >>>
> > >
> >
> https://github.com/netty/netty/blob/4.1/codec-mqtt/src/main/java/io/netty/handler/codec/mqtt/MqttDecoder.java#L534
> > > )
> > > >>> but changing this part to introduce a callback that allows to
> change
> > > the
> > > >>> decoded clientId is out of the scope of this layer IMHO.
> > > >>>
> > > >>> If someone has suggestion or, better, a solution please tell me!
> > > >>>
> > > >>> Thanks!
> > > >>>
> > > >>> Riccardo Modanese
> > > >>>
> > > >>>
> > >
> >
>