8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions #665

skarsaune · 2025-07-12T14:59:27Z

Note: The majority of the work in this PR is to make it work well with RMI connections. For Jolokia however, it should work out of the box as setting the env var will force https.

Progress

Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JMC-8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions (Bug - P3)

Reviewers

Alex Macdonald (@aptmac - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmc.git pull/665/head:pull/665
$ git checkout pull/665

Update a local copy of the PR:
$ git checkout pull/665
$ git pull https://git.openjdk.org/jmc.git pull/665/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 665

View PR using the GUI difftool:
$ git pr show -t 665

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jmc/pull/665.diff

Using Webrev

Link to Webrev Comment

…ror connection if it fails

bridgekeeper · 2025-07-12T14:59:51Z

👋 Welcome back skarsaune! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

skarsaune · 2025-07-12T15:00:37Z

....openjdk.jmc.browser/src/main/java/org/openjdk/jmc/browser/wizards/ConnectionWizardPage.java


 		fieldStack = new Composite(inner, SWT.NONE);
-		fieldStack.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false));
+		fieldStack.setLayoutData(new GridData(SWT.FILL, SWT.FILL, true, false, 2, 1));


Appearance:

openjdk · 2025-07-12T15:00:44Z

@skarsaune This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions

Reviewed-by: aptmac

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@aptmac) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

skarsaune · 2025-07-12T15:01:17Z

...k.jmc.console.ui/src/main/java/org/openjdk/jmc/console/ui/editor/internal/ConsoleEditor.java

 				// FIXME: Show stacktrace? (Need to show our own ExceptionDialog in that case, or maybe create our own DetailsAreaProvider, see WorkbenchStatusDialogManager.setDetailsAreaProvider)
 				return new Status(IStatus.ERROR, ConsolePlugin.PLUGIN_ID, IStatus.ERROR,
-						NLS.bind(Messages.ConsoleEditor_COULD_NOT_CONNECT, getEditorInput().getName(), e.getMessage()),
+						NLS.bind(e.getLocalizedMessage(), getEditorInput().getName(), e.getMessage()),


Apparently all the more detailed error message is in this method on the exception class

skarsaune · 2025-07-12T15:01:40Z

application/org.openjdk.jmc.jolokia/src/main/java/org/openjdk/jmc/jolokia/JmcJolokiaPlugin.java

 		JolokiaServiceManager serviceManager = JolokiaServiceManagerFactory.createJolokiaServiceManager(configuration,
 				new JulLogHandler(PLUGIN_ID), new AllowAllRestrictor(),
-				() -> new TreeSet<ServerDetector>(Arrays.asList(ServerDetector.FALLBACK)));
+				logHandler -> new TreeSet<ServerDetector>(Arrays.asList(ServerDetector.FALLBACK)));


Unrelated compiltion erro

Reverted this to separate concerns

mlbridge · 2025-07-29T10:57:15Z

Webrevs

bridgekeeper · 2025-08-26T15:39:19Z

@skarsaune This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

skarsaune · 2025-08-27T14:08:23Z

Is this perhaps something you might have a look at @aptmac ?

bridgekeeper · 2025-09-24T19:08:52Z

@skarsaune This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

aptmac

Overall looks good to me, commented with a couple of minor things

....openjdk.jmc.browser/src/main/java/org/openjdk/jmc/browser/wizards/ConnectionWizardPage.java

…cure-connection

skarsaune · 2025-10-14T14:43:56Z

/integrate

openjdk · 2025-10-14T14:45:33Z

@skarsaune
Your change (at version 850a233) is now ready to be sponsored by a Committer.

aptmac · 2025-10-14T14:47:34Z

/sponsor

openjdk · 2025-10-14T14:47:57Z

Going to push as commit 2bb16fa.

openjdk · 2025-10-14T14:48:06Z

@aptmac @skarsaune Pushed as commit 2bb16fa.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

JMC-8365: Add option to require secure connection and give hint in er…

da8fc0d

…ror connection if it fails

skarsaune commented Jul 12, 2025

View reviewed changes

skarsaune changed the title ~~JMC-8365: Add option to require secure JMX connection and give hint in error message if it fails~~ JMC-8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions Jul 12, 2025

openjdk bot changed the title ~~JMC-8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions~~ 8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions Jul 12, 2025

skarsaune added 5 commits July 12, 2025 17:14

JMC-8365: Fixed spots

65fde6d

JMC-8365: Remove premature change

b6fe292

JMC-8369: revert JmcJolokiaPlugin.java

a98a95b

JMC-8365: Actually reflect require secure connection back in wizard

b332e12

JMC-8365: Make secure button updating null safe

59d6208

skarsaune marked this pull request as ready for review July 29, 2025 10:53

openjdk bot added the rfr label Jul 29, 2025

skarsaune added 2 commits July 29, 2025 13:02

JMC-8365: Fix spots

3e59aae

JMC-8365: Make secure button updating null safe

161f0d3

aptmac reviewed Oct 8, 2025

View reviewed changes

....openjdk.jmc.browser/src/main/java/org/openjdk/jmc/browser/wizards/ConnectionWizardPage.java Outdated Show resolved Hide resolved

....openjdk.jmc.browser/src/main/java/org/openjdk/jmc/browser/wizards/ConnectionWizardPage.java Outdated Show resolved Hide resolved

skarsaune added 2 commits October 11, 2025 09:10

JMC-8365: Review fixes: whitespace correction and better naming

af93702

Merge remote-tracking branch 'origin/master' into JMC-8365-require-se…

850a233

…cure-connection

aptmac approved these changes Oct 14, 2025

View reviewed changes

openjdk bot added the ready label Oct 14, 2025

openjdk bot added the sponsor label Oct 14, 2025

openjdk bot added the integrated label Oct 14, 2025

openjdk bot closed this Oct 14, 2025

openjdk bot removed ready rfr sponsor labels Oct 14, 2025

8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions #665

8365: Jolokia - Additional support for security mechanisms related to SSL / authentication, discovery restrictions #665

Uh oh!

Conversation

skarsaune commented Jul 12, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Jul 12, 2025

Uh oh!

skarsaune Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

openjdk bot commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skarsaune Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

skarsaune Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

skarsaune Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

mlbridge bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

bridgekeeper bot commented Aug 26, 2025

Uh oh!

skarsaune commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bridgekeeper bot commented Sep 24, 2025

Uh oh!

aptmac left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

skarsaune commented Oct 14, 2025

Uh oh!

openjdk bot commented Oct 14, 2025

Uh oh!

aptmac commented Oct 14, 2025

Uh oh!

openjdk bot commented Oct 14, 2025

Uh oh!

openjdk bot commented Oct 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

skarsaune commented Jul 12, 2025 •

edited by openjdk bot

Loading

openjdk bot commented Jul 12, 2025 •

edited

Loading

mlbridge bot commented Jul 29, 2025 •

edited

Loading

skarsaune commented Aug 27, 2025 •

edited

Loading