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
SOLR-15809 Get rid of blacklist/whitelist terminology #424
Conversation
IP_ACL_OPTS=("-Dsolr.jetty.inetaccess.includes=${SOLR_IP_WHITELIST}" \ | ||
"-Dsolr.jetty.inetaccess.excludes=${SOLR_IP_BLACKLIST}") | ||
IP_ACL_OPTS=("-Dsolr.jetty.inetaccess.includes=${SOLR_IP_ALLOWLIST}" \ | ||
"-Dsolr.jetty.inetaccess.excludes=${SOLR_IP_DENYLIST}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have tried to support both in parallel for some time and warned when detecting the old, but it's a hassle across bash and Windows - any strong feelings about this? We spell it out in upgrade notes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thought, since this is about security (IP filtering), I plan to make the bin/solr scripts print an error and exit if the old env.var is set. That is fail-fast, and users can re-configure to use the new variable names and start again. Already tested a fix for linux. Need to test same for bin/solr.cmd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented and tested also for Windows.
@@ -82,6 +82,9 @@ All the usages are replaced by BaseHttpSolrClient.RemoteSolrException and BaseHt | |||
|
|||
* SOLR-11623: Every request handler in Solr now implements PermissionNameProvider. Any custom or 3rd party request handler must also do this | |||
|
|||
* SOLR-15809: Get rid of blacklist/whitelist terminology. JWTAuthPlugin parameter `algWhitelist` is now `algAllowlist`. The old parameter will still |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe state that they will be removed in Solr 10? Or I guess we can wait for the Solr 10 upgrade notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That can go in solr 10 when we remove support...
@@ -113,11 +113,11 @@ function personality_modules | |||
case ${moduleType} in | |||
submodules) | |||
for module in "${CHANGED_MODULES[@]}"; do | |||
if [[ ! "${module}" =~ ^lucene/(licenses|site) ]]; then # blacklist lucene/ dirs that aren't modules |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this whole file is defunct post-split, but that's another conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, need some cleanup in the dev-tools folder. I think we'll get to that once 9.0 work progresses...
# Conflicts: # solr/CHANGES.txt # solr/solr-ref-guide/src/major-changes-in-solr-9.adoc
…when starting solr
https://issues.apache.org/jira/browse/SOLR-15809
A good bulk of changes to whitelist / blacklist terminology across the codebase and docs