SOLR-18527: Provide a example of a secure Solr using bin/solr start -e secure by epugh · Pull Request #5306 · apache/solr

epugh · 2026-11-06T12:18:12Z

https://issues.apache.org/jira/browse/SOLR-18527

Description

Introduces a bin/solr start -e secure that demonstrates how to set up a 2 node Solr with security.

I put this together for two reasons: 1) It's very time consuming to test out a feature against a secure Solr, so I don't often do that step. I'd like to make it easier to have a setup that is part of th eproject that i can test out changes against. 2) Make it easier for. an end user to evaluate what a secure solr looks like so they can make smarter deployment choices.

And then, maybe a bonus reason, having an operating example makes it easier to verify all the steps in the tutorial match to what the secure solr example does....

Solution

I first played around with lots of LLM support in #5107, and then out of that effort learned some stuff and then opened a thread on dev@ https://lists.apache.org/thread.html/t54kplk8k3ylw52mrzplr710bjdxd531. Plus did a brainstoriming session with @gerlowskija.

This example only runs on linux/osx, and requires openssl and some other utilities to be installed.

I made a concious effort to NOT change Solr by adding new features. I just watned to demo what is already threre.

Because of that, we currently don't have HTTP/TLS support for ZooKeeper... That required some fixes to Zookeeper server (see that original WIP), and so I will do that in it's own PR. I also didn't try to make the tooling around ssl easier with a bin/solr ssl tool at this point either.

Tests

Manual testing, plus a bats for the example. I have struggled to not let my LLM genreate sooooo many new tests!!!

github-actions · 2026-11-06T12:18:49Z

Automated PR triage (LLM-generated; may be wrong). Checked once, when the PR was opened or marked ready; not updated on later pushes.

1 possible mismatch(es) between what this change warrants and what the PR includes.

	Warranted?	In PR?	Confidence	Why
✅ Jira issue in title	—	yes	—	Already present; not judged.
⚠️ Changelog — missing	added	no entry	high	The PR introduces a new `secure` example for `bin/solr start -e`, which is a new opt-in feature for users to evaluate a secure Solr setup, warranting an 'added' changelog entry.
✅ Reference Guide update	yes	yes	high	The PR adds a new example type `secure` to the CLI, which is documented in the Reference Guide's getting started section and CLI reference, and the description explicitly mentions adding a tutorial page.
✅ Upgrade notes	no	no	high	The change is purely additive, introducing a new example option without modifying existing behavior, parameters, or defaults.
✅ Tests	no	no	high	The changes are primarily to the CLI example launcher (`RunExampleTool.java`) and shell scripts (`bin/solr`), which fall under developer tooling and build/CI infrastructure that does not require new unit tests, and the PR already includes comprehensive BATS integration tests.

⚠️ mismatch, ❔ uncertain or unanswered, ✅ agrees. Matching changelog, docs and test files were hidden from the model; the PR description may still mention them. File presence does not establish adequacy. Assessed commit 86818a97fdfd2b29d82cfcd50a7397769bdffafc.

dsmiley · 2026-11-06T15:06:16Z

Am I correct that you created this PR as non-draft but then you converted to draft? This is why the new PR Triage LLM looked at this (it was for a moment not a draft).

janhoy · 2026-11-06T16:22:23Z

Am I correct that you created this PR as non-draft but then you converted to draft? This is why the new PR Triage LLM looked at this (it was for a moment not a draft).

This is a good reason going forward to alwasy open new PRs in draft mode, and first when you want to let others in, flip the switch. I often failed at this myself, and now we have another reason.

A disadvantage is that external contributors may "waste" the entire LLM review the moment they open a draft PR. And they don't even get hints on what are the best practices that the triager will evaluate on, since we removed those from the template. But I guess you cannot have both?

epugh · 2026-11-06T16:22:31Z

Am I correct that you created this PR as non-draft but then you converted to draft? This is why the new PR Triage LLM looked at this (it was for a moment not a draft).

correct... I realized I still had a lot of editing to do on the tutorial text.

epugh · 2026-11-08T15:31:26Z

