You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/07/13 23:45:57 UTC
[GitHub] [solr] thelabdude opened a new pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
thelabdude opened a new pull request #219:
URL: https://github.com/apache/solr/pull/219
https://issues.apache.org/jira/browse/SOLR-15525
# Description
Get zk acl username and/or password from either a Java sys prop (current behavior) or from an environment variable. The code now looks for the `env:` prefix on the Java sys prop value and if found, reads the value from the environment variable, such as: `-DzkDigestReadonlyPassword=env:SOME_ENV_VAR` will get the password from the env var named `SOME_ENV_VAR`. If Java sys prop not set, then we look for a fallback default env var, such as `ZK_READ_ACL_PASSWORD`
# Solution
Please provide a short description of the approach taken to implement your solution.
# Tests
Need to figure out a way to set env vars for testing w/o overcomplicating the impl. as in I don't like exposing `setEnvVars` like `EnvSSLCredentialProvider` did ...
# Checklist
Please review the following and check all that apply:
- [ ] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [ ] I have created a Jira issue and added the issue ID to my pull request title.
- [ ] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [ ] I have developed this patch against the `main` branch.
- [ ] I have run `./gradlew check`.
- [ ] I have added tests for my changes.
- [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] thelabdude closed pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
thelabdude closed pull request #219:
URL: https://github.com/apache/solr/pull/219
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #219:
URL: https://github.com/apache/solr/pull/219#issuecomment-879886897
Just reading this, are we adding "YAWTCS": _Yet Another Way To Configure Solr? I wonder if it's worth taking a pass through Solr and figuring out how to use this same pattern across all our various system properties? For example, today we support `-Dsolr.auth.jwt.allowOutboundHttp=true`. Could we extend this pattern to allow `-Dsolr.auth.jwt.allowOutboundHttp=env:SOME_ENV_VAR`? Or is this a situation that ONLY would every apply to Zookeeper ACL and wouldn't make sense elsewhere.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] epugh commented on pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
epugh commented on pull request #219:
URL: https://github.com/apache/solr/pull/219#issuecomment-879933198
> > Just reading this, are we adding "YAWTCS": _Yet Another Way To Configure Solr? I wonder if it's worth taking a pass through Solr and figuring out how to use this same pattern across all our various system properties? For example, today we support `-Dsolr.auth.jwt.allowOutboundHttp=true`. Could we extend this pattern to allow `-Dsolr.auth.jwt.allowOutboundHttp=env:SOME_ENV_VAR`? Or is this a situation that ONLY would every apply to Zookeeper ACL and wouldn't make sense elsewhere.
>
> I think only sensitive properties like credentials need this pattern as the main problem here is exposing credentials in Java sys props
So if this works well, maybe apply this to other places like `-Djavax.net.ssl.keyStorePassword=secret`?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] cpoerschke commented on a change in pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
cpoerschke commented on a change in pull request #219:
URL: https://github.com/apache/solr/pull/219#discussion_r669515038
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/VMParamsAllAndReadonlyDigestZkACLProvider.java
##########
@@ -30,6 +30,12 @@
public static final String DEFAULT_DIGEST_READONLY_USERNAME_VM_PARAM_NAME = "zkDigestReadonlyUsername";
public static final String DEFAULT_DIGEST_READONLY_PASSWORD_VM_PARAM_NAME = "zkDigestReadonlyPassword";
+
+ // fallback env vars if system props not set
+ public static final String ZK_ALL_ACL_USERNAME = "ZK_ALL_ACL_USERNAME";
+ public static final String ZK_ALL_ACL_PASSWORD = "ZK_ALL_ACL_PASSWORD";
+ public static final String ZK_READ_ACL_USERNAME = "ZK_READ_ACL_USERNAME";
+ public static final String ZK_READ_ACL_PASSWORD = "ZK_READ_ACL_PASSWORD";
Review comment:
`zkDigestReadonly*` is mentioned in `zookeeper-access-control.adoc` - maybe also mention these alternatives there?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] thelabdude commented on pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #219:
URL: https://github.com/apache/solr/pull/219#issuecomment-880001765
Latest commit here drops the `env:` approach and introduces new providers that read from env vars ... seems a bit heavy handed to me but is more inline with the `EnvSSLCredentialProvider` approach already in the code base.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] thelabdude commented on pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #219:
URL: https://github.com/apache/solr/pull/219#issuecomment-879921695
> Just reading this, are we adding "YAWTCS": _Yet Another Way To Configure Solr? I wonder if it's worth taking a pass through Solr and figuring out how to use this same pattern across all our various system properties? For example, today we support `-Dsolr.auth.jwt.allowOutboundHttp=true`. Could we extend this pattern to allow `-Dsolr.auth.jwt.allowOutboundHttp=env:SOME_ENV_VAR`? Or is this a situation that ONLY would every apply to Zookeeper ACL and wouldn't make sense elsewhere.
I think only sensitive properties like credentials need this pattern as the main problem here is exposing credentials in Java sys props
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] thelabdude commented on pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #219:
URL: https://github.com/apache/solr/pull/219#issuecomment-879940832
You can use `EnvSSLCredentialProvider` for setting the keystore / truststore passwords from env vars. We could emulate that approach for ZK ACL creds by introducing new provider impls but then felt the `env:` was more expedient. I'm open to the alternative approach though (emulating EnvSSLCredentialProvider)
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] thelabdude commented on pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #219:
URL: https://github.com/apache/solr/pull/219#issuecomment-880157912
actually, the UI is not the only concern here, the output of commands like `ps waux` might also show the Java sys props, so going to proceed as planned
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] thelabdude commented on pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
thelabdude commented on pull request #219:
URL: https://github.com/apache/solr/pull/219#issuecomment-880030205
fwiw ~ I just checked 8.9 and it shows the zk passwords as REDACTED in the UI, so I'm not even sure how useful this change is at this point ?!? probably just going to leave it open for now vs. merging ...
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
[GitHub] [solr] thelabdude commented on a change in pull request #219: SOLR-15525: Allow Zookeeper ACL credentials to be passed via env vars instead of Java system properties
Posted by GitBox <gi...@apache.org>.
thelabdude commented on a change in pull request #219:
URL: https://github.com/apache/solr/pull/219#discussion_r669644632
##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/VMParamsAllAndReadonlyDigestZkACLProvider.java
##########
@@ -30,6 +30,12 @@
public static final String DEFAULT_DIGEST_READONLY_USERNAME_VM_PARAM_NAME = "zkDigestReadonlyUsername";
public static final String DEFAULT_DIGEST_READONLY_PASSWORD_VM_PARAM_NAME = "zkDigestReadonlyPassword";
+
+ // fallback env vars if system props not set
+ public static final String ZK_ALL_ACL_USERNAME = "ZK_ALL_ACL_USERNAME";
+ public static final String ZK_ALL_ACL_PASSWORD = "ZK_ALL_ACL_PASSWORD";
+ public static final String ZK_READ_ACL_USERNAME = "ZK_READ_ACL_USERNAME";
+ public static final String ZK_READ_ACL_PASSWORD = "ZK_READ_ACL_PASSWORD";
Review comment:
will do!
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org