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-5712] Update elytron-tool scripts to make use of jboss-modules #4879

Merged
merged 10 commits into from
Dec 2, 2021

Conversation

fjuma
Copy link
Contributor

@fjuma fjuma commented Nov 26, 2021

https://issues.redhat.com/browse/WFCORE-5712
https://issues.redhat.com/browse/WFCORE-5720
https://issues.redhat.com/browse/WFCORE-5730

Depends on #4878 (I've included those commits in this PR to also un-ignore ElytronToolScriptTestCase)

@wildfly-ci
Copy link

Core - Full Integration Build 11108 outcome was UNKNOWN using a merge of 27b0e97
Summary: Canceled (Error while applying patch; cannot find commit 1790120 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/4879/merge branch was updated and the commit selected for the ... Build time: 00:02:38

@fjuma
Copy link
Contributor Author

fjuma commented Nov 26, 2021

@jamezp When you are back next week, would it be possible to get a review on the updates to the elytron-tool.[sh|bat|ps1] scripts? Thanks.

@github-actions
Copy link

github-actions bot commented Nov 26, 2021

Dependency Tree Analyzer Output:

New Dependencies:

  • commons-cli:commons-cli:jar:1.4:compile
  • org.apache.commons:commons-lang3:jar:3.11:compile
  • org.wildfly.core:wildfly-elytron-tool-wrapper:jar:18.0.0.Beta6-SNAPSHOT:compile

CC @wildfly/prod

@github-actions github-actions bot added the deps-changed Dependencies have been checked, and there are changes highlighted in a comment label Nov 26, 2021
@fjuma fjuma force-pushed the WFCORE-5712 branch 2 times, most recently from 01d225e to 8103730 Compare November 26, 2021 19:16
@wildfly-ci
Copy link

Core - Full Integration Build 11109 outcome was FAILURE using a merge of 8103730
Summary: Tests failed: 1 (1 new), passed: 4078, ignored: 44 Build time: 02:32:15

Failed tests

org.jboss.as.test.layers.LayersTestCase.testServlet: java.lang.Exception: Some unreferenced modules are un-expected [org.apache.commons.cli, org.apache.commons.lang3, org.wildfly.security.elytron-tool]
	at org.jboss.as.test.layers.LayersTest.test(LayersTest.java:109)
	at org.jboss.as.test.layers.LayersTestCase.testServlet(LayersTestCase.java:146)


@fjuma
Copy link
Contributor Author

fjuma commented Nov 26, 2021

I can't reproduce the ElytronToolScriptTestCase failures locally on Linux. Need to look closer at what's causing this to fail on CI.

@wildfly-ci
Copy link

Core - Full Integration Build 11113 outcome was FAILURE using a merge of 137925a
Summary: Tests failed: 1, passed: 4079, ignored: 43 Build time: 02:32:31

Failed tests

org.jboss.as.test.layers.LayersTestCase.testServlet: java.lang.Exception: Some unreferenced modules are un-expected [org.apache.commons.cli, org.apache.commons.lang3, org.wildfly.security.elytron-tool]
	at org.jboss.as.test.layers.LayersTest.test(LayersTest.java:109)
	at org.jboss.as.test.layers.LayersTestCase.testServlet(LayersTestCase.java:146)


@wildfly-ci
Copy link

Core - Full Integration Build 11114 outcome was FAILURE using a merge of 137925a
Summary: Tests failed: 1, passed: 4077, ignored: 45 Build time: 02:13:07

Failed tests

org.jboss.as.test.layers.LayersTestCase.testServlet: java.lang.Exception: Some unreferenced modules are un-expected [org.apache.commons.cli, org.apache.commons.lang3, org.wildfly.security.elytron-tool]
	at org.jboss.as.test.layers.LayersTest.test(LayersTest.java:109)
	at org.jboss.as.test.layers.LayersTestCase.testServlet(LayersTestCase.java:146)


@wildfly-ci
Copy link

Core - Full Integration Build 11115 outcome was FAILURE using a merge of 137925a
Summary: Tests failed: 1, passed: 4079, ignored: 43 Build time: 02:11:47

Failed tests

org.jboss.as.test.layers.LayersTestCase.testServlet: java.lang.Exception: Some unreferenced modules are un-expected [org.apache.commons.cli, org.apache.commons.lang3, org.wildfly.security.elytron-tool]
	at org.jboss.as.test.layers.LayersTest.test(LayersTest.java:109)
	at org.jboss.as.test.layers.LayersTestCase.testServlet(LayersTestCase.java:146)


@gaol
Copy link
Contributor

gaol commented Nov 29, 2021

I am not sure about the ElytronToolScriptTestCase failures, but the failure of LayersTestCase.testServlet() in full integration should be fixed via wildfly/wildfly#14970.

@fjuma fjuma force-pushed the WFCORE-5712 branch 2 times, most recently from 050e055 to e6174a1 Compare November 29, 2021 15:48
@jamezp
Copy link
Member

jamezp commented Nov 29, 2021

@fjuma Is there a reason we remove then re-add the module? I assume it's for backwards compatibility, but will that work if the wrapper library is not provided?

@fjuma
Copy link
Contributor Author

fjuma commented Nov 29, 2021

@jamezp ELYTRON_TOOL_ADDONS allows a user to optionally specify their own JAR that contains a custom credential store implementation. Before we updated the script to make use of jboss-modules, if a user defined ELYTRON_TOOL_ADDONS, it was just added to the classpath by the elytron-tool script.

After updating the script to make use of jboss-modules, this classpath approach could no longer work. So instead, if a user defines ELYTRON_TOOL_ADDONS, then the script will just create a new module that contains this JAR using the module add command. For the module add command to succeed, the module cannot already exist. That's why the script first removes the custom module if it happened to exist and then adds it.

@wildfly-ci
Copy link

Core - Full Integration Build 11122 outcome was UNKNOWN using a merge of 0cfad96
Summary: Canceled (Error while applying patch; cannot find commit 62ac754 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/4879/merge branch was updated and the commit selected for the ... Build time: 00:00:16

@fjuma
Copy link
Contributor Author

fjuma commented Nov 29, 2021

@jamezp Thanks for the feedback! I've just merged a commit from @Ashpan that should address your comments on the .bat script.

@wildfly-ci
Copy link

Core - Full Integration Build 11236 outcome was UNKNOWN using a merge of ccfd744
Summary: Canceled (Error while applying patch; cannot find commit 62ac754 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/4879/merge branch was updated and the commit selected for the ... Build time: 00:00:17

@wildfly-ci
Copy link

Core - Full Integration Build 11125 outcome was UNKNOWN using a merge of ccfd744
Summary: Canceled (Error while applying patch; cannot find commit 1f7c4d5 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/4879/merge branch was updated and the commit selected for the ... Build time: 00:00:09

@wildfly-ci
Copy link

Core - Full Integration Build 11238 outcome was UNKNOWN using a merge of ccfd744
Summary: Canceled (Error while applying patch; cannot find commit 1f7c4d5 in the https://github.com/wildfly/wildfly-core.git repository, possible reason: refs/pull/4879/merge branch was updated and the commit selected for the ... Build time: 00:00:12

Copy link
Member

@jamezp jamezp left a comment

Choose a reason for hiding this comment

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

CI is still waiting to run, but the change itself looks okay to me. I did run the script tests on a Windows VM and Linux. They passed on both platforms.

@wildfly-ci
Copy link

Core - Full Integration Build 11127 outcome was FAILURE using a merge of ccfd744
Summary: Tests failed: 1, passed: 4092, ignored: 41 Build time: 02:41:07

Failed tests

org.jboss.as.test.layers.LayersTestCase.testServlet: java.lang.Exception: Some unreferenced modules are un-expected [org.apache.commons.cli, org.apache.commons.lang3, org.wildfly.security.elytron-tool]
	at org.jboss.as.test.layers.LayersTest.test(LayersTest.java:109)
	at org.jboss.as.test.layers.LayersTestCase.testServlet(LayersTestCase.java:146)


@jamezp
Copy link
Member

jamezp commented Nov 29, 2021

That galleon failure needs to be addressed. There is likely an issue on Windows too with the PowerShell script test. This can be fixed with a commit like e6dd928.

@fjuma
Copy link
Contributor Author

fjuma commented Nov 30, 2021

Thanks very much @jamezp! I've included your commit in this PR.

The galleon failure should be addressed by wildfly/wildfly#14970.

@wildfly-ci
Copy link

Core - Full Integration Build 11130 outcome was FAILURE using a merge of fb45e1a
Summary: Tests failed: 1, passed: 4074, ignored: 43 Build time: 02:35:51

Failed tests

org.jboss.as.test.layers.LayersTestCase.testServlet: java.lang.Exception: Some unreferenced modules are un-expected [org.apache.commons.cli, org.apache.commons.lang3, org.wildfly.security.elytron-tool]
	at org.jboss.as.test.layers.LayersTest.test(LayersTest.java:109)
	at org.jboss.as.test.layers.LayersTestCase.testServlet(LayersTestCase.java:146)


@wildfly-ci
Copy link

Core - Full Integration Build 11131 outcome was FAILURE using a merge of fb45e1a
Summary: Tests failed: 1, passed: 4092, ignored: 41 Build time: 02:27:15

Failed tests

org.jboss.as.test.layers.LayersTestCase.testServlet: java.lang.Exception: Some unreferenced modules are un-expected [org.apache.commons.cli, org.apache.commons.lang3, org.wildfly.security.elytron-tool]
	at org.jboss.as.test.layers.LayersTest.test(LayersTest.java:109)
	at org.jboss.as.test.layers.LayersTestCase.testServlet(LayersTestCase.java:146)


@wildfly-ci
Copy link

Core - Full Integration Build 11244 outcome was FAILURE using a merge of fb45e1a
Summary: Tests passed: 7058, ignored: 128; exit code 137 (Step: Build & test full (Maven)) (new) Build time: 03:10:13

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Dec 1, 2021
@yersan
Copy link
Collaborator

yersan commented Dec 1, 2021

@fjuma I did not realize it was going to conflict with another one merged. Could you rebase and resolve the conflict? thanks

@fjuma
Copy link
Contributor Author

fjuma commented Dec 1, 2021

@yersan I've rebased and resolved the conflict. Thanks!

@@ -49,6 +49,10 @@
"org.wildfly.event.logger",
// wildfly-elytron-http-stateful-basic
"org.wildfly.security.http.sfbasic"
// wildfly-elytron-tool
"org.apache.commons.cli",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello @fjuma , missed comma "," after "org.wildfly.security.http.sfbasic"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, fixed.

@yersan
Copy link
Collaborator

yersan commented Dec 1, 2021

The RemotingLegacySubsystemTestCase.testSubsystemWithConnectorPropertyChange failing on "Linux JDK 11" Job is an intermittent error that it is unrelated to this PR, see XNIO-385

@wildfly-ci
Copy link

Core - Full Integration Build 11248 outcome was FAILURE using a merge of b9e9d27
Summary: Tests failed: 1 (1 new), passed: 7124, ignored: 128 Build time: 03:14:04

Failed tests

org.wildfly.test.scripts.AppClientScriptTestCase: java.io.UncheckedIOException: java.nio.file.FileSystemException: /store/work/tc-work/37b47ae8b9c60325/full/testsuite/scripts/target/wildfly/docs/schema/wildfly-elytron_1_0.xsd -> /store/work/tc-work/37b47ae8b9c60325/full/testsuite/scripts/target/wildfly (core)/docs/schema/wildfly-elytron_1_0.xsd: No space left on device
	at org.wildfly.test.common.ServerConfigurator.copy(ServerConfigurator.java:60)
	at org.wildfly.test.common.ServerConfigurator.configure(ServerConfigurator.java:47)
	at org.wildfly.test.scripts.ScriptTestCase.configureEnvironment(ScriptTestCase.java:74)
Caused by: java.nio.file.FileSystemException: /store/work/tc-work/37b47ae8b9c60325/full/testsuite/scripts/target/wildfly/docs/schema/wildfly-elytron_1_0.xsd -> /store/work/tc-work/37b47ae8b9c60325/full/testsuite/scripts/target/wildfly (core)/docs/schema/wildfly-elytron_1_0.xsd: No space left on device
	at sun.nio.fs.UnixException.translateToIOException(UnixException.java:91)
	at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
	at sun.nio.fs.UnixCopyFile.copyFile(UnixCopyFile.java:253)
	at sun.nio.fs.UnixCopyFile.copy(UnixCopyFile.java:581)
	at sun.nio.fs.UnixFileSystemProvider.copy(UnixFileSystemProvider.java:253)
	at java.nio.file.Files.copy(Files.java:1274)
	at org.wildfly.test.common.ServerConfigurator$1.visitFile(ServerConfigurator.java:76)
	at org.wildfly.test.common.ServerConfigurator$1.visitFile(ServerConfigurator.java:65)
	at java.nio.file.Files.walkFileTree(Files.java:2670)
	at java.nio.file.Files.walkFileTree(Files.java:2742)
	at org.wildfly.test.common.ServerConfigurator.copyDirectory(ServerConfigurator.java:65)
	at org.wildfly.test.common.ServerConfigurator.copy(ServerConfigurator.java:57)
	... 21 more


@yersan yersan added the hold Do not merge this PR label Dec 2, 2021
@yersan
Copy link
Collaborator

yersan commented Dec 2, 2021

Holding this PR until see how it integrates with wildfly/wildfly#14988, at least at product branches there is some kind of relationship between them. We can follow the integration here:

https://ci.wildfly.org/viewLog.html?buildTypeId=WF_WildFlyCoreIntegrationExperiments&buildId=285378

@yersan yersan removed the hold Do not merge this PR label Dec 2, 2021
@yersan yersan merged commit cfb4d11 into wildfly:main Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-changed Dependencies have been checked, and there are changes highlighted in a comment ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
6 participants