Closed arthurdm closed 6 years ago
It's always been confusing where the location of config files is in a WAR file - JPA persistence.xml is supposed to be in classes/META-INF but CDI's beans.xml should be in WEB-INF/beans.xml.
Though, I haven't seen a solution where a file is expected in the root META-INF
folder in case of a WAR file as the MP Open API spec now mandates. In maven projects, META-INF
is usually placed in src/main/resources
which maps to WEB-INF/classes
in the WAR. In order to map to root META-INF
folder, the folder would have to be in src/main/webapp/META-INF
next to WEB-INF
, potentially leading to 2 META-INF
folders in case the same WAR uses e.g. persistence.xml
from JPA.
I don't mind too much but MP Specs should be consistent and support same options. All the following options meet that:
META-INF
should be located in WEB-INF/classes/META-INF
and obsoletes supporting META-INF
in the root of a WAR fileMETA-INF
in the root of a WAR file to match the current requirements of Open APIWEB-INF
instead of META-INF
in a WAR file (similar to the CDI spec which expects beans.xml in a WEB-INF
for WARs) - I've raised https://github.com/eclipse/microprofile-config/issues/362 to add support for this in MP ConfigWEB-INF
, root META-INF
, WEB-INF/classes/META-INF
) so that users can place their config in any of them without too much thinkingFrom the above, my preferred solution is 3. or if MP Config wouldn't agree then option 1.
The option 2. is supported by OpenAPI now but I don't think it's the best solution and I discourage from adopting it for MP Config and rather make it obsolete for OpenAPI.
thanks a lot for outlining these different alternatives @OndrejM - it helps to see them all in one place.
Part of the motivation for the root/META-INF
location was to make it simple to augment an existing WAR (with either a OAS3 doc or a properties file that configured the API scanner) so that it produced the resulting /openapi
endpoint and enabled this app to participate in the "API Economy". In this case we're talking about legacy/older applications where the developers don't want to (or can't) touch anything inside /classes
.
Perhaps going forward in the new 1.1
spec the solution 1
you outlined is less confusing though, so I will ask other members of the community to weigh in as well.
@leochr @EricWittmann @mrglavas
@arthurdm
In this case we're talking about legacy/older applications where the developers don't want to (or can't) touch anything inside /classes.
I wonder, is that a real scenario? If you can touch [web root]/META-INF
, why wouldn't you be able to touch [web root]/WEB-INF/classes
or even simpler [web root]/WEB-INF/
?
From #222
MP Open API currently specifies [war root]/META-INF for certain files to be placed as well as a MP Config properties file.
[war root]/META-INF is however a highly unusual location, and doesn't align with any existing conventions in either MicroProfile, Java EE, or related technologies.
In fact, the usage of this folder is so alien for war archives that I think we should consider it to be a spec bug and therefor should best deprecate it.
I'd like to propose to use the [war root]/WEB-INF folder for the MP Open API specific files, and do not put any requirements on the configuration file used by MP Config (just specify the properties, but leave it to MP Config where they ultimately originate from)
I though this issue (#221) was different, since this one just asks for clarification, while #222 specifically asks for deprecation, but if everything can be handled here that's fine too.
thanks @arjantijms - this issue will decide which way we go forward in 1.1
and explicitly clarify that decision in the spec.
@arjantijms / @OndrejM - one such example is Tomcat's usage of context.xml, which sets per-web-app configuration at the <root>/META-INF/context.xml
.
@arjantijms / @OndrejM - one such example is Tomcat's usage of context.xml, which sets per-web-app configuration at the
/META-INF/context.xml.
That one is however not a configuration file that belongs to a spec implemented by Tomcat and/or a spec module bundled in WEB-INF/lib.
context.xml is more akin to domain.xml in Payara and standalone.xml in JBoss. You typically put that file outside the archive (in a server folder). Tomcat in fact treats it like that, as in it copies context.xml from the war using low level code to the usual location outside in the server, but only if there isn't already a context.xml there.
So although I do appreciate you found one example, this is a highly exceptional one really.
So although I do appreciate you found one example, this is a highly exceptional one really.
It demonstrates there's precedence for using <root>/META-INF
beyond EE specs. WebSphere Liberty has also used <root>/META-INF
for its Swagger documents and <root>/META-INF/stub
for its partial Swagger documents.
Also, the fact that Java SE uses <root>/META-INF/services
for jars can also be applied to app server's FAT jars
, which is also relevant to the domain in question here - a lot of the MP OpenAPI properties have to do with something akin to a service reference (i.e. Filter, ModelReader, etc).
So - although <root>/META-INF
is not defined in a WAR spec, there are a few examples above where it does make sense to consider as a valid location.
I'll check the liberty example to see if they actually mean [war root]/META-INF
and not "a" META-INF
that's found on the classpath of the web module.
I don't think Tomcat is a valid example really. It doesn't process or read context.xml
from META-INF, but practically the war is extracted, the file is copied to its usual location (if there's no file there already), and only from there is the file read and processed. Liberty might be the only actual example, if it indeed does that.
Also, the fact that Java SE uses
/META-INF/services for jars can also be applied to app server's FAT jars, which is also relevant to the domain in question here - a lot of the MP OpenAPI properties have to do with something akin to a service reference (i.e. Filter, ModelReader, etc).
I don't quite agree with that. The WEB-INF/lib/[some jar]
basically each and every spec in Java EE and MP too makes use of that.
there are a few examples above where it does make sense to consider as a valid location.
The fact that it's possible and even that there might be some (weak?) precedence, does beg the question whether we -should-?
Obviously it has proven to be possible for JAX-RS to use @Context
instead of @Inject
, but is it wise and is the best for our customers and user? Do we choose things so that they are consistent with the rest of MP and Java EE so that it's easy and logical for user to use, or do we just choose something arbitrarily because we technically can, and just require the user to remember all the different ways?
One thing I'd like to make sure is that OpenAPI doesn't define only locations that are specific to WAR
files, as JAR
is still a perfectly valid deployment package type
+1 @kenfinnigan
I think a lot of the arguments in this thread are geared solely for WAR
, which is one of
the deployment patterns, but not the only one. Having the OAS3 doc in the <root>/META-INF
allows it to apply to any deployment pattern derived from jar
(WAR, OSGi app, Jar, etc), which is one of the reasons why Liberty chose that location even for the Swagger document support back in 2016.
Since the OAS3 doc is in that location, the MP OpenAPI spec allows the flexibility to put the OpenAPI config in that directory too - in addition to (not replacing) the other valid MP Config locations.
Hi,
On Mon, May 21, 2018 at 1:05 PM, Ken Finnigan notifications@github.com wrote:
One thing I'd like to make sure is that OpenAPI doesn't define only locations that are specific to WAR files, as JAR is still a perfectly valid deployment package type
You mean an EJB jar? Are these supported by MicroProfile?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eclipse/microprofile-open-api/issues/221#issuecomment-390635232, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTjN3V0TgWj9t5OloIO1xcEvafJ8Cks5t0q1ygaJpZM4UFgtF .
Hi,
On Mon, May 21, 2018 at 2:26 PM, Arthur De Magalhaes < notifications@github.com> wrote:
+1 @kenfinnigan https://github.com/kenfinnigan
I think a lot of the arguments in this thread are geared solely for WAR, which is one of the deployment patterns, but not the only one. Having the OAS3 doc in the
/META-INF allows it to apply to any deployment pattern derived from jar (WAR, OSGi app, Jar, etc), which is one of the reasons why Liberty chose that location even for the Swagger document support back in 2016.
Perhaps you're mistaken with "META-INF on the classpath"?
"META-INF on the classpath" works everywhere. It's
At any length, I thought MicroProfile only supported JAX-RS and with that WARs, but I might have missed something.
Kind regards, Arjan
Since the OAS3 doc is in that location, the MP OpenAPI spec allows the flexibility to put the OpenAPI config in that directory too - in addition to (not replacing) the other valid MP Config locations.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eclipse/microprofile-open-api/issues/221#issuecomment-390653109, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTgVa4Wdhx_5-FP2_XCbWpBGJZcN6ks5t0sB2gaJpZM4UFgtF .
No, I mean any old JAR.
MicroProfile does not dictate the packaging format as WAR or JAR, it could be either, and certainly some implementations support one or the other or both.
My concern was that it seemed to be pointing towards a WAR specific location being mentioned in the spec, which is I said that it needs to work for WAR or JAR structures
Consensus was reached today at the hangout that the current behaviour is correct. I will assign this to myself to clarify in the spec.
MicroProfile does not dictate the packaging format as WAR or JAR, it could be either, and certainly some implementations support one or the other or both.
I wonder if that should not be an overal concern then, since the other MP specs as far as I know don't really say anything about that, do they?
My concern was that it seemed to be pointing towards a WAR specific location being mentioned in the spec, which is I said that it needs to work for WAR or JAR structures
Which is really simple to solve, by using the META-INF folder that's in the class path. That will basically resolve automatically to whatever physical location is valid for that in whatever archive.
Consensus was reached today at the hangout that the current behaviour is correct.
I wonder, who was present in that hangout and based on which arguments was that consensus reached? If it were the same people who originally okay'ed that to be in the spec in the first place, it's perhaps not a surprise they reached consensus about being correct themselves.
I feel it might be better to bring this to the broader attention of an architecture board in MP.
@kenfinnigan @arthurdm, you're missing a few very important points:
Therefore specifying the location as "META-INF" on classpath is more correct and works for both JAR and WAR files. For JAR files, it would be also valid to specify that "META-INF" is in root, but this wouldn't work for WAR files.
I'm not suggesting we need to treat WAR files in a special way but we should specify a location that doesn't break WAR files. Specifying location as "META-INF" on classpath doesn't break WAR files while specifying the location as "META-INF" in the root of the package does because it exposes files to the outside world.
It would be convenient if WAR files used "WEB-INF" location as an alternative, but it's not necessary if you don't want to mention WAR files in the spec.
As a result, an alternative location in <classpath>/META-INF
should be supported, which works in both JAR and WAR files. Later, the location in <package-root>/META-INF
should be deprecated. It wouldn't affect standard JAR files because there both locations are the same.
This section of the spec needs to be modified:
...inside the application module’s (i.e. WAR artifact) META-INF folder
Should be
inside META-INF folder on the classpath, alternatively inside the application module’s (i.e. WAR artifact), but this is deprecated and discouraged.
And the PR #220 should be accepted, or better, a new test case should be created for the new location, while the old one deprecated.
thanks for the comments guys. The conclusion from the hangout today is that:
<root>/META-INF
WEB-INF
for WARs). <root>/META-INF
Thanks for the reference to the document. This clarifies that Ivan Junckes Filho from Tomitribe, Eric Wittmann from Red Hat, and yourself reach that consensus (I'm not sure who Marc S is though).
But since it was a video hangout, it doesn't reveal which arguments where used to reach that consensus. I'm still very curious why anyone would think <root>/META-INF
would be a good place to store files, unless those files as @OndrejM mentioned are intended to be publicly available to the outside world (via an http client).
So would great if you could tell us a little about which actual arguments were used.
Marc S is from Red Hat, and Andy G from Tomitribe was also present (updated attendance now).
The arguments used were the same described along this thread - we took into consideration the arguments in favour and against the current solution, so we did have a fair conversation about it.
I will go ahead with a PR for the agreed direction, but if you still feel strongly against it, please join the next hangout on June 18, 10am EST and we can chat.
I'll join the hangout because I'm not happy with the resolution. It's not consistent with other parts of MicroProfile and the Java EE ecosystem.
that's fair @OndrejM - will be good to have a chat to close on this.
One point though, is that resources under <root>/META-INF
are not externally accessible as claimed, i.e. can you fetch MANIFEST-MF
?
@arthurdm that's fair point, I thought that only WEB-INF folder isn't accessible but it also applies to META-INF, so it's safe to place files in that location.
Still, it's inconsistent to support this location only in the OpenAPI spec, as I commented in the PR here (which is already merged)
I understand you added it for convenience because the openapi
file is expected in that location. Is there a reason why the openapi
or other files should be in the root META-INF
and not on META-INF
on classpath?
I think that the openapi.yaml
file is an alternative to annotations so it should be OK to read it from classpath as annotations are also read from classes, which are on the classpath. Reading all resources from META-INF on classpath would be consistent with the Config spec and wouldn't require any extensions like we have currently.
I think defining in multiple locations are bad and cause merging issue.
This is why in MP Config, we just say it is under classpath of META-INF. In jar, it is root META-INF. In war, it should be under WEB-INF\classes\META-INF.
I think having a properties file in <root>/META-INF
is no different than having more than 1 properties file in the classpath, as MP Config states all META-INF/microprofile-config.properties files on the ClassPath
- which indicates there can be more than 1.
Root/meta-inf in a war isn’t on the class path...
On Monday, June 18, 2018, Arthur De Magalhaes notifications@github.com wrote:
I think having a properties file in
/META-INF is no different than having more than 1 property file in the classpath, as MP Config states all META-INF/microprofile-config.properties files on the ClassPath - which indicates there can be more than 1. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/eclipse/microprofile-open-api/issues/221#issuecomment-398143909, or mute the thread https://github.com/notifications/unsubscribe-auth/AC5XTscsx-PIG5Tzn0M4TkSvr5j7f4yEks5t9-v5gaJpZM4UFgtF .
Root/meta-inf in a war isn’t on the class path...
Right, which wasn't implied. The idea is that you can have more than 1 MP Config properties file in the classpath - so for a WAR, that would mean more than 1 properties file inside WEB-INF\classes\
As agreed in the chat, closing this for now.
Please reopen, meta-inf for war case is the least portable location and where vendors put descriptor, not any spec. Spec must use classpath meta-inf or web-inf. For MP it means the spec wording should be about descriptor natural location and not reinvent something used in not a single spec in the whole platform.
I've raise https://github.com/eclipse/microprofile/issues/46 for the Arch. board to address this globally.
To summarize the discussion here, the only argument for keeping the current state is mentioned in here
Part of the motivation for the root/META-INF location was to make it simple to augment an existing WAR (with either a OAS3 doc or a properties file that configured the API scanner) so that it produced the resulting /openapi endpoint and enabled this app to participate in the "API Economy". In this case we're talking about legacy/older applications where the developers don't want to (or can't) touch anything inside /classes.
I didn't find any other argument to support the decision, despite it was declared multiple times that the discussion in hangouts concluded with keeping the current state.
I can easily counter that argument. For JAR files, the situation doesn't change with having artifacts in META-INF on classpath as it's the same location as root/META-INF
. For WAR files, it's as easy to augment an existing package by adding a file into root/WEB-INF/classes/META-INF
as by adding it into root/META-INF
. So there's really no point against expecting files on the classpath which would work for both JAR and WAR files. The only issue is that an augmentation tool would have to treat WAR files differently to JAR files, because the files should be in different locations.
Another (invalid) argument was that there's already a precendence in using root/META-INF. But all examples of this are vendor-specific (tomcat, liberty), while there's a strong precedence in existing specifications to use either META-INF on classpath (works in both JAR and WAR) or WEB-INF (in case of WAR). Adding another META-INF location for WARs causes confusion, as you can see in this StackOverflow question: https://stackoverflow.com/questions/17997731/maven-war-has-meta-inf-folder-in-two-places. The answer pinpoints the servlet spec, which on one hand allows root/META-INF and says it isn't visible to the outside world, on the other hand it states that this is reserved when using the standard Java archive tools and implies that it shouldn't be used in any other way. So the precedence for root/META-INF is much smaller than for other locations and this argument thus isn't valid at all.
On the other hand, there are many arguments for changing the expected location to META-INF on the classpath instead of in the root of a package:
It's 5 valid arguments for changing the current behavior against 1 valid argument to keep it.
@OndrejM @arthurdm
In addition to agreeing with Ondrej's points raised above, I would like to call special attention to this:
I didn't find any other argument to support the decision, despite it was declared multiple times that the discussion in hangouts concluded with keeping the current state.
So far the discussion has given a distinct feel of "we've chosen it before, maybe not thought too much about it, but now that it's chosen it has been chosen".
MicroProfile however promised to be fast paced, and part of this fast pace mean mistakes slip in. To counter this, it was also promised to be agile with correcting mistakes, as opposed to the Java EE process where things that slipped into a spec release can not really be changed anymore without moving the world and many years of waiting (typically 6 years at least).
I personally think there's very little discussion needed about [war root]/meta-inf
being a mistake. We all know it's a mistake, and I think you do as well ;) We all make mistakes (I certainly made tons of them), so there's no shame at all in that.
I assume the issue is solely with the idea of having to correct something in a spec, not about it actually being a mistake. As such I think this should not be an issue at all, as the MP process allows for things to be corrected without too mush hassle.
For .war in CDI, beans.xml can be placed under WEB-INF\beans.xml or WEB-INF\classes\META-INF\beans.xml. If multiple files are found in both locations, an unportable behavior occurs. Both @struberg and I did not like it. Therefore, in MP Config, we simply said the microprofile-config.properties on the classpath will be loaded, which means WEB-INF\classes\META-NF\microprofile-config.properties for .war files or META-INF\microprofile-config.properties for .jar files.
It is unusual to load files from META-INF\ folder under .war. I think in MP we should adopt what MP config has done - classpath only, which is clear and neat. If any vendor wants to load files from different locations, they are free to do so. @arthurdm are you considering the migration issues for the ones who used Open API v3 migrating to MP Open API? I think in Java, it is uncommon to expect people to place places under META-INF under .war though.
And then .war/META-INF/*.properties seems to work across many/all(?) servers.
@pilhuhn nop, .war/META-INF/*.properties
is not portable at all and something impossible to use for vendors. .war/WEB-INF/classes/META-INF/*.properties
or jar META-INF works everywhere.
@rmannibucau I guess I was trying to make the distinction between 'de facto' and 'de jure'.
If .war/WEB-INF/classes/META-INF/*.properties
is the only sensible place, then to me the decision is clear. And I guess the specs that introduce such property files should also call that out explicitly.
I think this thread has focused too much on the EE classpath, specially for a WAR
file. It's important to note that the parent specification for MP OpenAPI is the OpenAPI v3 spec (OAS3), and static (YAML | JSON) OAS3 files come from an API-first design pattern, which is language agnostic (i.e. has nothing to do with Java, annotations, or classpaths). This is where customers design their API separately from their application and without any influence or dependency on what language will implement it - you can have polyglot microservices implementing the same OAS3 doc in HA fashion if you wanted.
So when figuring out the best place the store this static OAS3 doc we took in consideration the common denominator in all Java deployments, including EE, SE, Spring, etc, and JAR
is that common artifact - reminding ourselves that WAR
, EAR
, WAB
FAT Jar
are all derived from JAR
. This document doesn't need to be in the classpath, it just needs to be in a consistent location that vendors know where to look - without worrying about conflicts or changing its location if they change their programming model (i.e. if they move from Spring to EE, or vice versa).
As stated before, Liberty has been doing this for over 2 years now, and we have noticed that adopters of API-design first approach in the field usually have build-time tools which extract the OAS3 from Git, validate against the application (whichever that may be, JAR
, WAR
, whichever) to ensure the developer didn't break the API contact, and then insert that OAS3 static file into META-INF
for the exposure of that API - all in a black box
style, without knowledge or special code of classpaths or the type of application we just handled.
@arthurdm well, actually no. MP is about embrassing EE and being reversed to JakartaEE so if you do something against EE like using vendor specific descriptors or locations like .war/META-INF then you block that process. Functionally there are well defined and known location to meet the same requirement as mentionned previously so no need to reinvent the wheel and loose users.
EE ... .war/META-INF
Thanks for confirming my point. =)
@arthurdm the most important part being the root spec of mp-openapi is mp so EE and it is an openapi integration in EE and not the opposite ;)
@arthurdm
Isn't your reasoning a variant of the age old:
"There might be a person, deep in the jungles of Japan, who might want to use [insert EE tech] with Python/Spring/Ruby/What have you"
In general that leads to very bland, and overly complex designs, for no good. (Python users would never want to use an API that's designed for Java). JSR 77 is a prime example.
I don't mean this in any way as offence, so please bear with me, but if you like to do work for OpenAPI (OAS3) or Spring, wouldn't it be a better idea to work with or for them directly instead of polluting/compromising the EE targeted MP specs?
I think we've heard a lot of opinions already. @arthurdm as a s pre c lead has his own opinion. But the role of spec lead is not to make ultimate decisions but drive collaboration and facilitate agreements. Arthur is failing in this respect. I'm sorry to say so, especially because I haven't met Arthur to have and I'm sure we'd have a friendly talk together.
At this stage, the only reasonable option is to have a wider discussion in the MP community and ultimately vote on a decision among all MP committers where everybody has an equal vote, including spec leads.
drive collaboration and facilitate agreements
@OndrejM - we have discussed this topic in three different MP OpenAPI hangouts, where I invited the opinions of all members - so definitely opinions were heard and discussed. Just ask @EricWittmann or @ivanjunckes or @msavy or @AndyGee or @leochr
I could have just ignored this issue, like it happens in many other specs (in or outside MP), but instead I gave it the appropriate attention - three hangouts, plus all the discussions here..which don't just reflect my opinions, it reflects the consensus reached at the various hangouts.
We actually even agreed today (as a team) to defer this topic to whatever decision comes out of https://github.com/eclipse/microprofile/issues/46 - so I definitely don't see any problems with the way this particular issue was handled. We discussed, invited opinions, had chats, reached an agreement to move forward.
Ultimately no one on the MP OpenAPI hangout felt strongly enough about this issue one way or the other, so the agreement we reached was to defer to the decision of the arch. board. Also I noticed that the information included in https://github.com/eclipse/microprofile/issues/46 is unclear (at least to me). So I'll update that in order for the board to make an informed decision.
@EricWittmann [quote]Ultimately no one on the MP OpenAPI hangout felt strongly enough about this issue one way or the other[/quote]
But perhaps that was because all the people on that hangout were the same people who didn't care (or thought about it) in the first place?
Very likely. And without anyone else joining the call, I think a reasonable decision was made: abide by the decision reached on issue https://github.com/eclipse/microprofile/issues/46
Disagree?
Disagree?
It's now probably for the best to wait for 46 then. Still it surprises me how those on the hangout would not feel strongly about this given all the arguments raised by so many different people.
@arjantijms, it seems that your comment was add multiple times. Probably Github messed up, Github was messing up with me yesterday too. I deleted all the redundant comments.
I agree, @EricWittmann with moving the resolution to eclipse/microprofile#46 and many thanks for clarification in that issue, you described the problem very well!
Just one thing to add - it's not easy for all interested parties to join the hangout, especially at the same time. Therefore I wouldn't make a decision during any hangout if there's a wide dispute like this one. It's better to ask on the mailing list and let people vote. Or discuss this in gitter so that it's easier to join the discussion and also keep the history of the discussion.
Hangouts should be for better coordination and not for making decisions.
@OndrejM thanks, GitHub just has an outage in the middle of me posting that comment (https://twitter.com/githubstatus/status/1018911757245538304)
This issue will take a look at the spec, decide what is the appropriate design, and further clarify the location of the various artifacts, such as the OpenAPI static file / snippet, and property files.