You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2022/08/15 22:36:53 UTC
[GitHub] [activemq-artemis] rtista opened a new pull request, #4179: ARTEMIS-3915 Add support for proxy protocol on broker acceptors
rtista opened a new pull request, #4179:
URL: https://github.com/apache/activemq-artemis/pull/4179
Hey guys,
As discussed in a thread on the developers mailing list, I've created [ARTEMIS-3915](https://issues.apache.org/jira/browse/ARTEMIS-3915) and have started developing support for HAProxy PROXY protocol on Artemis acceptors.
I think I've modified all the right places, and the changes are fairly simple to review, I'm a bit unsure about the channel pipeline modification, but I think it makes sense for the PROXY protocol handler, if configured, to be the first in the pipeline as it represents the frontier protocol, sicne any other handler will need the packets to first be decoded from PROXY protocol into something else that they understand.
My work is mostly based on the [examples from Netty](https://github.com/netty/netty/blob/4.1/example/src/main/java/io/netty/example/haproxy/HAProxyServer.java#L59).
However I have some doubts:
1. Is there any way to run the broker locally, so I can test the workflow with an HAProxy local instance (i.e. docker)?
2. Which kind of tests should I develop for this change? I've noticed the NettyAcceptorTest.java class (and the createServer() function) which I think allows me to start a broker instance with PROXY protocol enabled configured acceptors, but I'm unsure of how to send PROXY protocol encoded messages to each of the acceptors and validate the correct operation of the broker. Below is a sketch of what I've locally added to the test class:
```java
@Test
public void testHAProxyProxyProtocolConfiguration() throws Exception {
// Create ActiveMQ Server
ActiveMQServer server = createServer(false, createDefaultInVMConfig());
// Add all supported acceptors with proxy protocol option enabled
server.getConfiguration().addAcceptorConfiguration("mqtt", "tcp://127.0.0.1:1883?proxyProtocolEnabled=true");
server.getConfiguration().addAcceptorConfiguration("hornetq", "tcp://127.0.0.1:5445?proxyProtocolEnabled=true");
server.getConfiguration().addAcceptorConfiguration("amqp", "tcp://127.0.0.1:5672?proxyProtocolEnabled=true");
server.getConfiguration().addAcceptorConfiguration("stomp", "tcp://127.0.0.1:61613?proxyProtocolEnabled=true");
server.getConfiguration().addAcceptorConfiguration("openwire", "tcp://127.0.0.1:61616?proxyProtocolEnabled=true");
// Start the server
server.start();
// TODO: Send packets into each of the configured protocol acceptors
}
```
I still intend to add some documentation about the this theme, and maybe an example of a cluster deployed with an HAProxy instance which should be of great aid to new users which require a robust high availability solution, but I'd like to first close the implementation details.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4179: ARTEMIS-3915 Add support for proxy protocol on broker acceptors
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4179:
URL: https://github.com/apache/activemq-artemis/pull/4179#issuecomment-1219867590
Couple of things:
- Ideally the broker would automatically detect the use of the proxy protocol by a client. We already do lots of automatic protocol detection so that a single `acceptor` can support multiple messaging protocols plus WebSockets and HTTP. See `org.apache.activemq.artemis.core.protocol.ProtocolHandler` for implementation details. Having a static configuration means that the `acceptor` could only be used by clients communicating via the proxy protocol (or not). This would increase configuration complexity and potentially lead to confusion for clients.
- I don't see any code that actually makes use of the proxy protocol's additional information. Is Netty supposed to handle this transparently so that `io.netty.channel.Channel#remoteAddress` returns the client's actual address rather than the address of the proxy?
> Is there any way to run the broker locally, so I can test the workflow with an HAProxy local instance (i.e. docker)?
I'm not quite sure what you're asking here. The broker can be run locally standalone using the normal distribution available from the website. The broker can be embedded and run just as you're doing in the test code you pasted. The broker can also be run via Docker as described [here](https://github.com/apache/activemq-artemis/tree/main/artemis-docker).
> Which kind of tests should I develop for this change?
A basic sanity check for each messaging protocol would be a good place to start. Something that just sends and consumes a message.
Of course, you'd start first with manual testing. I'd recommend using your own instance of HAProxy with the `artemis consumer` and `artemis producer` commands.
Ultimately, though, we need tests in the test-suite to validate the functionality for each release and to mitigate against regressions. However, this may be difficult as I don't see any kind of Java implementation for HAProxy which you could embed and use for testing. You may need to implement something of your own here to act like HAProxy using Netty and `io.netty.handler.codec.haproxy.HAProxyMessageEncoder`.
Hope that helps!
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4179: ARTEMIS-3915 Add support for proxy protocol on broker acceptors
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4179:
URL: https://github.com/apache/activemq-artemis/pull/4179#issuecomment-1353484542
@rtista, I think this would be a worthwhile feature to add, but I'm going to close the PR for now since it appears it's been abandoned. Feel free to re-open the PR if you return to it. Thanks!
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram closed pull request #4179: ARTEMIS-3915 Add support for proxy protocol on broker acceptors
Posted by GitBox <gi...@apache.org>.
jbertram closed pull request #4179: ARTEMIS-3915 Add support for proxy protocol on broker acceptors
URL: https://github.com/apache/activemq-artemis/pull/4179
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4179: ARTEMIS-3915 Add support for proxy protocol on broker acceptors
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4179:
URL: https://github.com/apache/activemq-artemis/pull/4179#issuecomment-1310835378
@rtista, any updates on this?
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ARTEMIS-3915 Add support for proxy protocol on broker acceptors [activemq-artemis]
Posted by "jbertram (via GitHub)" <gi...@apache.org>.
jbertram commented on PR #4179:
URL: https://github.com/apache/activemq-artemis/pull/4179#issuecomment-3255235514
Replaced by #5908.
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact