From d332bcc4d60868736716990660f388c648dd8546 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 14 Mar 2016 16:53:54 +0000 Subject: [PATCH] does not create a localhost named location any longer. the wizard (coming soon) will do this for us. --- .../core/location/BasicLocationRegistry.java | 30 +++------ .../localhost/LocalhostLocationResolver.java | 9 ++- .../core/location/LocationRegistryTest.java | 61 ++++++++++++++++++- .../ApplicationResourceIntegrationTest.java | 2 +- .../client/BrooklynApiRestClientTest.java | 2 +- .../rest/testing/BrooklynRestApiTest.java | 2 +- .../rest/testing/BrooklynRestApiTest.java | 2 +- 7 files changed, 79 insertions(+), 29 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java b/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java index 0dc88d0213..c7d7127499 100644 --- a/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java +++ b/core/src/main/java/org/apache/brooklyn/core/location/BasicLocationRegistry.java @@ -47,6 +47,7 @@ import org.apache.brooklyn.core.mgmt.internal.LocalLocationManager; import org.apache.brooklyn.core.mgmt.internal.ManagementContextInternal; import org.apache.brooklyn.core.typereg.RegisteredTypePredicates; +import org.apache.brooklyn.location.localhost.LocalhostLocationResolver; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.config.ConfigBag; @@ -278,16 +279,6 @@ public void updateDefinedLocations() { } if (log.isDebugEnabled()) log.debug("Found "+count+" defined locations from properties (*.named.* syntax): "+definedLocations.values()); - if (getDefinedLocationByName("localhost")==null && !BasicOsDetails.Factory.newLocalhostInstance().isWindows() - && LocationConfigUtils.isEnabled(mgmt, "brooklyn.location.localhost")) { - log.debug("Adding a defined location for localhost"); - // add 'localhost' *first* - ImmutableMap oldDefined = ImmutableMap.copyOf(definedLocations); - definedLocations.clear(); - String id = Identifiers.makeRandomId(8); - definedLocations.put(id, localhost(id)); - definedLocations.putAll(oldDefined); - } for (RegisteredType item: mgmt.getTypeRegistry().getMatching(RegisteredTypePredicates.IS_LOCATION)) { updateDefinedLocation(item); @@ -296,13 +287,7 @@ public void updateDefinedLocations() { } } - @VisibleForTesting - void disablePersistence() { - // persistence isn't enabled yet anyway (have to manually save things, - // defining the format and file etc) - } - - protected static BasicLocationDefinition localhost(String id) { + private static BasicLocationDefinition localhost(String id) { return new BasicLocationDefinition(id, "localhost", "localhost", null); } @@ -477,12 +462,15 @@ public void putProperties(Map vals) { } @VisibleForTesting - public static void setupLocationRegistryForTesting(ManagementContext mgmt) { - // ensure localhost is added (even on windows) + public static void addNamedLocationLocalhost(ManagementContext mgmt) { + if (!mgmt.getConfig().getConfig(LocalhostLocationResolver.LOCALHOST_ENABLED)) { + throw new IllegalStateException("Localhost is disabled."); + } + + // ensure localhost is added (even on windows, it's just for testing) LocationDefinition l = mgmt.getLocationRegistry().getDefinedLocationByName("localhost"); if (l==null) mgmt.getLocationRegistry().updateDefinedLocation( BasicLocationRegistry.localhost(Identifiers.makeRandomId(8)) ); - - ((BasicLocationRegistry)mgmt.getLocationRegistry()).disablePersistence(); } + } diff --git a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java index 81a05f3180..2c8f827399 100644 --- a/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java +++ b/core/src/main/java/org/apache/brooklyn/location/localhost/LocalhostLocationResolver.java @@ -23,7 +23,10 @@ import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationRegistry; import org.apache.brooklyn.api.location.LocationResolver; +import org.apache.brooklyn.config.ConfigKey; +import org.apache.brooklyn.core.config.ConfigKeys; import org.apache.brooklyn.core.location.AbstractLocationResolver; +import org.apache.brooklyn.core.location.LocationConfigKeys; import org.apache.brooklyn.core.location.LocationConfigUtils; import org.apache.brooklyn.util.core.config.ConfigBag; @@ -40,6 +43,10 @@ */ public class LocalhostLocationResolver extends AbstractLocationResolver implements LocationResolver.EnableableLocationResolver { + private static String BROOKLYN_LOCATION_LOCALHOST_PREFIX = "brooklyn.location.localhost"; + + public static ConfigKey LOCALHOST_ENABLED = ConfigKeys.newConfigKeyWithPrefix(BROOKLYN_LOCATION_LOCALHOST_PREFIX+".", LocationConfigKeys.ENABLED); + public static final String LOCALHOST = "localhost"; @Override @@ -49,7 +56,7 @@ public String getPrefix() { @Override public boolean isEnabled() { - return LocationConfigUtils.isEnabled(managementContext, "brooklyn.location.localhost"); + return LocationConfigUtils.isEnabled(managementContext, BROOKLYN_LOCATION_LOCALHOST_PREFIX); } @Override diff --git a/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java b/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java index abb4d2c1bc..59ba4b4bfe 100644 --- a/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/location/LocationRegistryTest.java @@ -20,6 +20,7 @@ import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationDefinition; +import org.apache.brooklyn.api.location.LocationSpec; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.internal.BrooklynProperties; import org.apache.brooklyn.core.location.BasicLocationRegistry; @@ -27,6 +28,9 @@ import org.apache.brooklyn.core.mgmt.internal.LocalManagementContext; import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; import org.apache.brooklyn.location.byon.FixedListMachineProvisioningLocation; +import org.apache.brooklyn.location.localhost.LocalhostLocationResolver; +import org.apache.brooklyn.test.Asserts; +import org.apache.brooklyn.util.guava.Maybe; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testng.Assert; @@ -110,7 +114,7 @@ public void testLocationGetsDefaultDisplayName() { @Test public void testSetupForTesting() { mgmt = LocalManagementContextForTests.newInstance(); - BasicLocationRegistry.setupLocationRegistryForTesting(mgmt); + BasicLocationRegistry.addNamedLocationLocalhost(mgmt); Assert.assertNotNull(mgmt.getLocationRegistry().getDefinedLocationByName("localhost")); } @@ -145,17 +149,68 @@ public void testLocalhostEnabled() { BrooklynProperties properties = BrooklynProperties.Factory.newEmpty(); properties.put("brooklyn.location.localhost.enabled", true); mgmt = LocalManagementContextForTests.newInstance(properties); + BasicLocationRegistry.addNamedLocationLocalhost(mgmt); + Assert.assertTrue( findLocationMatching("localhost") ); } @Test - public void testLocalhostDisabled() { + public void testLocalhostNotPresentByDefault() { BrooklynProperties properties = BrooklynProperties.Factory.newEmpty(); - properties.put("brooklyn.location.localhost.enabled", false); + properties.put(LocalhostLocationResolver.LOCALHOST_ENABLED.getName(), false); mgmt = LocalManagementContextForTests.newInstance(properties); + log.info("RESOLVERS: "+mgmt.getLocationRegistry().getDefinedLocations()); log.info("DEFINED LOCATIONS: "+mgmt.getLocationRegistry().getDefinedLocations()); Assert.assertFalse( findLocationMatching("localhost") ); } + + @Test + public void testLocalhostAllowedByDefault() { + BrooklynProperties properties = BrooklynProperties.Factory.newEmpty(); + properties.put("brooklyn.location.named.localhost_allowed", "localhost"); + mgmt = LocalManagementContextForTests.newInstance(properties); + + Assert.assertTrue( findLocationMatching("localhost_allowed") ); + Maybe l = mgmt.getLocationRegistry().resolve("localhost_allowed", false, null); + Assert.assertTrue( l.isPresent(), "Should have resolved: "+l ); + l.get(); + } + + @Test + public void testNonsenseParentSupported() { + BrooklynProperties properties = BrooklynProperties.Factory.newEmpty(); + properties.put(LocalhostLocationResolver.LOCALHOST_ENABLED.getName(), false); + properties.put("brooklyn.location.named.bogus_will_fail_eventually", "totally_bogus"); + mgmt = LocalManagementContextForTests.newInstance(properties); + + Assert.assertTrue( findLocationMatching("bogus_will_fail_eventually") ); + Maybe l = mgmt.getLocationRegistry().resolve("bogus_will_fail_eventually", false, null); + Assert.assertTrue( l.isAbsent(), "Should not have resolved: "+l ); + try { + l.get(); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + Asserts.expectedFailureContains(e, "bogus_will_fail", "totally_bogus"); + } + } + + @Test + public void testLocalhostDisallowedIfDisabled() { + BrooklynProperties properties = BrooklynProperties.Factory.newEmpty(); + properties.put(LocalhostLocationResolver.LOCALHOST_ENABLED.getName(), false); + properties.put("brooklyn.location.named.local_host_not_allowed", "localhost"); + mgmt = LocalManagementContextForTests.newInstance(properties); + + Assert.assertTrue( findLocationMatching("local_host_not_allowed") ); + Maybe l = mgmt.getLocationRegistry().resolve("local_host_not_allowed", false, null); + Assert.assertTrue( l.isAbsent(), "Should not have resolved: "+l ); + try { + l.get(); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + Asserts.expectedFailureContains(e, "local_host", "localhost"); + } + } } diff --git a/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/ApplicationResourceIntegrationTest.java b/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/ApplicationResourceIntegrationTest.java index 4db359b415..81d789f880 100644 --- a/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/ApplicationResourceIntegrationTest.java +++ b/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/ApplicationResourceIntegrationTest.java @@ -75,7 +75,7 @@ protected synchronized ManagementContext getManagementContext() throws Exception if (manager == null) { manager = new LocalManagementContext(); BrooklynRestApiLauncherTest.forceUseOfDefaultCatalogWithJavaClassPath(manager); - BasicLocationRegistry.setupLocationRegistryForTesting(manager); + BasicLocationRegistry.addNamedLocationLocalhost(manager); BrooklynRestApiLauncherTest.enableAnyoneLogin(manager); } return manager; diff --git a/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/BrooklynApiRestClientTest.java b/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/BrooklynApiRestClientTest.java index f933e57e1e..96c898e9b9 100644 --- a/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/BrooklynApiRestClientTest.java +++ b/rest/rest-client/src/test/java/org/apache/brooklyn/rest/client/BrooklynApiRestClientTest.java @@ -65,7 +65,7 @@ protected synchronized ManagementContext getManagementContext() throws Exception if (manager == null) { manager = new LocalManagementContext(); BrooklynRestApiLauncherTest.forceUseOfDefaultCatalogWithJavaClassPath(manager); - BasicLocationRegistry.setupLocationRegistryForTesting(manager); + BasicLocationRegistry.addNamedLocationLocalhost(manager); BrooklynRestApiLauncherTest.enableAnyoneLogin(manager); } return manager; diff --git a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java index 853afb07cf..1133080f72 100644 --- a/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java +++ b/rest/rest-resources/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java @@ -146,7 +146,7 @@ protected synchronized ManagementContext getManagementContext() { manager = new LocalManagementContextForTests(); } manager.getHighAvailabilityManager().disabled(); - BasicLocationRegistry.setupLocationRegistryForTesting(manager); + BasicLocationRegistry.addNamedLocationLocalhost(manager); new BrooklynCampPlatformLauncherNoServer() .useManagementContext(manager) diff --git a/rest/rest-server-jersey/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java b/rest/rest-server-jersey/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java index bf3f8aff8c..09d0a25963 100644 --- a/rest/rest-server-jersey/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java +++ b/rest/rest-server-jersey/src/test/java/org/apache/brooklyn/rest/testing/BrooklynRestApiTest.java @@ -85,7 +85,7 @@ protected synchronized ManagementContext getManagementContext() { manager = new LocalManagementContextForTests(); } manager.getHighAvailabilityManager().disabled(); - BasicLocationRegistry.setupLocationRegistryForTesting(manager); + BasicLocationRegistry.addNamedLocationLocalhost(manager); new BrooklynCampPlatformLauncherNoServer() .useManagementContext(manager)