I don't think it's needed for 5.15.x as I don't believe there are plans to do anymore of those releases. However for 5.16.x it makes a lot of sense since we are only going with Log4j2 on 5.17.x and newer.
So definitely +1 for me to replace it in 5.16.x. On Fri, Feb 4, 2022 at 6:20 AM Jonathan Gallimore < jonathan.gallim...@gmail.com> wrote: > I had wondered the same thing. I'd be +1 on it. > > Jon > > On Fri, Feb 4, 2022 at 11:17 AM Colm O hEigeartaigh <cohei...@apache.org> > wrote: > > > Hey JB/all, > > > > I'm wondering if we can also take the opportunity to consider > > switching from Apache Log4J 1.x to Reload4J for AMQ 5.15.x/5.16.x. > > https://reload4j.qos.ch/ is a fork of Log4J 1.x with the CVE fixes > > applied. Other Apache projects such as Apache Avro have switched over > > to use it, similarly pax logging > > ( > > > https://github.com/ops4j/org.ops4j.pax.logging/commit/a21b1b41fb107164522e587643e8daff2724ce86 > > ). > > > > There will be many users of AMQ who will not want or be able to > > migrate to AMQ 5.17 and hence Log4J 2. It's difficult to convey to > > users that the CVEs don't apply to the default configuration, as many > > users just see critical CVEs via their own scanning tools and insist > > on upgrades. In addition, Log4J 1.x is not maintained since 2015 and > > if a new CVE were to come out that affects the default configuration, > > it would be problematic. > > > > If you approve I can create a PR. > > > > Colm. > > > > On Fri, Feb 4, 2022 at 7:51 AM Jean-Baptiste Onofré <j...@nanthrax.net> > > wrote: > > > > > > Hi Clebert, > > > > > > Good idea ! > > > > > > If Chris agree with this plan, I'm fine with it. > > > > > > Let me work on the tests then (I will create PRs). > > > > > > Regards > > > JB > > > > > > On 04/02/2022 00:55, Clebert Suconic wrote: > > > > I would say.. write the tests... if they pass no need to cancel the > > > > release as they will not affect runtime. only cancel the release if > > > > you actually find an issue. > > > > > > > > On Thu, Feb 3, 2022 at 8:15 AM Jean-Baptiste Onofré <j...@nanthrax.net > > > > wrote: > > > >> > > > >> Hi Chris, > > > >> > > > >> OK, fair enough for me. I will cancel the vote and work with Matt to > > add > > > >> the tests. > > > >> > > > >> Thanks, > > > >> Regards > > > >> JB > > > >> > > > >> On 03/02/2022 13:57, Christopher Shannon wrote: > > > >>> JB, > > > >>> > > > >>> I would rather just add tests first and re-run the vote as it > > sounded like > > > >>> it wouldn't take too long to add a few tests. I wanted to verify > > some of > > > >>> those fixes work and make sense and it would be a lot easier not > > having to > > > >>> try and figure out how to set up a manual test that will most > likely > > be > > > >>> inconsistent from what others used depending on the config settings > > and > > > >>> testing scenarios. Furthermore, it's very common to think an issue > > is fixed > > > >>> from manual testing and then run through unit tests and then find > an > > issue > > > >>> (missing edge case, etc) > > > >>> > > > >>> Matt, for AMQ-8397 you are right that it would be tricky to test > the > > > >>> behavior itself but I don't think that's necessary. Since the fix > is > > really > > > >>> just about adding a new flag to disable sending to the DLQ I would > > say > > > >>> instead of testing the full behavior you could at least just write > a > > quick > > > >>> test that would verify the default is what you want (off by > default) > > and > > > >>> that changing the policy actually properly sets it on the > > destination as > > > >>> intended. > > > >>> > > > >>> On Thu, Feb 3, 2022 at 3:44 AM Jean-Baptiste Onofré < > j...@nanthrax.net> > > wrote: > > > >>> > > > >>>> Hi, > > > >>>> > > > >>>> As some features have been tested manually (you did a pass, I did > a > > > >>>> quick pass on some as well), it's good enough for the release. > > > >>>> Agree with Chris that we should add test to "secure" the code > > > >>>> coverage/regression, but it should be done. > > > >>>> Either way is fine for me: if we consider it's blocker, I can > > cancel the > > > >>>> current vote, include new tests and cut a new release. > > > >>>> Else, I would say, let's move forward with the current vote and > > include > > > >>>> tests asap (decoupled from this vote/release). > > > >>>> > > > >>>> Regards > > > >>>> JB > > > >>>> > > > >>>> On 02/02/2022 22:21, Matt Pavlovich wrote: > > > >>>>> Note— I did test these changes manually. > > > >>>>> > > > >>>>>> On Feb 2, 2022, at 2:55 PM, Matt Pavlovich <mattr...@gmail.com> > > wrote: > > > >>>>>> > > > >>>>>> Hey Christopher- > > > >>>>>> > > > >>>>>> I see your point on the wording of AMQ-8443. It is an edge case > > with > > > >>>> ConnectionControl, not failover in general. I’ll clear up the > > description. > > > >>>> I’m happy to add tests for a couple of them; however, AMQ-8397 is > > tricky. > > > >>>>>> > > > >>>>>> What do you suggest for a reliable test scenario for AMQ-8397? > > > >>>>>> > > > >>>>>> AMQ-8053 - This is a very minor change, I’ll add a test > > > >>>>>> AMQ-8443 - Edge case, I’ll add a test > > > >>>>>> AMQ-8397 - Note— this is ‘disabled by default in 5.16.x’. This > is > > a > > > >>>> reporting change on an edge case from being “move message to DLQ” > > to log > > > >>>> message and increment a counter. This is really difficult to > > consistently > > > >>>> create a unit test to produce the scenario on varying hardware. > > This is a > > > >>>> heavy contention scenario and I was not able to find any tests > that > > produce > > > >>>> the current behavior. I’d be happy to adapt verification if you > can > > provide > > > >>>> a stable base scenario (or point to an existing unit test that I > > may have > > > >>>> missed). > > > >>>>>> > > > >>>>>> Thanks, > > > >>>>>> Matt Pavlovich > > > >>>>>> > > > >>>>>>> On Feb 2, 2022, at 12:56 PM, Christopher Shannon < > > > >>>> christopher.l.shan...@gmail.com <mailto: > > christopher.l.shan...@gmail.com>> > > > >>>> wrote: > > > >>>>>>> > > > >>>>>>> Unfortunately I am -1. I just checked the release and several > > Jiras > > > >>>> don't > > > >>>>>>> have any unit tests for verification. > > > >>>>>>> > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8053 < > > > >>>> https://issues.apache.org/jira/browse/AMQ-8053> > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8372 > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8397 > > > >>>>>>> https://issues.apache.org/jira/browse/AMQ-8443 > > > >>>>>>> > > > >>>>>>> There might be more, I quit checking after the first several. > > All of > > > >>>> these > > > >>>>>>> fixes and improvements need tests. It's important to add some > > tests to > > > >>>>>>> verify the actual bug or issue and to make sure it doesn't get > > broken > > > >>>> in > > > >>>>>>> the future. I am not comfortable releasing this without having > > tests > > > >>>> for > > > >>>>>>> these issues. The same of course goes into anything with > 5.17.0. > > > >>>> Obviously > > > >>>>>>> we don't need tests for trivial stuff (dependency upgrades, > null > > > >>>> pointer > > > >>>>>>> checks, etc) but other stuff should be tested. > > > >>>>>>> > > > >>>>>>> The biggest issue I have is AMQ-8443 which claims this is a fix > > for the > > > >>>>>>> "failover transport reconnect not working" but no demonstration > > of the > > > >>>>>>> issue or how the fix works in a unit test. > > > >>>>>>> > > > >>>>>>> On Wed, Feb 2, 2022 at 3:29 AM Francois Papon < > > > >>>> francois.pa...@openobject.fr> > > > >>>>>>> wrote: > > > >>>>>>> > > > >>>>>>>> +1 (non-binding) > > > >>>>>>>> > > > >>>>>>>> Thanks! > > > >>>>>>>> > > > >>>>>>>> regards, > > > >>>>>>>> > > > >>>>>>>> François > > > >>>>>>>> > > > >>>>>>>> On 01/02/2022 21:41, Jean-Baptiste Onofre wrote: > > > >>>>>>>>> Hi everyone, > > > >>>>>>>>> > > > >>>>>>>>> I submit Apache ActiveMQ 5.16.4 release to your vote. > > > >>>>>>>>> > > > >>>>>>>>> This release includes important fixes and updates on the > 5.16.x > > > >>>> series, > > > >>>>>>>> especially: > > > >>>>>>>>> - fix couple of warnings/issues on JDK16+ > > > >>>>>>>>> - fix stack trace display on transports > > > >>>>>>>>> - better secure on WebConsole > > > >>>>>>>>> - several dependencies updates > > > >>>>>>>>> - and much more! > > > >>>>>>>>> > > > >>>>>>>>> Please take a look on the Release Notes for details: > > > >>>>>>>>> > > > >>>>>>>> > > > >>>> > > > https://issues.apache.org/jira/secure/ReleaseNote.jspa?projectId=12311210&version=12350483 > > > >>>>>>>>> > > > >>>>>>>>> Maven staging repository is: > > > >>>>>>>>> > > > >>>>>>>> > > > >>>> > > > https://repository.apache.org/content/repositories/orgapacheactivemq-1244/ > > > >>>>>>>>> > > > >>>>>>>>> Dist staging repository is: > > > >>>>>>>>> > > https://dist.apache.org/repos/dist/dev/activemq/activemq/5.16.4/ > > > >>>>>>>>> > > > >>>>>>>>> Git tag: > > > >>>>>>>>> activemq-5.16.4 > > > >>>>>>>>> > > > >>>>>>>>> Please vote to approve this release: > > > >>>>>>>>> > > > >>>>>>>>> [ ] +1 Approve the release > > > >>>>>>>>> [ ] -1 Don't approve the release (please provide specific > > comments) > > > >>>>>>>>> > > > >>>>>>>>> This vote will be open for at least 72 hours. > > > >>>>>>>>> > > > >>>>>>>>> Thanks ! > > > >>>>>>>>> Regards > > > >>>>>>>>> JB > > > >>>>>>>> > > > >>>>>> > > > >>>>> > > > >>>>> > > > >>>> > > > >>> > > > > > > > > > > > > > > >
