ARTEMIS-3139 Anonymous types in Artemis config XSD

[ryan-highley]

Moves embedded XSD complexType definitions from XSD element definitions to top-level types available for XML schema validation in IDEs.

[michaelandrepearce]

Please see https://activemq.apache.org/components/artemis/documentation/latest/configuration-index.html we already support Modularising the broker xml with import

[michaelandrepearce]

I see in your JIRA you say you want to have diverts separated, this already works, in fact i know a few orgs where this is done successfully no changes to source / upstream are required what so ever. If you are running lint tools, for validation as per the aforementioned docs,

" Note: if you use xmllint to validate the XML against the schema you should enable xinclude flag when running.

--xinclude
"

As such if you're using an IDE you just need to add this to your lint'ing.

[michaelpearce-gain]

Im just testing this in some real env sandboxes. Something is breaking when doing a cluster upgrade with this change, trying to figure what its not liking to feed back.

[ryan-highley]

@michaelandrepearce Any update or feedback from your further testing?

[michaelandrepearce]

@ryan-highley so issue is occurring when applied in air gapped clusters, but haven't worked out why its not happy. Scratching our heads still atm, because reviewing the change it should be ok, you provided the xsd for offline which is all that should be needed, but its clearly not happy about something

[michaelandrepearce]

Only thought someone had in our team was if the xinclude.xsd that is added for offline need to reference xml.xsd

e.g. the xinclude.xsd needeed -> <xsd:import namespace="http://www.w3.org/XML/1998/namespace" schemaLocation="xml.xsd"/>

But that then seems not to be liked by xmllint tools

[ryan-highley]

@michaelandrepearce Finally able to circle back to this. (Stupid day job....)

Could you attach your config files so I can attempt to recreate the issue?

[michaelandrepearce]

@michaelandrepearce Finally able to circle back to this. (Stupid day job....)

Could you attach your config files so I can attempt to recreate the issue?

I cannot, it's an air gapped systems for a reason. I can only suggest you try make an air gapped setup and see if you can recreate the issues.

[jbertram]

@ryan-highley, the test failure was unrelated and I've pushed a fix for it. Please git commit --amend and push your branch again to trigger a rebuild.

[ryan-highley]

Thanks @jbertram

Noticed several curious build failures in a row. I've incorporated the updated test logic and another build is underway.

[jbertram]

@michaelandrepearce, are you still -1 on this change? It looks good to me.

[michaelandrepearce]

@jbertram has anything been done to fix the airgap issue? I don't see any changes so i don't see how it could have been... Has someone else tested fully in an airgap setup?

[jbertram]

@michaelandrepearce, I haven't done any direct testing, just code review. Can you elaborate on what you mean by "tested fully"? You mentioned doing an upgrade previously. Is that what you mean? Or is it sufficient to just create an instance and start it up?

[michaelandrepearce]

@jbertram so in a true fully air gapped environment linux tools like xmllint don't seem to like this. Which is used quite a bit in places to validate xml changes before applying to broker to ensure change doesn't foobar running system, and like wise when the team here tried it, it failed also on broker upgrade process they had.

Someone in our team did think possibly what it could be as we commented earlier:

https://github.com/apache/activemq-artemis/pull/4122#issuecomment-1184968423

but it needs someone who is driving this change to test that idea or find alternative solution, we spent as much time as we could trying to figure it out, but have to cut the time spent vs the benefit here, so i just left it, but in current state im a -1 on the change as from prev testing it will break any airgapped env we have.

[jbertram]

@michaelandrepearce, so in order to reproduce the issue I need to create an instance of the broker on an air-gapped system using the schema updates in this PR and then run xmllint on broker.xml?

[michaelandrepearce]

@jbertram so air gapped, yes. Two things the team here found

1. xmllint didn't like it
2. we ignored our system process and bypassed xmllint pre check, and forced it in still just to see, and then upgraded broker from pre-existing 2.16 broker didn't go well. Its been 6 months now so exact error i cannot remember.

but it needs some good air gap testing, and working out why these things.

[jbertram]

@michaelandrepearce, I can't even get xmllint to validate the normal broker.xml on a non-air-gapped system. Is there some trick to this? Here's what I did:

1. Created a fresh instance of Artemis 2.27.1 called test.
2. Ran this command from ARTEMIS_HOME

$ xmllint --valid --noout --schema schema/artemis-server.xsd path/to/test/etc/broker.xml
path/to/test/etc/broker.xml:24: validity error : Validation failed: no DTD found !
	       xsi:schemaLocation="urn:activemq /schema/artemis-configuration.xsd">
	                                                                          ^
path/to/test/etc/broker.xml validates

Is this a success or a failure?

[michaelandrepearce]

--valid is for DTD validation as such it expects DTD's to be given. were doing XSD which you're correctly passing schema, remove --valid

[ryan-highley]

I removed the XInclude.xsd file and references in the updated schema definitions some time ago. Hopefully that appeases xmllint.

Was never able to reproduce any issues with air-gapped hosts, but without a better idea of what steps caused the issues in the first place, I won't know how to address them.

[jbertram]

I created a release from this PR and copied it to an air-gapped system. I created an instance of the broker and then modified that instance's broker.xml to use an xinclude. Then I ran this from Artemis' home directory:

$ xmllint --noout --xinclude --schema schema/artemis-server.xsd /path/to/etc/broker.xml
/path/to/etc/broker.xml validates

As you can see, the validation succeeded. I will try performing an upgrade next.

[jbertram]

I performed an upgrade from 2.23.0 to the release I built from this PR and everything worked as expected. The broker.xml also validates fine with xmllint. @michaelandrepearce, what are your thoughts here? Any chance you could perform your own testing to confirm everything works as expected and if not provided specific details of what failed?

[michaelandrepearce]

@jbertram if latest is working for you, then i can have to assume it would for us, original version didn't. Unfortunately i spent all my internal credits on this PR early on testing it, and thus why asked if anyone else is testing this. If you're sure that its good then i remove my -1, to a 0, and let you merge if you're happy.