From 357d3fab9136a9a279f9d19ead929a864f5641f2 Mon Sep 17 00:00:00 2001 From: Alex Rudyy Date: Fri, 3 May 2013 11:20:54 +0000 Subject: QPID-4802: In management mode set state to ERRORED for failing to activate authentication providers, group providers and acl providers in order to allow editing of attributes preventing normal startup git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1478731 13f79535-47bb-0310-9956-ffa450edef68 --- .../store/ManagementModeStoreHandler.java | 11 +- .../qpid/server/model/adapter/AbstractAdapter.java | 23 +- .../adapter/AccessControlProviderAdapter.java | 23 +- .../adapter/AuthenticationProviderAdapter.java | 81 +++++-- .../server/model/adapter/GroupProviderAdapter.java | 72 +++++- .../qpid/server/model/adapter/PortAdapter.java | 4 +- .../model/ConfiguredObjectStateTransitionTest.java | 245 +++++++++++++++++++++ 7 files changed, 418 insertions(+), 41 deletions(-) create mode 100644 qpid/java/broker/src/test/java/org/apache/qpid/server/model/ConfiguredObjectStateTransitionTest.java (limited to 'qpid/java/broker') diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java index abf1537ef7..d08deee29b 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/store/ManagementModeStoreHandler.java @@ -13,6 +13,8 @@ import org.apache.qpid.server.configuration.ConfigurationEntry; import org.apache.qpid.server.configuration.ConfigurationEntryStore; import org.apache.qpid.server.configuration.IllegalConfigurationException; import org.apache.qpid.server.model.AccessControlProvider; +import org.apache.qpid.server.model.AuthenticationProvider; +import org.apache.qpid.server.model.GroupProvider; import org.apache.qpid.server.model.Port; import org.apache.qpid.server.model.Protocol; import org.apache.qpid.server.model.State; @@ -27,9 +29,12 @@ public class ManagementModeStoreHandler implements ConfigurationEntryStore private static final String PORT_TYPE = Port.class.getSimpleName(); private static final String VIRTUAL_HOST_TYPE = VirtualHost.class.getSimpleName(); private static final String ACCESS_CONTROL_PROVIDER_TYPE = AccessControlProvider.class.getSimpleName(); + private static final String GROUP_PROVIDER_TYPE = GroupProvider.class.getSimpleName(); + private static final String AUTHENTICATION_PROVIDER_TYPE = AuthenticationProvider.class.getSimpleName(); private static final String ATTRIBUTE_STATE = VirtualHost.STATE; private static final Object MANAGEMENT_MODE_AUTH_PROVIDER = "mm-auth"; + private final ConfigurationEntryStore _store; private final Map _cliEntries; private final Map _quiescedEntries; @@ -255,11 +260,7 @@ public class ManagementModeStoreHandler implements ConfigurationEntryStore { quiesce = true; } - else if (ACCESS_CONTROL_PROVIDER_TYPE.equals(entryType)) - { - quiesce = true; - } - else if (PORT_TYPE.equalsIgnoreCase(entryType)) + else if (PORT_TYPE.equals(entryType)) { if (attributes == null) { diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java index 51fd3a5c78..a8c3f54530 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AbstractAdapter.java @@ -351,15 +351,7 @@ abstract class AbstractAdapter implements ConfiguredObject protected void changeAttributes(final Map attributes) { - if (attributes.containsKey(ID)) - { - UUID id = getId(); - Object idAttributeValue = attributes.get(ID); - if (idAttributeValue != null && !idAttributeValue.equals(id)) - { - throw new IllegalConfigurationException("Cannot change existing configured object id"); - } - } + validateChangeAttributes(attributes); Collection names = getAttributeNames(); for (String name : names) { @@ -375,6 +367,19 @@ abstract class AbstractAdapter implements ConfiguredObject } } + protected void validateChangeAttributes(final Map attributes) + { + if (attributes.containsKey(ID)) + { + UUID id = getId(); + Object idAttributeValue = attributes.get(ID); + if (idAttributeValue != null && !idAttributeValue.equals(id.toString())) + { + throw new IllegalConfigurationException("Cannot change existing configured object id"); + } + } + } + protected void authoriseSetDesiredState(State currentState, State desiredState) throws AccessControlException { // allowed by default diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java index 75b80eb56c..a6fe191523 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AccessControlProviderAdapter.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; +import org.apache.log4j.Logger; import org.apache.qpid.server.model.AccessControlProvider; import org.apache.qpid.server.model.Broker; import org.apache.qpid.server.model.ConfiguredObject; @@ -43,6 +44,8 @@ import org.apache.qpid.server.util.MapValueConverter; public class AccessControlProviderAdapter extends AbstractAdapter implements AccessControlProvider { + private static final Logger LOGGER = Logger.getLogger(AccessControlProviderAdapter.class); + protected AccessControl _accessControl; protected final Broker _broker; @@ -217,8 +220,23 @@ public class AccessControlProviderAdapter extends AbstractAdapter implements Acc { if ((state == State.INITIALISING || state == State.QUIESCED) && _state.compareAndSet(state, State.ACTIVE)) { - _accessControl.open(); - return true; + try + { + _accessControl.open(); + return true; + } + catch(RuntimeException e) + { + _state.compareAndSet(State.ACTIVE, State.ERRORED); + if (_broker.isManagementMode()) + { + LOGGER.warn("Failed to activate ACL provider: " + getName(), e); + } + else + { + throw e; + } + } } else { @@ -235,7 +253,6 @@ public class AccessControlProviderAdapter extends AbstractAdapter implements Acc return false; } - return false; } diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java index 24f4757a18..5c4c8a53cd 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderAdapter.java @@ -30,6 +30,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; import javax.security.auth.login.AccountNotFoundException; @@ -58,6 +59,7 @@ import org.apache.qpid.server.security.auth.database.PrincipalDatabase; import org.apache.qpid.server.security.auth.manager.AuthenticationManager; import org.apache.qpid.server.security.auth.manager.PrincipalDatabaseAuthenticationManager; import org.apache.qpid.server.security.SecurityManager; +import org.apache.qpid.server.util.MapValueConverter; public abstract class AuthenticationProviderAdapter extends AbstractAdapter implements AuthenticationProvider { @@ -68,6 +70,7 @@ public abstract class AuthenticationProviderAdapter _supportedAttributes; protected Map _factories; + private AtomicReference _state; private AuthenticationProviderAdapter(UUID id, Broker broker, final T authManager, Map attributes, Collection attributeNames) { @@ -76,6 +79,9 @@ public abstract class AuthenticationProviderAdapter(state); addParent(Broker.class, broker); // set attributes now after all attribute names are known @@ -117,7 +123,7 @@ public abstract class AuthenticationProviderAdapter effectiveAttributes = super.generateEffectiveAttributes(attributes); AuthenticationManager manager = validateAttributes(effectiveAttributes); manager.initialise(); - _authManager = (T)manager; - String type = (String)effectiveAttributes.get(AuthenticationManagerFactory.ATTRIBUTE_TYPE); - AuthenticationManagerFactory managerFactory = _factories.get(type); - _supportedAttributes = createSupportedAttributes(managerFactory.getAttributeNames()); super.changeAttributes(attributes); + _authManager = (T)manager; + + // if provider was previously in ERRORED state then set its state to ACTIVE + _state.compareAndSet(State.ERRORED, State.ACTIVE); } private Map getAuthenticationManagerFactories() @@ -287,6 +340,8 @@ public abstract class AuthenticationProviderAdapter attributes) { + super.validateChangeAttributes(attributes); + String newName = (String)attributes.get(NAME); String currentName = getName(); if (!currentName.equals(newName)) diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java index a0e5bdb0e8..3bf62dc96b 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java @@ -28,7 +28,9 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.log4j.Logger; import org.apache.qpid.server.model.Broker; import org.apache.qpid.server.model.ConfiguredObject; import org.apache.qpid.server.model.Group; @@ -43,13 +45,17 @@ import org.apache.qpid.server.configuration.updater.TaskExecutor; import org.apache.qpid.server.security.access.Operation; import org.apache.qpid.server.security.group.GroupManager; import org.apache.qpid.server.security.SecurityManager; +import org.apache.qpid.server.util.MapValueConverter; public class GroupProviderAdapter extends AbstractAdapter implements GroupProvider { + private static Logger LOGGER = Logger.getLogger(GroupProviderAdapter.class); + private final GroupManager _groupManager; private final Broker _broker; private Collection _supportedAttributes; + private AtomicReference _state; public GroupProviderAdapter(UUID id, Broker broker, GroupManager groupManager, Map attributes, Collection attributeNames) { @@ -62,6 +68,8 @@ public class GroupProviderAdapter extends AbstractAdapter implements _groupManager = groupManager; _broker = broker; _supportedAttributes = createSupportedAttributes(attributeNames); + State state = MapValueConverter.getEnumAttribute(State.class, STATE, attributes, State.INITIALISING); + _state = new AtomicReference(state); addParent(Broker.class, broker); // set attributes now after all attribute names are known @@ -104,7 +112,7 @@ public class GroupProviderAdapter extends AbstractAdapter implements @Override public State getActualState() { - return null; + return _state.get(); } @Override @@ -180,7 +188,7 @@ public class GroupProviderAdapter extends AbstractAdapter implements } else if (STATE.equals(name)) { - return State.ACTIVE; // TODO + return getActualState(); } else if (TIME_TO_LIVE.equals(name)) { @@ -252,21 +260,67 @@ public class GroupProviderAdapter extends AbstractAdapter implements @Override protected boolean setState(State currentState, State desiredState) { + State state = _state.get(); if (desiredState == State.ACTIVE) { - _groupManager.open(); - return true; + if ((state == State.INITIALISING || state == State.QUIESCED || state == State.STOPPED) + && _state.compareAndSet(state, State.ACTIVE)) + { + try + { + _groupManager.open(); + return true; + } + catch(RuntimeException e) + { + _state.compareAndSet(State.ACTIVE, State.ERRORED); + if (_broker.isManagementMode()) + { + LOGGER.warn("Failed to activate group provider: " + getName(), e); + } + else + { + throw e; + } + } + } + else + { + throw new IllegalStateException("Cannot activate group provider in state: " + state); + } } else if (desiredState == State.STOPPED) { - _groupManager.close(); - return true; + if (_state.compareAndSet(state, State.STOPPED)) + { + _groupManager.close(); + return true; + } + else + { + throw new IllegalStateException("Cannot stop group provider in state: " + state); + } } else if (desiredState == State.DELETED) { - _groupManager.close(); - _groupManager.onDelete(); - return true; + if ((state == State.INITIALISING || state == State.ACTIVE || state == State.STOPPED || state == State.QUIESCED || state == State.ERRORED) + && _state.compareAndSet(state, State.DELETED)) + { + _groupManager.close(); + _groupManager.onDelete(); + return true; + } + else + { + throw new IllegalStateException("Cannot delete group provider in state: " + state); + } + } + else if (desiredState == State.QUIESCED) + { + if (state == State.INITIALISING && _state.compareAndSet(state, State.QUIESCED)) + { + return true; + } } return false; } diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java index 388427678e..de6ae06b94 100644 --- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java +++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/PortAdapter.java @@ -309,7 +309,7 @@ public class PortAdapter extends AbstractAdapter implements Port State state = _state.get(); if (desiredState == State.DELETED) { - if (state == State.INITIALISING || state == State.ACTIVE || state == State.STOPPED || state == State.QUIESCED) + if (state == State.INITIALISING || state == State.ACTIVE || state == State.STOPPED || state == State.QUIESCED || state == State.ERRORED) { return _state.compareAndSet(state, State.DELETED); } @@ -328,7 +328,7 @@ public class PortAdapter extends AbstractAdapter implements Port } catch(RuntimeException e) { - _state.compareAndSet(State.ACTIVE, state); + _state.compareAndSet(State.ACTIVE, State.ERRORED); throw e; } return true; diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/model/ConfiguredObjectStateTransitionTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/model/ConfiguredObjectStateTransitionTest.java new file mode 100644 index 0000000000..40f98b3b09 --- /dev/null +++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/model/ConfiguredObjectStateTransitionTest.java @@ -0,0 +1,245 @@ +package org.apache.qpid.server.model; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.File; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.UUID; + +import org.apache.qpid.server.BrokerOptions; +import org.apache.qpid.server.configuration.ConfigurationEntry; +import org.apache.qpid.server.configuration.ConfigurationEntryStore; +import org.apache.qpid.server.configuration.ConfiguredObjectRecoverer; +import org.apache.qpid.server.configuration.RecovererProvider; +import org.apache.qpid.server.configuration.startup.DefaultRecovererProvider; +import org.apache.qpid.server.configuration.updater.TaskExecutor; +import org.apache.qpid.server.security.auth.manager.AnonymousAuthenticationManagerFactory; +import org.apache.qpid.server.security.group.FileGroupManagerFactory; +import org.apache.qpid.server.stats.StatisticsGatherer; +import org.apache.qpid.server.util.BrokerTestHelper; +import org.apache.qpid.test.utils.QpidTestCase; + +public class ConfiguredObjectStateTransitionTest extends QpidTestCase +{ + private Broker _broker; + private RecovererProvider _recovererProvider; + private ConfigurationEntryStore _store; + private File _resourceToDelete; + + @Override + public void setUp() throws Exception + { + super.setUp(); + BrokerTestHelper.setUp(); + + _broker = BrokerTestHelper.createBrokerMock(); + StatisticsGatherer statisticsGatherer = mock(StatisticsGatherer.class); + TaskExecutor executor = mock(TaskExecutor.class); + when(executor.isTaskExecutorThread()).thenReturn(true); + when(_broker.getTaskExecutor()).thenReturn(executor); + + _recovererProvider = new DefaultRecovererProvider(statisticsGatherer, _broker.getVirtualHostRegistry(), + _broker.getLogRecorder(), _broker.getRootMessageLogger(), executor, new BrokerOptions()); + + _store = mock(ConfigurationEntryStore.class); + + _resourceToDelete = new File(TMP_FOLDER, getTestName()); + } + + @Override + public void tearDown() throws Exception + { + try + { + BrokerTestHelper.tearDown(); + if (_resourceToDelete.exists()) + { + _resourceToDelete.delete(); + } + } + finally + { + super.tearDown(); + } + } + + public void testGroupProviderValidStateTransitions() throws Exception + { + ConfigurationEntry providerEntry = getGroupProviderConfigurationEntry(); + ConfiguredObject provider = createConfiguredObject(providerEntry); + provider.setDesiredState(State.INITIALISING, State.QUIESCED); + assertValidStateTransition(provider, State.QUIESCED, State.STOPPED); + + provider = createConfiguredObject(providerEntry); + assertValidStateTransition(provider, State.INITIALISING, State.DELETED); + + providerEntry = getGroupProviderConfigurationEntry(); + provider = createConfiguredObject(providerEntry); + provider.setDesiredState(State.INITIALISING, State.QUIESCED); + assertValidStateTransition(provider, State.QUIESCED, State.DELETED); + + providerEntry = getGroupProviderConfigurationEntry(); + provider = createConfiguredObject(providerEntry); + provider.setDesiredState(State.INITIALISING, State.ACTIVE); + assertValidStateTransition(provider, State.ACTIVE, State.DELETED); + } + + public void testGroupProviderInvalidStateTransitions() throws Exception + { + ConfigurationEntry providerEntry = getGroupProviderConfigurationEntry(); + assertAllInvalidStateTransitions(providerEntry); + } + + public void testAuthenticationProviderValidStateTransitions() + { + ConfigurationEntry providerEntry = getAuthenticationProviderConfigurationEntry(); + assertAllValidStateTransitions(providerEntry); + } + + public void testAuthenticationProviderInvalidStateTransitions() + { + ConfigurationEntry providerEntry = getAuthenticationProviderConfigurationEntry(); + assertAllInvalidStateTransitions(providerEntry); + } + + public void testPortValidStateTransitions() + { + ConfigurationEntry providerEntry = getPortConfigurationEntry(); + assertAllValidStateTransitions(providerEntry); + } + + public void testPortInvalidStateTransitions() + { + ConfigurationEntry providerEntry = getPortConfigurationEntry(); + assertAllInvalidStateTransitions(providerEntry); + } + + private void assertAllInvalidStateTransitions(ConfigurationEntry providerEntry) + { + ConfiguredObject provider = createConfiguredObject(providerEntry); + assertInvalidStateTransition(provider, State.INITIALISING, State.REPLICA); + + provider.setDesiredState(State.INITIALISING, State.QUIESCED); + assertInvalidStateTransition(provider, State.QUIESCED, State.INITIALISING); + + provider.setDesiredState(State.QUIESCED, State.ACTIVE); + assertInvalidStateTransition(provider, State.ACTIVE, State.INITIALISING); + + provider.setDesiredState(State.ACTIVE, State.DELETED); + assertInvalidStateTransition(provider, State.DELETED, State.INITIALISING); + assertInvalidStateTransition(provider, State.DELETED, State.QUIESCED); + assertInvalidStateTransition(provider, State.DELETED, State.ACTIVE); + assertInvalidStateTransition(provider, State.DELETED, State.REPLICA); + assertInvalidStateTransition(provider, State.DELETED, State.ERRORED); + } + + private void assertAllValidStateTransitions(ConfigurationEntry providerEntry) + { + ConfiguredObject provider = createConfiguredObject(providerEntry); + assertNormalStateTransition(provider); + + provider = createConfiguredObject(providerEntry); + provider.setDesiredState(State.INITIALISING, State.QUIESCED); + assertValidStateTransition(provider, State.QUIESCED, State.STOPPED); + + provider = createConfiguredObject(providerEntry); + assertValidStateTransition(provider, State.INITIALISING, State.DELETED); + + provider = createConfiguredObject(providerEntry); + provider.setDesiredState(State.INITIALISING, State.QUIESCED); + assertValidStateTransition(provider, State.QUIESCED, State.DELETED); + + provider = createConfiguredObject(providerEntry); + provider.setDesiredState(State.INITIALISING, State.ACTIVE); + assertValidStateTransition(provider, State.ACTIVE, State.DELETED); + } + + private void assertInvalidStateTransition(ConfiguredObject object, State initialState, State... invalidStates) + { + assertEquals("Unepxceted state", initialState, object.getActualState()); + for (State state : invalidStates) + { + try + { + object.setDesiredState(initialState, state); + } + catch (IllegalStateException e) + { + // expected + } + assertEquals("Transition from state " + initialState + " into state " + state + " did occur", initialState, + object.getActualState()); + } + } + + private void assertValidStateTransition(ConfiguredObject object, State initialState, State... validStateSequence) + { + assertEquals("Unexpected state", initialState, object.getActualState()); + State currentState = initialState; + for (State state : validStateSequence) + { + object.setDesiredState(currentState, state); + assertEquals("Transition from state " + currentState + " into state " + state + " did not occur", state, + object.getActualState()); + currentState = state; + } + } + + private void assertNormalStateTransition(ConfiguredObject object) + { + assertValidStateTransition(object, State.INITIALISING, State.QUIESCED, State.ACTIVE, State.STOPPED, State.DELETED); + } + + private ConfiguredObject createConfiguredObject(ConfigurationEntry entry) + { + @SuppressWarnings("unchecked") + ConfiguredObjectRecoverer recoverer = + (ConfiguredObjectRecoverer)_recovererProvider.getRecoverer(entry.getType()); + return recoverer.create(_recovererProvider, entry, _broker); + } + + private ConfigurationEntry createConfigurationEntry(String type, Map attributes, ConfigurationEntryStore store) + { + return new ConfigurationEntry(UUID.randomUUID(), type, attributes, Collections.emptySet(), store); + } + + private ConfigurationEntry getAuthenticationProviderConfigurationEntry() + { + Map attributes = new HashMap(); + attributes.put(AuthenticationProvider.NAME, getTestName()); + attributes.put(AuthenticationProvider.TYPE, AnonymousAuthenticationManagerFactory.PROVIDER_TYPE); + return createConfigurationEntry(AuthenticationProvider.class.getSimpleName(), attributes , _store); + } + + private ConfigurationEntry getGroupProviderConfigurationEntry() throws Exception + { + Map attributes = new HashMap(); + attributes.put(GroupProvider.NAME, getTestName()); + attributes.put(GroupProvider.TYPE, FileGroupManagerFactory.GROUP_FILE_PROVIDER_TYPE); + attributes.put(FileGroupManagerFactory.PATH, _resourceToDelete.getAbsolutePath()); + if (!_resourceToDelete.exists()) + { + _resourceToDelete.createNewFile(); + } + return createConfigurationEntry(GroupProvider.class.getSimpleName(), attributes , _store); + } + + private ConfigurationEntry getPortConfigurationEntry() + { + ConfigurationEntry authProviderEntry = getAuthenticationProviderConfigurationEntry(); + AuthenticationProvider authProvider = (AuthenticationProvider)createConfiguredObject(authProviderEntry); + + Map attributes = new HashMap(); + attributes.put(Port.NAME, getTestName()); + attributes.put(Port.PROTOCOLS, Collections.singleton(Protocol.HTTP)); + attributes.put(Port.AUTHENTICATION_PROVIDER, authProvider.getName()); + attributes.put(Port.PORT, 0); + + when(_broker.findAuthenticationProviderByName(authProvider.getName())).thenReturn(authProvider); + return createConfigurationEntry(Port.class.getSimpleName(), attributes , _store); + } + +} -- cgit v1.2.1