Thanks for the clarification Georg. I've closed the SLING-7764 ticket as obsolete with the migration steps as a comment.
I don't mean to nitpick, but when reviewing the source code of ServiceUnavailableFilter#getResponseText method, it looks to me that in the use case where the responseTextFor503 configuration attribute has an invalid or non-existing "classpath:**" value, the filter sends back too many details to the end user in the response. Strict security folks would interpret returning those administrative details about the misconfigurated filter to the (anonymous) end users as an information disclosure security vulnerability. The misconfiguration details should really only be known to the server administrators. What do you think about just logging the details of the error and return some more generic response (perhaps the RESPONSE_TEXT_DEFAULT value?) in the http response send back to the end users in this scenario? Regards, Eric On Fri, May 24, 2019 at 12:34 AM Georg Henzler <slin...@ghenzler.de> wrote: > > Hi Eric, > > I improved [1] quite a bit. For your migration: a) is correct, it can > load an entry of any bundle (does not even have to be started yet) or > just give the text in plain text. The syntax is > "classpath:<symbolic-bundle-id>:/path/to/file.html" (your b) is slightly > wrong). > > > I guess the "custom" solution above is also a way to solve a custom > > build > > that does not include the "org.apache.sling.starter.content" bundle in > > the > > distribution? Or did you have something else in mind to provide better > > default behavior when that bundle is missing and the > > > classpath:org.apache.sling.starter.content:content/content/startup/index.html > > file does not exist in the runtime? > > There is error handling in place if the file is not found, but in > general I think using > "classpath:<symbolic-bundle-id>:/path/to/file.html" is just the easiest > and most straight-forward approach to load the file. Also that way it > can reside with the rest of the html file (which is great from a > maintenance point of view I think). > > - Georg > > [1] > > https://github.com/apache/felix/blob/trunk/healthcheck/README.md#service-unavailable-filter > > > > On 2019-05-23 23:45, Eric Norman wrote: > > Hi Georg, > > > > As far as I know, the only new documentation for SLING-7764 was the > > paragraph that was added to the README.md file at [1] > > > > 1. > > > https://github.com/apache/sling-org-apache-sling-starter-startup/commit/389e0cbb58f33346f97bf730ae795269abf64d03 > > > > > > I did see the configuration options for the ServiceUnavailableFilter > > component and see that I can provide an alternate configuration value > > for > > the responseTextFor503 attribute to use some other custom page. > > > > So just to be clear, the migration path from the SLING-7764 solution to > > the > > new solution would be something like: > > > > a) Change the custom bundle that contained the custom_index.html file > > to no > > longer be a fragment bundle attached to the > > org.apache.sling.starter.startup artifact that no longer exists. The > > fragment can become a regular non-fragment bundle or the > > custom_index.html > > file can be moved somewhere else and get rid of the fragment bundle > > entirely. > > > > b) Modify the > > > org.apache.felix.hc.core.impl.filter.ServiceUnavailableFilter-startupandshutdown > > service configuration to provide the alternate path to the custom page. > > For example, in the starter project (or a fork of it), change the > > responseTextFor503 value in the > > org-apache-sling-starter/src/main/provisioning/healthcheck.txt file. > > > > > > > > I guess the "custom" solution above is also a way to solve a custom > > build > > that does not include the "org.apache.sling.starter.content" bundle in > > the > > distribution? Or did you have something else in mind to provide better > > default behavior when that bundle is missing and the > > > classpath:org.apache.sling.starter.content:content/content/startup/index.html > > file does not exist in the runtime? > > > > Regards, > > Eric > > > > On Thu, May 23, 2019 at 1:01 AM Georg Henzler <slin...@ghenzler.de> > > wrote: > > > >> Hi Eric, > >> > >> so there is [1] which I will extend a bit to be more specific about > >> the > >> way to configure it (see [2] for metatype description). Is the > >> mechanism > >> from SLING-7764 somewhere documented on Sling side? I would fix that > >> documentation then as well.... > >> > >> -Georg > >> > >> [1] > >> "It is possible to configure a custom response text/html" > >> > >> > https://github.com/apache/felix/blob/trunk/healthcheck/README.md#service-unavailable-filter > >> > >> [2] > >> > >> > https://github.com/apache/felix/blob/trunk/healthcheck/core/src/main/java/org/apache/felix/hc/core/impl/filter/ServiceUnavailableFilter.java#L118 > >> > >> On 2019-05-23 04:00, Eric Norman wrote: > >> > Hi Georg, > >> > > >> > I'm trying to digest what has been done here. It looks like this > makes > >> > the > >> > solution that was done for > >> > https://issues.apache.org/jira/browse/SLING-7764 > >> > obsolete and breaks compatibility for any customization done via a > >> > fragment > >> > attached to the previous o.a.sling.starter.startup snapshot bundle? > >> > > >> > Have you provided any documentation on the expected technique for > >> > customizing the content of the page that is displayed when starting up > >> > or > >> > some guidance on how people would migrate from the old way to the new > >> > solution? > >> > > >> > Also what should happen for custom builds that don't include > >> > the org.apache.sling.starter.content artifact and still want a default > >> > "starting up" page to be displayed? > >> > > >> > > >> > Regards, > >> > Eric > >> > > >> > > >> > On Wed, May 22, 2019 at 3:35 PM Georg Henzler <ghenz...@apache.org> > >> > wrote: > >> > > >> >> Hi all, > >> >> > >> >> I finished this, mostly with commit [1]. I moved some bundles into > >> >> different start levels for better startup behaviour (that way I got > >> >> could finish this without fixing SLING-8355 first). > >> >> > >> >> -Georg > >> >> > >> >> [1] > >> >> > >> >> > >> > https://github.com/apache/sling-org-apache-sling-starter/commit/a16fb43f1d0333f74b066844e0377d93ca1e1e08 > >> >> > >> >> On 2019-05-14 11:05, Georg Henzler wrote: > >> >> > So I'll move forward with this and I created [1] to track this > change. > >> >> > > >> >> > -Georg > >> >> > > >> >> > [1] https://issues.apache.org/jira/browse/SLING-8418 > >> >> > > >> >> > On 2019-04-08 10:49, Jörg Hoh wrote: > >> >> >> Hi Georg, > >> >> >> > >> >> >> ok, wasn't aware of the "100% compatibility mode" :-) So +1 from > my > >> >> >> side. > >> >> >> > >> >> >> Jörg > >> >> >> > >> >> >> Am Mo., 8. Apr. 2019 um 00:28 Uhr schrieb Georg Henzler > >> >> >> <ghenz...@apache.org > >> >> >>> : > >> >> >> > >> >> >>> Hi Jörg, > >> >> >>> > >> >> >>> there is the option "autoDisableFilter" [1] in the > >> >> >>> ServiceUnavailableFilter - if true the filter automatically > >> >> >>> unregisters > >> >> >>> itself upon first non-503 result. Once unregistered it listens to > >> >> >>> FrameworkEvent.STARTLEVEL_CHANGED events to reregister once > needed > >> >> >>> (the > >> >> >>> shutdown case). During normal operations (that includes > deployments > >> >> >>> that > >> >> >>> might cause the selected "systemalive" checks to be temporarily > >> >> >>> unavailable) the filter is not active (which is also good from a > >> >> >>> performance perspective). > >> >> >>> > >> >> >>> So really, all that would be needed to replace oas-startupfilter > + > >> >> >>> -disabler and oas-starter-startup is the simple config [2] in the > >> >> >>> oas-starter provisioning model. > >> >> >>> > >> >> >>> -Georg > >> >> >>> > >> >> >>> [1] > >> >> >>> > >> >> >>> > >> >> > >> > https://github.com/apache/felix/blob/d5245ba3ba306b57b83c4d5c6cfe499d955b0acb/healthcheck/core/src/main/java/org/apache/felix/hc/core/impl/filter/ServiceUnavailableFilter.java#L98 > >> >> >>> > >> >> >>> [2] > >> >> >>> > >> >> >>> org.apache.felix.hc.core.impl.filter.ServiceUnavailableFilter > >> >> >>> tags=["systemalive] > >> >> >>> > >> >> >>> osgi.http.whiteboard.context.select="(&( > >> >> osgi.http.whiteboard.context.name > >> >> >>> \=*)(!(osgi.http.whiteboard.context.name > \=org.osgi.service.http)))" > >> >> >>> osgi.http.whiteboard.filter.regex=".*" > >> >> >>> autoDisableFilter=B"true" > >> >> >>> > >> >> >>> > >> >> >>> On 2019-04-07 19:49, Jörg Hoh wrote: > >> >> >>> > Hi Georg, > >> >> >>> > > >> >> >>> > Merging the 2 implementations oas-startupfilter + -disabler and > >> >> >>> > oas-starter-startup makes fully sense to me, although I am bit > >> >> hesitant > >> >> >>> > by > >> >> >>> > replacing the basic approach. > >> >> >>> > > >> >> >>> > these 2 have a rather static assumption, when the application > is > >> up, > >> >> >>> > and > >> >> >>> > their sole purpose is to indicate that. And once the system is > up, > >> >> it's > >> >> >>> > up > >> >> >>> > and this state remains unchanged until the > startupfilterDisabler > >> is > >> >> >>> > deactivated. A healthcheck based implementation can changed its > >> mind > >> >> >>> > based > >> >> >>> > on many factors, you already mentioned the fact that key > services > >> >> >>> > should be > >> >> >>> > available. If these services go down during runtime, the > status of > >> >> this > >> >> >>> > check changes. > >> >> >>> > > >> >> >>> > If I understand you correctly, then the semantic of the startup > >> >> itself > >> >> >>> > might not change, but it's more likely, that the > unavailability of > >> >> such > >> >> >>> > a > >> >> >>> > key service would cause the Filter to kick again, changing > >> >> >>> > runtime-behavior; currently it does not. > >> >> >>> > > >> >> >>> > Is this intended? > >> >> >>> > > >> >> >>> > Jörg > >> >> >>> > > >> >> >>> > > >> >> >>> > Am Sa., 6. Apr. 2019 um 00:24 Uhr schrieb Georg Henzler > >> >> >>> > <ghenz...@apache.org > >> >> >>> >> : > >> >> >>> > > >> >> >>> >> Hi all, > >> >> >>> >> > >> >> >>> >> we have currently two mechanisms in Sling to wait for startup: > >> >> >>> >> > >> >> >>> >> * sling-org-apache-sling-startupfilter + > >> >> >>> >> sling-org-apache-sling-startupfilter-disabler > >> >> >>> >> * sling-org-apache-sling-starter-startup (only used for the > >> starter > >> >> >>> >> application AFAIK) > >> >> >>> >> > >> >> >>> >> Now that systemready in Felix is fully migrated to Felix > Health > >> >> Checks > >> >> >>> >> I > >> >> >>> >> would like to rely on ServiceUnavailableFilter [1] instead. > The > >> >> >>> >> advantage is that this filter can be configured in a > declarative > >> >> >>> >> fashion > >> >> >>> >> (what is required to be alive can be configured, e.g. for a > >> customer > >> >> >>> >> project it's easy to wait for additional custom services) and > >> 503 is > >> >> >>> >> also correctly sent upon shutdown. > >> >> >>> >> > >> >> >>> >> Is this approach something everyone is ok with? > >> >> >>> >> > >> >> >>> >> -Georg > >> >> >>> >> > >> >> >>> >> [1] > >> >> >>> >> > >> >> >>> >> > >> >> >>> > >> >> > >> > https://github.com/apache/felix/blob/trunk/healthcheck/core/src/main/java/org/apache/felix/hc/core/impl/filter/ServiceUnavailableFilter.java > >> >> >>> >> > >> >> >>> > >> >> > >> >
