You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2014/11/13 00:47:28 UTC

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

GitHub user sjcorbett opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/323

    Tomcat HTTPS

    * Templates and copies `server.xml` and `web.xml` files to server rather than relying on sed.
    * Extends `TomcatServer` to support HTTPS.
    * Hoists some useful bits from `Jboss7ServerImpl` to `JavaWebAppSoftwareProcessImpl`.
    
    I verified that the Tomcat integration tests all ran succesfully.
    ```mvn clean install -PIntegration -Dtest=TomcatServerWebAppFixtureIntegrationTest```
    
    The Jboss integration tests failed, but they fail for me on master too. Please run the following command from `software/webapp` and do not merge this pull request if it does not complete successfully. You should only need to test JBoss 7: disable the JBoss 6 test by commenting out lines 40-42 and 49 of `JbossServerWebAppFixtureIntegrationTest`.
    ```mvn clean install -PIntegration -Dtest=JbossServerWebAppFixtureIntegrationTest#canStartAndStop```

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sjcorbett/incubator-brooklyn tomcat-https

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/323.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #323
    
----
commit 46dfc41088d0b84f930b5fc98ceaf60a67d05a83
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date:   2014-11-12T14:03:01Z

    Un-sed Tomcat configuration

commit ed00435c7434496bf5d1ec6e7b3f679712804bd6
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date:   2014-11-12T23:26:09Z

    Tomcat entity supports HTTPS

commit 0dc8e38c4acb510046a3ae5674aab86d96065fb0
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date:   2014-11-12T23:26:43Z

    Documentation improvements

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62883848
  
    no, the driver doesn't need it since it isn't proxied, but the entity does; but it looks like methods are sometimes on both the driver and the entity -- the entity is the more logical place for anything which would be driver-independent
    
    have things all working nicely BTW -- looks great after a couple minor changes, will merge after mvn clean bill of health


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#discussion_r20283387
  
    --- Diff: software/webapp/src/test/java/brooklyn/entity/webapp/tomcat/TomcatServerWebAppFixtureIntegrationTest.java ---
    @@ -52,9 +57,24 @@
             TestApplication tomcatApp = newTestApplication();
             TomcatServer tomcat = tomcatApp.createAndManageChild(EntitySpec.create(TomcatServer.class)
                     .configure(TomcatServer.HTTP_PORT, PortRanges.fromString(DEFAULT_HTTP_PORT)));
    -        
    +
    +
    +        File keystoreFile;
    +        try {
    +            keystoreFile = createTemporaryKeyStore("myname", "mypass");
    +            keystoreFile.deleteOnExit();
    +        } catch (Exception e) {
    +            throw Exceptions.propagate(e);
    +        }
    +
    +        TomcatServer httpsTomcat = tomcatApp.createAndManageChild(EntitySpec.create(TomcatServer.class)
    +                .configure(TomcatServer.HTTP_PORT, PortRanges.fromString(DEFAULT_HTTP_PORT))
    +                .configure(TomcatServer.ENABLED_PROTOCOLS, ImmutableSet.of("https"))
    +                .configure(TomcatServer.HTTPS_SSL_CONFIG,
    +                        new HttpsSslConfig().keyAlias("myname").keystorePassword("mypass").keystoreUrl(keystoreFile.getAbsolutePath())));
    +
             return new JavaWebAppSoftwareProcess[][] {
    -                new JavaWebAppSoftwareProcess[] {tomcat}
    +                new JavaWebAppSoftwareProcess[] { tomcat, httpsTomcat }
    --- End diff --
    
    should be
    
            return new JavaWebAppSoftwareProcess[][] {
                    new JavaWebAppSoftwareProcess[] { tomcat },
                    new JavaWebAppSoftwareProcess[] { httpsTomcat }
    
    (i'll fix)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62872707
  
    oh i see - it's `web.xml` w ~4600 additions


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62881317
  
    Maybe it's a difference between the entity and the driver? Freemarker definitely doesn't need methods on the driver impl to be on its interface too. 
    
    As I said initially I ran the Tomcat integration tests and got a clean bill. I commented out the non-HTTPS `tomcat` in the data provider so I don't think the mis-definition should have been an issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62875410
  
    integration tests fail, and when i deploy manually i get
    
        Failure running task customize (wmpATzbp): freemarker.core.InvalidReferenceException: Expression entity.httpsSslKeystorePassword is undefined on line 95, column 78 in config.
    
    i'll debug but @sjcorbett if this is missing something please let me know!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#discussion_r20280936
  
    --- Diff: software/webapp/src/main/resources/brooklyn/entity/webapp/tomcat/server.xml ---
    @@ -0,0 +1,151 @@
    +[#ftl]
    +<?xml version='1.0' encoding='utf-8'?>
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    Can you remove the license header from server.xml and web.xml, and add them to software/webapp/pom.xml to the `apache-rat-plugin` exclusion section.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62923184
  
    @ahgittin Thanks for your additions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#discussion_r20283257
  
    --- Diff: software/webapp/src/test/java/brooklyn/entity/webapp/tomcat/TomcatServerWebAppFixtureIntegrationTest.java ---
    @@ -52,9 +57,24 @@
             TestApplication tomcatApp = newTestApplication();
             TomcatServer tomcat = tomcatApp.createAndManageChild(EntitySpec.create(TomcatServer.class)
                     .configure(TomcatServer.HTTP_PORT, PortRanges.fromString(DEFAULT_HTTP_PORT)));
    -        
    +
    +
    +        File keystoreFile;
    +        try {
    +            keystoreFile = createTemporaryKeyStore("myname", "mypass");
    +            keystoreFile.deleteOnExit();
    +        } catch (Exception e) {
    +            throw Exceptions.propagate(e);
    +        }
    +
    +        TomcatServer httpsTomcat = tomcatApp.createAndManageChild(EntitySpec.create(TomcatServer.class)
    +                .configure(TomcatServer.HTTP_PORT, PortRanges.fromString(DEFAULT_HTTP_PORT))
    --- End diff --
    
    not needed, but no harm


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62880960
  
    quite a few errors running the tomcat integration tests but i'm working my way through them.  the problem may actually have been that it was returning null, but normally methods have to be on an interface (so they are proxied) in order for freemarker to access them.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#discussion_r20281328
  
    --- Diff: software/webapp/src/main/resources/brooklyn/entity/webapp/tomcat/server.xml ---
    @@ -0,0 +1,151 @@
    +[#ftl]
    +<?xml version='1.0' encoding='utf-8'?>
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    I can do, but to be clear they were already Apache licensed. I didn't add the header. Shall I go ahead?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62879870
  
    @ahgittin Strange. I would expect it to call `JavaWebAppSoftwareProcessImpl.getHttpsSslKeystorePassword`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#discussion_r20281351
  
    --- Diff: software/webapp/src/main/resources/brooklyn/entity/webapp/tomcat/server.xml ---
    @@ -0,0 +1,151 @@
    +[#ftl]
    +<?xml version='1.0' encoding='utf-8'?>
    +<!--
    +  Licensed to the Apache Software Foundation (ASF) under one or more
    --- End diff --
    
    I take it back! You didn't add these; they were already there in the original tomcat it looks like?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62885032
  
    merged, with some repairs in 7ef195f004bd7e4e718e1af744eaa31e5cf1015b


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/323


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: Tomcat HTTPS

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/323#issuecomment-62872463
  
    The github "Files changed" tab shows "with 4,924 additions and 91 deletions."  I see a lot fewer additions and a lot more deletions.  What's going on?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---