You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/14 02:28:07 UTC
[GitHub] [pulsar] skysiders opened a new pull request, #15381: [Broker] modify permission in metadata RocksdbMetadataStore
skysiders opened a new pull request, #15381:
URL: https://github.com/apache/pulsar/pull/15381
PULSAR-18 modify permission in RocksdbMetadataStore
### Motivation
While RocksdbMetadataStore init , it will create directory from dataPath use Files.createDirectories(dataPath); this path create with too loose that any one can read file in this directory.This may cause everyone can read logs, perhaps strict permission is more safe.
### Modifications
set permission for dataPath.
- [x] `no-need-doc`
this is just a permission update
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders commented on a diff in pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders commented on code in PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#discussion_r862421661
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -220,6 +223,8 @@ private RocksdbMetadataStore(String metadataURL, MetadataStoreConfig metadataSto
Path dataPath = FileSystems.getDefault().getPath(dataDir);
try {
Files.createDirectories(dataPath);
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
Review Comment:
I think directory should have x permission here
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- closed pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
Technoboy- closed pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
URL: https://github.com/apache/pulsar/pull/15381
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on code in PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#discussion_r862415249
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -220,6 +223,8 @@ private RocksdbMetadataStore(String metadataURL, MetadataStoreConfig metadataSto
Path dataPath = FileSystems.getDefault().getPath(dataDir);
try {
Files.createDirectories(dataPath);
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
+ Files.setPosixFilePermissions(dataPath, perms);
} catch (IOException e) {
e.printStackTrace();
Review Comment:
Could you help to change `log.error` ?
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -220,6 +223,8 @@ private RocksdbMetadataStore(String metadataURL, MetadataStoreConfig metadataSto
Path dataPath = FileSystems.getDefault().getPath(dataDir);
try {
Files.createDirectories(dataPath);
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
+ Files.setPosixFilePermissions(dataPath, perms);
} catch (IOException e) {
e.printStackTrace();
Review Comment:
Could you help to change to `log.error` ?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders commented on pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1146757749
Hi @Technoboy- ,could you please have a look at 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
Technoboy- commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1154639445
/pulsarbot run-failure-checks
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15381: PULSAR-18 modify permission in RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1112900130
@skysiders:Thanks for providing doc info!
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders commented on a diff in pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders commented on code in PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#discussion_r862731841
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -313,6 +318,8 @@ private void configLog(Options options) throws IOException {
logPathSetting = FileSystems.getDefault().getPath(logPath + "/rocksdb-log");
Files.createDirectories(logPathSetting);
options.setDbLogDir(logPathSetting.toString());
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
Review Comment:
Hi @Jason918 , this fix and line 226 are both set permission for directory, perhaps "rwxr-x---" is better? And for file "rw-r-----" is better.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders commented on a diff in pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders commented on code in PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#discussion_r862909160
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -313,6 +318,8 @@ private void configLog(Options options) throws IOException {
logPathSetting = FileSystems.getDefault().getPath(logPath + "/rocksdb-log");
Files.createDirectories(logPathSetting);
options.setDbLogDir(logPathSetting.toString());
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
Review Comment:
Thanks for review, and I found some permission problem for PULSAR-19 and PR for #15411 ,can you please take a look?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mattisonchao commented on pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1118040578
@skysiders We should fix the checkstyle issue.
```java
Error: src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:[70,1] (imports) ImportOrder: Extra separation in import group before 'java.nio.file.attribute.PosixFilePermission'
Error: src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:[70,1] (imports) ImportOrder: Import java.nio.file.attribute.PosixFilePermission appears after other imports that it should precede
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a diff in pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#discussion_r862419369
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -313,6 +318,8 @@ private void configLog(Options options) throws IOException {
logPathSetting = FileSystems.getDefault().getPath(logPath + "/rocksdb-log");
Files.createDirectories(logPathSetting);
options.setDbLogDir(logPathSetting.toString());
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
Review Comment:
Same here.
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -220,6 +223,8 @@ private RocksdbMetadataStore(String metadataURL, MetadataStoreConfig metadataSto
Path dataPath = FileSystems.getDefault().getPath(dataDir);
try {
Files.createDirectories(dataPath);
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
Review Comment:
"rw-r-----" is better?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a diff in pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#discussion_r862419615
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -220,6 +223,8 @@ private RocksdbMetadataStore(String metadataURL, MetadataStoreConfig metadataSto
Path dataPath = FileSystems.getDefault().getPath(dataDir);
try {
Files.createDirectories(dataPath);
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
+ Files.setPosixFilePermissions(dataPath, perms);
} catch (IOException e) {
e.printStackTrace();
Review Comment:
> Could you help to change to `log.error` ?
We should throw exception here. I will fix 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders commented on pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1118135738
Hi @mattisonchao , thanks for your review, I use check style to fix style issue, but perhaps some problem in my environment, I get `[WARNING] Unable to locate Source XRef to link to - DISABLED` after run `mvn checkstyle:checkstyle` so I check style by myself until get no error after run `mvn checkstyle:checkstyle`.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders commented on pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1114018144
In this patch, I set 700 permission for data path and log path.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders commented on pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1118139630
> Hi @mattisonchao , thanks for your review, I use check style to fix style issue, but perhaps some problem in my environment, I get `[WARNING] Unable to locate Source XRef to link to - DISABLED` after run `mvn checkstyle:checkstyle` so I check style by myself until get no error after run `mvn checkstyle:checkstyle`.
I use `mvn checkstyle:check` solve it.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #15381:
URL: https://github.com/apache/pulsar/pull/15381
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1146724683
The pr had no activity for 30 days, mark with Stale label.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #15381: PULSAR-18 modify permission in RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#issuecomment-1112897123
@skysiders:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] skysiders closed pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
skysiders closed pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
URL: https://github.com/apache/pulsar/pull/15381
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a diff in pull request #15381: [Broker] modify permission in metadata RocksdbMetadataStore
Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #15381:
URL: https://github.com/apache/pulsar/pull/15381#discussion_r862866794
##########
pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/RocksdbMetadataStore.java:
##########
@@ -313,6 +318,8 @@ private void configLog(Options options) throws IOException {
logPathSetting = FileSystems.getDefault().getPath(logPath + "/rocksdb-log");
Files.createDirectories(logPathSetting);
options.setDbLogDir(logPathSetting.toString());
+ Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwx------");
Review Comment:
"rwxr-x---" +1
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org