You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by al...@apache.org on 2016/11/16 22:33:05 UTC

[1/7] brooklyn-server git commit: Initialize REST API security correctly for integration tests

Repository: brooklyn-server
Updated Branches:
  refs/heads/master be89a8dd2 -> 2e4114259


Initialize REST API security correctly for integration tests

Also add checks for it in the tests.

Tests had different behaviour depending on whether brooklyn-ui existed along brooklyn-server on disk. When brooklyn-ui was there tests would find it and use the web app code from there, including the web.xml which restricts requests to only authorized users. When brooklyn-ui is missing though a default web app without web.xml was being created. This would result in non-authorized requests succeeding.  In this case requests including the authorization header would be accepted and rejected if the password is invalid. But in order to include the authorization header the server must first respond with a 401 which didn't happen. Moving web-security.xml to rest-server allows us to force request authentication for tests even if there's no web.xml in the web app.
Should be fine moving web-security.xml to rest-server (which is only used in classic) because it's only inserted by classic related code. Karaf Brooklyn doesn't have control over the web apps.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/cd07d816
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/cd07d816
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/cd07d816

Branch: refs/heads/master
Commit: cd07d8161ae4688900b824a449760986806c814f
Parents: 9b24f7d
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Nov 14 12:08:39 2016 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Mon Nov 14 12:08:39 2016 +0200

----------------------------------------------------------------------
 launcher/src/main/resources/web-security.xml    | 51 ----------------
 .../launcher/BrooklynWebServerTest.java         | 61 +++++++++++++++-----
 .../src/main/resources/web-security.xml         | 51 ++++++++++++++++
 .../brooklyn/rest/BrooklynRestApiLauncher.java  |  1 +
 .../AbstractRestApiEntitlementsTest.java        | 18 ++++--
 .../ActivityApiEntitlementsTest.java            | 17 +++---
 .../EntityConfigApiEntitlementsTest.java        |  2 +
 .../entitlement/ScriptApiEntitlementsTest.java  | 14 +++--
 .../entitlement/SensorApiEntitlementsTest.java  |  2 +
 .../entitlement/ServerApiEntitlementsTest.java  |  3 +
 .../ServerResourceIntegrationTest.java          |  2 +-
 11 files changed, 139 insertions(+), 83 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/launcher/src/main/resources/web-security.xml
----------------------------------------------------------------------
diff --git a/launcher/src/main/resources/web-security.xml b/launcher/src/main/resources/web-security.xml
deleted file mode 100644
index 2311458..0000000
--- a/launcher/src/main/resources/web-security.xml
+++ /dev/null
@@ -1,51 +0,0 @@
-<!--
-    Licensed to the Apache Software Foundation (ASF) under one
-    or more contributor license agreements.  See the NOTICE file
-    distributed with this work for additional information
-    regarding copyright ownership.  The ASF licenses this file
-    to you under the Apache License, Version 2.0 (the
-    "License"); you may not use this file except in compliance
-    with the License.  You may obtain a copy of the License at
-    
-     http://www.apache.org/licenses/LICENSE-2.0
-    
-    Unless required by applicable law or agreed to in writing,
-    software distributed under the License is distributed on an
-    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-    KIND, either express or implied.  See the License for the
-    specific language governing permissions and limitations
-    under the License.
--->
-<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"
-         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-         xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
-         version="3.1">
-
-  <security-constraint>
-    <web-resource-collection>
-      <web-resource-name>Logout</web-resource-name>
-      <url-pattern>/v1/logout</url-pattern>
-    </web-resource-collection>
-  </security-constraint>
-
-  <!-- Ignored programmatically if noConsoleSecurity -->
-  <security-constraint>
-    <web-resource-collection>
-      <web-resource-name>All</web-resource-name>
-      <url-pattern>/</url-pattern>
-    </web-resource-collection>
-    <auth-constraint>
-      <role-name>webconsole</role-name>
-    </auth-constraint>
-  </security-constraint>
-
-  <login-config>
-    <auth-method>BASIC</auth-method>
-    <realm-name>webconsole</realm-name>
-  </login-config>
-
-  <security-role>
-    <role-name>webconsole</role-name>
-  </security-role>
-
-</web-app>

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java
index 2b8406e..e1eb1b3 100644
--- a/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/launcher/BrooklynWebServerTest.java
@@ -18,17 +18,13 @@
  */
 package org.apache.brooklyn.launcher;
 
-import org.apache.brooklyn.core.entity.Entities;
-import org.apache.brooklyn.core.internal.BrooklynProperties;
-import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
-import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
-
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
 import java.io.File;
 import java.io.FileInputStream;
+import java.net.SocketException;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.security.KeyStore;
@@ -38,24 +34,31 @@ import java.util.Map;
 
 import javax.net.ssl.SSLHandshakeException;
 
