You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/10/21 09:31:44 UTC
[GitHub] [dolphinscheduler] ruanwenjun opened a new pull request, #12486: Set tenantDir permission
ruanwenjun opened a new pull request, #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486
## Purpose of the pull request
Right now, we cannot touch file in our shell task, since the task directory's owner is created by deploy user not tenant.
And the directory permission is `755`, we need to change the task directory's owner to tenant.
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
## Verify this pull request
<!--*(Please pick either of the following options)*-->
This pull request is code cleanup without any test coverage.
*(or)*
This pull request is already covered by existing tests, such as *(please describe tests)*.
(or)
This change added tests and can be verified as follows:
<!--*(example:)*
- *Added dolphinscheduler-dao tests for end-to-end.*
- *Added CronUtilsTest to verify the change.*
- *Manually verified the change by testing locally.* -->
(or)
If your pull request contain incompatible change, you should also add it to `docs/docs/en/guide/upgrede/incompatible.md`
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #12486: [Bug] Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#discussion_r1002498672
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -131,4 +138,23 @@ public static void downloadResourcesIfNeeded(StorageOperate storageOperate,
}
}
}
+
+ private static void createDirectoryWithOwner(Path filePath, String tenant) {
+ if (Files.exists(filePath)) {
+ return;
+ }
+ try {
+ Files.createDirectories(filePath);
+ if (!OSUtils.isSudoEnable()) {
+ // we need to open sudo, then we can change the owner.
+ return;
Review Comment:
If we throw an exception here, it means we only support deploy with `sudo`, then this switch is meaningless. We don't need to throw exception 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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12486: [Bug] Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#issuecomment-1287822751
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12486)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list) [27.3% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list)
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #12486: [Bug] Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#discussion_r1002498096
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -85,26 +85,36 @@ public static String getUploadFilename(String tenantCode, String filename) {
return fileName;
}
+ public static String getTenantDir(String tenant) {
+ return DATA_BASEDIR + "/exec/process/" + tenant;
Review Comment:
I remove this method, since we don't need this method.
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun merged pull request #12486: [Bug] Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
ruanwenjun merged PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #12486: [Bug] Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
caishunfeng commented on code in PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#discussion_r1002493661
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -85,26 +85,36 @@ public static String getUploadFilename(String tenantCode, String filename) {
return fileName;
}
+ public static String getTenantDir(String tenant) {
+ return DATA_BASEDIR + "/exec/process/" + tenant;
Review Comment:
```suggestion
return DATA_BASEDIR + EXEC_PROCESS + tenant;
```
##########
dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/utils/FileUtils.java:
##########
@@ -85,26 +85,36 @@ public static String getUploadFilename(String tenantCode, String filename) {
return fileName;
}
+ public static String getTenantDir(String tenant) {
+ return DATA_BASEDIR + "/exec/process/" + tenant;
+ }
+
/**
* directory of process execution
*
- * @param projectCode project code
- * @param processDefineCode process definition Code
+ * @param tenant tenant
+ * @param projectCode project code
+ * @param processDefineCode process definition Code
* @param processDefineVersion process definition version
- * @param processInstanceId process instance id
- * @param taskInstanceId task instance id
+ * @param processInstanceId process instance id
+ * @param taskInstanceId task instance id
* @return directory of process execution
*/
- public static String getProcessExecDir(long projectCode, long processDefineCode, int processDefineVersion,
- int processInstanceId, int taskInstanceId) {
- String fileName = String.format("%s/exec/process/%d/%s/%d/%d", DATA_BASEDIR,
- projectCode, processDefineCode + "_" + processDefineVersion, processInstanceId, taskInstanceId);
- File file = new File(fileName);
- if (!file.getParentFile().exists()) {
- file.getParentFile().mkdirs();
- }
-
- return fileName;
+ public static String getProcessExecDir(String tenant,
+ long projectCode,
+ long processDefineCode,
+ int processDefineVersion,
+ int processInstanceId,
+ int taskInstanceId) {
+ return String.format(
+ "%s/exec/process/%s/%d/%d_%d/%d/%d",
+ DATA_BASEDIR,
+ tenant,
+ projectCode,
+ processDefineCode,
+ processDefineVersion,
+ processInstanceId,
+ taskInstanceId);
Review Comment:
```suggestion
return String.format(
"%s/%s/%s/%d/%d_%d/%d/%d",
DATA_BASEDIR,
EXEC_PROCESS,
tenant,
projectCode,
processDefineCode,
processDefineVersion,
processInstanceId,
taskInstanceId);
```
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -131,4 +138,23 @@ public static void downloadResourcesIfNeeded(StorageOperate storageOperate,
}
}
}
+
+ private static void createDirectoryWithOwner(Path filePath, String tenant) {
+ if (Files.exists(filePath)) {
+ return;
+ }
+ try {
+ Files.createDirectories(filePath);
+ if (!OSUtils.isSudoEnable()) {
+ // we need to open sudo, then we can change the owner.
+ return;
Review Comment:
Is is better to throw a exception here?
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -131,4 +138,23 @@ public static void downloadResourcesIfNeeded(StorageOperate storageOperate,
}
}
}
+
+ private static void createDirectoryWithOwner(Path filePath, String tenant) {
+ if (Files.exists(filePath)) {
+ return;
+ }
+ try {
+ Files.createDirectories(filePath);
+ if (!OSUtils.isSudoEnable()) {
+ // we need to open sudo, then we can change the owner.
+ return;
+ }
+ UserPrincipalLookupService userPrincipalLookupService =
+ FileSystems.getDefault().getUserPrincipalLookupService();
+ UserPrincipal tenantPrincipal = userPrincipalLookupService.lookupPrincipalByName(tenant);
+ Files.setOwner(filePath, tenantPrincipal);
Review Comment:
Does it also work in mac or windows?
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12486: Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#issuecomment-1287573057
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12486)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list) [26.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list)
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12486: Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#issuecomment-1287572339
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12486)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list) [26.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list)
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #12486: [Bug] Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#discussion_r1002498286
##########
dolphinscheduler-worker/src/main/java/org/apache/dolphinscheduler/server/worker/utils/TaskExecutionCheckerUtils.java:
##########
@@ -131,4 +138,23 @@ public static void downloadResourcesIfNeeded(StorageOperate storageOperate,
}
}
}
+
+ private static void createDirectoryWithOwner(Path filePath, String tenant) {
+ if (Files.exists(filePath)) {
+ return;
+ }
+ try {
+ Files.createDirectories(filePath);
+ if (!OSUtils.isSudoEnable()) {
+ // we need to open sudo, then we can change the owner.
+ return;
+ }
+ UserPrincipalLookupService userPrincipalLookupService =
+ FileSystems.getDefault().getUserPrincipalLookupService();
+ UserPrincipal tenantPrincipal = userPrincipalLookupService.lookupPrincipalByName(tenant);
+ Files.setOwner(filePath, tenantPrincipal);
Review Comment:
Yes, we use standard jdk api, this can work in mac.
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #12486: [Bug] Set tenantDir permission
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #12486:
URL: https://github.com/apache/dolphinscheduler/pull/12486#issuecomment-1287821703
SonarCloud Quality Gate failed. [](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12486)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12486&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list) [27.3% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_coverage&view=list)
[](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12486&metric=new_duplicated_lines_density&view=list)
--
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@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org