we got sto figure out how to make bats tests run faster! the dupliccated picocli tests add 8 more minutes!

dsmiley · 2026-11-08T22:24:58Z

8 more minutes!

Out of control!
It's not surprising the tests are slow as they are the most integration-y tests we have. I have some ideas but this issue isn't the place.

The double running of commons cli and picocli tests under bats pushing us over limits.

I was struggling to retest things because of the need for the --filter-tags which is a temporary artifact of our cli split and can be removed then.

… in startSolr The secure example's port-range check (1024-64535) and whitespace-in-path guard were specific to runSecureExample, but every example type shares the same port and solr-home handling. The port check now runs once in the shared dispatch path (both the commons-cli and picocli implementations) so cloud/techproducts/schemaless/films get the same friendly validation. The whitespace guard masked a real bug: startSolr() built its bin/solr invocation as a single string and re-tokenized it with CommandLine.parse(), which silently mis-splits a quoted --solr-home segment containing whitespace. Rewriting it to build the command as discrete arguments (the way the secure example's runSecureCommand already does) fixes the underlying issue directly, so the guard is no longer needed and is removed. StartSolrParams.extraArgs moves from a pre-joined String to a List<String> for the same reason - a joined string relies on re-splitting by whitespace, which is exactly the bug being fixed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RwsEje8u9apx3SRzissZm

epugh · 2026-11-09T15:07:30Z

8 more minutes!

Out of control! It's not surprising the tests are slow as they are the most integration-y tests we have. I have some ideas but this issue isn't the place.

I moved the new secure example bats test to being run only Nightly. I also tweaked our tooling a bit so that a nightly bats test can be run by --tests to make running them easier.

Merging upstream/main (16 commits, including apache#5289's slim-distribution example-dir handling) brought in a new TestSolrCLIRunExample test that asserted the pre-secure-example "Unsupported example" message. Our branch's RunExampleTool already lists "secure" among the valid examples, so the merged test's expected string needs the same update. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RwsEje8u9apx3SRzissZm

TestSolrCLIRunExample picked ports via `new ServerSocket(0)`, which lets the OS hand back anything in its ephemeral range (49152-65535 on this machine). bin/solr derives the embedded ZooKeeper port as port + 1000, so a base port above 64535 overflows past the valid port range and now fails RunExampleTool's port-range validation (and would have failed via bin/solr's own SOLR_PORT_LISTEN check regardless). Retry until the OS hands back a port within the valid range instead of occasionally hitting this rare, non-deterministic flake. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RwsEje8u9apx3SRzissZm

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RwsEje8u9apx3SRzissZm

…Line build - Extract validatePort(int) so the commons-cli and picocli dispatch paths share one port-range check instead of two copy-pasted blocks. - Compute String.join(" ", command) once in startSolr() and reuse it across the echo and all three failure messages instead of rejoining the list each time. - Build the commons-exec CommandLine after the checkPortConflict() early-return instead of before it, restoring the original lazy construction so the "already running" path doesn't build and discard it for nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RwsEje8u9apx3SRzissZm

epugh · 2026-11-10T00:22:26Z

Okay, all bats tests pass locally that time out on ci.

I would love a review!!!

gerlowskija

Hey, this looks great. I've left a number of changes inline that I'd love responses on, but otherwise really cool to see this added!

gerlowskija · 2026-11-10T16:58:43Z