+import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.internal.BrooklynProperties;
+import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext;
+import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
+import org.apache.brooklyn.rest.BrooklynWebConfig;
+import org.apache.brooklyn.util.collections.MutableMap;
+import org.apache.brooklyn.util.exceptions.Exceptions;
+import org.apache.brooklyn.util.http.HttpTool;
+import org.apache.brooklyn.util.http.HttpToolResponse;
+import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.conn.ssl.SSLSocketFactory;
-import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.client.BasicCredentialsProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
-import org.apache.brooklyn.rest.BrooklynWebConfig;
-import org.apache.brooklyn.util.collections.MutableMap;
-import org.apache.brooklyn.util.http.HttpTool;
-import org.apache.brooklyn.util.http.HttpToolResponse;
-import org.apache.brooklyn.util.exceptions.Exceptions;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
-import java.net.SocketException;
 
 public class BrooklynWebServerTest {
 
@@ -92,13 +95,45 @@ public class BrooklynWebServerTest {
         try {
             webServer.start();
 
-            HttpToolResponse response = HttpTool.execAndConsume(new DefaultHttpClient(), new HttpGet(webServer.getRootUrl()));
+            HttpToolResponse response = HttpTool.execAndConsume(HttpTool.httpClientBuilder().build(), new HttpGet(webServer.getRootUrl()));
             assertEquals(response.getResponseCode(), 200);
         } finally {
             webServer.stop();
         }
     }
 
+    @Test
+    public void verifySecurityInitialized() throws Exception {
+        webServer = new BrooklynWebServer(newManagementContext(brooklynProperties));
+        webServer.start();
+        try {
+            HttpToolResponse response = HttpTool.execAndConsume(HttpTool.httpClientBuilder().build(), new HttpGet(webServer.getRootUrl()));
+            assertEquals(response.getResponseCode(), 401);
+        } finally {
+            webServer.stop();
+        }
+    }
+
+    @Test
+    public void verifySecurityInitializedExplicitUser() throws Exception {
+        webServer = new BrooklynWebServer(newManagementContext(brooklynProperties));
+        webServer.start();
+
+        CredentialsProvider credentialsProvider = new BasicCredentialsProvider();
+        credentialsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("myuser", "somepass"));
+        HttpClient client = HttpTool.httpClientBuilder()
+            .credentials(new UsernamePasswordCredentials("myuser", "somepass"))
+            .uri(webServer.getRootUrl())
+            .build();
+
+        try {
+            HttpToolResponse response = HttpTool.execAndConsume(client, new HttpGet(webServer.getRootUrl()));
+            assertEquals(response.getResponseCode(), 401);
+        } finally {
+            webServer.stop();
+        }
+    }
+
     @DataProvider(name="keystorePaths")
     public Object[][] getKeystorePaths() {
         return new Object[][] {

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/main/resources/web-security.xml
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/main/resources/web-security.xml b/rest/rest-server/src/main/resources/web-security.xml
new file mode 100644
index 0000000..2311458
--- /dev/null
+++ b/rest/rest-server/src/main/resources/web-security.xml
@@ -0,0 +1,51 @@
+<!--
+    Licensed to the Apache Software Foundation (ASF) under one
+    or more contributor license agreements.  See the NOTICE file
+    distributed with this work for additional information
+    regarding copyright ownership.  The ASF licenses this file
+    to you under the Apache License, Version 2.0 (the
+    "License"); you may not use this file except in compliance
+    with the License.  You may obtain a copy of the License at
+    
+     http://www.apache.org/licenses/LICENSE-2.0
+    
+    Unless required by applicable law or agreed to in writing,
+    software distributed under the License is distributed on an
+    "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+    KIND, either express or implied.  See the License for the
+    specific language governing permissions and limitations
+    under the License.
+-->
+<web-app xmlns="http://xmlns.jcp.org/xml/ns/javaee"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
+         version="3.1">
+
+  <security-constraint>
+    <web-resource-collection>
+      <web-resource-name>Logout</web-resource-name>
+      <url-pattern>/v1/logout</url-pattern>
+    </web-resource-collection>
+  </security-constraint>
+
+  <!-- Ignored programmatically if noConsoleSecurity -->
+  <security-constraint>
+    <web-resource-collection>
+      <web-resource-name>All</web-resource-name>
+      <url-pattern>/</url-pattern>
+    </web-resource-collection>
+    <auth-constraint>
+      <role-name>webconsole</role-name>
+    </auth-constraint>
+  </security-constraint>
+
+  <login-config>
+    <auth-method>BASIC</auth-method>
+    <realm-name>webconsole</realm-name>
+  </login-config>
+
+  <security-role>
+    <role-name>webconsole</role-name>
+  </security-role>
+
+</web-app>

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
index 4dc3d66..b600159 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
@@ -194,6 +194,7 @@ public class BrooklynRestApiLauncher {
         if (securityProvider != null && securityProvider != AnyoneSecurityProvider.class) {
             ((BrooklynProperties) mgmt.getConfig()).put(
                     BrooklynWebConfig.SECURITY_PROVIDER_CLASSNAME, securityProvider.getName());
+            ((WebAppContext)context).addOverrideDescriptor(getClass().getResource("/web-security.xml").toExternalForm());
         } else if (context instanceof WebAppContext) {
             ((WebAppContext)context).setSecurityHandler(new NopSecurityHandler());
         }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
index 4a0d568..c858799 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
@@ -37,6 +37,7 @@ import org.apache.brooklyn.rest.BrooklynRestApiLauncher;
 import org.apache.brooklyn.rest.BrooklynRestApiLauncherTestFixture;
 import org.apache.brooklyn.util.http.HttpAsserts;
 import org.apache.brooklyn.util.http.HttpTool;
+import org.apache.brooklyn.util.http.HttpTool.HttpClientBuilder;
 import org.apache.brooklyn.util.http.HttpToolResponse;
 import org.apache.http.auth.UsernamePasswordCredentials;
 import org.apache.http.client.HttpClient;
@@ -65,7 +66,7 @@ public abstract class AbstractRestApiEntitlementsTest extends BrooklynRestApiLau
         props.put(PerUserEntitlementManager.PER_USER_ENTITLEMENTS_CONFIG_PREFIX+".myUser", "user");
         props.put(PerUserEntitlementManager.PER_USER_ENTITLEMENTS_CONFIG_PREFIX+".myCustom", StaticDelegatingEntitlementManager.class.getName());
         
-        mgmt = LocalManagementContextForTests.builder(true).useProperties(props).build();
+        mgmt = LocalManagementContextForTests.builder(false).useProperties(props).build();
         app = mgmt.getEntityManager().createEntity(EntitySpec.create(TestApplication.class)
                 .child(EntitySpec.create(TestEntity.class))
                         .configure(TestEntity.CONF_NAME, "myname"));
@@ -84,10 +85,12 @@ public abstract class AbstractRestApiEntitlementsTest extends BrooklynRestApiLau
     }
     
     protected HttpClient newClient(String user) throws Exception {
-        return httpClientBuilder()
-                .uri(getBaseUriRest())
-                .credentials(new UsernamePasswordCredentials(user, "ignoredPassword"))
-                .build();
+        HttpClientBuilder builder = httpClientBuilder()
+                .uri(getBaseUriRest());
+        if (user != null) {
+            builder.credentials(new UsernamePasswordCredentials(user, "ignoredPassword"));
+        }
+        return builder.build();
     }
 
     protected String httpGet(String user, String path) throws Exception {
@@ -130,6 +133,11 @@ public abstract class AbstractRestApiEntitlementsTest extends BrooklynRestApiLau
         assertStatusCodeEquals(response, 404);
     }
 
+    protected void assert401(String path) throws Exception {
+        HttpToolResponse response = HttpTool.httpGet(newClient(null), URI.create(getBaseUriRest()).resolve(path), ImmutableMap.<String, String>of());
+        assertStatusCodeEquals(response, 401);
+    }
+
     protected void assertStatusCodeEquals(HttpToolResponse response, int expected) {
         assertEquals(response.getResponseCode(), expected,
                 "code="+response.getResponseCode()+"; reason="+response.getReasonPhrase()+"; content="+response.getContentAsString());

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ActivityApiEntitlementsTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ActivityApiEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ActivityApiEntitlementsTest.java
index 4a7a0b3..0b61e43 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ActivityApiEntitlementsTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ActivityApiEntitlementsTest.java
@@ -75,6 +75,7 @@ public class ActivityApiEntitlementsTest extends AbstractRestApiEntitlementsTest
     @Test(groups = "Integration")
     public void testGetTask() throws Exception {
         String path = "/v1/activities/"+subTask.getId();
+        assert401(path);
         assertPermitted("myRoot", path);
         assertPermitted("myUser", path);
         assertPermitted("myReadonly", path);
@@ -88,18 +89,20 @@ public class ActivityApiEntitlementsTest extends AbstractRestApiEntitlementsTest
         for (Map.Entry<String, String> entry : streams.entrySet()) {
             String streamId = entry.getKey();
             String expectedStream = entry.getValue();
+            String path = pathPrefix+streamId;
 
-            assertEquals(httpGet("myRoot", pathPrefix+streamId), expectedStream);
-            assertEquals(httpGet("myUser", pathPrefix+streamId), expectedStream);
-            assertEquals(httpGet("myReadonly", pathPrefix+streamId), expectedStream);
-            assertForbidden("myMinimal", pathPrefix+streamId);
-            assertForbidden("unrecognisedUser", pathPrefix+streamId);
+            assert401(path);
+            assertEquals(httpGet("myRoot", path), expectedStream);
+            assertEquals(httpGet("myUser", path), expectedStream);
+            assertEquals(httpGet("myReadonly", path), expectedStream);
+            assertForbidden("myMinimal", path);
+            assertForbidden("unrecognisedUser", path);
             
             StaticDelegatingEntitlementManager.setDelegate(new SeeSelectiveStreams(streamId));
-            assertEquals(httpGet("myCustom", pathPrefix+streamId), expectedStream);
+            assertEquals(httpGet("myCustom", path), expectedStream);
             
             StaticDelegatingEntitlementManager.setDelegate(new SeeSelectiveStreams("differentStreamId"));
-            assertForbidden("myCustom", pathPrefix+streamId);
+            assertForbidden("myCustom", path);
         }
     }
     

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/EntityConfigApiEntitlementsTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/EntityConfigApiEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/EntityConfigApiEntitlementsTest.java
index b95392b..fd2ffef 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/EntityConfigApiEntitlementsTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/EntityConfigApiEntitlementsTest.java
@@ -49,6 +49,7 @@ public class EntityConfigApiEntitlementsTest extends AbstractRestApiEntitlements
         String path = "/v1/applications/"+app.getId()+"/entities/"+entity.getId()+"/config/"+TestEntity.CONF_NAME.getName();
         String val = "\"myname\"";
         
+        assert401(path);
         assertEquals(httpGet("myRoot", path), val);
         assertEquals(httpGet("myUser", path), val);
         assertEquals(httpGet("myReadonly", path), val);
@@ -68,6 +69,7 @@ public class EntityConfigApiEntitlementsTest extends AbstractRestApiEntitlements
         String confName = TestEntity.CONF_NAME.getName();
         String regex = ".*"+confName+".*myname.*";
         
+        assert401(path);
         Asserts.assertStringMatchesRegex(httpGet("myRoot", path), regex);
         Asserts.assertStringMatchesRegex(httpGet("myUser", path), regex);
         Asserts.assertStringMatchesRegex(httpGet("myReadonly", path), regex);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ScriptApiEntitlementsTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ScriptApiEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ScriptApiEntitlementsTest.java
index 5f6498a..7f76e0c 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ScriptApiEntitlementsTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ScriptApiEntitlementsTest.java
@@ -36,14 +36,16 @@ public class ScriptApiEntitlementsTest extends AbstractRestApiEntitlementsTest {
     @Test(groups = "Integration")
     public void testGroovy() throws Exception {
         String script = "1 + 1";
-        HttpToolResponse rootRepsonse = httpPost("myRoot", "/v1/script/groovy", script.getBytes());
+        String path = "/v1/script/groovy";
+        HttpToolResponse rootRepsonse = httpPost("myRoot", path, script.getBytes());
         assertHealthyStatusCode(rootRepsonse);
-        Map groovyOutput = new Gson().fromJson(rootRepsonse.getContentAsString(), Map.class);
+        Map<?, ?> groovyOutput = new Gson().fromJson(rootRepsonse.getContentAsString(), Map.class);
         assertEquals(groovyOutput.get("result"), "2");
-        assertForbiddenPost("myUser", "/v1/script/groovy", script.getBytes());
-        assertForbiddenPost("myReadonly", "/v1/script/groovy", script.getBytes());
-        assertForbiddenPost("myMinimal", "/v1/script/groovy", script.getBytes());
-        assertForbiddenPost("unrecognisedUser", "/v1/script/groovy", script.getBytes());
+        assert401(path);
+        assertForbiddenPost("myUser", path, script.getBytes());
+        assertForbiddenPost("myReadonly", path, script.getBytes());
+        assertForbiddenPost("myMinimal", path, script.getBytes());
+        assertForbiddenPost("unrecognisedUser", path, script.getBytes());
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/SensorApiEntitlementsTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/SensorApiEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/SensorApiEntitlementsTest.java
index 931b7ae..3a60a86 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/SensorApiEntitlementsTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/SensorApiEntitlementsTest.java
@@ -52,6 +52,7 @@ public class SensorApiEntitlementsTest extends AbstractRestApiEntitlementsTest {
         String path = "/v1/applications/"+app.getId()+"/entities/"+entity.getId()+"/sensors/"+sensorName;
         String val = "\"myval\"";
         
+        assert401(path);
         assertEquals(httpGet("myRoot", path), val);
         assertEquals(httpGet("myUser", path), val);
         assertEquals(httpGet("myReadonly", path), val);
@@ -73,6 +74,7 @@ public class SensorApiEntitlementsTest extends AbstractRestApiEntitlementsTest {
         String sensorName = TestEntity.NAME.getName();
         String regex = ".*"+sensorName+".*myval.*";
         
+        assert401(path);
         Asserts.assertStringMatchesRegex(httpGet("myRoot", path), regex);
         Asserts.assertStringMatchesRegex(httpGet("myUser", path), regex);
         Asserts.assertStringMatchesRegex(httpGet("myReadonly", path), regex);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ServerApiEntitlementsTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ServerApiEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ServerApiEntitlementsTest.java
index ca53976..fd01654 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ServerApiEntitlementsTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/ServerApiEntitlementsTest.java
@@ -27,6 +27,7 @@ public class ServerApiEntitlementsTest extends AbstractRestApiEntitlementsTest {
     @Test(groups = "Integration")
     public void testGetHealthy() throws Exception {
         String path = "/v1/server/up";
+        assert401(path);
         assertPermitted("myRoot", path);
         assertPermitted("myUser", path);
         assertForbidden("myReadonly", path);
@@ -37,6 +38,7 @@ public class ServerApiEntitlementsTest extends AbstractRestApiEntitlementsTest {
     @Test(groups = "Integration")
     public void testReloadProperties() throws Exception {
         String resource = "/v1/server/properties/reload";
+        assert401(resource);
         assertPermittedPost("myRoot", resource, null);
         assertForbiddenPost("myUser", resource, null);
         assertForbiddenPost("myReadonly", resource, null);
@@ -48,6 +50,7 @@ public class ServerApiEntitlementsTest extends AbstractRestApiEntitlementsTest {
     public void testGetConfig() throws Exception {
         // Property set in test setup.
         String path = "/v1/server/config/" + Entitlements.GLOBAL_ENTITLEMENT_MANAGER.getName();
+        assert401(path);
         assertPermitted("myRoot", path);
         assertForbidden("myUser", path);
         assertForbidden("myReadonly", path);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/cd07d816/rest/rest-server/src/test/java/org/apache/brooklyn/rest/resources/ServerResourceIntegrationTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/resources/ServerResourceIntegrationTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/resources/ServerResourceIntegrationTest.java
index 604d1eb..2436b91 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/resources/ServerResourceIntegrationTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/resources/ServerResourceIntegrationTest.java
@@ -87,7 +87,7 @@ public class ServerResourceIntegrationTest extends BrooklynRestApiLauncherTestFi
                     uri, args, args);
             HttpAsserts.assertHealthyStatusCode(response.getResponseCode());
     
-            // Has no gone back to credentials from brooklynProperties; TestSecurityProvider credentials no longer work
+            // Has now gone back to credentials from brooklynProperties; TestSecurityProvider credentials no longer work
             response = HttpTool.httpPost(httpClientBuilder().uri(baseUri).credentials(defaultCredential).build(), 
                     uri, args, args);
             HttpAsserts.assertHealthyStatusCode(response.getResponseCode());


[3/7] brooklyn-server git commit: Non-HOT HA modes don't allow app-related requests in any more.

Posted by al...@apache.org.
Non-HOT HA modes don't allow app-related requests in any more.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/113f53b0
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/113f53b0
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/113f53b0

Branch: refs/heads/master
Commit: 113f53b01804b2a9e08f5135dfb7887605257446
Parents: de0af81
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Nov 14 12:10:52 2016 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Mon Nov 14 12:10:52 2016 +0200

----------------------------------------------------------------------
 .../java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java    | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/113f53b0/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
index db58f99..83d00f5 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/HaMasterCheckFilterTest.java
@@ -78,7 +78,7 @@ public class HaMasterCheckFilterTest extends BrooklynRestApiLauncherTestFixture
     @Test(groups = "Integration")
     public void testEntitiesExistOnMasterPromotion() throws Exception {
         initHaCluster(HighAvailabilityMode.AUTO, HighAvailabilityMode.AUTO);
-        assertEntityNotFound(new ReturnCodeNotRetry());
+        assertEquals(getAppResponseCode(), 403);
         stopWriteNode();
         assertEntityExists(new ReturnCodeNotRetryAndNodeIsMaster());
         assertReadIsMaster();
@@ -122,7 +122,6 @@ public class HaMasterCheckFilterTest extends BrooklynRestApiLauncherTestFixture
     private String createApp(ManagementContext mgmt) {
         EntityManager entityMgr = mgmt.getEntityManager();
         Entity app = entityMgr.createEntity(EntitySpec.create(BasicApplication.class));
-        entityMgr.manage(app);
         return app.getId();
     }
 


[2/7] brooklyn-server git commit: Don't cache the delegatinProvider in a static field as that's causing test failures (when reloading properties with a different security provider). DelegatingSecurityProvider caches the delegate in mgmt so re-creating it

Posted by al...@apache.org.
Don't cache the delegatinProvider in a static field as that's causing test failures (when reloading properties with a different security provider). DelegatingSecurityProvider caches the delegate in mgmt so re-creating it for the same mgmt is cheap.


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/de0af81a
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/de0af81a
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/de0af81a

Branch: refs/heads/master
Commit: de0af81a09aeec28d9b6dcef498299fd379c095c
Parents: cd07d81
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Nov 14 12:09:57 2016 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Mon Nov 14 12:09:57 2016 +0200

----------------------------------------------------------------------
 .../rest/security/jaas/BrooklynLoginModule.java      | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/de0af81a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/BrooklynLoginModule.java
----------------------------------------------------------------------
diff --git a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/BrooklynLoginModule.java b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/BrooklynLoginModule.java
index 75ddd32..6fc2efe 100644
--- a/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/BrooklynLoginModule.java
+++ b/rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/jaas/BrooklynLoginModule.java
@@ -139,7 +139,6 @@ public class BrooklynLoginModule implements LoginModule {
     private Map<String, ?> options;
     private BundleContext bundleContext;
 
-    private static DelegatingSecurityProvider defaultProvider;
     private HttpSession providerSession;
 
     private SecurityProvider provider;
@@ -152,18 +151,8 @@ public class BrooklynLoginModule implements LoginModule {
     public BrooklynLoginModule() {
     }
 
-    private SecurityProvider getDefaultProvider() {
-        if (defaultProvider == null) {
-            createDefaultSecurityProvider(getManagementContext());
-        }
-        return defaultProvider;
-    }
-
     private synchronized static SecurityProvider createDefaultSecurityProvider(ManagementContext mgmt) {
-        if (defaultProvider == null) {
-            defaultProvider = new DelegatingSecurityProvider(mgmt);
-        }
-        return defaultProvider;
+        return new DelegatingSecurityProvider(mgmt);
     }
 
     private ManagementContext getManagementContext() {
@@ -218,7 +207,7 @@ public class BrooklynLoginModule implements LoginModule {
             }
         } else {
             log.debug("Delegating security provider loading to Brooklyn.");
-            provider = getDefaultProvider();
+            provider = createDefaultSecurityProvider(getManagementContext());
         }
 
         log.debug("Using security provider " + provider);


[7/7] brooklyn-server git commit: This closes #434

Posted by al...@apache.org.
This closes #434


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/2e411425
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/2e411425
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/2e411425

Branch: refs/heads/master
Commit: 2e41142591b40744efcf8efa21148204eb609d15
Parents: be89a8d 23117fc
Author: Aled Sage <al...@gmail.com>
Authored: Wed Nov 16 22:32:49 2016 +0000
Committer: Aled Sage <al...@gmail.com>
Committed: Wed Nov 16 22:32:49 2016 +0000

----------------------------------------------------------------------
 launcher/src/main/resources/web-security.xml    |  51 -----
 .../BrooklynEntityMirrorIntegrationTest.java    |   4 +-
 .../launcher/BrooklynWebServerTest.java         |  61 +++--
 pom.xml                                         |   2 +-
 .../rest/security/jaas/BrooklynLoginModule.java |  15 +-
 .../src/main/resources/web-security.xml         |  51 +++++
 .../brooklyn/rest/BrooklynRestApiLauncher.java  |  15 +-
 .../brooklyn/rest/HaMasterCheckFilterTest.java  |   3 +-
 .../AbstractRestApiEntitlementsTest.java        |  21 +-
 .../ActivityApiEntitlementsTest.java            |  17 +-
 .../EntityConfigApiEntitlementsTest.java        |   2 +
 .../entitlement/ScriptApiEntitlementsTest.java  |  14 +-
 .../entitlement/SensorApiEntitlementsTest.java  |   2 +
 .../entitlement/ServerApiEntitlementsTest.java  |   3 +
 .../ServerResourceIntegrationTest.java          |   2 +-
 ...rooklynJacksonSerializerIntegrationTest.java | 223 ++++++++++---------
 ...leShellCommandDeprecatedIntegrationTest.java |   5 +-
 17 files changed, 281 insertions(+), 210 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2e411425/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------


[5/7] brooklyn-server git commit: Don't lose the security provider configured in tests on reload

Posted by al...@apache.org.
Don't lose the security provider configured in tests on reload


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/bfdf522c
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/bfdf522c
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/bfdf522c

Branch: refs/heads/master
Commit: bfdf522c452df262b3b4bb55cd567ddf45b31fa7
Parents: 819f725
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Nov 14 14:43:28 2016 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Mon Nov 14 14:49:52 2016 +0200

----------------------------------------------------------------------
 .../apache/brooklyn/rest/BrooklynRestApiLauncher.java | 14 +++++++++++---
 .../entitlement/AbstractRestApiEntitlementsTest.java  |  3 ++-
 2 files changed, 13 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bfdf522c/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
index b600159..910ee8f 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java
@@ -120,6 +120,9 @@ public class BrooklynRestApiLauncher {
         return this;
     }
 
+    /**
+     * Note: Lost on brooklyn.properties reload
+     */
     public BrooklynRestApiLauncher securityProvider(Class<? extends SecurityProvider> securityProvider) {
         this.securityProvider = securityProvider;
         return this;
@@ -191,10 +194,15 @@ public class BrooklynRestApiLauncher {
                     : "from custom context";
         }
 
-        if (securityProvider != null && securityProvider != AnyoneSecurityProvider.class) {
-            ((BrooklynProperties) mgmt.getConfig()).put(
-                    BrooklynWebConfig.SECURITY_PROVIDER_CLASSNAME, securityProvider.getName());
+        Maybe<Object> configSecurityProvider = mgmt.getConfig().getConfigLocalRaw(BrooklynWebConfig.SECURITY_PROVIDER_CLASSNAME);
+        boolean hasConfigSecurityProvider = configSecurityProvider.isPresent();
+        boolean hasOverrideSecurityProvider = (securityProvider != null && securityProvider != AnyoneSecurityProvider.class);
+        if (hasOverrideSecurityProvider || hasConfigSecurityProvider) {
             ((WebAppContext)context).addOverrideDescriptor(getClass().getResource("/web-security.xml").toExternalForm());
+            if (hasOverrideSecurityProvider) {
+                ((BrooklynProperties) mgmt.getConfig()).put(
+                        BrooklynWebConfig.SECURITY_PROVIDER_CLASSNAME, securityProvider.getName());
+            }
         } else if (context instanceof WebAppContext) {
             ((WebAppContext)context).setSecurityHandler(new NopSecurityHandler());
         }

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/bfdf522c/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
index c858799..a750d89 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/entitlement/AbstractRestApiEntitlementsTest.java
@@ -35,6 +35,7 @@ import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.rest.BrooklynRestApiLauncher;
 import org.apache.brooklyn.rest.BrooklynRestApiLauncherTestFixture;
+import org.apache.brooklyn.rest.BrooklynWebConfig;
 import org.apache.brooklyn.util.http.HttpAsserts;
 import org.apache.brooklyn.util.http.HttpTool;
 import org.apache.brooklyn.util.http.HttpTool.HttpClientBuilder;
@@ -65,6 +66,7 @@ public abstract class AbstractRestApiEntitlementsTest extends BrooklynRestApiLau
         props.put(PerUserEntitlementManager.PER_USER_ENTITLEMENTS_CONFIG_PREFIX+".myMinimal", "minimal");
         props.put(PerUserEntitlementManager.PER_USER_ENTITLEMENTS_CONFIG_PREFIX+".myUser", "user");
         props.put(PerUserEntitlementManager.PER_USER_ENTITLEMENTS_CONFIG_PREFIX+".myCustom", StaticDelegatingEntitlementManager.class.getName());
+        props.put(BrooklynWebConfig.SECURITY_PROVIDER_CLASSNAME, AuthenticateAnyoneSecurityProvider.class.getName());
         
         mgmt = LocalManagementContextForTests.builder(false).useProperties(props).build();
         app = mgmt.getEntityManager().createEntity(EntitySpec.create(TestApplication.class)
@@ -75,7 +77,6 @@ public abstract class AbstractRestApiEntitlementsTest extends BrooklynRestApiLau
         useServerForTest(BrooklynRestApiLauncher.launcher()
                 .managementContext(mgmt)
                 .forceUseOfDefaultCatalogWithJavaClassPath(true)
-                .securityProvider(AuthenticateAnyoneSecurityProvider.class)
                 .start());
     }
 


[6/7] brooklyn-server git commit: Minor integration test fixes

Posted by al...@apache.org.
Minor integration test fixes

* Request the correct "skipSecurity" mode
* Platform independent ssh result test
* Upgrade surefire to latest available


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/23117fce
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/23117fce
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/23117fce

Branch: refs/heads/master
Commit: 23117fce983b1be6f53434da93bd31bbf3b61c04
Parents: bfdf522
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Nov 14 15:42:48 2016 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Mon Nov 14 15:42:48 2016 +0200

----------------------------------------------------------------------
 .../brooklynnode/BrooklynEntityMirrorIntegrationTest.java       | 4 ++--
 pom.xml                                                         | 2 +-
 .../framework/SimpleShellCommandDeprecatedIntegrationTest.java  | 5 +++--
 3 files changed, 6 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/23117fce/launcher/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorIntegrationTest.java
----------------------------------------------------------------------
diff --git a/launcher/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorIntegrationTest.java b/launcher/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorIntegrationTest.java
index 921de9e..e013884 100644
--- a/launcher/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorIntegrationTest.java
+++ b/launcher/src/test/java/org/apache/brooklyn/entity/brooklynnode/BrooklynEntityMirrorIntegrationTest.java
@@ -78,7 +78,7 @@ public class BrooklynEntityMirrorIntegrationTest {
 
     
     protected void setUpServer() {
-        setUpServer(new LocalManagementContextForTests(), false);
+        setUpServer(new LocalManagementContextForTests(), true);
     }
     protected void setUpServer(ManagementContext mgmt, boolean skipSecurity) {
         try {
@@ -140,7 +140,7 @@ public class BrooklynEntityMirrorIntegrationTest {
         mgmtHttps.getBrooklynProperties().put("brooklyn.webconsole.security.users", "admin");
         mgmtHttps.getBrooklynProperties().put("brooklyn.webconsole.security.user.admin.password", "P5ssW0rd");
 
-        setUpServer(mgmtHttps, true);
+        setUpServer(mgmtHttps, false);
         Assert.assertTrue(getBaseUri().startsWith("https:"), "URL is not https: "+getBaseUri());
         // check auth is required
         HttpTestUtils.assertHttpStatusCodeEquals(getBaseUri(), 401);

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/23117fce/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 01861bf..51618fa 100644
--- a/pom.xml
+++ b/pom.xml
@@ -179,7 +179,7 @@
         <mockito.version>1.10.8</mockito.version>
         <assertj.version>2.2.0</assertj.version> <!-- v 2.2.0 is being used as v 3.20 introduces Java8 dependencies-->
         <cobertura.plugin.version>2.7</cobertura.plugin.version>
-        <surefire.version>2.18.1</surefire.version>
+        <surefire.version>2.19.1</surefire.version>
         <plantuml.version>6121</plantuml.version>
         <ant.version>1.8.4</ant.version>
 

http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/23117fce/test-framework/src/test/java/org/apache/brooklyn/test/framework/SimpleShellCommandDeprecatedIntegrationTest.java
----------------------------------------------------------------------
diff --git a/test-framework/src/test/java/org/apache/brooklyn/test/framework/SimpleShellCommandDeprecatedIntegrationTest.java b/test-framework/src/test/java/org/apache/brooklyn/test/framework/SimpleShellCommandDeprecatedIntegrationTest.java
index f8fdbde..ab24c9f 100644
--- a/test-framework/src/test/java/org/apache/brooklyn/test/framework/SimpleShellCommandDeprecatedIntegrationTest.java
+++ b/test-framework/src/test/java/org/apache/brooklyn/test/framework/SimpleShellCommandDeprecatedIntegrationTest.java
@@ -116,7 +116,7 @@ public class SimpleShellCommandDeprecatedIntegrationTest extends BrooklynAppUnit
 
         SimpleShellCommandTest uptime = app.createAndManageChild(EntitySpec.create(SimpleShellCommandTest.class)
             .configure(TARGET_ENTITY, testEntity)
-            .configure(COMMAND, "uptime"));
+            .configure(COMMAND, "date"));
 
         app.start(ImmutableList.<Location>of());
 
@@ -138,7 +138,8 @@ public class SimpleShellCommandDeprecatedIntegrationTest extends BrooklynAppUnit
         try {
             app.start(ImmutableList.<Location>of());
         } catch (Throwable t) {
-            Asserts.expectedFailureContains(t, "exit code expected equals 0 but found 1");
+            // Used to be "but found 1", I'm getting "but found 2". Could be OS specific.
+            Asserts.expectedFailureContains(t, "exit code expected equals 0 but found ");
         }
 
         assertThat(uptime.sensors().get(SERVICE_UP)).isFalse()


[4/7] brooklyn-server git commit: Clean up BrooklynJacksonSerializerIntegrationTest and mark test as Broken

Posted by al...@apache.org.
Clean up BrooklynJacksonSerializerIntegrationTest and mark test as Broken


Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/819f7252
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/819f7252
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/819f7252

Branch: refs/heads/master
Commit: 819f7252ffbb394036da169fe48aac8205ae7fc5
Parents: 113f53b
Author: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Authored: Mon Nov 14 13:54:29 2016 +0200
Committer: Svetoslav Neykov <sv...@cloudsoftcorp.com>
Committed: Mon Nov 14 13:54:29 2016 +0200

----------------------------------------------------------------------
 ...rooklynJacksonSerializerIntegrationTest.java | 223 ++++++++++---------
 1 file changed, 120 insertions(+), 103 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/819f7252/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/json/BrooklynJacksonSerializerIntegrationTest.java
----------------------------------------------------------------------
diff --git a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/json/BrooklynJacksonSerializerIntegrationTest.java b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/json/BrooklynJacksonSerializerIntegrationTest.java
index 1cf79e8..2bb3203 100644
--- a/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/json/BrooklynJacksonSerializerIntegrationTest.java
+++ b/rest/rest-server/src/test/java/org/apache/brooklyn/rest/util/json/BrooklynJacksonSerializerIntegrationTest.java
@@ -27,11 +27,14 @@ import javax.ws.rs.core.MediaType;
 import org.apache.brooklyn.api.entity.Entity;
 import org.apache.brooklyn.api.mgmt.ManagementContext;
 import org.apache.brooklyn.core.entity.Entities;
+import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal;
+import org.apache.brooklyn.core.test.BrooklynAppUnitTestSupport;
 import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests;
 import org.apache.brooklyn.core.test.entity.TestApplication;
 import org.apache.brooklyn.core.test.entity.TestEntity;
 import org.apache.brooklyn.rest.BrooklynRestApiLauncher;
 import org.apache.brooklyn.rest.util.json.BrooklynJacksonSerializerTest.SelfRefNonSerializableClass;
+import org.apache.brooklyn.util.exceptions.Exceptions;
 import org.apache.brooklyn.util.http.HttpTool;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.utils.URIBuilder;
@@ -40,15 +43,49 @@ import org.eclipse.jetty.server.Server;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.testng.Assert;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.gson.Gson;
 
-public class BrooklynJacksonSerializerIntegrationTest {
+public class BrooklynJacksonSerializerIntegrationTest extends BrooklynAppUnitTestSupport {
 
     private static final Logger log = LoggerFactory.getLogger(BrooklynJacksonSerializerIntegrationTest.class);
     
+    private Server server;
+    private HttpClient client;
+    URI entityUrl;
+    URI configUri;
+
+    @BeforeMethod(alwaysRun = true)
+    public void setUp() throws Exception {
+        super.setUp();
+        server = BrooklynRestApiLauncher.launcher().managementContext(mgmt).start();
+        client = HttpTool.httpClientBuilder().build();
+
+        String serverAddress = "http://localhost:"+((NetworkConnector)server.getConnectors()[0]).getLocalPort();
+        String appUrl = serverAddress + "/v1/applications/" + app.getId();
+        entityUrl = URI.create(appUrl + "/entities/" + app.getId());
+        configUri = new URIBuilder(entityUrl + "/config/" + TestEntity.CONF_OBJECT.getName())
+                .addParameter("raw", "true")
+                .build();
+
+    }
+    
+    @AfterMethod(alwaysRun = true)
+    @Override
+    public void tearDown() throws Exception {
+        try {
+            if (server != null) server.stop();
+        } catch (Exception e) {
+            Exceptions.propagateIfFatal(e);
+            log.warn("failed to stop server: "+e);
+        }
+        super.tearDown();
+    }
+    
     // Ensure TEXT_PLAIN just returns toString for ManagementContext instance.
     // Strangely, testWithLauncherSerializingListsContainingEntitiesAndOtherComplexStuff ended up in the 
     // EntityConfigResource.getPlain code, throwing a ClassCastException.
@@ -57,117 +94,97 @@ public class BrooklynJacksonSerializerIntegrationTest {
     // testWithLauncherSerializingListsContainingEntitiesAndOtherComplexStuff was calling it.
     @Test(groups="Integration") //because of time
     public void testWithAcceptsPlainText() throws Exception {
-        ManagementContext mgmt = LocalManagementContextForTests.newInstance();
-        Server server = null;
-        try {
-            server = BrooklynRestApiLauncher.launcher().managementContext(mgmt).start();
-            HttpClient client = HttpTool.httpClientBuilder().build();
-
-            TestApplication app = TestApplication.Factory.newManagedInstanceForTests(mgmt);
-
-            String serverAddress = "http://localhost:"+((NetworkConnector)server.getConnectors()[0]).getLocalPort();
-            String appUrl = serverAddress + "/v1/applications/" + app.getId();
-            String entityUrl = appUrl + "/entities/" + app.getId();
-            URI configUri = new URIBuilder(entityUrl + "/config/" + TestEntity.CONF_OBJECT.getName())
-                    .addParameter("raw", "true")
-                    .build();
-
-            // assert config here is just mgmt.toString()
-            app.config().set(TestEntity.CONF_OBJECT, mgmt);
-            String content = get(client, configUri, ImmutableMap.of("Accept", MediaType.TEXT_PLAIN));
-            log.info("CONFIG MGMT is:\n"+content);
-            Assert.assertEquals(content, mgmt.toString(), "content="+content);
-            
-        } finally {
-            try {
-                if (server != null) server.stop();
-            } catch (Exception e) {
-                log.warn("failed to stop server: "+e);
-            }
-            Entities.destroyAll(mgmt);
-        }
+        // assert config here is just mgmt.toString()
+        setConfig(mgmt);
+        String content = getConfigValueAsText();
+        log.info("CONFIG MGMT is:\n"+content);
+        Assert.assertEquals(content, mgmt.toString(), "content="+content);
     }
-        
+
     @Test(groups="Integration") //because of time
-    public void testWithLauncherSerializingListsContainingEntitiesAndOtherComplexStuff() throws Exception {
-        ManagementContext mgmt = LocalManagementContextForTests.newInstance();
-        Server server = null;
-        try {
-            server = BrooklynRestApiLauncher.launcher().managementContext(mgmt).start();
-            HttpClient client = HttpTool.httpClientBuilder().build();
-
-            TestApplication app = TestApplication.Factory.newManagedInstanceForTests(mgmt);
-
-            String serverAddress = "http://localhost:"+((NetworkConnector)server.getConnectors()[0]).getLocalPort();
-            String appUrl = serverAddress + "/v1/applications/" + app.getId();
-            String entityUrl = appUrl + "/entities/" + app.getId();
-            URI configUri = new URIBuilder(entityUrl + "/config/" + TestEntity.CONF_OBJECT.getName())
-                    .addParameter("raw", "true")
-                    .build();
-
-            // assert config here is just mgmt
-            app.config().set(TestEntity.CONF_OBJECT, mgmt);
-            String content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
-            log.info("CONFIG MGMT is:\n"+content);
-            @SuppressWarnings("rawtypes")
-            Map values = new Gson().fromJson(content, Map.class);
-            Assert.assertEquals(values, ImmutableMap.of("type", LocalManagementContextForTests.class.getCanonicalName()), "values="+values);
-
-            // assert normal API returns the same, containing links
-            content = get(client, entityUrl, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
-            log.info("ENTITY is: \n"+content);
-            values = new Gson().fromJson(content, Map.class);
-            Assert.assertTrue(values.size()>=3, "Map is too small: "+values);
-            Assert.assertTrue(values.size()<=6, "Map is too big: "+values);
-            Assert.assertEquals(values.get("type"), TestApplication.class.getCanonicalName(), "values="+values);
-            Assert.assertNotNull(values.get("links"), "Map should have contained links: values="+values);
-
-            // but config etc returns our nicely json serialized
-            app.config().set(TestEntity.CONF_OBJECT, app);
-            content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
-            log.info("CONFIG ENTITY is:\n"+content);
-            values = new Gson().fromJson(content, Map.class);
-            Assert.assertEquals(values, ImmutableMap.of("type", Entity.class.getCanonicalName(), "id", app.getId()), "values="+values);
-
-            // and self-ref gives error + toString
-            SelfRefNonSerializableClass angry = new SelfRefNonSerializableClass();
-            app.config().set(TestEntity.CONF_OBJECT, angry);
-            content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
-            log.info("CONFIG ANGRY is:\n"+content);
-            assertErrorObjectMatchingToString(content, angry);
-            
-            // as does Server
-            app.config().set(TestEntity.CONF_OBJECT, server);
-            content = get(client, configUri, ImmutableMap.of("Accept", MediaType.APPLICATION_JSON));
-            // NOTE, if using the default visibility / object mapper, the getters of the object are invoked
-            // resulting in an object which is huge, 7+MB -- and it wreaks havoc w eclipse console regex parsing!
-            // (but with our custom VisibilityChecker server just gives us the nicer error!)
-            log.info("CONFIG SERVER is:\n"+content);
-            assertErrorObjectMatchingToString(content, server);
-            Assert.assertTrue(content.contains(NotSerializableException.class.getCanonicalName()), "server should have contained things which are not serializable");
-            Assert.assertTrue(content.length() < 1024, "content should not have been very long; instead was: "+content.length());
-            
-        } finally {
-            try {
-                if (server != null) server.stop();
-            } catch (Exception e) {
-                log.warn("failed to stop server: "+e);
-            }
-            Entities.destroyAll(mgmt);
-        }
+    public void testWithMgmt() throws Exception {
+        setConfig(mgmt);
+        Map<?, ?> values = getConfigValueAsJson();
+        Assert.assertEquals(values, ImmutableMap.of("type", LocalManagementContextForTests.class.getCanonicalName()), "values="+values);
+
+        // assert normal API returns the same, containing links
+        values = getRestValueAsJson(entityUrl);
+        Assert.assertTrue(values.size()>=3, "Map is too small: "+values);
+        Assert.assertTrue(values.size()<=6, "Map is too big: "+values);
+        Assert.assertEquals(values.get("type"), TestApplication.class.getCanonicalName(), "values="+values);
+        Assert.assertNotNull(values.get("links"), "Map should have contained links: values="+values);
+
+    }
+
+    @Test(groups="Integration") //because of time
+    public void testWithApp() throws Exception {
+        // but config etc returns our nicely json serialized
+        setConfig(app);
+        Map<?, ?> values = getConfigValueAsJson();
+        Assert.assertEquals(values, ImmutableMap.of("type", Entity.class.getCanonicalName(), "id", app.getId()), "values="+values);
     }
 
-    private void assertErrorObjectMatchingToString(String content, Object expected) {
-        Object value = new Gson().fromJson(content, Object.class);
-        Assert.assertTrue(value instanceof Map, "Expected map, got: "+value);
-        Assert.assertEquals(((Map<?,?>)value).get("toString"), expected.toString());
+    @Test(groups="Integration") //because of time
+    public void testWithCyclic() throws Exception {
+        // and self-ref gives error + toString
+        SelfRefNonSerializableClass angry = new SelfRefNonSerializableClass();
+        setConfig(angry);
+        Map<?, ?> values = getConfigValueAsJson();
+        assertErrorObjectMatchingToString(values, angry);
+        
     }
 
-    private String get(HttpClient client, String uri, Map<String, String> headers) {
-        return get(client, URI.create(uri), headers);
+    // Broken. Serialization is failing because of "group" -> "threads" -> "group" cycle inside server (through the thread pool).
+    // It's doing best effort though - still serializing what it was able to before giving up. This results in a huge string
+    // which fails the assertions.
+    @Test(groups={"Integration", "Broken"}) //because of time
+    public void testWithServer() throws Exception {
+        // as does Server
+        setConfig(server);
+        Map<?, ?> values = getConfigValueAsJson();
+        // NOTE, if using the default visibility / object mapper, the getters of the object are invoked
+        // resulting in an object which is huge, 7+MB -- and it wreaks havoc w eclipse console regex parsing!
+        // (but with our custom VisibilityChecker server just gives us the nicer error!)
+        String content = getRestValue(configUri, MediaType.APPLICATION_JSON);
+        log.info("CONFIG is:\n"+content);
+        values = new Gson().fromJson(content, Map.class);
+        assertErrorObjectMatchingToString(values, server);
+        Assert.assertTrue(content.contains(NotSerializableException.class.getCanonicalName()), "server should have contained things which are not serializable");
+        Assert.assertTrue(content.length() < 1024, "content should not have been very long; instead was: "+content.length());
+    }
+
+    private void assertErrorObjectMatchingToString(Map<?, ?> content, Object expected) {
+        Assert.assertEquals(content.get("toString"), expected.toString());
     }
 
     private String get(HttpClient client, URI uri, Map<String, String> headers) {
         return HttpTool.httpGet(client, uri, headers).getContentAsString();
     }
+
+    protected String getConfigValueAsText() {
+        return getRestValueAsText(configUri);
+    }
+
+    protected String getRestValueAsText(URI url) {
+        return getRestValue(url, MediaType.TEXT_PLAIN);
+    }
+
+    protected Map<?, ?> getConfigValueAsJson() {
+        return getRestValueAsJson(configUri);
+    }
+
+    protected Map<?, ?> getRestValueAsJson(URI url) {
+        String content = getRestValue(url, MediaType.APPLICATION_JSON);
+        log.info("CONFIG is:\n"+content);
+        return new Gson().fromJson(content, Map.class);
+    }
+
+    protected String getRestValue(URI url, String contentType) {
+        return get(client, url, ImmutableMap.of("Accept", contentType));
+    }
+        
+    protected void setConfig(Object value) {
+        app.config().set(TestEntity.CONF_OBJECT, value);
+    }
+
 }