Hi, as I've written on the pull request. Great work, thank you. Feel free to merge any time. The other day I gave you full access to the sources. Again welcome aboard and happy coding :)
regards, Achim 2017-03-06 16:57 GMT+01:00 Stephan Siano <[email protected]>: > Hi Achim, > > ok you are right, on a careful configuration most of the use cases should > be possible. > > I have adapted the matching logic from the jetty ServerControllerImpl to > the EmbeddedTomcat implementation. > > I will create a pull request for the change. Can you have a look? > https://github.com/ops4j/org.ops4j.pax.web/pull/75 > > The change does some changes in the way the tomcat-server.xml is parsed > and in the merge logic how configurations from tomcat-server.xml and jetty > are merged. > > Furthermore I have added a few more entities to the config-server.xml for > in the tomcat-config-fragment sample (to have more features to test in > there) and added some tests to the TomcatConfigurationIntegrationTest. > > I also enabled some deactivated tests, even though I am not sure which of > these tests require my changes and which would even run without them. > > The el-stuff and the welcome and error pages still do not work, but I > don't think that this is related to the tomcat-sever-xml parsing > (PAXWEB-630). > > Best regards > Stephan > > Am Samstag, 4. März 2017 20:10:47 UTC+1 schrieb Achim Nierbeck: >> >> Hi Stephan, >> >> thanks for your time to look into this. >> Your use-cases A, looks like what we have right now. >> Use-case B, should already be possible, we have a bunch of settings >> available from the Pax-Web configurations which should cover >> most use-cases from a settings.xml. For example take a look the following >> [1]. >> >> regarding Use-Case C, this sounds like something which is already >> possible with Jetty as Container. [2] >> You register extra connectors and reference those as virtual host >> connectors. [3] >> >> Regarding your proposal, I'd rather not have a different behavior between >> jetty and tomcat regarding OSGi-Config first. >> In the end we're running inside an OSGi environment, people expect it to >> respect that, therefore the tomcat is just an Add-On. >> >> regards, Achim >> >> [1] - https://github.com/ops4j/org.ops4j.pax.web/blob/master/pax >> -web-runtime/src/main/resources/OSGI-INF/metatype/metatype.xml >> [2] - https://nierbeck.de/2013/05/bind-certain-web-applications- >> to-specific-httpconnectors-ii/ >> [3] - https://github.com/ops4j/org.ops4j.pax.web/blob/master/sam >> ples/whiteboard-extended/src/main/java/org/ops4j/pax/web/ >> extender/samples/whiteboard/internal/Activator.java >> >> >> 2017-03-04 10:24 GMT+01:00 Stephan Siano <[email protected]>: >> >>> Hi, >>> >>> after having implemented some merge algorithm, I figured out that this >>> is all wrong because the host entity does not define to which addresses the >>> tomcat instance listens, but allows to have different configurations (e.g. >>> valves) for different virtual hosts, so the listening addresses and the >>> tomcat hosts are actually completely unrelated. For the normal >>> configuration without tomcat-server.xml it is completely sufficient to have >>> one single default host that serves requests to all virtual hosts (as the >>> config admin information does not contain any host specidic configuration >>> anyway). Instead the listening addresses have to go into the connectors. >>> >>> The current implementation makes sure that there is one connector for >>> the HTTP and/or HTTPS port configured in the config admin settings. If no >>> such connector exists (from the configuration) one is created (without >>> address which means that it listens to all addresses). >>> >>> So my proposal would be to remove all the host merge logic and just make >>> sure that the default host of the engine configured in the tomcat instance >>> exists (and create one if it doesn't). >>> >>> Additionally the connector merge logic needs to be extended, but I am a >>> but unsure about what makes most sense here. I might have a closer look >>> into the jetty implementation again, but from an outside point of view I >>> see the folloeing use cases: >>> 1. User A does not care about listening address configuration at all. He >>> has defined a port and wants the container to listen to all his addresses >>> in his port. He may or may not have a tomcat-config.xml and if he defined a >>> connector in there, it uses the same port as the port configured in the >>> config admin service and did not configure any address. For this user the >>> current setup is just fine, the connector is either instantiated from the >>> tomcat-server.xml or created afterwards from the config admin properties. >>> (It does not matter that the listening addresses from the config admin >>> properties are ignored as they both default to listen to all addresses). >>> 2. User B wants to limit the listening address to one address (for HTTP >>> and HTTPS). With the current implementation he can achieve this by defining >>> connectors with the HTTP and HTTPS ports and configure them appropriately >>> in config-server.xml. he cannot configure that via config admin properties. >>> 3. User C wants to limit the listening address to one address for HTTP >>> only, but not for HTTPS (e.g. HTTP is supposed to be accessible only from >>> the internal network, HTTPS is supposed to be accessible from all >>> networks). With the current implementation he can also configure that by >>> defining the appropriate connectors in the tomcat-server.xml. >>> >>> I would guess for user A everything we can think up is ok. For user B >>> ist might be good to have an option to configure address specifc connectors >>> also from config/admin information, so he does not need a config-server.xml >>> at all. >>> >>> To create a solution that can also make user C happy is more difficutl. >>> He might have some elaborate and different configuration for his connectors >>> and he cannot un-define the config/admin properties. >>> >>> So my idea would be to do the following: >>> if there is already a connector for the port available, we just use it >>> as it is (the user has configured it in his config-server.xml, so this is >>> likely what he wants). This is the current behaviour. If there is no such >>> connector, we create connectors honoring the listening addresses. This >>> might kind of violate the config admin configuration first paradigm, but >>> will give users usually what they want. >>> >>> What do you think? >>> >>> Best regards >>> Stephan >>> >>> Am Freitag, 3. März 2017 20:25:10 UTC+1 schrieb Achim Nierbeck: >>>> >>>> Hi Stephan, >>>> >>>> see my comments inline :-) >>>> >>>> regards, Achim >>>> >>>> >>>> 2017-03-03 14:51 GMT+01:00 Stephan Siano <[email protected]>: >>>> >>>>> Hi, >>>>> >>>>> I am working on [PAXWEB-630] Interpret and use the tomcat-server.xml. >>>>> While doing so, I came across an interesting problem. >>>>> >>>>> The tomcat-server.xml can configure host entries (and this does make >>>>> sense, since that allows to add additional configurations like valves to >>>>> the host). >>>>> >>>>> However, after the config file parsing the mergeConfiguration() method >>>>> of EmbeddedTomcat will change the name of the host configured with the >>>>> EmbeddedTomcatInstance to the first listeningAddress in the configuration >>>>> (and will add new hosts to the engine configured at the engine attribute >>>>> of >>>>> the EmbeddedTomcatInstance). The first host will be set as defaultHost to >>>>> the engine. Does that really make sense? If I understand the jetty coding >>>>> correctly they try to match the listeningAddresses from the configuration >>>>> with those from the configration file (and add the missing ones). Wouldn't >>>>> that make more sense or is there a reason why the mechanism is as it is >>>>> (except that it is easier to implement that way)? >>>>> >>>>> >>>> AFAICR when I created a first shot for this, I basically moved the way >>>> it's handled with jetty to tomcat. >>>> But what you describe here, that sounds more like a bug, cause the way >>>> it's handled for jetty is the way it's supposed to be. >>>> >>>> >>>>> For the connectors the org.osgi.service.http.enabled and >>>>> org.osgi.service.https.enabled properties take precedence over everything >>>>> else (if they are set to false, all http or https connectors are removed). >>>>> Aside from that a connector for org.osgi.service.http.port and >>>>> org.osgi.service.https.port is added if it does not already exist in the >>>>> config file. I would guess that makes sense. >>>>> >>>> >>>> >>>> >>>>> >>>>> Can you explain how the merge behaviour of the hosts is supposed to be? >>>>> >>>> >>>> The idea behind this is >>>> A) osgi properties are always master >>>> B) external jetty or tomcat configurations are always add-ons >>>> as we're running in an OSGi env, the OSGi spec rules for the >>>> configuration, external configurations are an "add-on" from Pax-Web. >>>> >>>> Therefore if there is a configuration available from an native >>>> configuration which collides with the "default" configuration coming from >>>> the osgi properties, the Host configuration from the OSGi properties >>>> rules. >>>> All other configurations are add-ons. >>>> >>>> >>>> >>>>> >>>>> Are my assumptions about the connectors correct? >>>>> >>>>> Best regards >>>>> Stephan >>>>> >>>>> -- >>>>> -- >>>>> ------------------ >>>>> OPS4J - http://www.ops4j.org - [email protected] >>>>> >>>>> --- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "OPS4J" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> >>>> >>>> >>>> -- >>>> >>>> Apache Member >>>> Apache Karaf <http://karaf.apache.org/> Committer & PMC >>>> OPS4J Pax Web <http://wiki.ops4j.org/display/paxweb/Pax+Web/> >>>> Committer & Project Lead >>>> blog <http://notizblog.nierbeck.de/> >>>> Co-Author of Apache Karaf Cookbook <http://bit.ly/1ps9rkS> >>>> >>>> Software Architect / Project Manager / Scrum Master >>>> >>>> -- >>> -- >>> ------------------ >>> OPS4J - http://www.ops4j.org - [email protected] >>> >>> --- >>> You received this message because you are subscribed to the Google >>> Groups "OPS4J" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> >> >> -- >> >> Apache Member >> Apache Karaf <http://karaf.apache.org/> Committer & PMC >> OPS4J Pax Web <http://wiki.ops4j.org/display/paxweb/Pax+Web/> Committer >> & Project Lead >> blog <http://notizblog.nierbeck.de/> >> Co-Author of Apache Karaf Cookbook <http://bit.ly/1ps9rkS> >> >> Software Architect / Project Manager / Scrum Master >> >> -- > -- > ------------------ > OPS4J - http://www.ops4j.org - [email protected] > > --- > You received this message because you are subscribed to the Google Groups > "OPS4J" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- Apache Member Apache Karaf <http://karaf.apache.org/> Committer & PMC OPS4J Pax Web <http://wiki.ops4j.org/display/paxweb/Pax+Web/> Committer & Project Lead blog <http://notizblog.nierbeck.de/> Co-Author of Apache Karaf Cookbook <http://bit.ly/1ps9rkS> Software Architect / Project Manager / Scrum Master -- -- ------------------ OPS4J - http://www.ops4j.org - [email protected] --- You received this message because you are subscribed to the Google Groups "OPS4J" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
