janhoy commented on PR #854: URL: https://github.com/apache/solr-operator/pull/854#issuecomment-6765444621
🤖 Thanks @danielmeint — you're right, and for a sharper reason than "`JAVA_TOOL_OPTIONS` is awkward": `bin/solr` derives the SolrJ auth mechanism from `SOLR_AUTH_TYPE`, and that mapping changed in Solr 10, while we hardcode the 9-era one. | Solr | what `SOLR_AUTH_TYPE=basic` sets | |---|---| | 8.11, 9.11 | `-Dsolr.httpclient.builder.factory=…impl.PreemptiveBasicAuthClientBuilderFactory` (what we hardcode today) | | 10.1, 11.0 | `-Dsolr.solrj.http.jetty.customizer=…jetty.PreemptiveBasicAuthClientCustomizer` | Verified against authenticated Solr instances (`blockUnknown: true`, probing `/solr/admin/info/system`): | mechanism | 8.11 | 9.11 | 10.1 | (main) | |---|---|---|---|---| | `JAVA_TOOL_OPTIONS` + `-Dbasicauth` + builder factory (today) | ✅ banner, creds in output | ✅ banner, creds in output | ❌ 401 | ❌ | | `SOLR_AUTH_TYPE=basic` + `SOLR_AUTHENTICATION_OPTS="-Dbasicauth=u:p"` | ✅ | ✅ | ✅ | ✅ (deprecation WARN) | | `SOLR_AUTH_TYPE=basic` + `SOLR_SECURITY_AUTH_BASICAUTH_CREDENTIALS=u:p` (env only) | ❌ | ❌ | ✅ | ✅ | | `solr api --credentials u:p` | flag absent | flag absent | ✅ | ✅ | Every working row exits 0 on success and 1 on a wrong password, with no banner and no credentials in the output — so #799 is fixed by construction, with no output filter and no saved-exit-status dance. Plan, assuming this merges after #827: 1. Pass the credentials through `bin/solr`'s own env vars and drop the pipeline entirely, so the probe exit status is the CLI's: `SOLR_AUTH_TYPE=basic SOLR_AUTHENTICATION_OPTS="-Dbasicauth=$(cat …/username):$(cat …/password)" solr api <url-flag> "<url>"`. 2. Remove what that makes dead: the hardcoded `-Dsolr.httpclient.builder.factory` sysprop, the `grep -v JAVA_TOOL_OPTIONS` filter, the `probe_output`/`probe_status` capture, and the unit tests asserting the filter. 3. Drop #827's `--credentials` branch for Solr 10 — one mechanism now covers 8 through 11, and `--credentials` puts the credentials in the process arguments anyway. 4. Keep #827's major-version gate for the URL flag only, which is the one thing that genuinely needs branching: `-get` on 8, `--solr-url` on 9+ (`-get` is deprecated for removal since 9.7 and rejected outright in 10). Optional, in the same PR or later: 5. On Solr 10+, set the credentials via the `SOLR_SECURITY_AUTH_BASICAUTH_CREDENTIALS` env var instead of a sysprop, which keeps them out of the java argv (both `SOLR_AUTHENTICATION_OPTS` and `--credentials` show up in `ps` inside the pod). It does not work on 8/9, so it needs the version gate. Worth noting either way: the credentials stop reaching the probe output that Kubernetes surfaces in pod events, which was the more serious exposure in #799 — argv is readable only with a shell in the pod, pod events with namespace read access. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
