[ELY-2343] Add a web option to the Elytron Tool #1715

petrberan · 2022-06-09T13:24:32Z

https://issues.redhat.com/browse/ELY-2343

petrberan · 2022-11-30T16:36:01Z

Hi @Skyllarr @fjuma can I ask for review please?

fjuma · 2022-12-02T15:56:20Z

@petrberan Sorry for the delay! We will try to take a look next week. Thanks!

Skyllarr · 2022-12-02T18:57:04Z

tool/src/main/java/org/wildfly/security/tool/CredentialStoreCommand.java

+                Desktop desktop = Desktop.getDesktop();
+                if (desktop.isSupported(Desktop.Action.BROWSE)){
+                    try {
+                        desktop.browse(new URI("https://docs.wildfly.org/26.1/WildFly_Elytron_Security.html#CredentialStore"));


@petrberan Just a minor, since the version is in each URL and it will have to be updated in the future, it would be better to have it parametrized. Also https://docs.wildfly.org can be made into a constant

@Skyllarr Good point. Do you know what would be a good way to pass the parameter?

@petrberan Sorry I might have used a wrong word. I did not mean adding a parameter to the elytron tool that would specify the version. That would get more complicated to specify the version when using the tool with EAP for example etc. I meant it's better to make a constant out of "https://docs.wildfly.org" and eg. DOCS_VERSION="27"

@petrberan Otherwise this PR LGTM, thank you!

@Skyllarr I was also wondering how the version would pass from WFLY to WFCORE and then to ELY. Created a constants for the doc URI in each command class, should be alright now

We'd need to remember to update the DOCS_VERSION after each WildFly release. It would be nice if there was a latest URL that we could point to instead but I don't think we have something like that.

@fjuma When using tool with EAP, it is okay that it directs users to wildfly docs instead of EAP right?

Let's see what @OndrejKotek thinks when he's back from PTO.

We also need to check with @OndrejKotek to see if he thinks this should be handled as an RFE since it introduces a new option for Elytron Tool.

Regarding upstream docs in product. I'm not sure, asking higher instances. However, I think there should be a possibility to change the URLs easily, e.g. by building with a provided properties file with URLs.

Regarding having the WF version (and maybe even doc URL) hard-coded. Can this parametrization be moved a bit closer to releasing so that we don't forget to update to the latest? E.g. as property in pom.xml? Or as a properties file maintained along to WF documentation, referenced in pom.xml?

Regarding RFE process. The change is quite simple, however if we would go the way with different URLs for the product we needed some agreements and tests. Maybe still doable in scope of an JBEAP. Let's discuss this on the next Ely team meeting.

petrberan · 2022-12-08T15:00:20Z

@Skyllarr @fjuma Looking at the command classes and how some of the them share the same constants, what is your opinion on creating some form of ElytronToolConstants class that would contain all constants for the Tool commands?

Got this idea when copying the same URI and docs version for 4 classes as the parent Command class was a bad place to hold the constant. That way - for example when changing the version of docs - you don't have to change 5 classes but just 1.

In case that's a good idea, feel free to create a new JIRA issue and assign it to me

petrberan · 2023-02-13T13:41:05Z

Hi @fjuma can I ask for review please?

fjuma · 2023-03-16T20:23:10Z

@Skyllarr Just to check, have you tried this out manually?

@cam-rod If you have some time and could try this out as well, that would be great.

Skyllarr · 2023-03-16T20:34:11Z

@fjuma yes I checked it manually before and it was working well. But would be great for @cam-rod to try out as well

jessicarod7

Tested it with the shaded JAR, looks good! A couple things I noticed:

This error message appeared each time. I'm not sure if it's due to me using the shaded JAR or something else, but it didn't appear when running --help

Mar 16, 2023 4:42:02 PM org.jboss.logmanager.JBossLoggerFinder getLogger
ERROR: The LogManager accessed before the "java.util.logging.manager" system property was set to "org.jboss.logmanager.LogManager". Results may be unexpected.
Opening in existing browser session.

Tested in a container to try when a browser isn't available, didn't seem to lead to major errors. The error message java.io.IOException: Unable to open the browser. only appeared when using the --debug flag, it might be nicer to show that all the time.

Otherwise, nice work!

fjuma · 2023-03-16T20:53:21Z

Tested it with the shaded JAR, looks good! A couple things I noticed:
This error message appeared each time. I'm not sure if it's due to me using the shaded JAR or something else, but it didn't appear when running --help
Mar 16, 2023 4:42:02 PM org.jboss.logmanager.JBossLoggerFinder getLogger
ERROR: The LogManager accessed before the "java.util.logging.manager" system property was set to "org.jboss.logmanager.LogManager". Results may be unexpected.

Thanks for trying that out! I think #1843 fixed the LogManager issue, this branch might need a rebase.

bstansberry · 2023-03-23T13:41:51Z

@petrberan @fjuma This ties the Elytron project to the WildFly project in new and problematic ways.

The WildFly project doesn't make guarantees to the Elytron project regarding the structure of its documentation URLs, even for bits like the "WildFly_Elytron_Security" path element and subsequent anchors that are generally (but not entirely) under the control of people working on Elytron. Of course the two projects are intimately related and people cooperate, but this introduces a long term coordination requirement between the projects.
It creates a chicken and egg problem. For example "27" is not the correct value for the Elytron release that would include this work. Once WF 28 is released, Elytron would need to change to 28, but that would be wrong too, as that changed code would go into 29. The "27" is sure to be an issue, but a similar problem would apply if WildFly made any change to its docs URLs that impact what's done here.
It ties use of Elytron to WildFly. But Elytron is used in other contexts, e.g. JBoss EAP.

Skyllarr · 2023-03-24T14:29:27Z

@fjuma @bstansberry Just an idea to throw out there. Maybe we could create a short blog posts on https://wildfly-security.github.io/wildfly-elytron/blog/ for each of the tool commands and that would be where the web option would point. The URL of our blogs is under our control and should not be changing any time soon, and we can make edits to the posts without waiting for releases.

Disadvantage still is that users, mostly Jboss EAP users, would expect proper documentation links instead of blog posts, and we would not make new posts for each WF release so on older versions it could have missing options.

fjuma · 2023-03-27T14:31:28Z

@bstansberry @Skyllarr Thanks for the feedback here!

The Elytron website has a Guides section. Building on @Skyllarr's idea, that might be an option as well, i.e., add guides for Elytron Tool commands to our website. These could be kept up to date as changes are made. Just something to consider. But we'd still want a way to replace the URL.

petrberan · 2023-11-01T11:26:50Z

@fjuma Getting back to this again. Wouldn't it make sense to have the help in the Docs section on the web - https://wildfly-security.github.io/wildfly-elytron/docs/ ? There it could be even split into versions, the version in the command would either change depending on the string or it could be derived from the version in pom.xml

petrberan · 2023-11-23T12:15:16Z

Hi @fjuma just sending a reminder about his PR

fjuma · 2023-12-22T21:13:27Z

Hi @petrberan, thanks for the reminder and apologies for the delayed response. There have been a lot of discussions recently about having a latest URL for the WildFly docs (which would prevent us from having to update the URL for each release), a new Guides page for wildfly.org, etc. However, as was pointed out before, these options would still result in us depending on the WildFly project which isn't ideal.

The docs page you mentioned (https://wildfly-security.github.io/wildfly-elytron/docs/) points to the JavaDoc for Elytron so that could be a good candidate. The Elytron Tool commands are private API, their JavaDoc appears here:

https://wildfly-security.github.io/wildfly-elytron/documentation/api/next/org/wildfly/security/tool/package-summary.html

We could potentially update the JavaDoc to add more details and then the web option could point there. That way, this would be under our control and as you mentioned, this would be versioned.

@petrberan @Skyllarr WDYT?

fjuma · 2023-12-22T21:14:41Z

Just a reminder for the new year, please update this PR so it's against the 2.x branch. Thanks!

ivassile · 2024-02-27T20:14:32Z

@petrberan Can you please update the PR to be against 2.x? Thanks!

ivassile · 2024-08-20T19:25:55Z

@petrberan Can you please update the PR to be against 2.x so we can proceed? Thanks!

rsearls · 2024-09-18T17:48:38Z

This very old PR has been rebased to 2.x current. The update has been submitted
as new PR #2198

This PR can be closed.

fjuma · 2024-09-18T18:57:19Z

Thanks @rsearls, closing this one.

petrberan · 2024-09-25T15:17:35Z

Thanks for opening a PR for 2.x @rsearls . I originally waited for update on to where this option should point to, which I don't think I see and Wildfly documentation for version 27 seems outdated after 2 years since this PR was opened.

Still can't find where the link in there should lead, sadly.

Also please bear in mind that this PR requires wildfly/wildfly-core#5124 if I'm not mistaken

Skyllarr reviewed Dec 2, 2022

View reviewed changes

[ELY-2343] Add a web option to the Elytron Tool

8d1d651

petrberan force-pushed the ELY-2343 branch from 85b9c07 to 8d1d651 Compare December 8, 2022 14:53

petrberan requested a review from fjuma as a code owner December 8, 2022 14:53

jessicarod7 reviewed Mar 16, 2023

View reviewed changes

fjuma added the rebase this label Dec 22, 2023

rsearls mentioned this pull request Sep 18, 2024

[ELY-2343] Add a web option to the Elytron Tool #2198

Open

fjuma closed this Sep 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ELY-2343] Add a web option to the Elytron Tool #1715

[ELY-2343] Add a web option to the Elytron Tool #1715

petrberan commented Jun 9, 2022

petrberan commented Nov 30, 2022

fjuma commented Dec 2, 2022

Skyllarr Dec 2, 2022

petrberan Dec 6, 2022

Skyllarr Dec 8, 2022 •

edited

Loading

Skyllarr Dec 8, 2022

petrberan Dec 8, 2022

fjuma Mar 16, 2023

Skyllarr Mar 16, 2023

fjuma Mar 16, 2023

OndrejKotek Mar 20, 2023

petrberan commented Dec 8, 2022

petrberan commented Feb 13, 2023

fjuma commented Mar 16, 2023

Skyllarr commented Mar 16, 2023

jessicarod7 left a comment

fjuma commented Mar 16, 2023

bstansberry commented Mar 23, 2023

Skyllarr commented Mar 24, 2023 •

edited

Loading

fjuma commented Mar 27, 2023

petrberan commented Nov 1, 2023

petrberan commented Nov 23, 2023

fjuma commented Dec 22, 2023 •

edited

Loading

fjuma commented Dec 22, 2023

ivassile commented Feb 27, 2024

ivassile commented Aug 20, 2024

rsearls commented Sep 18, 2024

fjuma commented Sep 18, 2024

petrberan commented Sep 25, 2024

[ELY-2343] Add a web option to the Elytron Tool #1715

[ELY-2343] Add a web option to the Elytron Tool #1715

Conversation

petrberan commented Jun 9, 2022

petrberan commented Nov 30, 2022

fjuma commented Dec 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Skyllarr Dec 8, 2022 • edited Loading

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

Choose a reason for hiding this comment

petrberan commented Dec 8, 2022

petrberan commented Feb 13, 2023

fjuma commented Mar 16, 2023

Skyllarr commented Mar 16, 2023

jessicarod7 left a comment

Choose a reason for hiding this comment

fjuma commented Mar 16, 2023

bstansberry commented Mar 23, 2023

Skyllarr commented Mar 24, 2023 • edited Loading

fjuma commented Mar 27, 2023

petrberan commented Nov 1, 2023

petrberan commented Nov 23, 2023

fjuma commented Dec 22, 2023 • edited Loading

fjuma commented Dec 22, 2023

ivassile commented Feb 27, 2024

ivassile commented Aug 20, 2024

rsearls commented Sep 18, 2024

fjuma commented Sep 18, 2024

petrberan commented Sep 25, 2024

Skyllarr Dec 8, 2022 •

edited

Loading

Skyllarr commented Mar 24, 2023 •

edited

Loading

fjuma commented Dec 22, 2023 •

edited

Loading