Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WFLY-2527 Don't require a generic (foo=*) ManagementResourceRegistration to execute wildcard ops #5572

Merged
merged 6 commits into from
Dec 17, 2013
Merged

WFLY-2527 Don't require a generic (foo=*) ManagementResourceRegistration to execute wildcard ops #5572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b41435e
[WFLY-2527] Resolve OSHs for wildcard addresses even without a wildca…
bstansberry Dec 3, 2013
1d52228
[WFLY-2527] Use correct address when resolving ManagementResourceRegi…
bstansberry Dec 4, 2013
e7aba6c
[WFLY-2527] For multitarget requests, use a separate access control c…
bstansberry Dec 4, 2013
4f85b8d
[WFLY-2527] Support wildcard ops in domain mode when there is no wild…
bstansberry Dec 4, 2013
8e4cd68
[WFLY-2527] Be more lenient about read-operation-description for inva…
bstansberry Dec 6, 2013
af17a5d
[WFLY-2527] Tests
bstansberry Dec 8, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 38 additions & 2 deletions controller/src/main/java/org/jboss/as/controller/ModelControllerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
import org.jboss.as.controller.persistence.ConfigurationPersister;
import org.jboss.as.controller.registry.ImmutableManagementResourceRegistration;
import org.jboss.as.controller.registry.ManagementResourceRegistration;
import org.jboss.as.controller.registry.OperationEntry;
import org.jboss.as.controller.registry.Resource;
import org.jboss.as.core.security.AccessMechanism;
import org.jboss.dmr.ModelNode;
Expand Down Expand Up @@ -655,14 +656,15 @@ private class DefaultPrepareStepHandler implements OperationStepHandler {
@Override
public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
if (MGMT_OP_LOGGER.isTraceEnabled()) {
MGMT_OP_LOGGER.trace("Executing " + operation.get(OP) + " " + operation.get(OP_ADDR));
MGMT_OP_LOGGER.tracef("Executing %s %s", operation.get(OP), operation.get(OP_ADDR));
}
final PathAddress address = PathAddress.pathAddress(operation.get(OP_ADDR));
final String operationName = operation.require(OP).asString();
final OperationStepHandler stepHandler = rootRegistration.getOperationHandler(address, operationName);
final OperationStepHandler stepHandler = resolveOperationHandler(address, operationName);
if(stepHandler != null) {
context.addStep(stepHandler, OperationContext.Stage.MODEL);
} else {

ImmutableManagementResourceRegistration child = rootRegistration.getSubModel(address);
if (child == null) {
context.getFailureDescription().set(MESSAGES.noSuchResourceType(address));
Expand All @@ -674,6 +676,40 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
}
}

private OperationStepHandler resolveOperationHandler(final PathAddress address, final String operationName) {
OperationStepHandler result = rootRegistration.getOperationHandler(address, operationName);
if (result == null && address.size() > 0) {
// For wildcard elements, check specific registrations where the same OSH is used
// for all such registrations
PathElement pe = address.getLastElement();
if (pe.isWildcard()) {
String type = pe.getKey();
PathAddress parent = address.subAddress(0, address.size() - 1);
Set<PathElement> children = rootRegistration.getChildAddresses(parent);
if (children != null) {
OperationStepHandler found = null;
for (PathElement child : children) {
if (type.equals(child.getKey())) {
OperationEntry oe = rootRegistration.getOperationEntry(parent.append(child), operationName);
OperationStepHandler osh = oe == null ? null : oe.getOperationHandler();
if (osh == null || (found != null && !found.equals(osh))) {
// Not all children have the same handler; give up
found = null;
break;
}
// We have a candidate OSH
found = osh;
}
}
if (found != null) {
result = found;
}
}
}
}
return result;
}

/**
* The root resource, maintains a read-only reference to the current model. All write operations have to performed
* after acquiring the write lock on a clone of the underlying model.
Expand Down
86 changes: 80 additions & 6 deletions .../main/java/org/jboss/as/controller/operations/global/ReadOperationDescriptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@
import static org.jboss.as.controller.operations.global.GlobalOperationAttributes.LOCALE;
import static org.jboss.as.controller.operations.global.GlobalOperationAttributes.NAME;

import java.util.Locale;
import java.util.Set;

import org.jboss.as.controller.OperationContext;
import org.jboss.as.controller.OperationDefinition;
import org.jboss.as.controller.OperationFailedException;
import org.jboss.as.controller.OperationStepHandler;
import org.jboss.as.controller.PathAddress;
import org.jboss.as.controller.PathElement;
import org.jboss.as.controller.ProcessType;
import org.jboss.as.controller.SimpleAttributeDefinition;
import org.jboss.as.controller.SimpleAttributeDefinitionBuilder;
Expand Down Expand Up @@ -67,7 +69,7 @@ public class ReadOperationDescriptionHandler implements OperationStepHandler {


static final OperationDefinition DEFINITION = new SimpleOperationDefinitionBuilder(READ_OPERATION_DESCRIPTION_OPERATION, ControllerResolver.getResolver("global"))
.setParameters(NAME, LOCALE)
.setParameters(NAME, LOCALE, ACCESS_CONTROL)
.setReplyType(ModelType.OBJECT)
.setReadOnly()
.setRuntimeOnly()
Expand All @@ -81,14 +83,13 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
String operationName = NAME.resolveModelAttribute(context, operation).asString();
boolean accessControl = ACCESS_CONTROL.resolveModelAttribute(context, operation).asBoolean();

final ImmutableManagementResourceRegistration registry = context.getResourceRegistration();
OperationEntry operationEntry = registry.getOperationEntry(PathAddress.EMPTY_ADDRESS, operationName);
if (operationEntry == null || (context.getProcessType() == ProcessType.DOMAIN_SERVER && !operationEntry.getFlags().contains(OperationEntry.Flag.RUNTIME_ONLY))) {
final DescribedOp describedOp = getDescribedOp(context, operationName, operation, !accessControl);
if (describedOp == null || (context.getProcessType() == ProcessType.DOMAIN_SERVER && !describedOp.flags.contains(OperationEntry.Flag.RUNTIME_ONLY))) {
throw new OperationFailedException(new ModelNode().set(MESSAGES.operationNotRegistered(operationName,
PathAddress.pathAddress(operation.require(OP_ADDR)))));
} else {
final ModelNode result = operationEntry.getDescriptionProvider().getModelDescription(GlobalOperationHandlers.getLocale(context, operation));
Set<OperationEntry.Flag> flags = operationEntry.getFlags();
final ModelNode result = describedOp.description;
Set<OperationEntry.Flag> flags = describedOp.flags;
boolean readOnly = flags.contains(OperationEntry.Flag.READ_ONLY);
result.get(READ_ONLY).set(readOnly);
if (!readOnly) {
Expand All @@ -114,4 +115,77 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
}
context.stepCompleted();
}

private static DescribedOp getDescribedOp(OperationContext context, String operationName, ModelNode operation, boolean lenient) throws OperationFailedException {
DescribedOp result = null;
ImmutableManagementResourceRegistration registry = context.getResourceRegistration();
if (registry != null) {
OperationEntry operationEntry = registry.getOperationEntry(PathAddress.EMPTY_ADDRESS, operationName);
if (operationEntry != null) {
Locale locale = GlobalOperationHandlers.getLocale(context, operation);
result = new DescribedOp(operationEntry, locale);
}
} else if (lenient) {
PathAddress address = PathAddress.pathAddress(operation.get(OP_ADDR));
if (address.size() > 0) {
// For wildcard elements, check specific registrations where the same OSH is used
// for all such registrations
PathElement pe = address.getLastElement();
if (pe.isWildcard()) {
ImmutableManagementResourceRegistration rootRegistration = context.getRootResourceRegistration();
String type = pe.getKey();
PathAddress parent = address.subAddress(0, address.size() - 1);
Set<PathElement> children = rootRegistration.getChildAddresses(parent);
if (children != null) {
Locale locale = GlobalOperationHandlers.getLocale(context, operation);
DescribedOp found = null;
for (PathElement child : children) {
if (type.equals(child.getKey())) {
OperationEntry oe = rootRegistration.getOperationEntry(parent.append(child), operationName);
DescribedOp describedOp = oe == null ? null : new DescribedOp(oe, locale);
if (describedOp == null || (found != null && !found.equals(describedOp))) {
// Not all children have the same handler; give up
found = null;
break;
}
// We have a candidate OSH
found = describedOp;
}
}
result = found;
}
}

}
}
return result;
}

private static class DescribedOp {
private final ModelNode description;
private final Set<OperationEntry.Flag> flags;

private DescribedOp(OperationEntry operationEntry, Locale locale) {
this.description = operationEntry.getDescriptionProvider().getModelDescription(locale);
this.flags = operationEntry.getFlags();
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;

DescribedOp that = (DescribedOp) o;

return description.equals(that.description) && flags.equals(that.flags);

}

@Override
public int hashCode() {
int result = description.hashCode();
result = 31 * result + flags.hashCode();
return result;
}
}
}
38 changes: 20 additions & 18 deletions ...c/main/java/org/jboss/as/controller/operations/global/ReadResourceDescriptionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ ReadResourceDescriptionAccessControlContext getAccessControlContext() {
@Override
public void execute(final OperationContext context, final ModelNode operation) throws OperationFailedException {
final PathAddress address = PathAddress.pathAddress(operation.get(OP_ADDR));
ReadResourceDescriptionAccessControlContext accessControlContext = getAccessControlContext() == null ? new ReadResourceDescriptionAccessControlContext(address, null) : getAccessControlContext();
if (getAccessControlContext() == null && address.isMultiTarget()) {
executeMultiTarget(context, operation, accessControlContext);
executeMultiTarget(context, operation);
} else {
ReadResourceDescriptionAccessControlContext accessControlContext = getAccessControlContext() == null ? new ReadResourceDescriptionAccessControlContext(address, null) : getAccessControlContext();
doExecute(context, operation, accessControlContext);
}
}
Expand Down Expand Up @@ -175,6 +175,7 @@ private void doExecuteInternal(final OperationContext context, final ModelNode o
}

final String opName = operation.require(OP).asString();
PathAddress opAddr = PathAddress.pathAddress(operation.get(OP_ADDR));
final int recursiveDepth = RECURSIVE_DEPTH.resolveModelAttribute(context, operation).asInt();
final boolean recursive = recursiveDepth > 0 || RECURSIVE.resolveModelAttribute(context, operation).asBoolean();
final boolean proxies = PROXIES.resolveModelAttribute(context, operation).asBoolean();
Expand All @@ -184,7 +185,7 @@ private void doExecuteInternal(final OperationContext context, final ModelNode o
final AccessControl accessControl = AccessControl.forName(ACCESS_CONTROL.resolveModelAttribute(context, operation).asString());


final ImmutableManagementResourceRegistration registry = getResourceRegistrationCheckForAlias(context, accessControlContext.opAddress, accessControlContext);
final ImmutableManagementResourceRegistration registry = getResourceRegistrationCheckForAlias(context, opAddr, accessControlContext);

final DescriptionProvider descriptionProvider = registry.getModelDescription(PathAddress.EMPTY_ADDRESS);
final Locale locale = GlobalOperationHandlers.getLocale(context, operation);
Expand All @@ -194,7 +195,7 @@ private void doExecuteInternal(final OperationContext context, final ModelNode o
final Map<PathElement, ModelNode> childResources = recursive ? new HashMap<PathElement, ModelNode>() : Collections.<PathElement, ModelNode>emptyMap();

if (accessControl != AccessControl.NONE) {
accessControlContext.initLocalResourceAddresses(context, operation);
accessControlContext.initLocalResourceAddresses(context, opAddr);
}


Expand Down Expand Up @@ -269,7 +270,7 @@ private void doExecuteInternal(final OperationContext context, final ModelNode o
final ModelNode rrOp = operation.clone();
final PathAddress address;
try {
address = PathAddress.pathAddress(accessControlContext.opAddress, element);
address = PathAddress.pathAddress(opAddr, element);
} catch (Exception e) {
continue;
}
Expand Down Expand Up @@ -304,7 +305,7 @@ public void handleRollback(OperationContext context, ModelNode operation) {

private OperationStepHandler getRecursiveStepHandler(ImmutableManagementResourceRegistration childReg, String opName, ReadResourceDescriptionAccessControlContext accessControlContext, PathAddress address) {
OperationStepHandler overrideHandler = childReg.getOperationHandler(PathAddress.EMPTY_ADDRESS, opName);
if (overrideHandler != null && overrideHandler.getClass() == ReadResourceDescriptionHandler.class || overrideHandler.getClass() == AliasStepHandler.class) {
if (overrideHandler != null && (overrideHandler.getClass() == ReadResourceDescriptionHandler.class || overrideHandler.getClass() == AliasStepHandler.class)) {
// not an override
overrideHandler = null;
}
Expand Down Expand Up @@ -332,14 +333,16 @@ private ImmutableManagementResourceRegistration getResourceRegistrationCheckForA
}


private void executeMultiTarget(final OperationContext context, final ModelNode operation, final ReadResourceDescriptionAccessControlContext accessControlContext) {
private void executeMultiTarget(final OperationContext context, final ModelNode operation) {
// Format wildcard queries as list
final ModelNode result = context.getResult().setEmptyList();
context.addStep(new ModelNode(), GlobalOperationHandlers.AbstractMultiTargetHandler.FAKE_OPERATION.clone(),
new GlobalOperationHandlers.RegistrationAddressResolver(operation, result,
new OperationStepHandler() {
@Override
public void execute(final OperationContext context, final ModelNode operation) throws OperationFailedException {
final PathAddress address = PathAddress.pathAddress(operation.get(OP_ADDR));
ReadResourceDescriptionAccessControlContext accessControlContext = getAccessControlContext() == null ? new ReadResourceDescriptionAccessControlContext(address, null) : getAccessControlContext();
// step handler bypassing further wildcard resolution
doExecute(context, operation, accessControlContext);
}
Expand Down Expand Up @@ -406,8 +409,8 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
ModelNode result = new ModelNode();
boolean customDefaultCheck = operation.get(OP).asString().equals(GlobalOperationHandlers.CHECK_DEFAULT_RESOURCE_ACCESS);
ResourceAuthorization authResp = context.authorizeResource(true, customDefaultCheck);
if (authResp.getResourceResult(ActionEffect.ADDRESS).getDecision() == Decision.DENY) {
if (!defaultSetting) {
if (authResp == null || authResp.getResourceResult(ActionEffect.ADDRESS).getDecision() == Decision.DENY) {
if (!defaultSetting || authResp == null) {
//We are not allowed to see the resource, so we don't set the accessControlResult, meaning that the ReadResourceAssemblyHandler will ignore it for this address
} else {
result.get(ActionEffect.ADDRESS.toString()).set(false);
Expand Down Expand Up @@ -621,31 +624,30 @@ static final class ReadResourceDescriptionAccessControlContext {
this.parentAddresses = parent != null ? parent.parentAddresses : null;
}

private void initLocalResourceAddresses(OperationContext context, ModelNode operation){
localResourceAddresses = getLocalResourceAddresses(context, operation);
private void initLocalResourceAddresses(OperationContext context, PathAddress opAddress){
localResourceAddresses = getLocalResourceAddresses(context, opAddress);
}

private List<PathAddress> getLocalResourceAddresses(OperationContext context, ModelNode operation){
private List<PathAddress> getLocalResourceAddresses(OperationContext context, PathAddress opAddr){
List<PathAddress> localResourceAddresses = null;
PathAddress opAddr = PathAddress.pathAddress(operation.require(OP_ADDR));
if (parentAddresses == null) {
if (opAddr.size() == 0) {
return Collections.singletonList(PathAddress.EMPTY_ADDRESS);
} else {
localResourceAddresses = new ArrayList<>();
getAllActualResourceAddresses(context, operation, localResourceAddresses, PathAddress.EMPTY_ADDRESS, opAddr);
getAllActualResourceAddresses(context, localResourceAddresses, PathAddress.EMPTY_ADDRESS, opAddr);
}
} else {
localResourceAddresses = new ArrayList<>();
for (PathAddress pathAddress : parentAddresses) {
getAllActualResourceAddresses(context, operation, localResourceAddresses, pathAddress, opAddr);
getAllActualResourceAddresses(context, localResourceAddresses, pathAddress, opAddr);
}
}
return localResourceAddresses;

}

private void getAllActualResourceAddresses(OperationContext context, ModelNode operation, List<PathAddress> addresses, PathAddress currentAddress, PathAddress opAddress) {
private void getAllActualResourceAddresses(OperationContext context, List<PathAddress> addresses, PathAddress currentAddress, PathAddress opAddress) {
if (opAddress.size() == 0) {
return;
}
Expand Down Expand Up @@ -687,7 +689,7 @@ private void getAllActualResourceAddresses(OperationContext context, ModelNode o
if (address.size() == opAddress.size()) {
addresses.add(address);
} else {
getAllActualResourceAddresses(context, operation, addresses, address, opAddress);
getAllActualResourceAddresses(context, addresses, address, opAddress);
}
}
}
Expand All @@ -698,7 +700,7 @@ private void getAllActualResourceAddresses(OperationContext context, ModelNode o
if (address.size() == opAddress.size()) {
addresses.add(address);
} else {
getAllActualResourceAddresses(context, operation, addresses, address, opAddress);
getAllActualResourceAddresses(context, addresses, address, opAddress);
}
}
}
Expand Down