Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for filters (Maven variables) & WAR overlays - PART 1: WAR modules #1104

Closed
scottkurz opened this issue Mar 9, 2021 · 30 comments
Closed

Comments

@scottkurz
Copy link
Member

ISSUE

This use case was brought up first as a the runtime issue: OpenLiberty/open-liberty#16133

I think a useful starting point, rather than requiring even more invention unique to liberty-maven-plugin, is to assume the user is maven-war-plugin configuration:

<configuration>
         <filteringDeploymentDescriptors>true</filteringDeploymentDescriptors>

PROPOSAL OUTLINE

So dev mode and loose app support in general would need to be enhanced to support a "filtered" web.xml, beans.xml, etc.

At a lower level, the algorithm we'd need to use could be something like:

  1. adjust the loose app XML construction to look at the maven-war-plugin (pluginMgmt) config for filteringDeploymentDescriptors
  2. If found, write the filtered output to some temp location in /target
  3. Add the XML entry pointing to this temp location instead of or in addition to, eg.. src/main/webapp
  4. Make sure dev mode incorporates this into the iterative loop (I'm not looking at the code but if it assumes it doesn't need to "redeploy" a loose app..that could be an issue; make sure it can react to changes in the source: today a /src change doesn't require any updates to the loose app XML).

Unclear to me if 2. is already captured in some function we can call (either goal or at least API) or do we have to reverse engineer it? Maybe https://maven.apache.org/plugins/maven-war-plugin/exploded-mojo.html could help?

MORE CONTEXT

For src/main/resources content, like persistence.xml, OTOH.. I think this already "just works", since loose app is pointing to "target/classes" and I'm assuming the 'resources' goal already in dev mode loop does the right thing. It would be worth spending a bit of effort manually confirming

THOUGHTS ?

@melloware - does this outline sound like it would be useful to you? Would use of filteringDeploymentDescriptors fit your needs? If not would you be able to give some explanation and maybe examples of your Maven POM showing what might be a better fit instead? (Thank you.)

@scottkurz scottkurz changed the title Add support for "filtered" DDs (web.xml, beans.xml) to loose app construction Add support for "filtered" Deployment Descriptor DDs (web.xml, beans.xml) to loose app construction Mar 9, 2021
@melloware
Copy link

@scottkurz That completely works for me. I can then control it with Maven Profiles whether its Development or Production mode.

@scottkurz scottkurz changed the title Add support for "filtered" Deployment Descriptor DDs (web.xml, beans.xml) to loose app construction Add support for "filtered" Deployment Descriptor DDs to loose app construction Mar 9, 2021
@scottkurz
Copy link
Member Author

Actually the filteringDeploymentDescriptors only seems to handle web.xml but not other EE files like beans.xml or faces-config.xml that might go into WEB-INF.

We'll have to think about that a bit.

@melloware thanks for the quick feedback before. What's your opinion on this latest point? Is this useful to you if it only performs filtering in the web.xml file but not the beans.xml or faces-config.xml files?

@melloware
Copy link

I think web.xml is definitely important but I am not sure if others will run into the same issue with their other XML configs I can definitely see the need.

@scottkurz
Copy link
Member Author

Thinking about this a bit more... I think we probably should include beans.xml or faces-config.xml in the solution, to handle war plugin config like:

                <webResources>
                    <resource>
                        <directory>${basedir}/src/main/webapp</directory>
                        <filtering>true</filtering>

I think we have to decide roughly between:

  1. Detecting that the war-plugin is doing some filtering and auto-constructing the loose app config based on the 'exploded' output (which incorporates filtering) OR
  2. Adding a new (non-default) config saying basically: "call the exploded goal and build the loose app from related (filtered) paths"

(or some variant in between somehow).

ANOTHER ISSUE

Finally, I don't think the dev mode loop is handling filtering even in, e.g. src/main/resources/META-INF/persistence.xml    with a simple POM snippet like:

    <resources>
        <resource>
            <directory>src/main/resources</directory>
            <filtering>true</filtering>
        </resource>
    </resources>

which maybe should also be fixed at the same time here.

@scottkurz
Copy link
Member Author

I have been giving this some more thought.

I'm wondering if we should instead use the .../target/<finalName> directory into which the war plugin lays things down before finally zipping up the .war in the loose app definition.

E.g. build the loose app as:

<archive>
    <dir sourceOnDisk=".../project/target/myapp-1.0" targetInArchive="/"/>
    <file sourceOnDisk=".../project/target/tmp/META-INF/MANIFEST.MF" targetInArchive="/META-INF/MANIFEST.MF"/>
</archive>

We could modify the dev mode loop by adding new calls to:

  1. resources:resources
  2. war:exploded (everything the war plugin does except the final war zip).

This one change would add support for "filters" both in src/main/resources as well as src/main/webapp since both get copied into this directory. It would also allow us to support WAR overlays which https://dzone.com/articles/mavens-war-overlay-what-are the loose app format currently doesn't seem to handle.

I think that the war plugin is "smart" about only copying new dependencies and deleting "stale" dependencies... I think the performance hit should be minimal.

Talking more internally on the subject but wanted to post this update.

@melloware
Copy link

I believe that is how the Maven Jetty plugin works it allows you to use overlays.

@scottkurz
Copy link
Member Author

Noting another request to allow overlays: #593

@scottkurz
Copy link
Member Author

Was thinking whether I should look at the plugin-level configuration (of maven-war-plugin) or pick a specific goal's configuration.

Though the change in #767 adding support for webResources looks at plugin-level config.. I think I'm going to start looking at the 'exploded' goal config.. in case for some reason someone wanted to differentiate the exploded vs war (package/zip) behavior. The downside is if someone just naively configured the war goal, I wouldn't pick up their config.. but that's going through a bit more trouble than adding plugin-level config, which would apply/merge to the exploded goal config as well.

@scottkurz
Copy link
Member Author

Made some progress here:
https://github.com/scottkurz/ci.common/tree/next-time
https://github.com/scottkurz/ci.maven/tree/next-time

I haven't spent any time on trying to detect when/how to use this loose app config, so am assuming it's triggered by a new config parm: "exploded=true" (defaults to 'false')

@scottkurz
Copy link
Member Author

Prototype seems to be working fairly well.

Two points for further investigation:

  1. For a very large WAR, with many dependencies, does this new design using war:exploded scale or does it slow down ? With a handful: 5-10 dependency JARs the response seems basically instant. But one advantage of the current loose app structure is it points to the dep JARs in .m2/repository. If we have to copy them into target/myapp-1.0 is this a noticeable perf. hit? We may only need to copy them once but how long does even that initial copy take ?

  2. Can we auto-detect that we want to use the new behavior based on config? Something like if any of these have filtering enabled we use the new path:

  • build/resources/resource (any resource)
  • war/configuration/filteringDeploymentDescriptors
  • war/configuration/webResources/resource (any resource)

Even if we can auto-detect, we still might need an explicit switch, e.g. to support overlays. That could lead to a choice of "exploded=true | false | auto" perhaps ? Getting ahead of myself, that's a bit further down the road.

@scottkurz
Copy link
Member Author

One final point we might need to clarify is that we're talking about a build time, not a run time substitution. That is, we're not saying you can deploy a web.xml with contents including: <param-value>${jsf.stage}</param-value> and filter/substitute for the jsf.stage value when the maven plugin starts up Liberty (somehow).

@scottkurz
Copy link
Member Author

SETUP FOR BIG WAR

To address performance concerns, I did some measurements enlarging my test WAR in two ways:

  • Adding 15 dependency JARs on the large side, totaling about 150MB.
  • Adding the 5000 or so files, (also around 150MB) in the openliberty.io website WAR that come from the jekyll build

Observations:

  1. There is indeed a penalty for the extra copy we need to make now, both from the web resources source dirs and the local m2 repo dependencies, but it's not too bad:
  • The extra copy of the 150MB of JAR deps is barely noticable (a few seconds maybe)
  • The extra copy of the 5000 files took longer: best case around 7s on my Linux VM, 30 sec. on my Win10 laptop (1:08 before I turned off antivirus scanning)

If you were going to package a WAR eventually you'd have had to have paid this later, but if you were just going to push to Git after your changes this is new, extra. You don't have to pay iteratively because the war plugin uses a "smart" copy, so it's akin to installing Liberty and the features .

  1. Though the copying's not so bad the additional scanning/watching of all these web resources is bad. Monitoring all the openliberty.io paths made dev mode grind to a halt. Before we never had to watch src/main/webapp, now this and other webResources might contain filtered resources, which is a lot more to watch.

We seem to need to do something like only watch web resource paths when filtering is enabled (parsing the war/resources config). What I'm not sure of is if we would then simply build the loose app from the single "exploded" directory: <dir sourceOnDisk="C:\app\target\myapp-1.0" targetInArchive="/"/> with a mix of monitored/updated resources and resources that only get updated when something else triggers the loop.... or if we have a more complicated, fine-grained loose app construction from the different webResources locations

OVERLAY

Finally, it's worth nothing that for this particular app, io.openliberty... it might fit well into an "overlay" type of use case. I realize it might not be worth modifying the build now, but interesting to realize it'd be an option if we went ahead with this new function... in which case none of these webResources are "source" at all for this particular module (so don't need to be monitored).. but rather the jekyll build becomes more like a dependency.

@scottkurz
Copy link
Member Author

What I'm not sure of is if we would then simply build the loose app from the single "exploded" directory: <dir sourceOnDisk="C:\app\target\myapp-1.0" targetInArchive="/"/> with a mix of monitored/updated resources and resources that only get updated when something else triggers the loop.... or if we have a more complicated, fine-grained loose app construction from the different webResources locations

Yeah, now that the design suggests not watching/monitoring non-"filtered" web resources dirs, the running app would no longer pick up any changes to those resources (neither static HTML or something like beans.xml).

So the loose app needs to be defined with a more fine-grained mix from the different locations. Not a complete rewrite needed, but have to rework and retest.

@scottkurz
Copy link
Member Author

PROTOTYPE UPDATE

Reworked to only monitor "filtered" source resource directories:
https://github.com/scottkurz/ci.common/tree/exploded
https://github.com/scottkurz/ci.maven/tree/exploded

The fact that loose apps guarantee loading the "virtual" contents in element order within the loose app XML file, per:
https://www.ibm.com/docs/en/was-liberty/base?topic=liberty-loose-applications
means that we can correctly overlay the "non-filtered" source resource dirs (for which dev mode won't be watching for updates) and load them before the new target webappdir (which only gets updated when something the dev mode watcher is watching triggers a new 'exploded' call.

<archive>
    <!-- One dir of unfiltered web resources, as source -->
    <dir sourceOnDisk="C:\git\sample.batch.bonuspayout\src\main\more-unfiltered-resources" targetInArchive="/"/>
    <!-- Another dir of unfiltered web resources, as source -->
    <dir sourceOnDisk="C:\git\sample.batch.bonuspayout\target\jekyll-webapp" targetInArchive="/"/>
    <!-- The target web app dir, which has classes, resources, and filtered web resources (with lower precedence than the previous two)  -->
    <dir sourceOnDisk="C:\git\sample.batch.bonuspayout\target\myapp-1.0" targetInArchive="/"/>
    <file sourceOnDisk="C:\git\sample.batch.bonuspayout\target\tmp\META-INF\MANIFEST.MF" targetInArchive="/META-INF/MANIFEST.MF"/>

PERFORMANCE

Now that we have eliminated scanning webResources dirs for which filtering is disabled, the only remaining performance concern is the ONE-TIME copying of dependencies and other resources into the target webapp dir.

It is a one-time hit because the war:exploded logic is "mostly" smart about avoiding unnecessary re-copying. As mentioned before, a few bigger smaller files aren't a big deal but many, even smaller files, can add time.. e.g. up to 30s for a large app like the openlibery.io website jekyll deps.

Note that if you were ever going to do a mvn package or war:war you'd pay this cost anyway, and so it's just a matter of shifting when you would pay this cost, upfront during initial execution of dev mode or later when doing the package.

It's only in the case where you never build the WAR package that this is a purely extra penalty....though this case would still be fairly common so it is a concern.

But this could be addressed too by not making this the default, and requiring a new parameter like -Dexploded (as in this prototype), to enable the new behavior

@scottkurz
Copy link
Member Author

@melloware can I just ask you to confirm one thing here ? I think we are clear on this and have been following along on the same page since you started the discussion but... I just want to be clear that what I'm proposing involves resolving the filter-type substitutions at build time... rather than at run time.

If you are running out of dev mode, of course, the distinction collapses, since dev mode involves building and running in one easy step.

But I'm just trying to clarify that we're NOT proposing allowing you to package a server with an app deployed include a web.xml with a ${var} type of substitution to be resolved later. You'd have already resolved the substitutions when building/deploying/packaging in this type of scenario.

Does that sound like we are still on the same page? Thanks for your input here.

@melloware
Copy link

Correct I am just saying when running in DEV the substitution should be applied and when packaging for PRD the substitution should be applied not that is is dynamic in a packaged WAR file.

@scottkurz scottkurz added the Epic label May 17, 2021
@scottkurz
Copy link
Member Author

Since we are discussing filtering at the Epic, high-level, we should probably consider server.xml filtering at the same time, e.g. this issue: #587.

Though the implementation may be different we might want to say, enable both with a common external (plugin parameter).

@scottkurz scottkurz changed the title Add support for "filtered" Deployment Descriptor DDs to loose app construction Support for filters (Maven variables) & WAR overlays Jul 29, 2021
@scottkurz
Copy link
Member Author

scottkurz commented Aug 3, 2021

UFO (design) link: https://ibm.box.com/s/k6zinwfzvwfzfp0fl4mv7fv9mbmuubfa

@tevans78
Copy link
Member

UFO Review Recording - https://ibm.ent.box.com/file/846043148864
Comments: Slide 33: case may not be correct on "serverXMLfiltered". Also check if the property name is consistent with other areas.

@scottkurz
Copy link
Member Author

Comments: Slide 33: case may not be correct on "serverXMLfiltered". Also check if the property name is consistent with other areas.

Changed property name to: serverXmlFiltering

@scottkurz
Copy link
Member Author

One subtlety I thought of since the design review: in the existing loose app format, a change to a dep in a local m2 repo will cause the app to recycle...since the loose app points to the dep location in the local m2.

Under the new exploded loose app design this would not be the case, since the deps would typically now come from the target webapp dir.

This could in theory affect a "shared library" type of use case in which a separate module is updated and built (and installed into m2 repo) while dev mode is active for an app consuming this "shared lib" (as a dep).

The user could sort of workaround this limitation by making a trivial pom.xml change to force another call to war:exploded.

Perhaps that's a good question to ask more generally even..is there a way to force another call to war:exploded?

@scottkurz scottkurz self-assigned this Oct 18, 2021
@scottkurz
Copy link
Member Author

UPDATE: having merged: #1289

Roughly we are DONE:

  • overlay & resource filtering enabled for single-WAR-modules

We did NOT yet get to:

  • resource filtering enabled for multi-mod dev mode
  • server.xml filtering

One thing I'm not sure about is:

  • overlay/filtering in multi-mod NON-dev-mode.
    So e.g. if a multi-mod app was happily using the existing loose app format, even though resource filtering / overlay wasn't completely supported (per this feature).. what happens with the current code in the stream? Does it work, even though dev mode can't handle this case? Ideally we would check before releasing the plugin next.

@scottkurz
Copy link
Member Author

@melloware - I just wanted to give you in particular an update, since you opened the original issue.

In the 3.5.2-SNAPSHOT version we made available you should now be able to meet the needs of your original use case for a single WAR module application, and you can now leverage "filtering" in web resources and "deployment descriptor" files like web.xml when working with "loose applications" (e.g. in "dev mode").

As I maybe hinted at in my earlier comment we are likely to release this level of support in our next release of the plugin and implement the rest at a later date.

If you are interested in trying it out, you could download the snapshot by adding to your POM:

    <pluginRepositories>
        <!-- Configure Sonatype OSS Maven snapshots repository -->
        <pluginRepository>
            <id>sonatype-nexus-snapshots</id>
            <name>Sonatype Nexus Snapshots</name>
            <url>https://oss.sonatype.org/content/repositories/snapshots/</url>
            <snapshots>
                <enabled>true</enabled>
            </snapshots>
            <releases>
                <enabled>false</enabled>
            </releases>
        </pluginRepository>
    </pluginRepositories>

No special config is required to use the new function; the liberty-maven-plugin should auto-detect based on your WAR plugin config, and generate the new "loose application" format to enable this use case. (One caveat is that we expect the configuration to be added to the plugin-level rather than to execution-level, since in dev mode we don't actually execute an individual execution).

If you are able to give it a try and have any feedback we would be happy to hear. Thank you!

@melloware
Copy link

Nice work!

@scottkurz
Copy link
Member Author

Nice work!

@melloware - Thanks for the quick response. Can I take that to mean you had a chance to try it out ?

@melloware
Copy link

yep it works for my use case of switching the JSF Development stage

@melloware
Copy link

I will monitor once this is released and update my Liberty Faces example project to do this.

https://github.com/melloware/liberty-faces

@yeekangc
Copy link
Member

yeekangc commented Mar 14, 2022

See #1104 (comment) for where we are.

We will look to make available "overlay & resource filtering enabled for single-WAR-modules" in the next release of the plugin.

@scottkurz
Copy link
Member Author

Let me summarize again since the earlier comment summarizing: #1104 (comment) included a maybe confusing question.

@scottkurz
Copy link
Member Author

scottkurz commented Aug 16, 2023

Delivered the final fix: #1714 targeted for liberty-maven-plugin v3.8.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 23.0.0.10
Development

Successfully merging a pull request may close this issue.

5 participants