You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/03/11 08:47:35 UTC

[GitHub] [skywalking] wu-sheng opened a new pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

wu-sheng opened a new pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493
 
 
   ### Secrets Management File Of ElasticSearch Username and Password 
   The value of `secretsManagementFile` should point to the secrets management file is the file including username and password of ElasticSearch server. 
   The file uses the properties format.
   ```properties
   user=xxx
   password=yyy
   ```
   
   The major difference between using `user/password` configs in the `application.yaml` and this file is, this file is being watched by the OAP server. 
   Once it is changed manually or through 3rd party tool, such as [Vault](https://github.com/hashicorp/vault), 
   the storage provider will use the new username and password to establish the connection and close the old one. If the information exist in the file,
   the `user/password` will be overrided.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#discussion_r390977827
 
 

 ##########
 File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchProvider.java
 ##########
 @@ -117,6 +120,31 @@ public void prepare() throws ServiceNotProvidedException {
         if (config.getDayStep() > 1) {
             TimeSeriesUtils.setDAY_STEP(config.getDayStep());
         }
+
+        if (!StringUtil.isEmpty(config.getSecretsManagementFile())) {
+            MultipleFilesChangeMonitor monitor = new MultipleFilesChangeMonitor(
+                10, readableContents -> {
+                    final byte[] secretsFileContent = readableContents.get(0);
+                    if (secretsFileContent == null) {
+                        return;
+                    }
+                    Properties userAndPass = new Properties();
+                    userAndPass.load(new ByteArrayInputStream(secretsFileContent));
+                    config.setUser(userAndPass.getProperty("user"));
+                    config.setPassword(userAndPass.getProperty("password"));
+
+                    if (elasticSearchClient == null) {
+                        //In the startup process, we just need to change the username/password
 
 Review comment:
   Added.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] JaredTan95 merged pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

JaredTan95 merged pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#discussion_r390964865
 
 

 ##########
 File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
 ##########
 @@ -119,6 +119,13 @@ public ElasticSearchClient(String clusterNodes,
     @Override
     public void connect() throws IOException, KeyStoreException, NoSuchAlgorithmException, KeyManagementException, CertificateException {
         List<HttpHost> hosts = parseClusterNodes(protocol, clusterNodes);
+        if (client != null) {
+            try {
+                client.close();
+            } catch (Throwable t) {
+                log.error("ElasticSearch client reconnection fails based on new config", t);
+            }
+        }
 
 Review comment:
   Good point. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#discussion_r390880429
 
 

 ##########
 File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchProvider.java
 ##########
 @@ -117,6 +120,31 @@ public void prepare() throws ServiceNotProvidedException {
         if (config.getDayStep() > 1) {
             TimeSeriesUtils.setDAY_STEP(config.getDayStep());
         }
+
+        if (!StringUtil.isEmpty(config.getSecretsManagementFile())) {
+            MultipleFilesChangeMonitor monitor = new MultipleFilesChangeMonitor(
+                10, readableContents -> {
+                    final byte[] secretsFileContent = readableContents.get(0);
+                    if (secretsFileContent == null) {
+                        return;
+                    }
+                    Properties userAndPass = new Properties();
+                    userAndPass.load(new ByteArrayInputStream(secretsFileContent));
+                    config.setUser(userAndPass.getProperty("user"));
+                    config.setPassword(userAndPass.getProperty("password"));
+
+                    if (elasticSearchClient == null) {
+                        //In the startup process, we just need to change the username/password
 
 Review comment:
   Was thinking about this. But not sure. Does across Thread cache will keep forever? Because we are not sensitive this changed immediately, several seconds later is fine.
   What do you think about this?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

wu-sheng commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-597650492

Features added. I add the JKS and pass into this secret management machenism.

### ElasticSearch 6 With Https SSL Encrypting communications.
- File at `trustStorePath` is being monitored, once it is changed, the ElasticSearch client will do reconnecting.
- `trustStorePass` could be changed on the runtime through **Secrets Management File Of ElasticSearch Authentication**

### Secrets Management File Of ElasticSearch Authentication
The value of `secretsManagementFile` should point to the secrets management file absolute path.
The file includes username, password and JKS password of ElasticSearch server in the properties format.
```properties
user=xxx
password=yyy
trustStorePass=zzz
```

The major difference between using `user, password, trustStorePass` configs in the `application.yaml` and this file is, this file is being watched by the OAP server.
Once it is changed manually or through 3rd party tool, such as [Vault](https://github.com/hashicorp/vault),
the storage provider will use the new username, password and JKS password to establish the connection and close the old one. If the information exist in the file,
the `user/password` will be overrided.

----------------------------------------------------------------
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.

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

With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-597552700
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=h1) Report
   > Merging [#4493](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/37e93a6f4167e16990e959c3d61eab5853c67ffb?src=pr&el=desc) will **decrease** coverage by `0.04%`.
   > The diff coverage is `5.08%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4493/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4493      +/-   ##
   ==========================================
   - Coverage   25.55%   25.51%   -0.05%     
   ==========================================
     Files        1244     1244              
     Lines       28843    28893      +50     
     Branches     3944     3953       +9     
   ==========================================
   + Hits         7371     7372       +1     
   - Misses      20794    20842      +48     
   - Partials      678      679       +1
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...lasticsearch/StorageModuleElasticsearchConfig.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hDb25maWcuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
   | [...in/elasticsearch7/client/ElasticSearch7Client.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9jbGllbnQvRWxhc3RpY1NlYXJjaDdDbGllbnQuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...rary/client/elasticsearch/ElasticSearchClient.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvY2xpZW50L2VsYXN0aWNzZWFyY2gvRWxhc3RpY1NlYXJjaENsaWVudC5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...sticsearch/StorageModuleElasticsearchProvider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...icsearch7/StorageModuleElasticsearch7Provider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9TdG9yYWdlTW9kdWxlRWxhc3RpY3NlYXJjaDdQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...erver/library/util/MultipleFilesChangeMonitor.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvTXVsdGlwbGVGaWxlc0NoYW5nZU1vbml0b3IuamF2YQ==) | `69.51% <37.5%> (-4.18%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=footer). Last update [37e93a6...5cac4e5](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

codecov-io commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-597552700
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=h1) Report
   > Merging [#4493](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/6723f349c144b79ac7c8d291a82d62e84f84ed4d?src=pr&el=desc) will **decrease** coverage by `<.01%`.
   > The diff coverage is `4.25%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4493/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4493      +/-   ##
   ==========================================
   - Coverage   25.52%   25.51%   -0.01%     
   ==========================================
     Files        1243     1243              
     Lines       28837    28876      +39     
     Branches     3943     3951       +8     
   ==========================================
   + Hits         7360     7368       +8     
   - Misses      20800    20831      +31     
     Partials      677      677
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...lasticsearch/StorageModuleElasticsearchConfig.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hDb25maWcuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
   | [...in/elasticsearch7/client/ElasticSearch7Client.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9jbGllbnQvRWxhc3RpY1NlYXJjaDdDbGllbnQuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...rary/client/elasticsearch/ElasticSearchClient.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvY2xpZW50L2VsYXN0aWNzZWFyY2gvRWxhc3RpY1NlYXJjaENsaWVudC5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...sticsearch/StorageModuleElasticsearchProvider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...icsearch7/StorageModuleElasticsearch7Provider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9TdG9yYWdlTW9kdWxlRWxhc3RpY3NlYXJjaDdQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...erver/library/util/MultipleFilesChangeMonitor.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvTXVsdGlwbGVGaWxlc0NoYW5nZU1vbml0b3IuamF2YQ==) | `72.15% <50%> (-1.54%)` | :arrow_down: |
   | [...walking/oap/server/core/analysis/Downsampling.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvcmUvYW5hbHlzaXMvRG93bnNhbXBsaW5nLmphdmE=) | `100% <0%> (+100%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=footer). Last update [6723f34...2806553](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

wu-sheng commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#discussion_r392101147
 
 

 ##########
 File path: docs/en/setup/backend/backend-storage.md
 ##########
 @@ -124,6 +126,20 @@ Such as, if dayStep == 11,
 
 NOTICE, TTL deletion would be affected by these. You should set an extra more dayStep in your TTL. Such as you want to TTL == 30 days and dayStep == 10, you actually need to set TTL = 40;
 
+### Secrets Management File Of ElasticSearch Authentication
+The value of `secretsManagementFile` should point to the secrets management file absolute path. 
+The file includes username, password and JKS password of ElasticSearch server in the properties format.
+```properties
+user=xxx
+password=yyy
+trustStorePass=zzz
+```
+
+The major difference between using `user, password, trustStorePass` configs in the `application.yaml` and this file is, this file is being watched by the OAP server. 
 
 Review comment:
   Yes. And fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] JaredTan95 edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

JaredTan95 edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-598688452
 
 
   tested `user/password` changed scenario, and LGTM:
   
   ```bash
   ## pasword changed:
   ······
   	Caused by: org.elasticsearch.client.ResponseException: method [POST], host [http://localhost:9200], URI [/profile_task/type/_search?typed_keys=true&ignore_unavailable=false&expand_wildcards=open&allow_no_indices=true&search_type=query_then_fetch&batched_reduce_size=512], status line [HTTP/1.1 401 Unauthorized]
   {"error":{"root_cause":[{"type":"security_exception","reason":"failed to authenticate user [elastic]","header":{"WWW-Authenticate":"Basic realm=\"security\" charset=\"UTF-8\""}}],"type":"security_exception","reason":"failed to authenticate user [elastic]","header":{"WWW-Authenticate":"Basic realm=\"security\" charset=\"UTF-8\""}},"status":401}
   ······
   
   ## config file changed and password is right:
   org.apache.skywalking.oap.server.library.client.elasticsearch.ElasticSearchClient - 176 [pool-2-thread-1] INFO  [] - elasticsearch cluster nodes: localhost:9200
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] JaredTan95 commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

JaredTan95 commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-598688452
 
 
   tested `user/password` changed scenario, and LGTM:
   
   ```bash
   ······
   	Caused by: org.elasticsearch.client.ResponseException: method [POST], host [http://localhost:9200], URI [/profile_task/type/_search?typed_keys=true&ignore_unavailable=false&expand_wildcards=open&allow_no_indices=true&search_type=query_then_fetch&batched_reduce_size=512], status line [HTTP/1.1 401 Unauthorized]
   {"error":{"root_cause":[{"type":"security_exception","reason":"failed to authenticate user [elastic]","header":{"WWW-Authenticate":"Basic realm=\"security\" charset=\"UTF-8\""}}],"type":"security_exception","reason":"failed to authenticate user [elastic]","header":{"WWW-Authenticate":"Basic realm=\"security\" charset=\"UTF-8\""}},"status":401}
   ······
   org.apache.skywalking.oap.server.library.client.elasticsearch.ElasticSearchClient - 176 [pool-2-thread-1] INFO  [] - elasticsearch cluster nodes: localhost:9200
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] wu-sheng commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

wu-sheng commented on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-597643157
 
 
   @kezhenxu94 @JaredTan95 @hanahmily Could you help on testing on this? Change the JKS or ES username/password and trigger the secrets file changed, check whether the connection reestablishes successfully.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-597552700
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=h1) Report
   > Merging [#4493](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/37e93a6f4167e16990e959c3d61eab5853c67ffb&el=desc) will **decrease** coverage by `0.04%`.
   > The diff coverage is `5.08%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4493/graphs/tree.svg?width=650&height=150&src=pr&token=qrILxY5yA8)](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4493      +/-   ##
   ==========================================
   - Coverage   25.55%   25.51%   -0.05%     
   ==========================================
     Files        1244     1244              
     Lines       28843    28893      +50     
     Branches     3944     3953       +9     
   ==========================================
   + Hits         7371     7372       +1     
   - Misses      20794    20842      +48     
   - Partials      678      679       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...rary/client/elasticsearch/ElasticSearchClient.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvY2xpZW50L2VsYXN0aWNzZWFyY2gvRWxhc3RpY1NlYXJjaENsaWVudC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...lasticsearch/StorageModuleElasticsearchConfig.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hDb25maWcuamF2YQ==) | `0.00% <ø> (ø)` | |
   | [...sticsearch/StorageModuleElasticsearchProvider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hQcm92aWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...icsearch7/StorageModuleElasticsearch7Provider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9TdG9yYWdlTW9kdWxlRWxhc3RpY3NlYXJjaDdQcm92aWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...in/elasticsearch7/client/ElasticSearch7Client.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9jbGllbnQvRWxhc3RpY1NlYXJjaDdDbGllbnQuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...erver/library/util/MultipleFilesChangeMonitor.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvTXVsdGlwbGVGaWxlc0NoYW5nZU1vbml0b3IuamF2YQ==) | `69.51% <37.50%> (-4.18%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=footer). Last update [37e93a6...5cac4e5](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] JaredTan95 commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

JaredTan95 commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#discussion_r392087102
 
 

 ##########
 File path: docs/en/setup/backend/backend-storage.md
 ##########
 @@ -124,6 +126,20 @@ Such as, if dayStep == 11,
 
 NOTICE, TTL deletion would be affected by these. You should set an extra more dayStep in your TTL. Such as you want to TTL == 30 days and dayStep == 10, you actually need to set TTL = 40;
 
+### Secrets Management File Of ElasticSearch Authentication
+The value of `secretsManagementFile` should point to the secrets management file absolute path. 
+The file includes username, password and JKS password of ElasticSearch server in the properties format.
+```properties
+user=xxx
+password=yyy
+trustStorePass=zzz
+```
+
+The major difference between using `user, password, trustStorePass` configs in the `application.yaml` and this file is, this file is being watched by the OAP server. 
 
 Review comment:
   typo duplicated with `this file is`?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

kezhenxu94 commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#discussion_r390959130
 
 

 ##########
 File path: oap-server/server-library/library-client/src/main/java/org/apache/skywalking/oap/server/library/client/elasticsearch/ElasticSearchClient.java
 ##########
 @@ -119,6 +119,13 @@ public ElasticSearchClient(String clusterNodes,
     @Override
     public void connect() throws IOException, KeyStoreException, NoSuchAlgorithmException, KeyManagementException, CertificateException {
         List<HttpHost> hosts = parseClusterNodes(protocol, clusterNodes);
+        if (client != null) {
+            try {
+                client.close();
+            } catch (Throwable t) {
+                log.error("ElasticSearch client reconnection fails based on new config", t);
+            }
+        }
 
 Review comment:
   Same thread visibility problem for `client`, if the monitor thread failed to detect the `client` is initialized below(in line 129), the `client` is not closed and left open forever

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-597552700
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=h1) Report
   > Merging [#4493](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/cd2fb513a67a42a8946d8fd57cef987cb0f79287?src=pr&el=desc) will **decrease** coverage by `0.04%`.
   > The diff coverage is `5.08%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4493/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4493      +/-   ##
   ==========================================
   - Coverage   25.52%   25.48%   -0.05%     
   ==========================================
     Files        1244     1244              
     Lines       28842    28892      +50     
     Branches     3944     3953       +9     
   ==========================================
     Hits         7363     7363              
   - Misses      20800    20848      +48     
   - Partials      679      681       +2
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...lasticsearch/StorageModuleElasticsearchConfig.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hDb25maWcuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
   | [...in/elasticsearch7/client/ElasticSearch7Client.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9jbGllbnQvRWxhc3RpY1NlYXJjaDdDbGllbnQuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...rary/client/elasticsearch/ElasticSearchClient.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvY2xpZW50L2VsYXN0aWNzZWFyY2gvRWxhc3RpY1NlYXJjaENsaWVudC5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...sticsearch/StorageModuleElasticsearchProvider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...icsearch7/StorageModuleElasticsearch7Provider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9TdG9yYWdlTW9kdWxlRWxhc3RpY3NlYXJjaDdQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...erver/library/util/MultipleFilesChangeMonitor.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvTXVsdGlwbGVGaWxlc0NoYW5nZU1vbml0b3IuamF2YQ==) | `69.51% <37.5%> (-4.18%)` | :arrow_down: |
   | [...ache/skywalking/apm/agent/core/jvm/JVMService.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvanZtL0pWTVNlcnZpY2UuamF2YQ==) | `53.33% <0%> (-1.67%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=footer). Last update [cd2fb51...66a69ea](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

codecov-io edited a comment on issue #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#issuecomment-597552700
 
 
   # [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=h1) Report
   > Merging [#4493](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/fdb816c799d468458c51ee019916b9afb75e4528?src=pr&el=desc) will **decrease** coverage by `0.04%`.
   > The diff coverage is `5.08%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/skywalking/pull/4493/graphs/tree.svg?width=650&token=qrILxY5yA8&height=150&src=pr)](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #4493      +/-   ##
   ==========================================
   - Coverage   25.55%   25.51%   -0.05%     
   ==========================================
     Files        1244     1244              
     Lines       28843    28893      +50     
     Branches     3944     3953       +9     
   ==========================================
   + Hits         7371     7372       +1     
   - Misses      20794    20842      +48     
   - Partials      678      679       +1
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...lasticsearch/StorageModuleElasticsearchConfig.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hDb25maWcuamF2YQ==) | `0% <ø> (ø)` | :arrow_up: |
   | [...in/elasticsearch7/client/ElasticSearch7Client.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9jbGllbnQvRWxhc3RpY1NlYXJjaDdDbGllbnQuamF2YQ==) | `0% <0%> (ø)` | :arrow_up: |
   | [...rary/client/elasticsearch/ElasticSearchClient.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2xpYnJhcnkvY2xpZW50L2VsYXN0aWNzZWFyY2gvRWxhc3RpY1NlYXJjaENsaWVudC5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...sticsearch/StorageModuleElasticsearchProvider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoLXBsdWdpbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL3N0b3JhZ2UvcGx1Z2luL2VsYXN0aWNzZWFyY2gvU3RvcmFnZU1vZHVsZUVsYXN0aWNzZWFyY2hQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...icsearch7/StorageModuleElasticsearch7Provider.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItc3RvcmFnZS1wbHVnaW4vc3RvcmFnZS1lbGFzdGljc2VhcmNoNy1wbHVnaW4vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9zdG9yYWdlL3BsdWdpbi9lbGFzdGljc2VhcmNoNy9TdG9yYWdlTW9kdWxlRWxhc3RpY3NlYXJjaDdQcm92aWRlci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
   | [...erver/library/util/MultipleFilesChangeMonitor.java](https://codecov.io/gh/apache/skywalking/pull/4493/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItbGlicmFyeS9saWJyYXJ5LXV0aWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvb2FwL3NlcnZlci9saWJyYXJ5L3V0aWwvTXVsdGlwbGVGaWxlc0NoYW5nZU1vbml0b3IuamF2YQ==) | `69.51% <37.5%> (-4.18%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=footer). Last update [fdb816c...b08bf83](https://codecov.io/gh/apache/skywalking/pull/4493?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage

Posted by GitBox <gi...@apache.org>.

kezhenxu94 commented on a change in pull request #4493: Support Secrets Management File in the ElasticSearch 6/7 storage
URL: https://github.com/apache/skywalking/pull/4493#discussion_r390877727
 
 

 ##########
 File path: oap-server/server-storage-plugin/storage-elasticsearch-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/elasticsearch/StorageModuleElasticsearchProvider.java
 ##########
 @@ -117,6 +120,31 @@ public void prepare() throws ServiceNotProvidedException {
         if (config.getDayStep() > 1) {
             TimeSeriesUtils.setDAY_STEP(config.getDayStep());
         }
+
+        if (!StringUtil.isEmpty(config.getSecretsManagementFile())) {
+            MultipleFilesChangeMonitor monitor = new MultipleFilesChangeMonitor(
+                10, readableContents -> {
+                    final byte[] secretsFileContent = readableContents.get(0);
+                    if (secretsFileContent == null) {
+                        return;
+                    }
+                    Properties userAndPass = new Properties();
+                    userAndPass.load(new ByteArrayInputStream(secretsFileContent));
+                    config.setUser(userAndPass.getProperty("user"));
+                    config.setPassword(userAndPass.getProperty("password"));
+
+                    if (elasticSearchClient == null) {
+                        //In the startup process, we just need to change the username/password
 
 Review comment:
   Make `elasticSearchClient` volatile?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services