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

WFLY-16951 Add necessary individual elytron component jars to jboss-client.jar #15979

Merged
merged 1 commit into from Sep 20, 2022

Conversation

chengfang
Copy link
Contributor

@chengfang chengfang commented Aug 24, 2022

This is a follow-up of 26.x PR #15962 in the main branch.

https://issues.redhat.com/browse/WFLY-16951

It was reported in the above jira that jboss-client.jar worked in 26.0.0 but failed in 26.1.1 for this user's application. In 26.0.0, jboss-client.jar includes everything in wildfly-elytron.jar, which is an uber jar including many of its dependencies and submodules. In 26.1.1, some components like jboss-ejb-client and wildfly-naming-client split their dependency on wildfly-elytron into its more modular individual submodules. Therefore, when building jboss-client.jar, these individual elytron-* submodules are bundled in jboss-client.jar, which omits some artifacts unreachable via transitive dependency.

In order to maintain some kind of compatibility with 26.0.0, this PR proposes to add some additional elytron-* submodules to jboss-client.jar. Even though those additional artifacts are not required at each component client level (e.g., jboss-ejb-client, or wildfly-naming-client), they are needed to support a wide range of non-maven-based applications with various security mechanisms.

The following artifacts are not added to jboss-client.jar in this PR, as they appear not applicable on the client side, but I may be wrong:

elytron-audit
elytron-jacc
elytron-oidc
wildfly-elytron-jose-util
wildfly-elytron-jose-jwk
elytron--deprecated
elytron-
-http-*

Comment from Farah copied here for easy reference:

@chengfang Thanks for working on this! To make sure that we aren't accidentally missing anything here, I'm wondering if for WildFly 26.1.2, it would be safer to just add a dependency on org.wildfly.security:wildfly-elytron to mimic the behaviour that was present in WildFly 26.0.0.Final? What do you think? That would preserve compatibility.

We can then working on fixing this properly in the main branch. As a starting point, I've added some comments about dependencies that I think can be removed.

I've incorporated Farah's suggestions in the other PR (#15962).

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Aug 24, 2022
@bstansberry
Copy link
Contributor

@chengfang Note that the module that we're currently using is ee-9/source-transform/client/shade.

I don't see anything in the WF build currently using wildfly-client-all so if you wanted you could update client/shade to the EE 10 deps and then remove ee-9/source-transform/client/shade. We'd need to notify folks a bit in case anyone outside the WF build is using wildfly-client-all-jakarta.

@chengfang
Copy link
Contributor Author

chengfang commented Aug 25, 2022

@bstansberry thanks, I'll update it.

Update: the task of dropping ee-9/source-transform/client/shade is done via #15995

@chengfang chengfang marked this pull request as draft August 30, 2022 19:58
@chengfang chengfang marked this pull request as ready for review September 6, 2022 21:16
@chengfang
Copy link
Contributor Author

@fjuma in my manual testing, I found the following deps are still needed on the client side: (you commented in the other 26.x PR that they could be removed, if I read it correctly):

        <dependency>
            <groupId>org.wildfly.security</groupId>
            <artifactId>wildfly-elytron-digest</artifactId>
        </dependency>
        <dependency>
            <groupId>org.wildfly.security</groupId>
            <artifactId>wildfly-elytron-mechanism-digest</artifactId>
        </dependency>
        <dependency>
            <groupId>org.wildfly.security</groupId>
            <artifactId>wildfly-elytron-sasl-digest</artifactId>
        </dependency>

@chengfang chengfang changed the title WFLY-16734 jboss-client.jar not working with Wildfly 26.1.1 WFLY-16951 Add necessary individual elytron component jars to jboss-client.jar Sep 6, 2022
@fjuma
Copy link
Contributor

fjuma commented Sep 7, 2022

My comment on the other PR was referring to wildfly-elytron-json-util specifically in that block. Sorry for the confusion, @chengfang!

@bstansberry
Copy link
Contributor

@chengfang @fjuma Can we get this one done in the next day or two? It seems like a good thing to get bake for in 27 Beta1

@bstansberry bstansberry added the Critical Doesn't block a release but deserves higher priority than most. Use sparingly. label Sep 19, 2022
@chengfang
Copy link
Contributor Author

I don't have anything else to add. @fjuma can you review again?

@chengfang
Copy link
Contributor Author

@Skyllarr can you please review it?

@Skyllarr
Copy link
Contributor

The following artifacts are not added to jboss-client.jar in this PR, as they appear not applicable on the client side, but I may be wrong:
elytron-audit
elytron-jacc
elytron-oidc
wildfly-elytron-jose-util
wildfly-elytron-jose-jwk
elytron--deprecated
elytron--http-*

@chengfang Yes these are not applicable on the client side. The PR looks good to me.

@bstansberry bstansberry merged commit 0f58896 into wildfly:main Sep 20, 2022
@bstansberry
Copy link
Contributor

Thanks @chengfang and @Skyllarr!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Critical Doesn't block a release but deserves higher priority than most. Use sparingly. deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
4 participants