Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature][Task] K8s namespace auth manager #9303

Merged
merged 14 commits into from
Apr 20, 2022

Conversation

qianli2022
Copy link
Contributor

Purpose of the pull request

Namespaces are managed with permissions that are created by the administrator and authorized to be used by ordinary users

Brief change log

  • Add unauth-namespace,authed-namespace,available-list to K8sNamespaceController
  • Add K8sNamespaceUserMapper to dao
  • Update some code to k8s api,service

Verify this pull request

This change added tests and can be verified as follows:

  • Added some tests to K8sNamespaceControllerTest.
  • Added some tests to UsersControllerTest.
  • Added some tests to K8SNamespaceServiceTest.

close #9252

@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2022

Codecov Report

Merging #9303 (6e36383) into dev (4a29c6a) will decrease coverage by 0.27%.
The diff coverage is 43.13%.

@@             Coverage Diff              @@
##                dev    #9303      +/-   ##
============================================
- Coverage     39.98%   39.71%   -0.28%     
- Complexity     4375     4411      +36     
============================================
  Files           822      826       +4     
  Lines         32924    33327     +403     
  Branches       3656     3698      +42     
============================================
+ Hits          13165    13236      +71     
- Misses        18532    18858     +326     
- Partials       1227     1233       +6     
Impacted Files Coverage Δ
...lugin/alert/wechat/WeChatAlertParamsConstants.java 0.00% <ø> (ø)
...uler/plugin/alert/wechat/WechatAppChatMessage.java 0.00% <0.00%> (ø)
...cheduler/plugin/alert/wechat/WechatAppMessage.java 0.00% <0.00%> (ø)
...inscheduler/api/controller/ExecutorController.java 40.00% <ø> (ø)
...lphinscheduler/api/controller/UsersController.java 59.25% <0.00%> (-2.28%) ⬇️
...cheduler/api/service/impl/ExecutorServiceImpl.java 41.96% <ø> (ø)
...r/api/service/impl/ProcessInstanceServiceImpl.java 60.22% <ø> (-0.12%) ⬇️
...er/api/service/impl/TaskDefinitionServiceImpl.java 24.01% <ø> (+0.05%) ⬆️
.../org/apache/dolphinscheduler/common/Constants.java 80.95% <ø> (ø)
...che/dolphinscheduler/common/enums/AlertStatus.java 0.00% <0.00%> (ø)
... and 42 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9d7223b...6e36383. Read the comment docs.

@qianli2022 qianli2022 requested a review from caishunfeng April 1, 2022 08:40
@caishunfeng caishunfeng requested a review from songjianet April 7, 2022 10:01
@caishunfeng
Copy link
Contributor

The backend part is good to me. But you need to change UI in next-ui. @qianli2022

@caishunfeng
Copy link
Contributor

@songjianet please take a look of UI part, thanks.

@caishunfeng
Copy link
Contributor

BTW, @qianli2022 please update the doc too, DS had put the doc part into code repository, so it should update the code and doc synchronously.

@caishunfeng caishunfeng added the miss:docs missing documents in PR label Apr 12, 2022
@songjianet
Copy link
Member

There are two problems here:

  1. We will not perform non-bug functional maintenance on the old ui, we will gradually eliminate the old ui
  2. For the new UI changes, you added an interface method, but it is not actually used, I don't know why

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

64.5% 64.5% Coverage
4.1% 4.1% Duplication

@caishunfeng
Copy link
Contributor

PTAL again @songjianet the next-ui had added.

@songjianet
Copy link
Member

The changes of the old ui can be reverted, because we will not do any non-bug processing on the old ui.

Copy link
Member

@songjianet songjianet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@caishunfeng caishunfeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@caishunfeng caishunfeng merged commit 165d7aa into apache:dev Apr 20, 2022
fengjian1129 pushed a commit to fengjian1129/dolphinscheduler that referenced this pull request Apr 23, 2022
* k8s auth

* remove log

* fix test

* use constants

* use constants K8S_LOCAL_TEST_CLUSTER

* simple auth get

* change test

* add namespace authorize in user page

* prettier code

* change test data

Co-authored-by: qianl4 <qianl4@cicso.com>
Co-authored-by: William Tong <weitong@cisco.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
miss:docs missing documents in PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature][api] Task can run on kubernetes - namespace auth manager
5 participants