+    echo("ZooKeeper is protected by digest ACLs; its transport remains plaintext on loopback.");
+    if (!params.noPrompt()) {
+      prompt(
+          new Scanner(userInput, StandardCharsets.UTF_8),


[0] runCloudExample already creates its own Scanner. Could we pull that Scanner-creation code into somewhere that'd be reusable?

Done — pulled it into a shared userInputScanner() helper used by both runCloudExample and runSecureExample.

gerlowskija · 2026-11-11T10:49:34Z

+Do not run these commands against an existing cluster.
+
+IMPORTANT: This is a local learning exercise with demonstration passwords and a shared self-signed certificate.
+Keep the cluster on loopback.


[0] "Keep the cluster on loopback" implies it, but IMO we should really say explicitly here: "this exact set up is not ready or appropriate for production"

Done, made it explicit.

gerlowskija · 2026-11-11T10:54:44Z

+== Create a Certificate and PKCS12 Stores
+
+Generate a private key and self-signed certificate directly in a PKCS12 keystore.
+The subject alternative names cover both names used in this exercise:


[0] "The subject alternative names"

I'm having trouble understanding what this means. I'd guess it's referring to the -subject flag given to the final openssl command below in some way, but I don't know what "alternative names" means since that flag isn't taking an argument.

Maybe some wording cleanup would be worthwhile here to make this clearer.

Done — reworded to point directly at the -ext 'SAN=...' flag so the connection is explicit.

gerlowskija · 2026-11-11T10:56:08Z

+== Enable HTTPS on Both Nodes
+
+Write an include file used only for this exercise.
+The unquoted heredoc expands the absolute working-directory path now; escaped dollar signs preserve variables for later launches:


[0] Might be simpler if we just give them the file content to copy/paste, rather than giving them a heredoc syntax and having to explain how various quotes are escaped or not escaped 😛

Didn't make this change — the heredocs aren't just a syntax choice, they're doing real work: expanding $TUTORIAL_HOME into an absolute path so every subsequent command works regardless of where the reader unpacked Solr. Switching to static copy-paste file content would mean either hardcoding a path (breaks if they used a different directory) or asking the reader to manually find/replace it themselves, which seems like a worse experience than the current escaping. Happy to reconsider if you see a cleaner way to thread that through — let me know if you'd like another pass at it.

gerlowskija · 2026-11-11T10:59:01Z

+Hostname verification remains enabled.
+Client certificates are not required; later steps authenticate users with Basic Authentication over HTTPS.
+
+Before restarting, set the cluster's advertised URL scheme while ZooKeeper is still running.


[0] "Before restarting" is the first mention that the we actually have to restart our nodes in order to pick up HTTPS. I imagine this'll be confusing and take some explaining for folks not already very familiar with Solr, who are likely to have the question: "Why didn't I just pick these settings before starting Solr the first time?"

[Q] Also, do we really need the bin/solr clutster --property urlScheme bit below? Maybe our docs are incorrect, but I don't see it in the Enabling SSL mini-tutorial. Are we sure this is really needed?

Both addressed. Added an explicit heads-up before the include-file rewrite that a restart is coming. On the urlScheme question: you were right to be suspicious — I removed the 'bin/solr cluster --property urlScheme --value https' step entirely from both the tool and the doc, and verified via a live run of test_secure_example.bats (including the assertions on https:// shard addresses) that it's unnecessary. This lines up with the Solr 10.1 upgrade note that SOLR_SSL_ENABLED now suffices for scheme detection — the property is redundant since then.

gerlowskija · 2026-11-11T11:05:50Z

[0] This page has some duplication with our enabling-ssl.adoc page, which also shares some very similar commands for turning on SSL, generating certs, etc.

I guess some duplication is inevitable when taking reference content and turning it into a more descriptive, readable README. But if you haven't yet it might be worth taking a look at the two pages to make sure there's not too much duplication...

epugh · 2026-11-11T13:46:15Z

Why is zkDigestPassword=SolrRocks unencrypted? How do we supply that encrypted?

- Generate a fresh random password per run instead of a hardcoded "SolrRocks" everywhere (keystore, HTTP basic-auth accounts, ZooKeeper digest accounts); print it once up front. Threaded through secureExampleRequest() and the bats test, which now extracts it from the command output instead of asserting on a literal. - Drop the "set urlScheme cluster property" step entirely. Verified by running test_secure_example.bats with and without it: identical pass, including the assertions on https:// shard addresses. This matches the Solr 10.1 upgrade note that SOLR_SSL_ENABLED now suffices for scheme detection, making the property redundant. - Extract the duplicated `new Scanner(userInput, ...)` construction (runCloudExample and runSecureExample each built their own) into a shared userInputScanner() helper. - Reword the "unsupported option" error message and add a comment in solr/bin/solr explaining why the example-detection loop re-scans raw args instead of reusing the $EXAMPLE variable set by the later option-parsing loop (that loop runs after the ulimit check, so it can't be reused for ulimit-suppression timing). - Tutorial doc: state explicitly that the setup isn't production-ready, clarify what "subject alternative names" refers to, foreshadow that nodes need restarting to pick up HTTPS, drop the now-unnecessary urlScheme step, and correct the claim that the secure example always uses a fixed password. Verified via a live run of test_secure_example.bats (both tests passing) after each substantive change, not just compilation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RwsEje8u9apx3SRzissZm

epugh · 2026-11-11T16:15:05Z

Really great commenting @gerlowskija !

epugh added 4 commits November 5, 2026 09:24

Document scoped secure Solr tutorial design

80f87c9

Plan two-node HTTPS setup and inter-node TLS verification

7c0f279

Add two-node secure Solr tutorial and shell example

a45af6b

Finally a first working draft.

86818a9

github-actions Bot added documentation Improvements or additions to documentation start-scripts examples cat:cli labels Nov 6, 2026

epugh commented Nov 6, 2026

View reviewed changes

Comment thread docs/rubypowers/plans/2026-11-05-secure-solr-tutorial.md Resolved

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/bin/solr Resolved

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/bin/solr Resolved

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/core/src/java/org/apache/solr/cli/RunExampleTool.java Resolved

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/core/src/java/org/apache/solr/cli/RunExampleTool.java Resolved

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/core/src/java/org/apache/solr/cli/RunExampleTool.java Resolved

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/core/src/java/org/apache/solr/cli/RunExampleTool.java Resolved

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/core/src/java/org/apache/solr/cli/RunExampleTool.java Resolved

epugh marked this pull request as draft November 6, 2026 12:44

dsmiley mentioned this pull request Nov 6, 2026

LLM-assisted PR triage #5281

Merged

Review again the tutorial

7f6a5fe

epugh marked this pull request as ready for review November 6, 2026 17:29

epugh added 3 commits November 6, 2026 12:30

dont need this stuff

5ff747a

doc the change

7964605

don't need this

e809fa2

epugh commented Nov 6, 2026

View reviewed changes

Comment thread solr/packaging/test/test_secure_example.bats Resolved

could more language tweaks

041181e

epugh requested a review from janhoy November 6, 2026 17:38

epugh requested a review from gerlowskija November 6, 2026 17:38

half the time, but still 60 seconds...

01c6518

dsmiley mentioned this pull request Nov 6, 2026

PR triage: compact bullet comment; judge again on every ready_for_review #5308

Merged

epugh added 2 commits November 9, 2026 07:17

Move this to nightly to get under the timeout limit in ci of 40 minutes

9da8c7b

The double running of commons cli and picocli tests under bats pushing us over limits.

Allow specific nightly test to run via --tests key only

15f0987

I was struggling to retest things because of the need for the --filter-tags which is a temporary artifact of our cli split and can be removed then.

github-actions Bot added the tool:build label Nov 9, 2026

epugh and others added 7 commits November 9, 2026 10:24

tweak text

786d72f

nicer comment

93bbacc

Merge remote-tracking branch 'upstream/main' into SOLR-18527

c47177f

Simplify pickFreePort javadoc to one line

f647d42

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RwsEje8u9apx3SRzissZm

github-actions Bot added the tests label Nov 9, 2026

gerlowskija approved these changes Nov 11, 2026

View reviewed changes

epugh merged commit 780653b into apache:main Nov 11, 2026
4 of 13 checks passed

epugh added this to the 10.x milestone Nov 11, 2026

Conversation

epugh commented Nov 6, 2026

Description

Solution

Tests

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dsmiley commented Nov 6, 2026

Uh oh!

janhoy commented Nov 6, 2026

Uh oh!

epugh commented Nov 6, 2026

Uh oh!

Uh oh!

epugh commented Nov 8, 2026

Uh oh!

dsmiley commented Nov 8, 2026

Uh oh!

epugh commented Nov 9, 2026

Uh oh!

epugh commented Nov 10, 2026

Uh oh!

gerlowskija left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

epugh commented Nov 11, 2026

Uh oh!

epugh commented Nov 11, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants