WFCORE-6472 Remove obsolete remoting subsystem transformers and long deprecated resources/attributes #5644

pferraro · 2023-09-01T20:09:06Z

https://issues.redhat.com/browse/WFCORE-6472
https://issues.redhat.com/browse/WFCORE-6473

Resources and attributes deprecated since WF23 have been dropped. This includes the weird handling described in WFCORE-6473.
I've added an enumeration of supported model versions, to be referenced by future transformers.

This PR also adds new 6.0 version of schema that prunes obsolete elements/attributes for which there is no longer any model in which to persist.
The parser/writer for version 6.0 is now based on PersistentResourceXMLDescription, and the 6.0 schema simplified accordingly.
Subsystem tests have also been parameterized to accommodate future schema versions, so that testing of legacy schema parsing does not get dropped.
The worker attribute has been made required, rather than a hidden default, since this is a capability reference.

~~Requires wildfly/wildfly#17145 to fix wildfly full integration test failures.~~ Merged.

pferraro · 2023-09-01T20:09:48Z

wildfly full integration tests will likely fail, requiring wildfly/wildfly#17145

bstansberry

@pferraro @fl4via

I've made some minor review comments below. I think the biggest issue to discuss are the various breaking changes.

The original WFCORE-6472 request; the legacy endpoint resource. As we discussed starting at WFCORE-6407 Deprecate non-default contructors from AbstractAddStepHandler and AbstractWriteAttributeHandler #5563 (comment) I think this is fine. It just removes an alternate view with no difference in runtime behavior.
The remote outbound connection removal stuff. I think that should have a different JIRA. This I think needs good discussion as it is fairly large and is a behavior change. When GenericOutboundConnectionResourceDefinition and LocalOutboundConnectionResourceDefinition were deprecated the advice was to replace with RemoteOutboundConnectionResourceDefinition with the protocol configured. But now that is not possible as 'protocol' is removed, so users have to configure an authentication context. Looking at how that works I don't get the impression it is possible to set the protocol. I think we may be limited to remote+http.
The removal of the default value for WORKER. You've handled that well with respect to domain mode and legacy schema parsing; the question here is if we're ok with breaking CLI scripts.

Thanks, Paul, for doing this as this extension was due for a general update.

remoting/subsystem/src/main/java/org/jboss/as/remoting/RemotingSubsystemModel.java

remoting/subsystem/src/main/java/org/jboss/as/remoting/RemotingSubsystemAdd.java

remoting/subsystem/src/main/java/org/jboss/as/remoting/RemotingSubsystemRootResource.java

bstansberry · 2023-09-06T18:23:11Z

core-feature-pack/galleon-common/src/main/resources/layers/standalone/remoting/layer-spec.xml

-        <param name="http-connector" value="http-remoting-connector"/>
-        <param name="connector-ref" value="default"/>
-        <param name="sasl-authentication-factory" value="application-sasl-authentication"/>
-    </feature>


Note to self:

This change requires https://github.com/wildfly/wildfly/pull/17145/files as that is where the remoting feature group used in full WF that provides the http-connector is configured. The 'remoting' feature group file here is not relevant in full WF.

…/subsystem=remoting/configuration=endpoint will be removed.

* Drop problematic static references to resource definitions and add operation handlers. * Replace racy write attribute handlers with ReloadRequiredWriteAttributeHandler * Drop deprecated ServiceName alias * OutboundConnection services do not provide the ServiceName associated with its RuntimeCapability. * Consolidate redundant service implementations for URI-only connections.

…F23.

Make RemotingSubsystem40Parser final to discourage any temptation to extend this class in the future.

…ources and attributes. Migrate Namespace enum to RemotingSubsystemSchema enum. Use PersistentResourceXMLDescription for parsing/writing 6.0 (and future) schemas. Drop unnecessary use of non-standard attribute marshallers. Parameterize subsystem test case to validate recent schemas.

…at require dependencies that are not avilable in wildfly-core.

pferraro · 2023-09-15T15:28:16Z

Rebased against main.

bstansberry · 2023-09-21T19:29:31Z

...bsystem/src/main/java/org/jboss/as/remoting/GenericOutboundConnectionResourceDefinition.java

                .setDeprecatedSince(RemotingSubsystemModel.VERSION_4_0_0.getVersion())
        );
    }

    @Override
    public void registerChildren(ManagementResourceRegistration resourceRegistration) {
+        // TODO why does this resource have properties that are never referenced?


@pferraro @fl4via This dates to:

7281132

which came in as part of the big Elytron integration PR #2148

due to its incorporation of wildfly-security-incubator#37

The child is kind of deprecated because this resource is deprecated but I suppose it should be deprecatd on its own. And if we decided to undeprecate this and local-outbound-connection for sure we should deprecate the child.

bstansberry · 2023-09-21T19:42:54Z

...subsystem/src/main/java/org/jboss/as/remoting/LocalOutboundConnectionResourceDefinition.java

@@ -40,38 +44,39 @@ class LocalOutboundConnectionResourceDefinition extends AbstractOutboundConnecti

    static final PathElement ADDRESS = PathElement.pathElement(CommonAttributes.LOCAL_OUTBOUND_CONNECTION);

-    public static final SimpleAttributeDefinition OUTBOUND_SOCKET_BINDING_REF = new SimpleAttributeDefinitionBuilder(CommonAttributes.OUTBOUND_SOCKET_BINDING_REF, ModelType.STRING, false)
+    // TODO This is never used - why is it required?


@pferraro @fl4via This dates to:

7281132

which came in as part of the big Elytron integration PR #2148

due to its incorporation of wildfly-security-incubator#37

I guess the local: protocol used to involve a host name in its URIs, but no longer does.

If this resource survives this attribute should be deprecated and made optional.

It also shouldn't allow expressions :)

bstansberry · 2023-09-21T19:47:25Z

remoting/subsystem/src/main/java/org/jboss/as/remoting/LocalOutboundConnectionWriteHandler.java

-            context.removeService(serviceName); // safe even if the service doesn't exist
-            // install the service with new values
-            LocalOutboundConnectionAdd.INSTANCE.installRuntimeService(context, fullModel);
-        }


Big +1 to getting rid of this.

bstansberry · 2023-09-21T19:48:28Z

...ting/subsystem/src/main/java/org/jboss/as/remoting/RemoteOutboundConnectionWriteHandler.java

-        }
-
-        return reloadRequired;
-    }


Big +1 to getting rid of this.

bstansberry · 2023-09-21T19:49:20Z

remoting/subsystem/src/main/java/org/jboss/as/remoting/RemotingExtension.java

        // local outbound connection
-        subsystem.registerSubModel(LocalOutboundConnectionResourceDefinition.INSTANCE);
+        subsystem.registerSubModel(new LocalOutboundConnectionResourceDefinition());
        // (generic) outbound connection
        subsystem.registerSubModel(new GenericOutboundConnectionResourceDefinition());


TODO -- this children wiring should be moved out of the Extension impl.

yersan · 2023-09-25T15:30:08Z

Thanks @fl4via @pferraro and @bstansberry

github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Sep 1, 2023

This comment was marked as outdated.

Sign in to view

pferraro force-pushed the WFCORE-6472 branch from 474fa54 to 1d04a20 Compare September 1, 2023 22:20

This comment was marked as outdated.

Sign in to view

wildfly deleted a comment from wildfly-ci Sep 2, 2023

pferraro requested a review from bstansberry September 2, 2023 14:43

This comment was marked as outdated.

Sign in to view

pferraro force-pushed the WFCORE-6472 branch from 1680da4 to 4c98398 Compare September 2, 2023 17:08

This comment was marked as outdated.

Sign in to view

pferraro force-pushed the WFCORE-6472 branch from 4c98398 to a4b976a Compare September 2, 2023 18:18

This comment was marked as outdated.

Sign in to view

pferraro force-pushed the WFCORE-6472 branch from a4b976a to 9d0924c Compare September 2, 2023 20:01

This comment was marked as outdated.

Sign in to view

pferraro requested a review from fl4via September 5, 2023 21:59

bstansberry requested changes Sep 6, 2023

View reviewed changes

pferraro force-pushed the WFCORE-6472 branch from 17a5bdf to 7fa3fd8 Compare September 14, 2023 16:33

pferraro mentioned this pull request Sep 14, 2023

WFCORE-6407 Deprecate non-default contructors from AbstractAddStepHandler and AbstractWriteAttributeHandler #5563

Merged

pferraro added 13 commits September 15, 2023 11:21

Enumerate supported subsystem model versions.

c676655

Remove redundant ModelVersion instances.

fa84148

Deprecate and remove use of RemotingServices.SUBSYSTEM_ENDPOINT

cb9e3bb

Deprecate and remove use of RemotingServices.HTTP_LISTENER_REGISTRY

cab1454

Update WildCardReadsTestCase to use a different test resource, since …

847e022

…/subsystem=remoting/configuration=endpoint will be removed.

WFCORE-6472 Removing obsolete remoting subsystem transformers

3141424

Remove selective remoting resources and attributes deprecated since W…

ab90442

…F23.

Remove pointless RemotingSubsystem50Parser class.

a3bd728

Make RemotingSubsystem40Parser final to discourage any temptation to extend this class in the future.

Bump remoting subsystem model version to 7.0.0.

1a261c4

Add missing remoting dependency to jmx-remoting layer.

e45c998

remoting feature group in wildfly-core should not contain features th…

48a7961

…at require dependencies that are not avilable in wildfly-core.

pferraro force-pushed the WFCORE-6472 branch from 7fa3fd8 to 48a7961 Compare September 15, 2023 15:27

pferraro requested a review from bstansberry September 15, 2023 15:28

This comment was marked as outdated.

Sign in to view

bstansberry reviewed Sep 21, 2023

View reviewed changes

bstansberry approved these changes Sep 25, 2023

View reviewed changes

yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Sep 25, 2023

yersan merged commit cdfabc4 into wildfly:main Sep 25, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WFCORE-6472 Remove obsolete remoting subsystem transformers and long deprecated resources/attributes #5644

WFCORE-6472 Remove obsolete remoting subsystem transformers and long deprecated resources/attributes #5644

pferraro commented Sep 1, 2023 •

edited

Loading

pferraro commented Sep 1, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

bstansberry left a comment

bstansberry Sep 6, 2023

pferraro Sep 6, 2023

pferraro commented Sep 15, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

bstansberry Sep 21, 2023

bstansberry Sep 21, 2023

bstansberry Sep 21, 2023

bstansberry Sep 21, 2023

bstansberry Sep 21, 2023

bstansberry Sep 21, 2023

yersan commented Sep 25, 2023

WFCORE-6472 Remove obsolete remoting subsystem transformers and long deprecated resources/attributes #5644

WFCORE-6472 Remove obsolete remoting subsystem transformers and long deprecated resources/attributes #5644

Conversation

pferraro commented Sep 1, 2023 • edited Loading

pferraro commented Sep 1, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

bstansberry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pferraro commented Sep 15, 2023

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yersan commented Sep 25, 2023

pferraro commented Sep 1, 2023 •

edited

Loading