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.

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

WFCORE-4041: supply default values for nested attributes #4853

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Expand Up @@ -745,9 +745,7 @@ private static ModelNode buildUsersResource(PropertiesRealmConfiguration config)
mn.get(Util.RELATIVE_TO).set(config.getRelativeTo());
}
mn.get(Util.DIGEST_REALM_NAME).set(config.getExposedRealmName());
if (config.getPlainText()) {
mn.get(Util.PLAIN_TEXT).set(config.getPlainText());
}
mn.get(Util.PLAIN_TEXT).set(config.getPlainText());
return mn;
}

Expand Down
Expand Up @@ -223,11 +223,12 @@ private void validateNestedAttributes(final ModelNode subModel, final ObjectType
return;
}

final Set<String> keys = subModel.keys();
final Set<String> definedKeys = new HashSet<>(keys.size());
for (String key : keys) {
if (subModel.hasDefined(key)) {
definedKeys.add(key);
// only top-level model contains all keys, we have to retrieve keys from the description
final AttributeDefinition[] keys = attr.getValueTypes();
final Set<String> definedKeys = new HashSet<>(keys.length);
for (AttributeDefinition key : keys) {
if (subModel.hasDefined(key.getName()) || key.getDefaultValue() != null) {
definedKeys.add(key.getName());
}
}
AttributeDefinition[] subAttrs = attr.getValueTypes();
Expand Down
Expand Up @@ -9,6 +9,7 @@
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.COMPOSITE;
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.FAILED;
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.FAILURE_DESCRIPTION;
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.INCLUDE_DEFAULTS;
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.NAME;
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.OP;
import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.OP_ADDR;
Expand Down Expand Up @@ -161,6 +162,12 @@ public static ModelNode getReadAttributeOperation(final PathAddress address, Str
return createAttributeOperation(READ_ATTRIBUTE_OPERATION, address, attributeName);
}

public static ModelNode getReadAttributeOperation(final PathAddress address, String attributeName, boolean includeDefaults) {
ModelNode op = getReadAttributeOperation(address, attributeName);
op.get(INCLUDE_DEFAULTS).set(includeDefaults);
return op;
}

public static ModelNode getReadResourceDescriptionOperation(final PathAddress address) {
ModelNode op = createEmptyOperation(READ_RESOURCE_DESCRIPTION_OPERATION, address);
return op;
Expand Down
Expand Up @@ -99,7 +99,7 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
}, OperationContext.Stage.MODEL, true);

// 1. read current attribute value
ModelNode readAttributeOperation = Util.getReadAttributeOperation(address, useEnhancedSyntax ? attributeExpression : attributeName);
ModelNode readAttributeOperation = Util.getReadAttributeOperation(address, useEnhancedSyntax ? attributeExpression : attributeName, false);
context.addStep(readResponse, readAttributeOperation, ReadAttributeHandler.INSTANCE, OperationContext.Stage.MODEL, true);
} else {
assert attributeAccess.getStorageType() == AttributeAccess.Storage.RUNTIME;
Expand Down
Expand Up @@ -15,6 +15,9 @@

import org.jboss.as.controller.AttributeDefinition;
import org.jboss.as.controller.ExpressionResolver;
import org.jboss.as.controller.ObjectListAttributeDefinition;
import org.jboss.as.controller.ObjectMapAttributeDefinition;
import org.jboss.as.controller.ObjectTypeAttributeDefinition;
import org.jboss.as.controller.OperationContext;
import org.jboss.as.controller.OperationDefinition;
import org.jboss.as.controller.OperationFailedException;
Expand Down Expand Up @@ -276,6 +279,9 @@ static void resolveAttribute(OperationContext context, AttributeDefinition attri
} else if (subModel.hasDefined(attribute.getName())) {
final ModelNode result = subModel.get(attribute.getName());
context.getResult().set(result);
if (defaults) {
handleObjectAttributes(context.getResult(), attribute);
}
} else if (defaults && attribute.getDefaultValue() != null) {
// No defined value in the model. See if we should reply with a default from the metadata,
// reply with undefined, or fail because it's a non-existent attribute name
Expand All @@ -287,6 +293,36 @@ static void resolveAttribute(OperationContext context, AttributeDefinition attri
}
}

private static void handleObjectAttributes(ModelNode model, AttributeDefinition attribute) {
if (attribute instanceof ObjectTypeAttributeDefinition) {
readNestedDefaults(model, (ObjectTypeAttributeDefinition) attribute);
} else if (attribute instanceof ObjectListAttributeDefinition) {
ObjectTypeAttributeDefinition valueType = ((ObjectListAttributeDefinition) attribute).getValueType();
for (int i = 0; i < model.asInt(); i++) {
readNestedDefaults(model.get(i), valueType);
}
} else if (attribute instanceof ObjectMapAttributeDefinition) {
ObjectTypeAttributeDefinition valueType = ((ObjectMapAttributeDefinition) attribute).getValueType();
for (String key : model.keys()) {
readNestedDefaults(model.get(key), valueType);
}
}
}

private static void readNestedDefaults(ModelNode model, ObjectTypeAttributeDefinition attribute) {
for (AttributeDefinition subAttribute : attribute.getValueTypes()) {
ModelNode defaultValue = subAttribute.getDefaultValue();
String subAttrName = subAttribute.getName();
if (defaultValue != null && !model.hasDefined(subAttrName)) {
model.get(subAttrName).set(defaultValue);
}

if (model.hasDefined(subAttrName)) {
handleObjectAttributes(model.get(subAttrName), subAttribute);
}
}
}

private static class Validator extends ParametersValidator {

private static final Validator RESOLVABLE = new Validator(true);
Expand Down
Expand Up @@ -101,7 +101,6 @@ static class AttributeMappingObjectDefinition {

static final SimpleAttributeDefinition RECURSIVE_SEARCH = new SimpleAttributeDefinitionBuilder(ElytronDescriptionConstants.SEARCH_RECURSIVE, ModelType.BOOLEAN, true)
.setRequires(ElytronDescriptionConstants.FILTER)
.setDefaultValue(ModelNode.TRUE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for making this change? By having the default defined which also matches our XML schema it means we can avoid complicating it with the isDefined check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an attribute A has a default value and requires attribute B then B has to have a default value as well, otherwise it will fail validation. Top-level attributes are being checked for this but I didn't implement it correctly for nested attributes.

If there is an available default value for FILTER and TO I can change that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@darranl how do we proceed with this?

.setAllowExpression(true)
.setFlags(AttributeAccess.Flag.RESTART_RESOURCE_SERVICES)
.build();
Expand Down Expand Up @@ -312,7 +311,6 @@ static class IdentityMappingObjectDefinition {

static final SimpleAttributeDefinition USE_RECURSIVE_SEARCH = new SimpleAttributeDefinitionBuilder(ElytronDescriptionConstants.USE_RECURSIVE_SEARCH, ModelType.BOOLEAN, true)
.setRequires(ElytronDescriptionConstants.SEARCH_BASE_DN)
.setDefaultValue(ModelNode.FALSE)
.setAllowExpression(true)
.setFlags(AttributeAccess.Flag.RESTART_RESOURCE_SERVICES)
.build();
Expand Down Expand Up @@ -497,7 +495,7 @@ private void configureIdentityMapping(OperationContext context, ModelNode model,
}

ModelNode useRecursiveSearchNode = IdentityMappingObjectDefinition.USE_RECURSIVE_SEARCH.resolveModelAttribute(context, identityMappingNode);
if (useRecursiveSearchNode.asBoolean()) {
if (useRecursiveSearchNode.isDefined() && useRecursiveSearchNode.asBoolean()) {
identityMappingBuilder.searchRecursive();
}

Expand Down
Expand Up @@ -131,7 +131,6 @@ class SaslServerDefinitions {
static final SimpleAttributeDefinition VERSION_COMPARISON = new SimpleAttributeDefinitionBuilder(ElytronDescriptionConstants.VERSION_COMPARISON, ModelType.STRING, false)
.setRequired(false)
.setAllowExpression(true)
.setDefaultValue(new ModelNode(ElytronDescriptionConstants.LESS_THAN))
.setRequires(ElytronDescriptionConstants.PROVIDER_VERSION)
.setAllowedValues(ElytronDescriptionConstants.LESS_THAN, ElytronDescriptionConstants.GREATER_THAN)
.setValidator(EnumValidator.create(Comparison.class))
Expand Down
Expand Up @@ -32,7 +32,6 @@
import org.jboss.as.host.controller.discovery.StaticDiscovery;
import org.jboss.as.host.controller.ignored.IgnoredDomainResourceRegistry;
import org.jboss.as.host.controller.model.host.AdminOnlyDomainConfigPolicy;
import org.jboss.as.remoting.Protocol;
import org.jboss.as.repository.ContentRepository;
import org.jboss.as.repository.HostFileRepository;
import org.jboss.dmr.ModelNode;
Expand Down Expand Up @@ -92,8 +91,6 @@ public abstract class DomainControllerWriteAttributeHandler extends ReloadRequir
new SimpleAttributeDefinitionBuilder(ModelDescriptionConstants.PROTOCOL, ModelType.STRING)
.setRequired(false)
.setAllowExpression(true)
.setValidator(EnumValidator.create(Protocol.class))
.setDefaultValue(org.jboss.as.remoting.Protocol.REMOTE.toModelNode())
.setFlags(AttributeAccess.Flag.RESTART_ALL_SERVICES)
.setRequires(ModelDescriptionConstants.HOST, ModelDescriptionConstants.PORT)
.build();
Expand Down
Expand Up @@ -30,7 +30,6 @@
import org.jboss.as.controller.registry.Resource;
import org.jboss.as.host.controller.descriptions.HostResolver;
import org.jboss.as.host.controller.model.host.AdminOnlyDomainConfigPolicy;
import org.jboss.as.remoting.Protocol;
import org.jboss.dmr.ModelNode;
import org.jboss.dmr.ModelType;
import org.wildfly.security.auth.client.AuthenticationContext;
Expand Down Expand Up @@ -62,8 +61,6 @@ public class RemoteDomainControllerAddHandler implements OperationStepHandler {
public static final SimpleAttributeDefinition PROTOCOL = new SimpleAttributeDefinitionBuilder(ModelDescriptionConstants.PROTOCOL, ModelType.STRING)
.setRequired(false)
.setAllowExpression(true)
.setValidator(EnumValidator.create(Protocol.class))
.setDefaultValue(Protocol.REMOTE.toModelNode())
.setFlags(AttributeAccess.Flag.RESTART_ALL_SERVICES)
.setRequires(ModelDescriptionConstants.HOST, ModelDescriptionConstants.PORT)
.build();
Expand Down
Expand Up @@ -196,6 +196,7 @@ public void testDCFailover() throws Exception {
changePrimaryOp.get(ModelDescriptionConstants.OP).set(RemoteDomainControllerAddHandler.OPERATION_NAME);
changePrimaryOp.get(ModelDescriptionConstants.HOST).set("${jboss.test.host.secondary.address}");
changePrimaryOp.get(ModelDescriptionConstants.PORT).set(MGMT_PORTS[1]);
changePrimaryOp.get(ModelDescriptionConstants.PROTOCOL).set("remote");
changePrimaryOp.get(ModelDescriptionConstants.AUTHENTICATION_CONTEXT).set("secondaryHostAContext");

hostUtils[2].executeForResult(changePrimaryOp);
Expand Down
Expand Up @@ -194,6 +194,7 @@ public void testDCFailover() throws Exception {
changePrimaryOp.get(ModelDescriptionConstants.OP).set(RemoteDomainControllerAddHandler.OPERATION_NAME);
changePrimaryOp.get(ModelDescriptionConstants.HOST).set("${jboss.test.host.secondary.address}");
changePrimaryOp.get(ModelDescriptionConstants.PORT).set(MGMT_PORTS[1]);
changePrimaryOp.get(ModelDescriptionConstants.PROTOCOL).set("remote");
changePrimaryOp.get(ModelDescriptionConstants.SECURITY_REALM).set("ManagementRealm");

hostUtils[2].executeForResult(changePrimaryOp);
Expand Down
Expand Up @@ -27,7 +27,7 @@
<!-- Remote domain controller configuration with a host and port -->
<remote authentication-context="secondaryHostAContext" ignore-unused-configuration="true" admin-only-policy="${jboss.test.admin-only-policy}">
<discovery-options>
<static-discovery name="primary" host="${jboss.test.host.primary.address}" port="9999"/>
<static-discovery name="primary" protocol="remote" host="${jboss.test.host.primary.address}" port="9999"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These additions look problematic, if we need to make configuration changes in test cases as a result of work in the management model this indicates that these changes will potentially break existing user configurations that don't have these attributes defined.

</discovery-options>
</remote>
</domain-controller>
Expand Down
Expand Up @@ -25,7 +25,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote authentication-context="secondaryHostAContext" ignore-unused-configuration="true" admin-only-policy="${jboss.test.admin-only-policy}">
<remote protocol="remote" authentication-context="secondaryHostAContext" ignore-unused-configuration="true" admin-only-policy="${jboss.test.admin-only-policy}">
<!--<discovery-options>
<static-discovery name="primary" host="${jboss.test.host.primary.address}" port="9999"/>
</discovery-options>-->
Expand Down
Expand Up @@ -30,7 +30,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" ignore-unused-configuration="true">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" ignore-unused-configuration="true">
<!--
<ignored-resources type="extension">
<instance name="org.jboss.as.jsr77"/>
Expand Down
Expand Up @@ -29,7 +29,7 @@
</management>

<domain-controller>
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext"/>
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext"/>
</domain-controller>

<!--
Expand Down
Expand Up @@ -29,7 +29,7 @@
</management>

<domain-controller>
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<ignored-resources type="extension">
<instance name="ignored"/>
</ignored-resources>
Expand Down
Expand Up @@ -30,7 +30,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" ignore-unused-configuration="false">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" ignore-unused-configuration="false">
</remote>
</domain-controller>

Expand Down
Expand Up @@ -49,7 +49,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<ignored-resources type="extension">
<instance name="org.jboss.as.jsr77"/>
</ignored-resources>
Expand Down
Expand Up @@ -30,7 +30,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" ignore-unused-configuration="false">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" ignore-unused-configuration="false">
</remote>
</domain-controller>

Expand Down
Expand Up @@ -51,7 +51,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<ignored-resources type="extension">
<instance name="org.jboss.as.threads"/>
</ignored-resources>
Expand Down
Expand Up @@ -64,7 +64,7 @@
<!-- Remote domain controller configuration with a host and port -->
<remote security-realm="ManagementRealm">
<discovery-options>
<static-discovery name="start-option" host="${jboss.test.host.primary.address}" port="9999" />
<static-discovery name="start-option" protocol="remote" host="${jboss.test.host.primary.address}" port="9999" />
</discovery-options>
</remote>
</domain-controller>
Expand Down
Expand Up @@ -49,7 +49,7 @@
</management>
<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext"/>
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext"/>
</domain-controller>
<interfaces>
<interface name="management">
Expand Down
Expand Up @@ -39,7 +39,7 @@
</management>
<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext"/>
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext"/>
</domain-controller>
<interfaces>
<interface name="management">
Expand Down
Expand Up @@ -49,7 +49,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" />
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext" />
</domain-controller>

<interfaces>
Expand Down
Expand Up @@ -50,7 +50,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<ignored-resources type="extension">
<instance name="org.jboss.as.threads"/>
</ignored-resources>
Expand Down
Expand Up @@ -49,7 +49,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<ignored-resources type="extension">
<instance name="org.jboss.as.jsr77"/>
</ignored-resources>
Expand Down
Expand Up @@ -49,7 +49,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<ignored-resources type="extension">
<instance name="org.jboss.as.jsr77"/>
</ignored-resources>
Expand Down
Expand Up @@ -51,7 +51,7 @@

<domain-controller>
<!-- Remote domain controller configuration with a host and port -->
<remote host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<remote protocol="remote" host="${jboss.test.host.primary.address}" port="9999" authentication-context="secondaryHostAContext">
<ignored-resources type="extension">
<instance name="org.jboss.as.threads"/>
</ignored-resources>
Expand Down