dsmiley commented on code in PR #3504:
URL: https://github.com/apache/solr/pull/3504#discussion_r2296327489


##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -132,10 +133,10 @@ public void handleRequestBody(SolrQueryRequest req, 
SolrQueryResponse rsp) throw
     String httpMethod = (String) req.getContext().get("httpMethod");
     Command command = new Command(req, rsp, httpMethod);
     if ("POST".equals(httpMethod)) {
-      if (configEditing_disabled || isImmutableConfigSet) {
+      if (!configEditingEnabled || isImmutableConfigSet) {
         final String reason =
-            configEditing_disabled
-                ? "due to " + CONFIGSET_EDITING_DISABLED_ARG
+            !configEditingEnabled
+                ? "due to " + CONFIGSET_EDITING_ENABLED_ARG
                 : "because ConfigSet is immutable";

Review Comment:
   to my point -- even "immutable" is the word chosen to communicate this to 
the user



##########
solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java:
##########
@@ -102,9 +103,9 @@
 public class SolrConfigHandler extends RequestHandlerBase
     implements SolrCoreAware, PermissionNameProvider {
   private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-  public static final String CONFIGSET_EDITING_DISABLED_ARG = 
"disable.configEdit";
-  public static final boolean configEditing_disabled =
-      Boolean.getBoolean(CONFIGSET_EDITING_DISABLED_ARG);
+  public static final String CONFIGSET_EDITING_ENABLED_ARG = 
"solr.configset.edit.enabled";

Review Comment:
   I'm not sure we can equate "config editing" with "configset editing".  
Maybe.  This flag controls the [Config API 
](https://solr.apache.org/guide/solr/latest/configuration-guide/config-api.html)
 which is conceptually similar to but not identical to configset.  It's a 
separate area of the code and documentation, so I think it's setting should be 
reflective of that.  Perhaps `solr.api.config.immutable` or 
`solr.configset.configapi.immutable`
   
   In my example there, I suggest `immutable` instead of `edit` as I think that 
a more general word and is in fact how we display a message to the user below...



##########
solr/solr-ref-guide/modules/configuration-guide/pages/configsets-api.adoc:
##########
@@ -88,7 +88,7 @@ The output will look like:
 Upload a configset, which is sent as a zipped file.
 A single, non-zipped file can also be uploaded with the `filePath` parameter.
 
-This functionality is enabled by default, but can be disabled via a runtime 
parameter `-Dconfigset.upload.enabled=false`.
+This functionality is enabled by default, but can be disabled via a runtime 
parameter `-Dsolr.configset.upload.enabled=false`.

Review Comment:
   Maybe `solr.configset.immutable` would be a more universal way to 
communicate.... immutability?  Instead of "updload", "edit", ....
   
   Oh, I see this flag only controls upload while it's possible to delete and 
clone.  @gerlowskija , shouldn't the underlying protection be about 
immutability generally instead of merely upload specifically?  Just upload 
seems like half of a security measure.



##########
solr/solrj/src/java/org/apache/solr/common/util/EnvUtils.java:
##########
@@ -232,6 +232,14 @@ static synchronized void init(
               deprecatedKey,
               key);
           setProperty(key, String.valueOf(!Boolean.getBoolean(deprecatedKey)));
+        } else if (deprecatedKey.equals("disable.config.edit")
+            || deprecatedKey.equals("disable.v2.api")) {

Review Comment:
   why do these need to be called out specifically as a special case here?  I 
thought you had a general mechanism, wherein you need to only list these names 
in a file.



-- 
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: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to