[Feature-11652][2022Q4 RoadMap] DS support remote SSH task. #12736

DarkAssassinator · 2022-11-06T09:19:43Z

Purpose of the pull request

This PR will close [Feature] DS can support task running on remote host, not just worker server. #11652
This is a part of [RoadMap][Year 2022 Q4] Community RoadMap #12436

Brief change log

1. Add a Task Remote Host Management

Add a table named t_ds_task_remote_host in DB
Add a Task Remote Host Management page in Security
Support use test connet this host (use SSH)

2. Add a column named remoteHostCode in TaskInstance, TaskDefinition and TaskDefinitionLog entities.
3. Add SSH code for shell task.

Use ACP to controll SSH Session. And the pooled object just is session not channel. If DS need channel control in the future, i will add channel pooled object.
Limit SFTP upload rate and max file size.
Add SFTP monitor process.

Limitations

DS SSH do not support CPU/Memory limit on the remote server. Because other non-DS cluster servers may not necessarily deploy this feature.
SSH pool just controll session, not channel.
Now just support Shell and Python.

Verify this pull request

This pull request is already covered by existing tests

codecov-commenter · 2022-11-06T10:11:04Z

Codecov Report

Merging #12736 (05afebe) into dev (7336afa) will decrease coverage by 0.24%.
The diff coverage is 21.35%.

@@             Coverage Diff              @@
##                dev   #12736      +/-   ##
============================================
- Coverage     39.37%   39.13%   -0.25%     
- Complexity     4271     4305      +34     
============================================
  Files          1067     1079      +12     
  Lines         40118    40661     +543     
  Branches       4603     4666      +63     
============================================
+ Hits          15797    15912     +115     
- Misses        22544    22955     +411     
- Partials       1777     1794      +17

Impacted Files	Coverage Δ
...r/api/constants/ApiFuncIdentificationConstant.java	`88.23% <ø> (ø)`
...lphinscheduler/common/enums/AuthorizationType.java	`0.00% <0.00%> (ø)`
...apache/dolphinscheduler/common/utils/NetUtils.java	`48.61% <0.00%> (-1.04%)`	⬇️
...er/master/builder/TaskExecutionContextBuilder.java	`63.79% <0.00%> (-2.28%)`	⬇️
...erver/master/runner/StreamTaskExecuteRunnable.java	`0.00% <0.00%> (ø)`
.../server/master/runner/WorkflowExecuteRunnable.java	`8.17% <ø> (ø)`
...pache/dolphinscheduler/service/model/TaskNode.java	`44.44% <ø> (ø)`
...hinscheduler/service/task/TaskInstanceHandler.java	`0.00% <0.00%> (ø)`
...duler/plugin/task/api/AbstractCommandExecutor.java	`0.00% <0.00%> (ø)`
...dolphinscheduler/plugin/task/api/ProcessUtils.java	`0.00% <0.00%> (ø)`
... and 22 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

...r-api/src/main/java/org/apache/dolphinscheduler/api/controller/TaskRemoteHostController.java

