showuon commented on a change in pull request #9947:
URL: https://github.com/apache/kafka/pull/9947#discussion_r563527151
##########
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/StateDirectory.java
##########
@@ -109,16 +109,27 @@ public StateDirectory(final StreamsConfig config, final
Time time, final boolean
log.warn("Using /tmp directory in the state.dir property can
cause failures with writing the checkpoint file" +
" due to the fact that this directory can be cleared by
the OS");
}
-
// change the dir permission to "rwxr-x---" to avoid world readable
- final Path basePath = Paths.get(baseDir.getPath());
- final Path statePath = Paths.get(stateDir.getPath());
+ configurePermissions(Paths.get(baseDir.getPath()));
+ configurePermissions(Paths.get(stateDir.getPath()));
+ }
+ }
+
+ private void configurePermissions(final Path path) {
+ if
(path.getFileSystem().supportedFileAttributeViews().contains("posix")) {
final Set<PosixFilePermission> perms =
PosixFilePermissions.fromString("rwxr-x---");
try {
- Files.setPosixFilePermissions(basePath, perms);
- Files.setPosixFilePermissions(statePath, perms);
+ Files.setPosixFilePermissions(path, perms);
} catch (final IOException e) {
- log.error("Error changing permissions for the state or base
directory {} ", stateDir.getPath(), e);
+ log.error("Error changing permissions for the directory {} ",
path, e);
+ }
+ } else {
+ final File file = path.toFile();
Review comment:
I know we need both `File` and `Path` instance to complete the
permission setting. What I meant is that:
1. We already had the `File` instances, i.e. baseDir, stateDir
2. `Paths.get(stateDir.getPath())`
--> what happened behind the scene, is we created a Path instance from the
path string in the file instance (i.e.`stateDir.getPath()`)
```java
public final Path getPath(String var1, String... var2) {
...
return new UnixPath(this, var3);
}
```
3. `path.toFile()`
--> what happened behind the scene, is we created a File instance from the
path string
```java
public final File toFile() {
return new File(this.toString());
}
```
So, currently, we will created an additional File instance via
`path.toFile()`. If we pass the File instance into `configurePermissions()`
directly, it'll only created a `Path` instance. ex:
```java
configurePermissions(stateDir); // pass the file instance directly
```
```java
private void configurePermissions(final File file) {
final Path path = Paths.get(file.getPath()); // created path instance
if
(path.getFileSystem().supportedFileAttributeViews().contains("posix")) {
....
}
else {
boolean set = file.setReadable(true, true); // won't create
additional File instance 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
