You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@submarine.apache.org by GitBox <gi...@apache.org> on 2021/08/08 17:21:28 UTC
[GitHub] [submarine] kevin85421 opened a new pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
kevin85421 opened a new pull request #702:
URL: https://github.com/apache/submarine/pull/702
### What is this PR for?
In our multi-tenant architecture, users only can create experiments in their namespaces, but the Submarine workbench enables users to specify namespaces.
This patch aims to
### What type of PR is it?
[Bug Fix]
### Todos
### What is the Jira issue?
https://issues.apache.org/jira/browse/SUBMARINE-965
### How should this be tested?
**Test1**
* Step1: Use submarine-operator to create a submarine service in the namespace "submarine-user-test"
* Step2: Create an experiment, and check whether the experiment is in the namespace "submarine-user-test" rather than "default".
<img width="1439" alt="截圖 2021-08-08 下午11 57 35" src="https://user-images.githubusercontent.com/20109646/128640083-61ffc46d-fc26-4f22-8769-8202092c6574.png">
**Test2**
* Remove the field "namespace" in the Submarine workbench
### Screenshots (if appropriate)
<img width="1439" alt="截圖 2021-08-08 下午11 57 35" src="https://user-images.githubusercontent.com/20109646/128640083-61ffc46d-fc26-4f22-8769-8202092c6574.png">
<img width="994" alt="截圖 2021-08-09 上午1 19 46" src="https://user-images.githubusercontent.com/20109646/128640160-58a94857-346d-4ce9-8d14-97edeb4de26e.png">
### Questions:
* Do the license files need updating? No
* Are there breaking changes for older versions? No
* Does this need new documentation? No
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] kevin85421 commented on pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
Posted by GitBox <gi...@apache.org>.
kevin85421 commented on pull request #702:
URL: https://github.com/apache/submarine/pull/702#issuecomment-894918603
@pingsutw Can you help me review this PR? 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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] KUAN-HSUN-LI commented on pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
Posted by GitBox <gi...@apache.org>.
KUAN-HSUN-LI commented on pull request #702:
URL: https://github.com/apache/submarine/pull/702#issuecomment-895695966
@kevin85421 I am confused about running `mvn install -DskipTests` again. I think it will take more time in the integration test.
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] kevin85421 commented on pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
Posted by GitBox <gi...@apache.org>.
kevin85421 commented on pull request #702:
URL: https://github.com/apache/submarine/pull/702#issuecomment-895709972
@KUAN-HSUN-LI The command `mvn install -DskipTests` is used to upload the latest package to the local repository. To elaborate, the module `submarine-test` has a dependency `submarine-server-core`, and thus we need to upload the latest package. On the other hand, without the command, maven will repost errors.
For more details, you can check these two pull requests.
https://github.com/apache/submarine/pull/695/files
https://github.com/apache/submarine/pull/683/files
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] asfgit closed pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #702:
URL: https://github.com/apache/submarine/pull/702
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] KUAN-HSUN-LI commented on pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
Posted by GitBox <gi...@apache.org>.
KUAN-HSUN-LI commented on pull request #702:
URL: https://github.com/apache/submarine/pull/702#issuecomment-895731402
@kevin85421 Thanks for your reply!
At first, I had mentioned this problem therefore I saved the dependency in the cache. I wished it will use the latest dependency but it didn't work as I expect. For the reason to reduce the CI time and stable tests, I think it is useful to build the submarine in the e2e and k8s jobs.
(i.e. build -> submarine-e2e => submarine-e2e and build -> submarine-k8s => submarine-k8s )
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] pingsutw commented on a change in pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
Posted by GitBox <gi...@apache.org>.
pingsutw commented on a change in pull request #702:
URL: https://github.com/apache/submarine/pull/702#discussion_r684969774
##########
File path: submarine-server/server-core/src/test/java/org/apache/submarine/server/rest/ExperimentRestApiTest.java
##########
@@ -224,7 +222,6 @@ private void verifyResult(Experiment experiment, String uid) {
assertEquals(experimentFinishedTime, experiment.getFinishedTime());
assertEquals(metaName, experiment.getSpec().getMeta().getName());
assertEquals(metaFramework, experiment.getSpec().getMeta().getFramework());
- assertEquals(metaNamespace, experiment.getSpec().getMeta().getNamespace());
Review comment:
I think we should assert that the experiment namespace is default. And we should add a new test to make sure the namespace is what we expect if we update `ENV_NAMESPACE`
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [submarine] kevin85421 commented on pull request #702: SUBMARINE-965. Users should not have the privilege to create experiments in different namespaces
Posted by GitBox <gi...@apache.org>.
kevin85421 commented on pull request #702:
URL: https://github.com/apache/submarine/pull/702#issuecomment-895331193
@pingsutw Thank you for your review. I have added the assertion to `ExperimentRestApiTest.java`. In addition, I updated the TODO in this pull request description.
The integration test is very slow in "apache/submarine"; thus, I use the CI result of "kevin85421/hadoop-submarine" to ensure the correctness of this patch.
https://github.com/kevin85421/hadoop-submarine/actions/runs/1113309003
--
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@submarine.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org