+    @ResponseStatus(HttpStatus.OK)
+    @ApiException(VARIFY_TASK_REMOTE_HOST_ERROR)
+    @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+    public Result verifyTaskRemoteHost(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser,


...r-api/src/main/java/org/apache/dolphinscheduler/api/controller/TaskRemoteHostController.java

+    @ResponseStatus(HttpStatus.OK)
+    @ApiException(TEST_CONNECT_ERROR)
+    @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+    public Result testConnect(@Parameter(hidden = true) @RequestAttribute(value = Constants.SESSION_USER) User loginUser,


...duler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/TaskRemoteHostMapperTest.java

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_mysql.sql

SbloodyS · 2022-11-07T02:29:20Z

...task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/ssh/SSHSessionHolder.java

+        SSHResponse response = execCommand("mkdir -p " + remoteDirPath);
+        return response.getExitCode() == 0;
+    }
+
+    public void clearPath(String path) {
+        if (SINGLE_SLASH.equals(path)) {
+            return;
+        }
+        execCommand("rm -rf " + path);


I think it's better to use File.delete to perform operations instead of shell commands.

I think it's better to use File.delete to perform operations instead of shell commands.

Becuase these codes need to clear the path on the remote SSH server, so use the rm -rf command, not File.delete

Please don't mark it as resolved if it's not addressed. cc @zhongjiajie @caishunfeng

Please don't mark it as resolved if it's not addressed. cc @zhongjiajie @caishunfeng

Sorry, sure, and i think that File.delete cannot delete the remote dir and files, so i think that this case has resolved. WDYT.

zhongjiajie · 2022-11-09T13:34:01Z

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_mysql.sql

+DROP TABLE IF EXISTS `t_ds_task_remote_host`;
+CREATE TABLE `t_ds_task_remote_host`


I do not think adding a table for a task type is a good idea, can we ship it to our data source center? I know that we will change its name from datasource center to connection center, which can have various types of connection instead of database only

Add a Task Remote Host Management page in Security

If we add it to connection center, we can also migrate it into connection center, which may keep our web UI simple and unify

Add a Task Remote Host Management page in Security

If we add it to connection center, we can also migrate it into connection center, which may keep our web UI simple and unify

yes, but the conclusion we discussed bi-week was to temporarily in Security, after Connection center fine, we will migrate it into Connection center

caishunfeng

incomplete review

...r-api/src/main/java/org/apache/dolphinscheduler/api/controller/TaskRemoteHostController.java

...pi/src/main/java/org/apache/dolphinscheduler/api/service/impl/TaskRemoteHostServiceImpl.java

dolphinscheduler-dao/src/main/resources/sql/dolphinscheduler_h2.sql

caishunfeng · 2022-11-14T03:05:15Z

...k-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/AbstractCommandExecutor.java

+            // because JSch .chmod is not stable, so use the 'chmod -R' instead
+            logger.info("update remote path's permission:{} on session:{} to 755", taskRequest.getExecutePath(),
+                    sessionHost.toString());
+            String chmodCommand = "chmod -R 755 " + taskRequest.getExecutePath();


Why it should chmod 755? I'm not sure whether it will cause some CVE? cc @ruanwenjun

Why it should chmod 755? I'm not sure whether it will cause some CVE? cc @ruanwenjun

because remote server should keep same as DS worker, and just chmod the execute path, if there are any worry, we can change to 600. WDYT

Why it should chmod 755? I'm not sure whether it will cause some CVE? cc @ruanwenjun

because remote server should keep same as DS worker, and just chmod the execute path, if there are any worry, we can change to 600. WDYT

If chmod 600 can work, I think it is better.

Why it should chmod 755? I'm not sure whether it will cause some CVE? cc @ruanwenjun

because remote server should keep same as DS worker, and just chmod the execute path, if there are any worry, we can change to 600. WDYT

If chmod 600 can work, I think it is better.

ohh sorry, my mistake, not 600, chmod 700 should be ok

...i/src/main/java/org/apache/dolphinscheduler/plugin/task/api/ssh/PooledSSHSessionFactory.java

caishunfeng · 2022-11-14T03:09:35Z

SonarCloud Quality Gate failed.

3 Bugs 0 Vulnerabilities 4 Security Hotspots 72 Code Smells

23.6% Coverage 0.0% Duplication

@DarkAssassinator please check the sonar result and fix it.

DarkAssassinator · 2022-11-14T13:56:13Z

SonarCloud Quality Gate failed.
3 Bugs 0 Vulnerabilities 4 Security Hotspots 72 Code Smells
23.6% Coverage 0.0% Duplication

@DarkAssassinator please check the sonar result and fix it.

Sure, but these 4 Security Hotspots are not really issue, just password word

… BashTaskExecutionContext

DarkAssassinator · 2022-11-15T12:05:09Z

Hi @SbloodyS @caishunfeng, please PTAL again, thx

Radeity

I have one question that in #12552, DS supports to define file parameter. It seems like you don't consider this scenario, output files might be created under remote host's execution directory, however, you only fetch stdout and neglect output files.

DarkAssassinator · 2022-11-21T11:19:49Z

I have one question that in #12552, DS supports to define file parameter. It seems like you don't consider this scenario, output files might be created under remote host's execution directory, however, you only fetch stdout and neglect output files.

this is a new add scenario, i will check it. and please help to review other codes.

sonarcloud · 2022-11-25T13:59:30Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
4 Security Hotspots
45 Code Smells

23.5% Coverage
0.0% Duplication

github-actions · 2023-04-04T00:20:06Z

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

github-actions · 2023-08-04T00:22:08Z

This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.

github-actions · 2023-08-17T00:17:46Z

This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.

DarkAssassinator requested review from kezhenxu94, zhongjiajie, Tianqi-Dotes, EricGao888, songjianet, Amy0104, caishunfeng, SbloodyS, zhuangchong and ruanwenjun as code owners November 6, 2022 09:19

github-actions bot added backend document UI ui and front end related labels Nov 6, 2022

DarkAssassinator force-pushed the dev_yann_feature_add_ssh branch from 5352b4b to 748af92 Compare November 6, 2022 09:33

github-advanced-security bot found potential problems Nov 6, 2022

View reviewed changes

SbloodyS reviewed Nov 7, 2022

View reviewed changes

SbloodyS assigned DarkAssassinator Nov 7, 2022

SbloodyS added this to the 3.2.0 milestone Nov 7, 2022

SbloodyS added the feature new feature label Nov 7, 2022

DarkAssassinator force-pushed the dev_yann_feature_add_ssh branch from f1ac7d5 to bf6fbe0 Compare November 7, 2022 11:50

zhongjiajie reviewed Nov 9, 2022

View reviewed changes

DarkAssassinator force-pushed the dev_yann_feature_add_ssh branch 2 times, most recently from dc8084b to fb3ae31 Compare November 12, 2022 15:03

DarkAssassinator mentioned this pull request Nov 12, 2022

[Refactor] Migrate environment and system resources limitations to task params #12873

Closed

3 tasks

caishunfeng reviewed Nov 14, 2022

View reviewed changes

DarkAssassinator force-pushed the dev_yann_feature_add_ssh branch from b1bb091 to b2af72b Compare November 14, 2022 13:20

caishunfeng added the 3.2.0 for 3.2.0 version label Nov 15, 2022

DarkAssassinator added 10 commits November 15, 2022 19:00

fix CI

0a76c3c

fix CI

ce963e0

add DB index

513b00f

remove remote_host_code from taskInstance and TaskDefinition, and add…

be2f749

… BashTaskExecutionContext

fix failed UT cases

71c02b8

rub spotless apply

09a1f79

fix failed UT

d50eb80

update API and fix SonarLint bug

82a3617

correct PLSql column

e61eae8

fix E2E

f03f8a1

DarkAssassinator force-pushed the dev_yann_feature_add_ssh branch from 71b8b00 to f03f8a1 Compare November 15, 2022 11:07

github-actions bot added the e2e e2e test label Nov 15, 2022

fix spotless

87fe4eb

DarkAssassinator added 2 commits November 15, 2022 20:20

fix english doc

dd2402e

Merge branch 'dev' into dev_yann_feature_add_ssh

fbb3e17

Radeity requested changes Nov 21, 2022

View reviewed changes

EricGao888 added the priority:middle label Nov 25, 2022

Merge branch 'dev' into dev_yann_feature_add_ssh

05afebe

jieguangzhou mentioned this pull request Mar 27, 2023

[Feature][Task Plugin] RemoteShell Task Plugin #13794

Closed

3 tasks

github-actions bot added the Stale label Apr 4, 2023

SbloodyS removed the Stale label Apr 4, 2023

github-actions bot added the Stale label Aug 4, 2023

kezhenxu94 removed their request for review August 6, 2023 07:34

github-actions bot closed this Aug 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature-11652][2022Q4 RoadMap] DS support remote SSH task. #12736

[Feature-11652][2022Q4 RoadMap] DS support remote SSH task. #12736

DarkAssassinator commented Nov 6, 2022

codecov-commenter commented Nov 6, 2022 •

edited

Loading

SbloodyS Nov 7, 2022

DarkAssassinator Nov 7, 2022

SbloodyS Nov 14, 2022

DarkAssassinator Nov 14, 2022

zhongjiajie Nov 9, 2022

zhongjiajie Nov 9, 2022 •

edited

Loading

DarkAssassinator Nov 9, 2022

caishunfeng left a comment

caishunfeng Nov 14, 2022

DarkAssassinator Nov 14, 2022

caishunfeng Nov 15, 2022

DarkAssassinator Nov 15, 2022

caishunfeng commented Nov 14, 2022

DarkAssassinator commented Nov 14, 2022

DarkAssassinator commented Nov 15, 2022

Radeity left a comment

DarkAssassinator commented Nov 21, 2022

sonarcloud bot commented Nov 25, 2022

github-actions bot commented Apr 4, 2023

github-actions bot commented Aug 4, 2023

github-actions bot commented Aug 17, 2023

		DROP TABLE IF EXISTS `t_ds_task_remote_host`;
		CREATE TABLE `t_ds_task_remote_host`

[Feature-11652][2022Q4 RoadMap] DS support remote SSH task. #12736

[Feature-11652][2022Q4 RoadMap] DS support remote SSH task. #12736

Conversation

DarkAssassinator commented Nov 6, 2022

Purpose of the pull request

Brief change log

Limitations

Verify this pull request

codecov-commenter commented Nov 6, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhongjiajie Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caishunfeng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caishunfeng commented Nov 14, 2022

DarkAssassinator commented Nov 14, 2022

DarkAssassinator commented Nov 15, 2022

Radeity left a comment

Choose a reason for hiding this comment

DarkAssassinator commented Nov 21, 2022

sonarcloud bot commented Nov 25, 2022

github-actions bot commented Apr 4, 2023

github-actions bot commented Aug 4, 2023

github-actions bot commented Aug 17, 2023

codecov-commenter commented Nov 6, 2022 •

edited

Loading

zhongjiajie Nov 9, 2022 •

edited

Loading