You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/03/09 10:02:24 UTC

[GitHub] [cloudstack] soreana opened a new pull request #4774: Added configuration and Integration test to restrict public template …

soreana opened a new pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774


   ### Description
   
   As a cloud provider, we don't want our customers to see other templates. This pr limits template access to the domain.
   
   <!-- For new features, provide link to FS, dev ML discussion etc. -->
   <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
   
   <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
   <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
   <!-- Fixes: # -->
   
   <!--- ********************************************************************************* -->
   <!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
   <!--- PLEASE PUT AN 'X' in only **ONE** box -->
   <!--- ********************************************************************************* -->
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [x] New feature (non-breaking change which adds functionality)
   - [ ] Bug fix (non-breaking change which fixes an issue)
   - [ ] Enhancement (improves an existing feature and functionality)
   - [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [x] Minor
   
   
   ### How Has This Been Tested?
   
   To test this feature, I created two domains named Test1 and Test2, each with their respective domain admins (test1 and test2).
   I used cloudmonkey command to list different combination of templateFilters ( **all**, **featured**, **self**, **selfexecutable**, **sharedexecutable**, **executable** and **community** ) and accounts ( **admin**, **test1** and **test2** ).
   
   #### To test this feature:
   
   1. Create two domain with their respective domain admin accounts.
   2. Register new template in one of those domains. e.g: Debian.
   3. Set global setting `restrict.public.access.to.templates` to **off**. This is the community's default. Justify output of cloudmonkey listTemplates with third column of last table.
   4. Set global setting `restrict.public.access.to.templates` to **on**. Justify output of cloudmonkey listTemplates with fourth column of last table.
   
   Template | Owner 
   --------------|--------------
   CentOS | Admin
   Debian | Test1
   SystemVM | Admin
   Ubuntu | Admin
   
   <table>
       <thead>
           <tr>
               <th> Account </th>
               <th> templatefilter </th>
               <th> global setting=false </th>
               <th> global setting=true </th>
       <tbody>
           <tr>
               <td rowspan=7>Admin</td>
               <td>all</td>
               <td>CentOs<br>Debian<br>SystemVm<br>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td>featured</td>
               <td>CentOs<br>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> self </td>
               <td>Ubuntu<br>SystemVm</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> selfexecutable </td>
               <td>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> sharedexecutable </td>
               <td>Nothing</td>
               <td>No change</td>
           </tr>
           <tr>
               <td>executable</td>
               <td>CentOs<br>Debian<br>SystemVm<br>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> community </td>
               <td>Debian</td>
               <td>No change</td>
           </tr>
           <tr>
               <td rowspan=7>test1</td>
               <td>all</td>
               <td>CentOs<br>Debian<br>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td>featured</td>
               <td>CentOs<br>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> self </td>
               <td>Debian</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> selfexecutable </td>
               <td> Debian </td>
               <td>No change</td>
           </tr>
           <tr>
               <td> sharedexecutable </td>
               <td>Nothing</td>
               <td>No change</td>
           </tr>
           <tr>
               <td>executable</td>
               <td>CentOs<br>Debian<br>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> community </td>
               <td>Debian</td>
               <td>No change</td>
           </tr>
           <tr>
               <td rowspan=7>test2</td>
               <td>all</td>
               <td>CentOs<br>Debian<br>Ubuntu</td>
               <td>CentOs<br>Ubuntu</td>
           </tr>
           <tr>
               <td>featured</td>
               <td>CentOs<br>Ubuntu</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> self </td>
               <td>Nothing</td>
               <td>No change</td>
           </tr>
           <tr>
               <td> selfexecutable </td>
               <td> Nothing </td>
               <td>No change</td>
           </tr>
           <tr>
               <td> sharedexecutable </td>
               <td>Nothing</td>
               <td>No change</td>
           </tr>
           <tr>
               <td>executable</td>
               <td>CentOs<br>Debian<br>Ubuntu</td>
               <td>CentOs<br>Ubuntu</td>
           </tr>
           <tr>
               <td> community </td>
               <td>Debian</td>
               <td> Nothing </td>
           </tr>
       </tbody>
   </table>
   
   
   I wrote [this](https://gist.github.com/soreana/7c20dd1b5bca00cfdfee5018c13add6a) script to test this pr, you can find it in the following link. You need the `cmk` command and you should put `admin`, `test1`, and `test2` users info in the cmk configuration file. How to run this?
   
   - Put account names in accounts array defined at top of the script
   - ./listTemplates.sh will list all filter for all accounts
   - ./listTemplates.sh <account name> list all templates using all possible filters for <account name>
   - ./listTemplates.sh <template filter> list all templates for accounts in ('admin' 'test' 'test2') using <template filter>
   - ./listTemplates.sh <account name> <template filter> list all templates for <accounts name> using <template filter>
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-793981648


   Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 59


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r590200925



##########
File path: engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
##########
@@ -42,14 +42,16 @@
 public interface TemplateManager {
     static final String AllowPublicUserTemplatesCK = "allow.public.user.templates";
     static final String TemplatePreloaderPoolSizeCK = "template.preloader.pool.size";
+    static final String RestrictPublicTemplateAccessToDomainCK = "restrict.public.template.access.to.domain";
 
     static final ConfigKey<Boolean> AllowPublicUserTemplates = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true",
         "If false, users will not be able to create public templates.", true, ConfigKey.Scope.Account);
 
     static final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, TemplatePreloaderPoolSizeCK, "8",
             "Size of the TemplateManager threadpool", false, ConfigKey.Scope.Global);
 
-
+    static final ConfigKey<Boolean> RestrictPublicTemplateAccessToDomain = new ConfigKey<>("Advanced", Boolean.class, RestrictPublicTemplateAccessToDomainCK, "false",
+            "If true, the public template wouldn't share between domains", true, ConfigKey.Scope.Global);

Review comment:
       Can this configuration parameter defined at Domain level, instead of Global, which gives flexibility at domain level.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-930703666


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-926332486


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929842391


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929089154


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1422


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-937628705


   @sureshanaparti I tried to fix the issue mentioned by you yesterday. It was unsuccessful. I'm currently trying to revert the changes and move the config to global scope.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1032023344


   @sureshanaparti IIt still has an access issue. I'm going to work on it in the current week and the week after. I will update you when the pr is ready for a 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r810140867



##########
File path: api/src/main/java/org/apache/cloudstack/query/QueryService.java
##########
@@ -111,8 +111,8 @@
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
-    static final ConfigKey<Boolean> RestrictPublicTemplatesOfOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.templates.of.other.domains", "false",
-            "If true, the public templates of other domains will not be visible in this domain.", true, ConfigKey.Scope.Domain);
+    static final ConfigKey<Boolean> SharePublicTemplates = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates", "true",

Review comment:
       What about this name?
   
   ```java
       static final ConfigKey<Boolean> SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true",
   
   ````




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-925592711


   @sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-793740038


   Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2890


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r590313126



##########
File path: engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
##########
@@ -42,14 +42,16 @@
 public interface TemplateManager {
     static final String AllowPublicUserTemplatesCK = "allow.public.user.templates";
     static final String TemplatePreloaderPoolSizeCK = "template.preloader.pool.size";
+    static final String RestrictPublicTemplateAccessToDomainCK = "restrict.public.template.access.to.domain";
 
     static final ConfigKey<Boolean> AllowPublicUserTemplates = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true",
         "If false, users will not be able to create public templates.", true, ConfigKey.Scope.Account);
 
     static final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, TemplatePreloaderPoolSizeCK, "8",
             "Size of the TemplateManager threadpool", false, ConfigKey.Scope.Global);
 
-
+    static final ConfigKey<Boolean> RestrictPublicTemplateAccessToDomain = new ConfigKey<>("Advanced", Boolean.class, RestrictPublicTemplateAccessToDomainCK, "false",
+            "If true, the public template wouldn't share between domains", true, ConfigKey.Scope.Global);

Review comment:
       I think domain scope will add more value, please consider doing that.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-938587273


   > @weizhouapache my concern is primarily that (a) avoid rushing features - we're close to cutting RC so any business layer changes around domains/accounts can cause BIG problems (of course I've not tested/reviewed the PR myself so these concerns may not be valid), (b) the issue you've raised has always existed, adding something new however can cause a regression, (c) we probably need a non-author to test this (not sure if it has been done already?).
   > 
   > That's just me being conservative and risk-aversive, I'm open to getting it in 4.16.1 or 4.17.0 if we can address above concerns.
   
   @rhtyd thanks for explanation. let's cut 4.16.0.0 RC1 and release it asap. 
   Moving this to 4.17.0.0 is ok to me.
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934333567


   @soreana Changes looks good now. The actual use case here is to limit template access to the domain (which has to be restricted using a config in that domain only by root/domain admin). But, the config in the domain here is changing access of other domains, to be visible in that domain. Am I Correct?


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-932060180


   Continued tests with domain level setting.
   
   (i)
   - Domain1 config "restrict.public.template.access.to.domain": false and Domain1 config "restrict.public.template.access.to.domain": true
   => The public templates of domain1 are not listed for domain2 admin.
   => The public templates of domain2 are listed for domain1 admin.
   
   (ii)
   - Domain1 config "restrict.public.template.access.to.domain": true and Domain1 config "restrict.public.template.access.to.domain": false
   => The public templates of domain1 are listed for domain2 admin.
   => The public templates of domain2 are not listed for domain1 admin.
   
   (iii)
   - Set the Domain1 config "restrict.public.template.access.to.domain": false and Domain2 config "restrict.public.template.access.to.domain": false
   => The public templates of domain1 are listed for domain2 admin
   => The public templates of domain2 are listed for domain1 admin
   
   (iv)
   - Set the Domain1 config "restrict.public.template.access.to.domain": true and Domain2 config "restrict.public.template.access.to.domain": true
   => The public templates of domain1 are not listed for domain2 admin
   => The public templates of domain2 are not listed for domain1 admin
   
   @soreana tests (i) and (ii) above, should list the public templates when the respective domain config  ""restrict.public.template.access.to.domain" is false, and it seems to be working in the other way. Please check.
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r722130337



##########
File path: engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
##########
@@ -48,6 +48,8 @@
     static final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, TemplatePreloaderPoolSizeCK, "8",
             "Size of the TemplateManager threadpool", false, ConfigKey.Scope.Global);
 
+

Review comment:
       white spaces, no relevant changes in this file. can remove all the changes in this file?




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1031054845


   > Hi @sureshanaparti I see you were assigned to this PR, could you complete your testing or still needs testing?
   
   @nvazquez this needs re-testing with the latest changes, will update here after my testing is done.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1047658226


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2682


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland closed pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland closed pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774


   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1084546418


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1084625014


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3041


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1056814429


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1057360332


   <b>Trillian test result (tid-3458)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 32130 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4774-t3458-kvm-centos7.zip
   Smoke tests completed. 92 look OK, 0 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r810140867



##########
File path: api/src/main/java/org/apache/cloudstack/query/QueryService.java
##########
@@ -111,8 +111,8 @@
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
-    static final ConfigKey<Boolean> RestrictPublicTemplatesOfOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.templates.of.other.domains", "false",
-            "If true, the public templates of other domains will not be visible in this domain.", true, ConfigKey.Scope.Domain);
+    static final ConfigKey<Boolean> SharePublicTemplates = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates", "true",

Review comment:
       What about this name?
   
   ```java
   static final ConfigKey<Boolean> SharePublicTemplatesWithOtherDomains = 
       new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true",
   
   ````




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-878741521


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r701786875



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+
+            cls._cleanup.append(cls.root_template)
+            cls._cleanup.append(cls.template1)
+            cls._cleanup.append(cls.template2)
+            cls._cleanup.append(cls.sub_template)
+        else:
+            return
+
+        cls._cleanup.append(cls.account1)
+        cls._cleanup.append(cls.account2)
+        cls._cleanup.append(cls.sub_account)
+        cls._cleanup.append(cls.sub_domain)
+        cls._cleanup.append(cls.domain1)
+        cls._cleanup.append(cls.domain2)
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.apiclient = super(
+                TestTemplateAccessAcrossDomains,
+                cls).getClsTestClient().getApiClient()
+            # Cleanup resources used
+            cleanup_resources(cls.apiclient, cls._cleanup)
+
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+        return
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_01_check_cross_domain_template_access(self):
+        """
+        Verify that templates belonging to one domain should not be accessible
+        by other domains except for parent and ROOT domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of domain2 should not be accessible by domain1
+        3. Make sure template of domain1 should not be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        self.validate_uploaded_template(self.apiclient, self.template1.id)
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        self.validate_uploaded_template(self.apiclient, self.template2.id)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Make sure root domain can still access all subdomain templates
+        # Step 4
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_02_create_template(self):
+        """
+        Verify that templates belonging to one domain can be accessible
+        by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false (default behavior)
+        2. Make sure template of domain2 can be accessible by domain1
+        3. Make sure template of domain1 can be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        5. Deploy virtual machine in domain1 using template from domain2
+        6. Make sure that virtual machine can be deployed and is in running state
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("false")
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, True)
+
+        # Step 4
+        # Make sure root domain can still access all subdomain templates
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        # Deploy new virtual machine using template
+        virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.services["virtual_machine"],
+            templateid=self.template2.id,
+            accountid=self.account1.name,
+            domainid=self.account1.domainid,
+            serviceofferingid=self.service_offering.id,
+        )
+        self.debug("creating an instance with template ID: %s" % self.template2.id)
+        vm_response = VirtualMachine.list(self.apiclient,
+                                          id=virtual_machine.id,
+                                          account=self.account1.name,
+                                          domainid=self.account1.domainid)
+        self.assertEqual(
+            isinstance(vm_response, list),
+            True,
+            "Check for list VMs response after VM deployment"
+        )
+        # Verify VM response to check whether VM deployment was successful
+        self.assertNotEqual(
+            len(vm_response),
+            0,
+            "Check VMs available in List VMs response"
+        )
+
+        # Step 6
+        vm = vm_response[0]
+        self.assertEqual(
+            vm.state,
+            'Running',
+            "Check the state of VM created from Template"
+        )
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_03_check_subdomain_template_access(self):
+        """
+        Verify that templates belonging to parent domain can be accessible
+        by sub domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of ROOT domain can be accessible by domain1
+        3. Make sure template of ROOT domain can be accessible by domain2
+        """
+
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.root_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_04_check_non_public_template_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template2.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain2)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.template2.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template2.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_05_check_non_public_template_subdomain_access(self):
+        """
+        Verify that non public templates belonging to ROOT domain
+        should not be accessible by sub domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+        self.update_configuration("true")
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.root_template.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.root_template.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, False)
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, False)
+
+        self.update_configuration("false")
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain2, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_06_check_sub_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="True")
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_07_check_default_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        # Step 1
+        self.update_configuration("false")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_08_check_non_public_template_sub_domain_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default except ROOT domain
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template1 to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template1.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain1)
+        for template_response in list_template_response:
+            if template_response.id == self.template1.id:
+                break
+
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template1.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Even child domain should not access non public template
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+    def validate_uploaded_template(self, apiclient, template_id, retries=70, interval=5):
+        """Check if template download will finish in 1 minute"""
+        while retries > -1:
+            time.sleep(interval)
+            template_response = Template.list(
+                apiclient,
+                id=template_id,
+                zoneid=self.zone.id,
+                templatefilter='self'
+            )
+
+            if isinstance(template_response, list):
+                template = template_response[0]
+                if not hasattr(template, 'status') or not template or not template.status:
+                    retries = retries - 1
+                    continue
+                if 'Failed' in template.status:
+                    raise Exception(
+                        "Failed to download template: status - %s" %
+                        template.status)
+
+                elif template.status == 'Download Complete' and template.isready:
+                    return
+
+                elif 'Downloaded' in template.status:
+                    retries = retries - 1
+                    continue
+
+                elif 'Installing' not in template.status:
+                    if retries >= 0:
+                        retries = retries - 1
+                        continue
+                    raise Exception(
+                        "Error in downloading template: status - %s" %
+                        template.status)
+
+            else:
+                retries = retries - 1
+        raise Exception("Template download failed exception.")
+
+    def list_templates(self, templatefilter, domain):
+        return Template.list(
+                    self.apiclient,
+                    templatefilter=templatefilter,
+                    zoneid=self.zone.id,
+                    domainid=domain.id)
+
+    def validate_template_ownership(self, template, owner, nonowner, include_cross_domain_template):
+        """List the template belonging to domain which created it
+           Make sure that other domain can't access it.
+        """
+        list_template_response = self.list_templates('all', owner)
+        if list_template_response is not None:
+            """If global setting is false then public templates of any domain should
+               be accessible by any other domain
+            """
+            if include_cross_domain_template:
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        return
+
+                raise Exception("Template %s belonging to domain %s should "
+                                "be accessible by domain %s"
+                                % (template.name, nonowner.name, owner.name))
+            else:
+                """If global setting is true then public templates of any domain should not
+                   be accessible by any other domain except for root domain
+                """
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        raise Exception("Template %s belonging to domain %s should "
+                                        "not be accessible by domain %s"
+                                        % (template.name, nonowner.name, owner.name))
+
+    def update_configuration(self, value):
+        """
+        Function to update the global setting "restrict.public.access.to.templates"
+        :param value:
+        :return:
+        """
+        update_configuration_cmd = updateConfiguration.updateConfigurationCmd()
+        update_configuration_cmd.name = "restrict.public.template.access.to.domain"

Review comment:
       @soreana Config is defined as domain scoped therefore shouldn't the tests update it for specific domain?




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-938412693


   I've security concerns about this PR, really haven't seen the code or tested it myself - should this be considered in the milestone so close to the RC - @nvazquez @sureshanaparti ?


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-995941391


   Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 1916


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-793812206


   @soreana, does this have any impact on upgrades, existing templates in the environment might have already shared and used across domains. Does a domain lose viewing or accessing the templates which are already used when the configuration parameter is set to false.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-796590655


   @DaanHoogland It is nice feature. But it will not address access right issue fixed in this pr.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r590255285



##########
File path: engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
##########
@@ -42,14 +42,16 @@
 public interface TemplateManager {
     static final String AllowPublicUserTemplatesCK = "allow.public.user.templates";
     static final String TemplatePreloaderPoolSizeCK = "template.preloader.pool.size";
+    static final String RestrictPublicTemplateAccessToDomainCK = "restrict.public.template.access.to.domain";
 
     static final ConfigKey<Boolean> AllowPublicUserTemplates = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true",
         "If false, users will not be able to create public templates.", true, ConfigKey.Scope.Account);
 
     static final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, TemplatePreloaderPoolSizeCK, "8",
             "Size of the TemplateManager threadpool", false, ConfigKey.Scope.Global);
 
-
+    static final ConfigKey<Boolean> RestrictPublicTemplateAccessToDomain = new ConfigKey<>("Advanced", Boolean.class, RestrictPublicTemplateAccessToDomainCK, "false",
+            "If true, the public template wouldn't share between domains", true, ConfigKey.Scope.Global);

Review comment:
       I'm checking that. That is an option.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-933391150


   @soreana 
   please have a look at @sureshanaparti 's comments .
   
   As I remember, it was a global setting , not a domain setting, right ?


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-926332657


   @sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r722131219



##########
File path: server/src/main/java/com/cloud/api/query/QueryManagerImpl.java
##########
@@ -3516,6 +3543,8 @@ private boolean isPermissible(Long accountDomainId, Long offeringDomainId) {
         }
         SearchCriteria<TemplateJoinVO> sc = sb.create();
 
+        boolean restrictPublicTemplatesToDomain = QueryService.RestrictPublicTemplatesOfOtherDomains.valueIn(caller.getDomainId());

Review comment:
       ```suggestion
           boolean restrictPublicTemplatesOfOtherDomains = QueryService.RestrictPublicTemplatesOfOtherDomains.valueIn(caller.getDomainId());
   ```




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1047607070


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-866887772


   Hi @soreana looks like the PR has a conflict, can you please resolve it?


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-800307808


   @harikrishna-patnala I updated the PR. I migrated the setting to the domain level. It is a limited code change required to do so :D. I tested that with subdomains as well. More details:
   
   ```
   Root (Ubuntu)
     |-- domain0 (Debian)
     |       |-- sub0 (Fedora)
     |       |-- sub1
     |-- domain1
   ```
   
   If you set `restrict.public.template.access.to.domain` to true for domain1, domain1 access to `Debian` and `Fedora` will be limited.
   
   If you set `restrict.public.template.access.to.domain` to true for sub1, sub1 access to `Fedora` will be limited.
   
   Let me know if you have other concerns.
   
   P.S: I'm updating the test cases.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-937628705


   @sureshanaparti I tried to fix the issue mentioned by you yesterday. It was unsuccessful. I'm currently trying to revert the changes and move the config to global scope.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929087394


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1420


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934673530


   Thanks for the effort @soreana, @sureshanaparti is this PR ready?


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-925614440


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1373


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-911460060


   @shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-932044871


   Manually tested with the below steps and verified the public template access restriction to other domains, when the global config "_restrict.public.template.access.to.domain_" is set to true.
   
   - Created to Domains: Domain1 and Domain2.
   - Created accounts/users: Domain1Admin and Domain2Admin.
   - Registered public templates from macchinina-admin from Admin account.
   - Registered public templates from macchinina-domain1, centos55-domain1 from Domain1Admin account.
   - Registered public templates from macchinina-domain2, centos55-domain2 from Domain2Admin account.
   - By default the global config "restrict.public.template.access.to.domain" is set to false, and the public templates of domain1/domain2 are listed for admin, domain1 admin, and domain2 admin. 
   - Set the global config "restrict.public.template.access.to.domain" to true, and verified the public templates of domain1 are not listed for domain2 admin, and the templates of domain2 are not listed for domain1 admin. But, all the public templates are listed for root admin.
   
   
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929086678


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1419


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-995882391


   @sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r826121575



##########
File path: server/src/main/java/com/cloud/acl/DomainChecker.java
##########
@@ -167,6 +168,16 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
                             throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates");
                         }
                     }
+                } else if (QueryService.SharePublicTemplatesWithOtherDomains.valueIn(owner.getDomainId()) && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { // public template can be used by other accounts in the same domain or in sub-domains, and domain admin of parent domains
+                    if (caller.getDomainId() != owner.getDomainId() && !_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
+                        if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL || caller.getType() == Account.ACCOUNT_TYPE_PROJECT) {
+                            throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                        } else if (caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) {
+                            if (!_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
+                                throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                            }
+                        }
+                    }

Review comment:
       Updated the code.




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1067013871


   > please apply this suggestion @soreana . It reverses the cleanup from how it was created and thus prevents any blockage due to dependencies in the infra/app-scape.
   
   
   @DaanHoogland Could you please clarify your comment. I didn't get that. Should I write it like this?
   
   ``` Python
               self.update_restrict_template_configuration(self.sub_domain.id, self.sub_domain_config)
               self.update_restrict_template_configuration(self.domain2.id, self.domain2_config)
               self.update_restrict_template_configuration(self.domain1.id, self.domain1_config)
               cleanup_resources(self.apiclient, self.cleanup)
   ```


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1077620344


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2977


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r810138864



##########
File path: api/src/main/java/org/apache/cloudstack/query/QueryService.java
##########
@@ -111,8 +111,8 @@
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
-    static final ConfigKey<Boolean> RestrictPublicTemplatesOfOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.templates.of.other.domains", "false",
-            "If true, the public templates of other domains will not be visible in this domain.", true, ConfigKey.Scope.Domain);
+    static final ConfigKey<Boolean> SharePublicTemplates = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates", "true",
+            "If false, templates of this domain with not show up in the list templates.", true, ConfigKey.Scope.Domain);

Review comment:
       It is a typo, `will` is correct.




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r590224553



##########
File path: engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
##########
@@ -42,14 +42,16 @@
 public interface TemplateManager {
     static final String AllowPublicUserTemplatesCK = "allow.public.user.templates";
     static final String TemplatePreloaderPoolSizeCK = "template.preloader.pool.size";
+    static final String RestrictPublicTemplateAccessToDomainCK = "restrict.public.template.access.to.domain";
 
     static final ConfigKey<Boolean> AllowPublicUserTemplates = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true",
         "If false, users will not be able to create public templates.", true, ConfigKey.Scope.Account);
 
     static final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, TemplatePreloaderPoolSizeCK, "8",
             "Size of the TemplateManager threadpool", false, ConfigKey.Scope.Global);
 
-
+    static final ConfigKey<Boolean> RestrictPublicTemplateAccessToDomain = new ConfigKey<>("Advanced", Boolean.class, RestrictPublicTemplateAccessToDomainCK, "false",
+            "If true, the public template wouldn't share between domains", true, ConfigKey.Scope.Global);

Review comment:
       We want to enforce that as a root admin. We don't want to allow our customer share templates. If the shared template caused an issue, we don't want to be responsible for it.
   Btw, we can move it to zone level.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-878723949


   @nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1084546687


   @nvazquez a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache closed pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774


   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-932060180


   Continued tests with domain level setting.
   
   (i)
   - Domain1 config "restrict.public.template.access.to.domain": false and Domain2 config "restrict.public.template.access.to.domain": true
   => The public templates of domain1 are not listed for domain2 admin.
   => The public templates of domain2 are listed for domain1 admin.
   
   (ii)
   - Domain1 config "restrict.public.template.access.to.domain": true and Domain2 config "restrict.public.template.access.to.domain": false
   => The public templates of domain1 are listed for domain2 admin.
   => The public templates of domain2 are not listed for domain1 admin.
   
   (iii)
   - Set the Domain1 config "restrict.public.template.access.to.domain": false and Domain2 config "restrict.public.template.access.to.domain": false
   => The public templates of domain1 are listed for domain2 admin
   => The public templates of domain2 are listed for domain1 admin
   
   (iv)
   - Set the Domain1 config "restrict.public.template.access.to.domain": true and Domain2 config "restrict.public.template.access.to.domain": true
   => The public templates of domain1 are not listed for domain2 admin
   => The public templates of domain2 are not listed for domain1 admin
   
   @soreana tests (i) and (ii) above, should list the public templates when the respective domain config  ""restrict.public.template.access.to.domain" is false, and it seems to be working in the other way. Please check.
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929842391


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-911459162


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934083593


   ping @soreana, can you address the outstanding comments.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-935780726


   Hi @soreana, If the config setting added to the domain level, then restrict templates visibility of the domain in another domains using that config change in that domain (so that the control is in that domain itself). Otherwise, move back to global config, which restricts templates visibility of all other domains in a domain. In any case, the last config name "restrict.public.template.access.to.domain" should be fine.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934221069


   @sureshanaparti  Sorry for the late we have an issue I've to check. Regarding your test case (i) and (ii)
   > (i)
   > 
   > Domain1 config "restrict.public.template.access.to.domain": false and Domain2 config "restrict.public.template.access.to.domain": true
   > => The public templates of domain1 are not listed for domain2 admin.
   > => The public templates of domain2 are listed for domain1 admin.
   > 
   > (ii)
   > 
   > Domain1 config "restrict.public.template.access.to.domain": true and Domain2 config "restrict.public.template.access.to.domain": false
   > => The public templates of domain1 are listed for domain2 admin.
   > => The public templates of domain2 are not listed for domain1 admin.
   > 
   
   (i) when you set Domain2's `restrict.public.template.access.to.domain` to true, that means Domain2's templates will be available to other domains. (current behavior). It doesn't care about what would be visible in this domain. Let's clear it up with an example:
   
   | Domain    | `restrict.public.template.access.to.domain`  | Templates |
   | -----         | ----                                                                     | ---------  |
   | Domain1   | false                                                                    |  X1, Y1      |
   | Domain2   | true                                                                     |  X2, Y2     |
   
   What they will see when their admin list domains:
   
   | Domain    |  List Templates Output |
   | -----         |  ---------                       |
   | Domain1   |  X1, Y1, X2, Y2               |
   | Domain2   |  X2, Y2                          |
   
   Cause `restrict.public.template.access.to.domain` sets to true in Domain2,  Domain2's access to see the public template was restricted. It is the same for the (ii) test case.
   
   @weizhouapache I got https://github.com/apache/cloudstack/pull/4774#issuecomment-799156990 request from @harikrishna-patnala to moved the setting to domain scope.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934285468


   > ain2's access to see the public template was restricted. It is the same for the (ii) test case.
   
   @soreana As per my understanding from config name, when the Domain2 config "restrict.public.template.access.to.domain" is true, the public templates of Domain2 are restricted to other domains, and other domains cannot see them. So, in the above case, it has to be as below.
   
   Domain | List Templates Output
   -- | --
   Domain1 | X1, Y1
   Domain2 | X1, Y1, X2, Y2
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934285468


   > ain2's access to see the public template was restricted. It is the same for the (ii) test case.
   
   @soreana As per my understanding, when the Domain2 config "restrict.public.template.access.to.domain" is true, the public templates of Domain2 are restricted to other domains, and other domains cannot see them. So, in the above case, it has to be as below.
   
   Domain | List Templates Output
   -- | --
   Domain1 | X1, Y1
   Domain2 | X1, Y1, X2, Y2
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934293736


   @soreana May be you can update the config name to be clear, something like "_restrict.public.templates.of.other.domains_"


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1031055334


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r810040742



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,627 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import (listZones,
+                                  deleteTemplate,
+                                  listConfigurations,
+                                  updateConfiguration)
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):

Review comment:
       Done.




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-995992973


   @sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-996092965


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_multiplication_x: suse15. SL-JID 1921


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1068066224


   Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 2888


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1067768321


   > > > please apply this suggestion @soreana . It reverses the cleanup from how it was created and thus prevents any blockage due to dependencies in the infra/app-scape.
   > > 
   > > 
   > > @DaanHoogland Could you please clarify your comment. I didn't get that. Should I write it like this?
   > > ```python
   > >             self.update_restrict_template_configuration(self.sub_domain.id, self.sub_domain_config)
   > >             self.update_restrict_template_configuration(self.domain2.id, self.domain2_config)
   > >             self.update_restrict_template_configuration(self.domain1.id, self.domain1_config)
   > >             cleanup_resources(self.apiclient, self.cleanup)
   > > ```
   > 
   > no @soreana , just the last line should be replaced by the one sugested.
   
   Sorry @DaanHoogland I wasn't visible in the conversation tab 😄🙈
   Added the changes.
   
   ![Screenshot 2022-03-15 at 10 41 50](https://user-images.githubusercontent.com/9499410/158350520-089d0517-6b63-4245-8a96-fc75f5b3d9bf.png)


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-796588262


   Not that I want to intrude on your discussion @soreana @harikrishna-patnala , but would it make sense to add a scope to the filter:
   
   all, featured, self, selfexecutable, sharedexecutable, executable, community, *domain*


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r722129557



##########
File path: api/src/main/java/org/apache/cloudstack/query/QueryService.java
##########
@@ -111,6 +111,9 @@
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
+    static final ConfigKey<Boolean> RestrictPublicTemplatesOfOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.templates.of.other.domains", "false",
+            "If true, other domains' public templates will not be visible in this domain.", true, ConfigKey.Scope.Domain);

Review comment:
       ```suggestion
               "If true, the public templates of other domains will not be visible in this domain.", true, ConfigKey.Scope.Domain);
   ```




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-926353200


   @sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-933410522


   > @nvazquez the config scope is Domain https://github.com/apache/cloudstack/pull/4774/files#diff-a7bfc6f26c3369d9ee68fec67d2c4fcdd228dbc3d67fb56ee3b1590c4a24c4f4R117
   
   @sureshanaparti yes.  changing it from domain scope setting to global setting might solve the issues in your testing.
   @soreana could you please consider it ? it can be improved in next major release (4.17).


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934318383


   > @sureshanaparti Thanks for the comments and review, I will update my pr. Should I force update it and make it one commit, too?
   
   @soreana it is not needed.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r724769139



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,627 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import (listZones,
+                                  deleteTemplate,
+                                  listConfigurations,
+                                  updateConfiguration)
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):

Review comment:
       @soreana scanning quickly - can you add this to .travis.yml?




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-938443164


   > I've security concerns about this PR, really haven't seen the code or tested it myself - should this be considered in the milestone so close to the RC - @nvazquez @sureshanaparti ?
   
   @rhtyd can you describe it ?
   
   in 4.16, there is a globals setting `allow.public.user.templates` . if it is set to true (default value), users are able to create public templates.
   
   However, the public templates are visible for all users on the platform. this is my security concern actually, this PR solves the problem by adding a new global setting to determine the public templates are visible to users in the domain or all users. I think it is a good idea.
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
rhtyd edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-938582892


   @weizhouapache my concern is primarily that (a) avoid rushing features - we're close to cutting RC so any business layer changes around domains/accounts can cause BIG problems (of course I've not tested/reviewed the PR myself so these concerns may not be valid), (b) the issue you've raised has always existed, adding something new however can cause a regression, (c) we probably need a non-author to test this (not sure if it has been done already?).
   
   That's just me being conservative and risk-aversive, I'm open to getting it in 4.16.1 or 4.17.0 if we can address above concerns.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1031035018


   Hi @sureshanaparti I see you were assigned to this PR, could you complete your testing or still needs testing?


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1047608036


   @soreana a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1056816244


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1067768321


   > > > please apply this suggestion @soreana . It reverses the cleanup from how it was created and thus prevents any blockage due to dependencies in the infra/app-scape.
   > > 
   > > 
   > > @DaanHoogland Could you please clarify your comment. I didn't get that. Should I write it like this?
   > > ```python
   > >             self.update_restrict_template_configuration(self.sub_domain.id, self.sub_domain_config)
   > >             self.update_restrict_template_configuration(self.domain2.id, self.domain2_config)
   > >             self.update_restrict_template_configuration(self.domain1.id, self.domain1_config)
   > >             cleanup_resources(self.apiclient, self.cleanup)
   > > ```
   > 
   > no @soreana , just the last line should be replaced by the one sugested.
   
   Sorry @DaanHoogland your suggestion wasn't visible in the conversation tab 😄🙈
   Added the changes.
   
   ![Screenshot 2022-03-15 at 10 41 50](https://user-images.githubusercontent.com/9499410/158350520-089d0517-6b63-4245-8a96-fc75f5b3d9bf.png)


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1084330052


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-996423146


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1077589167


   @soreana a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-867367307


   domain scope change looks good @soreana, please resolve the conflicts on the PR.


-- 
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-794334885


   @harikrishna-patnala I didn't noticed any issue. Here is what I tested today.
   
   1. Set the global setting to false (templates shared between domains). 
   2. Created the instance (named `INSTANCE`) in `test2` domain using Debian template (which belongs to `test1` domain)
   3. Set the global setting to false (templates shared between domains).  
   4. `Debian` Template was removed from the template list.
   5. I tested everything related to `INSTANCE` including: `VNC console`, `Migration`, `Stop`, `Start`, `Reboot`, `Snapshot`
   6. I found nothing.
   
   Let me know if you have other concerns.
   
   **Btw**, while I tried to reinstall the `INSTANCE` I lost access to `Debian` template which is legitimate behaviour. See attached.
   
   ### Without restriction (global setting sets to false)
   <kbd> ![Screenshot 2021-03-09 at 19 33 44](https://user-images.githubusercontent.com/9499410/110524763-19f8d900-8114-11eb-8b60-983c0f08f155.png)</kbd>
   
   ### With restriction (global setting sets to true)
   <kbd> ![Screenshot 2021-03-09 at 19 33 16](https://user-images.githubusercontent.com/9499410/110524785-20875080-8114-11eb-8a69-6a62277f4caa.png)</kbd>
   
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-930225821


   <b>Trillian test result (tid-2230)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 31597 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4774-t2230-kvm-centos7.zip
   Smoke tests completed. 88 look OK, 1 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_hostha_enable_ha_when_host_disabled | `Error` | 2.68 | test_hostha_kvm.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 304.89 | test_hostha_kvm.py
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-925592217


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-932044871


   Manually tested with the below steps and verified the public template access restriction to other domains, when the config "_restrict.public.template.access.to.domain_" is set to true.
   
   - Created to Domains: Domain1 and Domain2.
   - Created accounts/users: Domain1Admin and Domain2Admin.
   - Registered public templates from macchinina-admin from Admin account.
   - Registered public templates from macchinina-domain1, centos55-domain1 from Domain1Admin account.
   - Registered public templates from macchinina-domain2, centos55-domain2 from Domain2Admin account.
   - By default the global config "restrict.public.template.access.to.domain" is set to false, and the public templates of domain1/domain2 are listed for admin, domain1 admin, and domain2 admin. 
   - Set the global config "restrict.public.template.access.to.domain" to true, and verified the public templates of domain1 are not listed for domain2 admin, and the templates of domain2 are not listed for domain1 admin. But, all the public templates are listed for root admin.
   
   
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-931306548


   <b>Trillian test result (tid-2245)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36312 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4774-t2245-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r701846869



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+
+            cls._cleanup.append(cls.root_template)
+            cls._cleanup.append(cls.template1)
+            cls._cleanup.append(cls.template2)
+            cls._cleanup.append(cls.sub_template)
+        else:
+            return
+
+        cls._cleanup.append(cls.account1)
+        cls._cleanup.append(cls.account2)
+        cls._cleanup.append(cls.sub_account)
+        cls._cleanup.append(cls.sub_domain)
+        cls._cleanup.append(cls.domain1)
+        cls._cleanup.append(cls.domain2)
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.apiclient = super(
+                TestTemplateAccessAcrossDomains,
+                cls).getClsTestClient().getApiClient()
+            # Cleanup resources used
+            cleanup_resources(cls.apiclient, cls._cleanup)
+
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+        return
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_01_check_cross_domain_template_access(self):
+        """
+        Verify that templates belonging to one domain should not be accessible
+        by other domains except for parent and ROOT domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of domain2 should not be accessible by domain1
+        3. Make sure template of domain1 should not be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        self.validate_uploaded_template(self.apiclient, self.template1.id)
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        self.validate_uploaded_template(self.apiclient, self.template2.id)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Make sure root domain can still access all subdomain templates
+        # Step 4
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_02_create_template(self):
+        """
+        Verify that templates belonging to one domain can be accessible
+        by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false (default behavior)
+        2. Make sure template of domain2 can be accessible by domain1
+        3. Make sure template of domain1 can be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        5. Deploy virtual machine in domain1 using template from domain2
+        6. Make sure that virtual machine can be deployed and is in running state
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("false")
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, True)
+
+        # Step 4
+        # Make sure root domain can still access all subdomain templates
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        # Deploy new virtual machine using template
+        virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.services["virtual_machine"],
+            templateid=self.template2.id,
+            accountid=self.account1.name,
+            domainid=self.account1.domainid,
+            serviceofferingid=self.service_offering.id,
+        )
+        self.debug("creating an instance with template ID: %s" % self.template2.id)
+        vm_response = VirtualMachine.list(self.apiclient,
+                                          id=virtual_machine.id,
+                                          account=self.account1.name,
+                                          domainid=self.account1.domainid)
+        self.assertEqual(
+            isinstance(vm_response, list),
+            True,
+            "Check for list VMs response after VM deployment"
+        )
+        # Verify VM response to check whether VM deployment was successful
+        self.assertNotEqual(
+            len(vm_response),
+            0,
+            "Check VMs available in List VMs response"
+        )
+
+        # Step 6
+        vm = vm_response[0]
+        self.assertEqual(
+            vm.state,
+            'Running',
+            "Check the state of VM created from Template"
+        )
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_03_check_subdomain_template_access(self):
+        """
+        Verify that templates belonging to parent domain can be accessible
+        by sub domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of ROOT domain can be accessible by domain1
+        3. Make sure template of ROOT domain can be accessible by domain2
+        """
+
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.root_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_04_check_non_public_template_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template2.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain2)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.template2.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template2.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_05_check_non_public_template_subdomain_access(self):
+        """
+        Verify that non public templates belonging to ROOT domain
+        should not be accessible by sub domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+        self.update_configuration("true")
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.root_template.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.root_template.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, False)
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, False)
+
+        self.update_configuration("false")
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain2, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_06_check_sub_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="True")
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_07_check_default_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        # Step 1
+        self.update_configuration("false")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_08_check_non_public_template_sub_domain_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default except ROOT domain
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template1 to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template1.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain1)
+        for template_response in list_template_response:
+            if template_response.id == self.template1.id:
+                break
+
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template1.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Even child domain should not access non public template
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+    def validate_uploaded_template(self, apiclient, template_id, retries=70, interval=5):
+        """Check if template download will finish in 1 minute"""
+        while retries > -1:
+            time.sleep(interval)
+            template_response = Template.list(
+                apiclient,
+                id=template_id,
+                zoneid=self.zone.id,
+                templatefilter='self'
+            )
+
+            if isinstance(template_response, list):
+                template = template_response[0]
+                if not hasattr(template, 'status') or not template or not template.status:
+                    retries = retries - 1
+                    continue
+                if 'Failed' in template.status:
+                    raise Exception(
+                        "Failed to download template: status - %s" %
+                        template.status)
+
+                elif template.status == 'Download Complete' and template.isready:
+                    return
+
+                elif 'Downloaded' in template.status:
+                    retries = retries - 1
+                    continue
+
+                elif 'Installing' not in template.status:
+                    if retries >= 0:
+                        retries = retries - 1
+                        continue
+                    raise Exception(
+                        "Error in downloading template: status - %s" %
+                        template.status)
+
+            else:
+                retries = retries - 1
+        raise Exception("Template download failed exception.")
+
+    def list_templates(self, templatefilter, domain):
+        return Template.list(
+                    self.apiclient,
+                    templatefilter=templatefilter,
+                    zoneid=self.zone.id,
+                    domainid=domain.id)
+
+    def validate_template_ownership(self, template, owner, nonowner, include_cross_domain_template):
+        """List the template belonging to domain which created it
+           Make sure that other domain can't access it.
+        """
+        list_template_response = self.list_templates('all', owner)
+        if list_template_response is not None:
+            """If global setting is false then public templates of any domain should
+               be accessible by any other domain
+            """
+            if include_cross_domain_template:
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        return
+
+                raise Exception("Template %s belonging to domain %s should "
+                                "be accessible by domain %s"
+                                % (template.name, nonowner.name, owner.name))
+            else:
+                """If global setting is true then public templates of any domain should not
+                   be accessible by any other domain except for root domain
+                """
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        raise Exception("Template %s belonging to domain %s should "
+                                        "not be accessible by domain %s"
+                                        % (template.name, nonowner.name, owner.name))
+
+    def update_configuration(self, value):
+        """
+        Function to update the global setting "restrict.public.access.to.templates"
+        :param value:
+        :return:
+        """
+        update_configuration_cmd = updateConfiguration.updateConfigurationCmd()
+        update_configuration_cmd.name = "restrict.public.template.access.to.domain"

Review comment:
       @soreana 👍 
   I added a diff for some changes in my review if that helps in any way




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-877180363


   I fixed the conflict.
   @nvazquez @harikrishna-patnala  Sorry for the late response. As I was away for couple of weeks, I completely forgot about this.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-878741677


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-794912883


   @soreana I have raised the question exactly like Reinstall VM you mentioned above, loosing access to the template which are already in use. May be keeping the configuration scope to Domain level will give flexibility in allowing access in these cases.
   
   Let's hear from others too. 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929045423


   @shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-937674524


   > @sureshanaparti I tried to fix the issue mentioned by you yesterday. It was unsuccessful. I'm currently trying to revert the changes and move the config to global scope.
   
   ok, thank you for the update.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-937623496


   @soreana, any update, are you working on the changes?


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929041380


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-926352894


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-937623496






-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-878740420


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 528


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r810103090



##########
File path: api/src/main/java/org/apache/cloudstack/query/QueryService.java
##########
@@ -111,8 +111,8 @@
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
-    static final ConfigKey<Boolean> RestrictPublicTemplatesOfOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.templates.of.other.domains", "false",
-            "If true, the public templates of other domains will not be visible in this domain.", true, ConfigKey.Scope.Domain);
+    static final ConfigKey<Boolean> SharePublicTemplates = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates", "true",
+            "If false, templates of this domain with not show up in the list templates.", true, ConfigKey.Scope.Domain);

Review comment:
       with or will ?

##########
File path: api/src/main/java/org/apache/cloudstack/query/QueryService.java
##########
@@ -111,8 +111,8 @@
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
-    static final ConfigKey<Boolean> RestrictPublicTemplatesOfOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.templates.of.other.domains", "false",
-            "If true, the public templates of other domains will not be visible in this domain.", true, ConfigKey.Scope.Domain);
+    static final ConfigKey<Boolean> SharePublicTemplates = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates", "true",

Review comment:
       maybe add more content to configuration name ?




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1068074449


   @soreana you've got a build error:
   ```
   [ERROR] COMPILATION ERROR : 
   [INFO] -------------------------------------------------------------
   [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.17.0.0-SNAPSHOT/server/src/main/java/com/cloud/acl/DomainChecker.java:[128,39] error: cannot find symbol
     symbol:   variable ACCOUNT_TYPE_NORMAL
     location: interface Account
   [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.17.0.0-SNAPSHOT/server/src/main/java/com/cloud/acl/DomainChecker.java:[128,90] error: cannot find symbol
     symbol:   variable ACCOUNT_TYPE_PROJECT
     location: interface Account
   [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.17.0.0-SNAPSHOT/server/src/main/java/com/cloud/acl/DomainChecker.java:[130,46] error: cannot find symbol
     symbol:   variable ACCOUNT_TYPE_DOMAIN_ADMIN
     location: interface Account
   [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.17.0.0-SNAPSHOT/server/src/main/java/com/cloud/acl/DomainChecker.java:[130,103] error: cannot find symbol
     symbol:   variable ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN
     location: interface Account
   [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.17.0.0-SNAPSHOT/server/src/main/java/com/cloud/acl/DomainChecker.java:[203,54] error: cannot find symbol
     symbol:   variable ACCOUNT_TYPE_ADMIN
     location: interface Account
   [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.17.0.0-SNAPSHOT/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:[3874,38] error: cannot find symbol
     symbol:   variable ACCOUNT_TYPE_ADMIN
     location: interface Account
   ```


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r821695630



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,627 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import (listZones,
+                                  deleteTemplate,
+                                  listConfigurations,
+                                  updateConfiguration)
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+        cls._cleanup.append(cls.domain1)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+        cls._cleanup.append(cls.account1)
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+        cls._cleanup.append(cls.sub_domain)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+        cls._cleanup.append(cls.sub_account)
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+        cls._cleanup.append(cls.domain2)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+        cls._cleanup.append(cls.account2)
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.root_template.download(cls.apiclient)
+            cls._cleanup.append(cls.root_template)
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.template1.download(cls.apiclient)
+            cls._cleanup.append(cls.template1)
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.sub_template.download(cls.apiclient)
+            cls._cleanup.append(cls.sub_template)
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.template2.download(cls.apiclient)
+            cls._cleanup.append(cls.template2)
+        else:
+            return
+
+    @classmethod
+    def tearDownClass(cls):
+        super(TestTemplateAccessAcrossDomains, cls).tearDownClass()
+
+    def setUp(self):
+        self.apiclient = self.testClient.getApiClient()
+        self.domain1_config = self.get_restrict_template_configuration(self.domain1.id)
+        self.domain2_config = self.get_restrict_template_configuration(self.domain2.id)
+        self.sub_domain_config = self.get_restrict_template_configuration(self.sub_domain.id)
+        self.cleanup = []
+        return
+
+    def tearDown(self):
+        try:
+            self.update_restrict_template_configuration(self.domain1.id, self.domain1_config)
+            self.update_restrict_template_configuration(self.domain2.id, self.domain2_config)
+            self.update_restrict_template_configuration(self.sub_domain.id, self.sub_domain_config)
+            cleanup_resources(self.apiclient, self.cleanup)

Review comment:
       please apply this suggestion @soreana . It reverses the cleanup from how it was created and thus prevents any blockage due to dependencies in the infra/app-scape.

##########
File path: server/src/main/java/com/cloud/acl/DomainChecker.java
##########
@@ -167,6 +168,16 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
                             throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates");
                         }
                     }
+                } else if (QueryService.SharePublicTemplatesWithOtherDomains.valueIn(owner.getDomainId()) && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { // public template can be used by other accounts in the same domain or in sub-domains, and domain admin of parent domains
+                    if (caller.getDomainId() != owner.getDomainId() && !_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
+                        if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL || caller.getType() == Account.ACCOUNT_TYPE_PROJECT) {
+                            throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                        } else if (caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) {
+                            if (!_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
+                                throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                            }
+                        }
+                    }

Review comment:
       maybe using 
   ```
   // public template can be used by other accounts in the same domain or in sub-domains, and domain admin of parent domains
   ```
   as an inspiration for a method name or a javadoc on it?

##########
File path: server/src/main/java/com/cloud/acl/DomainChecker.java
##########
@@ -167,6 +168,16 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
                             throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates");
                         }
                     }
+                } else if (QueryService.SharePublicTemplatesWithOtherDomains.valueIn(owner.getDomainId()) && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { // public template can be used by other accounts in the same domain or in sub-domains, and domain admin of parent domains
+                    if (caller.getDomainId() != owner.getDomainId() && !_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
+                        if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL || caller.getType() == Account.ACCOUNT_TYPE_PROJECT) {
+                            throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                        } else if (caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) {
+                            if (!_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
+                                throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                            }
+                        }
+                    }

Review comment:
       can you put this block in a separate 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-996448808


   Packaging result: :heavy_check_mark: el7 :heavy_multiplication_x: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1931


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-995881625


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934319999


   @weizhouapache Cool, 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-935658282


   > > ain2's access to see the public template was restricted. It is the same for the (ii) test case.
   > 
   > @soreana As per my understanding from config name, when the Domain2 config "restrict.public.template.access.to.domain" is true, the public templates of Domain2 are restricted to other domains, and other domains cannot see them. So, in the above case, it has to be as below.
   > 
   > Domain	List Templates Output
   > Domain1	X1, Y1
   > Domain2	X1, Y1, X2, Y2
   
   @sureshanaparti 
   I agree with you.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929045423






-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-933397263


   @nvazquez the config scope is Domain
   https://github.com/apache/cloudstack/pull/4774/files#diff-a7bfc6f26c3369d9ee68fec67d2c4fcdd228dbc3d67fb56ee3b1590c4a24c4f4R117


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929842909






-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929842909


   @sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] rhtyd commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-938582892


   @weizhouapache my concern is primarily that (a) avoid rushing features - we're close to cutting RC so any business layer changes around domains/accounts can cause BIG problems (of course I've not tested/reviewed the PR myself so these concerns may not be valid), (b) the issue you've raised has always existed, adding something new however can cause a regression, (c) we probably need a non-author to test this (not sure if it has been done already?).


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1032023344


   @sureshanaparti It still has an access issue. I'm going to work on it in the current week and the week after. I will update you when the pr is ready for a 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r810140867



##########
File path: api/src/main/java/org/apache/cloudstack/query/QueryService.java
##########
@@ -111,8 +111,8 @@
             "allow.user.view.all.domain.accounts", "false",
             "Determines whether users can view all user accounts within the same domain", true, ConfigKey.Scope.Domain);
 
-    static final ConfigKey<Boolean> RestrictPublicTemplatesOfOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "restrict.public.templates.of.other.domains", "false",
-            "If true, the public templates of other domains will not be visible in this domain.", true, ConfigKey.Scope.Domain);
+    static final ConfigKey<Boolean> SharePublicTemplates = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates", "true",

Review comment:
       What about this name?
   
   ```java
   static final ConfigKey<Boolean> SharePublicTemplatesWithOtherDomains = new ConfigKey<>("Advanced", Boolean.class, "share.public.templates.with.other.domains", "true",
   
   ````




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-996423397


   @sureshanaparti a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1078581051


   <b>Trillian test result (tid-3713)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 30873 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4774-t3713-kvm-centos7.zip
   Smoke tests completed. 92 look OK, 0 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r825238349



##########
File path: server/src/main/java/com/cloud/acl/DomainChecker.java
##########
@@ -167,6 +168,16 @@ public boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
                             throw new PermissionDeniedException("Domain Admin and regular users can modify only their own Public templates");
                         }
                     }
+                } else if (QueryService.SharePublicTemplatesWithOtherDomains.valueIn(owner.getDomainId()) && caller.getType() != Account.ACCOUNT_TYPE_ADMIN) { // public template can be used by other accounts in the same domain or in sub-domains, and domain admin of parent domains
+                    if (caller.getDomainId() != owner.getDomainId() && !_domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) {
+                        if (caller.getType() == Account.ACCOUNT_TYPE_NORMAL || caller.getType() == Account.ACCOUNT_TYPE_PROJECT) {
+                            throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                        } else if (caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN || caller.getType() == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) {
+                            if (!_domainDao.isChildDomain(caller.getDomainId(), owner.getDomainId())) {
+                                throw new PermissionDeniedException(caller + "is not allowed to access the template " + template);
+                            }
+                        }
+                    }

Review comment:
       +1 to this suggestion




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1068030430


   @DaanHoogland a Jenkins job has been kicked to build packages. It will be bundled with  KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1067668670


   > > please apply this suggestion @soreana . It reverses the cleanup from how it was created and thus prevents any blockage due to dependencies in the infra/app-scape.
   > 
   > @DaanHoogland Could you please clarify your comment. I didn't get that. Should I write it like this?
   > 
   > ```python
   >             self.update_restrict_template_configuration(self.sub_domain.id, self.sub_domain_config)
   >             self.update_restrict_template_configuration(self.domain2.id, self.domain2_config)
   >             self.update_restrict_template_configuration(self.domain1.id, self.domain1_config)
   >             cleanup_resources(self.apiclient, self.cleanup)
   > ```
   
   no @soreana , just the last line should be replaced by the one sugested.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-926730441


   <b>Trillian test result (tid-2194)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36384 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4774-t2194-kvm-centos7.zip
   Smoke tests completed. 89 look OK, 0 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-800307808


   @harikrishna-patnala I updated the PR. I migrated the setting to domain level. It is limited code change required to do so :D .I tested that with subdomains as well. More details:
   
   ```
   Root (Ubuntu)
     |-- domain0 (Debian)
     |       |-- sub0 (Fedora)
     |       |-- sub1
     |-- domain1
   ```
   
   If you set `restrict.public.template.access.to.domain` to true for domain1, domain1 access to `Debian` and `Fedora` will be limited.
   
   If you set `restrict.public.template.access.to.domain` to true for sub1, sub1 access to `Fedora` will be limited.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-793812206


   @soreana, does this have any impact on upgrades, existing templates in the environment might have already shared and used across domains. Does a domain lose viewing or accessing the templates which are already used when the configuration parameter is set to true.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-878723831


   No problem, thanks @soreana
   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r701837601



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+
+            cls._cleanup.append(cls.root_template)
+            cls._cleanup.append(cls.template1)
+            cls._cleanup.append(cls.template2)
+            cls._cleanup.append(cls.sub_template)
+        else:
+            return
+
+        cls._cleanup.append(cls.account1)
+        cls._cleanup.append(cls.account2)
+        cls._cleanup.append(cls.sub_account)
+        cls._cleanup.append(cls.sub_domain)
+        cls._cleanup.append(cls.domain1)
+        cls._cleanup.append(cls.domain2)
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.apiclient = super(
+                TestTemplateAccessAcrossDomains,
+                cls).getClsTestClient().getApiClient()
+            # Cleanup resources used
+            cleanup_resources(cls.apiclient, cls._cleanup)
+
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+        return
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_01_check_cross_domain_template_access(self):
+        """
+        Verify that templates belonging to one domain should not be accessible
+        by other domains except for parent and ROOT domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of domain2 should not be accessible by domain1
+        3. Make sure template of domain1 should not be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        self.validate_uploaded_template(self.apiclient, self.template1.id)
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        self.validate_uploaded_template(self.apiclient, self.template2.id)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Make sure root domain can still access all subdomain templates
+        # Step 4
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_02_create_template(self):
+        """
+        Verify that templates belonging to one domain can be accessible
+        by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false (default behavior)
+        2. Make sure template of domain2 can be accessible by domain1
+        3. Make sure template of domain1 can be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        5. Deploy virtual machine in domain1 using template from domain2
+        6. Make sure that virtual machine can be deployed and is in running state
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("false")
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, True)
+
+        # Step 4
+        # Make sure root domain can still access all subdomain templates
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        # Deploy new virtual machine using template
+        virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.services["virtual_machine"],
+            templateid=self.template2.id,
+            accountid=self.account1.name,
+            domainid=self.account1.domainid,
+            serviceofferingid=self.service_offering.id,
+        )
+        self.debug("creating an instance with template ID: %s" % self.template2.id)
+        vm_response = VirtualMachine.list(self.apiclient,
+                                          id=virtual_machine.id,
+                                          account=self.account1.name,
+                                          domainid=self.account1.domainid)
+        self.assertEqual(
+            isinstance(vm_response, list),
+            True,
+            "Check for list VMs response after VM deployment"
+        )
+        # Verify VM response to check whether VM deployment was successful
+        self.assertNotEqual(
+            len(vm_response),
+            0,
+            "Check VMs available in List VMs response"
+        )
+
+        # Step 6
+        vm = vm_response[0]
+        self.assertEqual(
+            vm.state,
+            'Running',
+            "Check the state of VM created from Template"
+        )
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_03_check_subdomain_template_access(self):
+        """
+        Verify that templates belonging to parent domain can be accessible
+        by sub domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of ROOT domain can be accessible by domain1
+        3. Make sure template of ROOT domain can be accessible by domain2
+        """
+
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.root_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_04_check_non_public_template_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template2.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain2)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.template2.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template2.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_05_check_non_public_template_subdomain_access(self):
+        """
+        Verify that non public templates belonging to ROOT domain
+        should not be accessible by sub domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+        self.update_configuration("true")
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.root_template.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.root_template.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, False)
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, False)
+
+        self.update_configuration("false")
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain2, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_06_check_sub_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="True")
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_07_check_default_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        # Step 1
+        self.update_configuration("false")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_08_check_non_public_template_sub_domain_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default except ROOT domain
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template1 to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template1.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain1)
+        for template_response in list_template_response:
+            if template_response.id == self.template1.id:
+                break
+
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template1.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Even child domain should not access non public template
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+    def validate_uploaded_template(self, apiclient, template_id, retries=70, interval=5):
+        """Check if template download will finish in 1 minute"""
+        while retries > -1:
+            time.sleep(interval)
+            template_response = Template.list(
+                apiclient,
+                id=template_id,
+                zoneid=self.zone.id,
+                templatefilter='self'
+            )
+
+            if isinstance(template_response, list):
+                template = template_response[0]
+                if not hasattr(template, 'status') or not template or not template.status:
+                    retries = retries - 1
+                    continue
+                if 'Failed' in template.status:
+                    raise Exception(
+                        "Failed to download template: status - %s" %
+                        template.status)
+
+                elif template.status == 'Download Complete' and template.isready:
+                    return
+
+                elif 'Downloaded' in template.status:
+                    retries = retries - 1
+                    continue
+
+                elif 'Installing' not in template.status:
+                    if retries >= 0:
+                        retries = retries - 1
+                        continue
+                    raise Exception(
+                        "Error in downloading template: status - %s" %
+                        template.status)
+
+            else:
+                retries = retries - 1
+        raise Exception("Template download failed exception.")
+
+    def list_templates(self, templatefilter, domain):
+        return Template.list(
+                    self.apiclient,
+                    templatefilter=templatefilter,
+                    zoneid=self.zone.id,
+                    domainid=domain.id)
+
+    def validate_template_ownership(self, template, owner, nonowner, include_cross_domain_template):
+        """List the template belonging to domain which created it
+           Make sure that other domain can't access it.
+        """
+        list_template_response = self.list_templates('all', owner)
+        if list_template_response is not None:
+            """If global setting is false then public templates of any domain should
+               be accessible by any other domain
+            """
+            if include_cross_domain_template:
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        return
+
+                raise Exception("Template %s belonging to domain %s should "
+                                "be accessible by domain %s"
+                                % (template.name, nonowner.name, owner.name))
+            else:
+                """If global setting is true then public templates of any domain should not
+                   be accessible by any other domain except for root domain
+                """
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        raise Exception("Template %s belonging to domain %s should "
+                                        "not be accessible by domain %s"
+                                        % (template.name, nonowner.name, owner.name))
+
+    def update_configuration(self, value):
+        """
+        Function to update the global setting "restrict.public.access.to.templates"
+        :param value:
+        :return:
+        """
+        update_configuration_cmd = updateConfiguration.updateConfigurationCmd()
+        update_configuration_cmd.name = "restrict.public.template.access.to.domain"

Review comment:
       @shwstppr 
   Yeap, you are right. I was working on a different [RP](https://github.com/apache/cloudstack/pull/5296) , my next task is updating this PR's 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-905167235


   @soreana can you address the open comments on the marvin tests?


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1077843454


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1077588432


   @DaanHoogland I've finally found the issue. I had deviation with main branch :)
   Now it is working.
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-911498464


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1101


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti closed pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti closed pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774


   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-930703467


   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-932044871


   Manually tested with the below steps and verified the public template access restriction to other domains, when the config "_restrict.public.template.access.to.domain_" is set to true.
   
   - Created to Domains: Domain1 and Domain2.
   - Created accounts/users: Domain1Admin and Domain2Admin.
   - Registered public templates from macchinina-admin from Admin account.
   - Registered public templates from macchinina-domain1, centos55-domain1 from Domain1Admin account.
   - Registered public templates from macchinina-domain2, centos55-domain2 from Domain2Admin account.
   - By default the config "restrict.public.template.access.to.domain" is set to false, and the public templates of domain1/domain2 are listed for admin, domain1 admin, and domain2 admin. 
   - Set the config "restrict.public.template.access.to.domain" to true, and verified the public templates of domain1 are not listed for domain2 admin, and the templates of domain2 are not listed for domain1 admin. But, all the public templates are listed for root admin.
   
   
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] shwstppr commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
shwstppr commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929041380


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r590200925



##########
File path: engine/components-api/src/main/java/com/cloud/template/TemplateManager.java
##########
@@ -42,14 +42,16 @@
 public interface TemplateManager {
     static final String AllowPublicUserTemplatesCK = "allow.public.user.templates";
     static final String TemplatePreloaderPoolSizeCK = "template.preloader.pool.size";
+    static final String RestrictPublicTemplateAccessToDomainCK = "restrict.public.template.access.to.domain";
 
     static final ConfigKey<Boolean> AllowPublicUserTemplates = new ConfigKey<Boolean>("Advanced", Boolean.class, AllowPublicUserTemplatesCK, "true",
         "If false, users will not be able to create public templates.", true, ConfigKey.Scope.Account);
 
     static final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, TemplatePreloaderPoolSizeCK, "8",
             "Size of the TemplateManager threadpool", false, ConfigKey.Scope.Global);
 
-
+    static final ConfigKey<Boolean> RestrictPublicTemplateAccessToDomain = new ConfigKey<>("Advanced", Boolean.class, RestrictPublicTemplateAccessToDomainCK, "false",
+            "If true, the public template wouldn't share between domains", true, ConfigKey.Scope.Global);

Review comment:
       Can this configuration parameter defined at Domain level, instead of Global, which gives flexibility at domain level and check the access of templates based on the Owner domain configuration parameter value ?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-794187077


   Packaging result: :heavy_check_mark: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 63


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-800307808


   @harikrishna-patnala I updated the PR. I migrated the setting to the domain level. It is a limited code change required to do so :D. I tested that with subdomains as well. More details:
   
   ```
   Root (Ubuntu)
     |-- domain0 (Debian)
     |       |-- sub0 (Fedora)
     |       |-- sub1
     |-- domain1
   ```
   
   If you set `restrict.public.template.access.to.domain` to true for domain1, domain1 access to `Debian` and `Fedora` will be limited.
   
   If you set `restrict.public.template.access.to.domain` to true for sub1, sub1 access to `Fedora` will be limited.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-879255859


   <b>Trillian test result (tid-1246)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 46884 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4774-t1246-kvm-centos7.zip
   Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
   Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
   Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
   Smoke tests completed. 84 look OK, 4 have error(s)
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 72.00 | test_kubernetes_clusters.py
   test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 396.74 | test_routers_network_ops.py
   test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Failure` | 341.05 | test_routers_network_ops.py
   test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 539.81 | test_vpc_redundant.py
   test_02_redundant_VPC_default_routes | `Failure` | 373.90 | test_vpc_redundant.py
   test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 549.32 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Failure` | 553.14 | test_vpc_redundant.py
   test_05_rvpc_multi_tiers | `Error` | 553.16 | test_vpc_redundant.py
   test_01_vpc_site2site_vpn_multiple_options | `Failure` | 305.87 | test_vpc_vpn.py
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-796612990


   @harikrishna-patnala In regards to your `Reinstall VM` comment. If test1 user (who owned the template) makes it private, test2 user (who used the template) will lose access to the template and he/she can't see that template in the `Reinstall VM` list.
   
   As a result, with or without these changes, the test2 user might lose access to the template. It is an inherent risk in using community templates :D
   
   ### Reinstall VM **before** making Debian template private
   <kbd> <img width="519" alt="Screenshot 2021-03-11 at 10 47 24" src="https://user-images.githubusercontent.com/9499410/110768888-ea52e980-8257-11eb-9cec-82496189d22b.png"> </kbd>
   
   ### Reinstall VM **after** making Debian template private
   <kbd><img width="519" alt="Screenshot 2021-03-11 at 10 43 05" src="https://user-images.githubusercontent.com/9499410/110767944-0e61fb00-8257-11eb-930c-6f10cbeee3da.png">
   </kbd>
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-793699323


   @blueorangutan package


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1044593281


   @sureshanaparti I've updated the pr. It is ready for testing. I will update the integration test after we are done with the manual testing.
   Thanks in advance :)


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1032224680


   > @sureshanaparti It still has an access issue. I'm going to work on it in the current week and the week after. I will update you when the pr is ready for a test.
   
   ok @soreana , thanks for the update.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1068028969


   np @soreana 
   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1077844255


   @nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] weizhouapache commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1085517146


   
   @blueorangutan 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-799156990


   Thanks @soreana for testing further. If you can add scope to domain that will be great. 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-793700516


   @harikrishna-patnala a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] soreana edited a comment on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana edited a comment on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-796612990


   @harikrishna-patnala In regards to your `Reinstall VM` comment. If test1 user (who owned the template) makes it private, test2 user (who used the template) will lose access to the template and he/she can't see that template in the `Reinstall VM` list.
   
   As a result, with or without these changes, the test2 user might lose access to the template. It is an inherent risk in using community templates. :D
   
   ### Reinstall VM **before** making Debian template private
   <kbd> <img width="519" alt="Screenshot 2021-03-11 at 10 47 24" src="https://user-images.githubusercontent.com/9499410/110768888-ea52e980-8257-11eb-9cec-82496189d22b.png"> </kbd>
   
   ### Reinstall VM **after** making Debian template private
   <kbd><img width="519" alt="Screenshot 2021-03-11 at 10 43 05" src="https://user-images.githubusercontent.com/9499410/110767944-0e61fb00-8257-11eb-930c-6f10cbeee3da.png">
   </kbd>
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-929087741


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1421


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-995992184


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-926332839


   <b>Trillian Build Failed (tid-2189)<b/>


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-935671961


   > Thanks for the effort @soreana, @sureshanaparti is this PR ready?
   
   not yet @nvazquez 


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] nvazquez commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-938551104


   @soreana @rhtyd @weizhouapache @sureshanaparti thanks for the effort to try getting this in time for 4.16. Unfortunately, the PR is still not ready and will need to move it to the next milestone


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r724981785



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,627 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import (listZones,
+                                  deleteTemplate,
+                                  listConfigurations,
+                                  updateConfiguration)
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+        cls._cleanup.append(cls.domain1)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+        cls._cleanup.append(cls.account1)
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+        cls._cleanup.append(cls.sub_domain)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+        cls._cleanup.append(cls.sub_account)
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+        cls._cleanup.append(cls.domain2)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+        cls._cleanup.append(cls.account2)
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.root_template.download(cls.apiclient)
+            cls._cleanup.append(cls.root_template)
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.template1.download(cls.apiclient)
+            cls._cleanup.append(cls.template1)
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.sub_template.download(cls.apiclient)
+            cls._cleanup.append(cls.sub_template)
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.template2.download(cls.apiclient)
+            cls._cleanup.append(cls.template2)
+        else:
+            return
+
+    @classmethod
+    def tearDownClass(cls):
+        super(TestTemplateAccessAcrossDomains, cls).tearDownClass()
+
+    def setUp(self):
+        self.apiclient = self.testClient.getApiClient()
+        self.domain1_config = self.get_restrict_template_configuration(self.domain1.id)
+        self.domain2_config = self.get_restrict_template_configuration(self.domain2.id)
+        self.sub_domain_config = self.get_restrict_template_configuration(self.sub_domain.id)
+        self.cleanup = []
+        return
+
+    def tearDown(self):
+        try:
+            self.update_restrict_template_configuration(self.domain1.id, self.domain1_config)
+            self.update_restrict_template_configuration(self.domain2.id, self.domain2_config)
+            self.update_restrict_template_configuration(self.sub_domain.id, self.sub_domain_config)
+            cleanup_resources(self.apiclient, self.cleanup)

Review comment:
       better do 
   ```suggestion
               super(TestTemplateAccessAcrossDomains, self).tearDown()
   ```
   it does the same but reverses the cleanup list and does some extra checks for virrtual machines for instance.




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#discussion_r668557580



##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name

Review comment:
       ```suggestion
               cls.sub_template = Template.register(cls.apiclient,
                                                    cls.services["test_templates"]["kvm"],
                                                    zoneid=cls.zone.id,
                                                    account=cls.sub_account.name,
                                                    domainid=cls.sub_domain.id,
                                                    hypervisor=cls.hypervisor.lower())
               cls._cleanup.append(cls.template2)
               cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+
+            cls._cleanup.append(cls.root_template)
+            cls._cleanup.append(cls.template1)
+            cls._cleanup.append(cls.template2)
+            cls._cleanup.append(cls.sub_template)
+        else:
+            return
+
+        cls._cleanup.append(cls.account1)
+        cls._cleanup.append(cls.account2)
+        cls._cleanup.append(cls.sub_account)
+        cls._cleanup.append(cls.sub_domain)
+        cls._cleanup.append(cls.domain1)
+        cls._cleanup.append(cls.domain2)
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.apiclient = super(
+                TestTemplateAccessAcrossDomains,
+                cls).getClsTestClient().getApiClient()
+            # Cleanup resources used
+            cleanup_resources(cls.apiclient, cls._cleanup)
+
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+        return
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_01_check_cross_domain_template_access(self):
+        """
+        Verify that templates belonging to one domain should not be accessible
+        by other domains except for parent and ROOT domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of domain2 should not be accessible by domain1
+        3. Make sure template of domain1 should not be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        self.validate_uploaded_template(self.apiclient, self.template1.id)
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        self.validate_uploaded_template(self.apiclient, self.template2.id)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Make sure root domain can still access all subdomain templates
+        # Step 4
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_02_create_template(self):
+        """
+        Verify that templates belonging to one domain can be accessible
+        by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false (default behavior)
+        2. Make sure template of domain2 can be accessible by domain1
+        3. Make sure template of domain1 can be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        5. Deploy virtual machine in domain1 using template from domain2
+        6. Make sure that virtual machine can be deployed and is in running state
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("false")
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, True)
+
+        # Step 4
+        # Make sure root domain can still access all subdomain templates
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        # Deploy new virtual machine using template
+        virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.services["virtual_machine"],
+            templateid=self.template2.id,
+            accountid=self.account1.name,
+            domainid=self.account1.domainid,
+            serviceofferingid=self.service_offering.id,
+        )
+        self.debug("creating an instance with template ID: %s" % self.template2.id)
+        vm_response = VirtualMachine.list(self.apiclient,
+                                          id=virtual_machine.id,
+                                          account=self.account1.name,
+                                          domainid=self.account1.domainid)
+        self.assertEqual(
+            isinstance(vm_response, list),
+            True,
+            "Check for list VMs response after VM deployment"
+        )
+        # Verify VM response to check whether VM deployment was successful
+        self.assertNotEqual(
+            len(vm_response),
+            0,
+            "Check VMs available in List VMs response"
+        )
+
+        # Step 6
+        vm = vm_response[0]
+        self.assertEqual(
+            vm.state,
+            'Running',
+            "Check the state of VM created from Template"
+        )
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_03_check_subdomain_template_access(self):
+        """
+        Verify that templates belonging to parent domain can be accessible
+        by sub domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of ROOT domain can be accessible by domain1
+        3. Make sure template of ROOT domain can be accessible by domain2
+        """
+
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.root_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_04_check_non_public_template_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template2.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain2)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.template2.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template2.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_05_check_non_public_template_subdomain_access(self):
+        """
+        Verify that non public templates belonging to ROOT domain
+        should not be accessible by sub domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+        self.update_configuration("true")
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain)
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        for template_response in list_template_response:
+            if template_response.id == self.root_template.id:
+                break
+
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.root_template.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain, False)
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain, False)
+
+        self.update_configuration("false")
+        self.validate_template_ownership(self.root_template, self.domain1, self.domain2, False)
+        self.validate_template_ownership(self.root_template, self.domain2, self.domain2, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_06_check_sub_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        self.root_template.updatePermissions(self.apiclient,
+                                             ispublic="True")
+        # Step 1
+        self.update_configuration("true")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, False)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_07_check_default_public_template_sub_domain_access(self):
+        """
+        Verify that non root admin sub-domains can access parents templates by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false
+        2. Make sure that sub-domain account can access root templates
+        3. Make sure that sub-domain account can access parent templates
+        4. Make sure that ROOT domain can access the sub-domain template
+        5. Make sure that sibling domain cannot access templates of sub-domain
+        """
+
+        # Step 1
+        self.update_configuration("false")
+        # Make sure child domains can still access parent domain templates
+        self.validate_uploaded_template(self.apiclient, self.sub_template.id)
+
+        # Step 2
+        self.validate_template_ownership(self.root_template, self.sub_domain, self.domain, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, True)
+
+        # Step 4
+        self.validate_template_ownership(self.sub_template, self.domain, self.sub_domain, True)
+
+        # Step 5
+        self.validate_template_ownership(self.sub_template, self.domain2, self.sub_domain, True)
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_08_check_non_public_template_sub_domain_access(self):
+        """
+        Verify that non public templates belonging to one domain
+        should not be accessible by other domains by default except ROOT domain
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Change the permission level of "ispublic" of template1 to false
+        3. Make sure other domains should not be able to access the template
+        4. Make sure that ONLY ROOT domain can access the non public template
+        5. Set global setting restrict.public.access.to.templates to false
+        6. Repeat the steps 3 and 4
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        # Step 2
+        self.template1.updatePermissions(self.apiclient,
+                                         ispublic="False")
+
+        list_template_response = self.list_templates('all', self.domain1)
+        for template_response in list_template_response:
+            if template_response.id == self.template1.id:
+                break
+
+        self.assertEqual(
+            isinstance(list_template_response, list),
+            True,
+            "Check list response returns a valid list"
+        )
+        self.assertIsNotNone(
+            template_response,
+            "Check template %s failed" % self.template1.id
+        )
+        self.assertEqual(
+            template_response.ispublic,
+            int(False),
+            "Check ispublic permission of template"
+        )
+
+        # Step 3
+        # Other domains should not access non public template
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Even child domain should not access non public template
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+
+        # Step 4
+        # Only ROOT domain can access non public templates of child domain
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+        # Step 5
+        self.update_configuration("false")
+
+        # Step 6
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.sub_domain, self.domain1, False)
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+
+    def validate_uploaded_template(self, apiclient, template_id, retries=70, interval=5):
+        """Check if template download will finish in 1 minute"""
+        while retries > -1:
+            time.sleep(interval)
+            template_response = Template.list(
+                apiclient,
+                id=template_id,
+                zoneid=self.zone.id,
+                templatefilter='self'
+            )
+
+            if isinstance(template_response, list):
+                template = template_response[0]
+                if not hasattr(template, 'status') or not template or not template.status:
+                    retries = retries - 1
+                    continue
+                if 'Failed' in template.status:
+                    raise Exception(
+                        "Failed to download template: status - %s" %
+                        template.status)
+
+                elif template.status == 'Download Complete' and template.isready:
+                    return
+
+                elif 'Downloaded' in template.status:
+                    retries = retries - 1
+                    continue
+
+                elif 'Installing' not in template.status:
+                    if retries >= 0:
+                        retries = retries - 1
+                        continue
+                    raise Exception(
+                        "Error in downloading template: status - %s" %
+                        template.status)
+
+            else:
+                retries = retries - 1
+        raise Exception("Template download failed exception.")
+
+    def list_templates(self, templatefilter, domain):
+        return Template.list(
+                    self.apiclient,
+                    templatefilter=templatefilter,
+                    zoneid=self.zone.id,
+                    domainid=domain.id)
+
+    def validate_template_ownership(self, template, owner, nonowner, include_cross_domain_template):
+        """List the template belonging to domain which created it
+           Make sure that other domain can't access it.
+        """
+        list_template_response = self.list_templates('all', owner)
+        if list_template_response is not None:
+            """If global setting is false then public templates of any domain should
+               be accessible by any other domain
+            """
+            if include_cross_domain_template:
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        return
+
+                raise Exception("Template %s belonging to domain %s should "
+                                "be accessible by domain %s"
+                                % (template.name, nonowner.name, owner.name))
+            else:
+                """If global setting is true then public templates of any domain should not
+                   be accessible by any other domain except for root domain
+                """
+                for temp in list_template_response:
+                    if template.name == temp.name:
+                        raise Exception("Template %s belonging to domain %s should "
+                                        "not be accessible by domain %s"
+                                        % (template.name, nonowner.name, owner.name))
+
+    def update_configuration(self, value):
+        """
+        Function to update the global setting "restrict.public.access.to.templates"
+        :param value:
+        :return:
+        """
+        update_configuration_cmd = updateConfiguration.updateConfigurationCmd()
+        update_configuration_cmd.name = "restrict.public.template.access.to.domain"
+        update_configuration_cmd.value = value
+        return self.apiclient.updateConfiguration(update_configuration_cmd)

Review comment:
       We'll need to record the values before the suite (or each test) and reset it afterwards to the original value as well. (a nit and not implemented in every tests as neat as i would like, but it would enable us to run these tests in production environments.

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())

Review comment:
       ```suggestion
               cls.root_template = Template.register(cls.apiclient,
                                                     cls.services["test_templates"]["kvm"],
                                                     zoneid=cls.zone.id,
                                                     domainid=cls.domain.id,
                                                     hypervisor=cls.hypervisor.lower())
               cls._cleanup.append(cls.root_template)
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )

Review comment:
       ```suggestion
           cls.sub_account = Account.create(
               cls.apiclient,
               cls.services["acl"]["accountD11"],
               domainid=cls.sub_domain.id
           )
           cls._cleanup.append(cls.sub_account)
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)

Review comment:
       ```suggestion
           cls.sub_domain = Domain.create(
               cls.apiclient,
               services=cls.services["acl"]["domain11"],
               parentdomainid=cls.domain1.id)
           cls._cleanup.append(cls.sub_domain)
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)

Review comment:
       ```suggestion
           cls.domain1 = Domain.create(
               cls.apiclient,
               services=cls.services["acl"]["domain1"],
               parentdomainid=cls.domain.id)
           cls._cleanup.append(cls.domain1)
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+
+            cls._cleanup.append(cls.root_template)
+            cls._cleanup.append(cls.template1)
+            cls._cleanup.append(cls.template2)
+            cls._cleanup.append(cls.sub_template)
+        else:
+            return
+
+        cls._cleanup.append(cls.account1)
+        cls._cleanup.append(cls.account2)
+        cls._cleanup.append(cls.sub_account)
+        cls._cleanup.append(cls.sub_domain)
+        cls._cleanup.append(cls.domain1)
+        cls._cleanup.append(cls.domain2)
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.apiclient = super(
+                TestTemplateAccessAcrossDomains,
+                cls).getClsTestClient().getApiClient()
+            # Cleanup resources used
+            cleanup_resources(cls.apiclient, cls._cleanup)
+
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+        return

Review comment:
       ```suggestion
           super(TestTemplateAccessAcrossDomains, cls).tearDownClass()
   ```
   for this the items to clean up must be added in order of creation, they will be destroyed in reverse order.

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )

Review comment:
       ```suggestion
           cls.account1 = Account.create(
               cls.apiclient,
               cls.services["acl"]["accountD1"],
               domainid=cls.domain1.id
           )
           cls._cleanup.append(cls.account1)
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)

Review comment:
       ```suggestion
           cls.domain2 = Domain.create(
               cls.apiclient,
               services=cls.services["acl"]["domain2"],
               parentdomainid=cls.domain.id)
           cls._cleanup.append(cls.domain2)
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+
+            cls._cleanup.append(cls.root_template)
+            cls._cleanup.append(cls.template1)
+            cls._cleanup.append(cls.template2)

Review comment:
       ```suggestion
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )

Review comment:
       ```suggestion
           cls.account2 = Account.create(
               cls.apiclient,
               cls.services["acl"]["accountD2"],
               domainid=cls.domain2.id
           )
           cls._cleanup.append(cls.account2)
   ```

##########
File path: test/integration/component/test_template_access_across_domains.py
##########
@@ -0,0 +1,592 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# Import Local Modules
+from nose.plugins.attrib import attr
+from marvin.cloudstackTestCase import cloudstackTestCase, unittest
+from marvin.cloudstackAPI import listZones, deleteTemplate, updateConfiguration
+from marvin.lib.utils import (cleanup_resources)
+from marvin.lib.base import (Account,
+                             Domain,
+                             Network,
+                             NetworkOffering,
+                             Template,
+                             ServiceOffering,
+                             VirtualMachine,
+                             Snapshot,
+                             Volume)
+from marvin.lib.common import (get_domain,
+                               get_zone,
+                               get_template,
+                               get_builtin_template_info)
+# Import System modules
+import time
+import logging
+
+class TestTemplateAccessAcrossDomains(cloudstackTestCase):
+    @classmethod
+    def setUpClass(cls):
+        cls.testClient = super(TestTemplateAccessAcrossDomains, cls).getClsTestClient()
+        cls.apiclient = cls.testClient.getApiClient()
+
+        cls.services = cls.testClient.getParsedTestDataConfig()
+        # Get Zone, Domain and templates
+        cls.domain = get_domain(cls.apiclient)
+        cls.zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests())
+        cls.services['mode'] = cls.zone.networktype
+        cls.logger = logging.getLogger("TestRouterResources")
+        cls._cleanup = []
+        cls.unsupportedHypervisor = False
+        cls.hypervisor = cls.testClient.getHypervisorInfo()
+        if cls.hypervisor.lower() in ['lxc']:
+            cls.unsupportedHypervisor = True
+            return
+        cls.services["virtual_machine"]["zoneid"] = cls.zone.id
+
+        # Create new domain1
+        cls.domain1 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain1"],
+            parentdomainid=cls.domain.id)
+
+        # Create account1
+        cls.account1 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD1"],
+            domainid=cls.domain1.id
+        )
+
+        # Create new sub-domain
+        cls.sub_domain = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain11"],
+            parentdomainid=cls.domain1.id)
+
+        # Create account for sub-domain
+        cls.sub_account = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD11"],
+            domainid=cls.sub_domain.id
+        )
+
+        # Create new domain2
+        cls.domain2 = Domain.create(
+            cls.apiclient,
+            services=cls.services["acl"]["domain2"],
+            parentdomainid=cls.domain.id)
+
+        # Create account2
+        cls.account2 = Account.create(
+            cls.apiclient,
+            cls.services["acl"]["accountD2"],
+            domainid=cls.domain2.id
+        )
+
+        cls.service_offering = ServiceOffering.create(
+            cls.apiclient,
+            cls.services["service_offering"]
+        )
+        cls._cleanup.append(cls.service_offering)
+        if cls.hypervisor.lower() in ['kvm']:
+            # register template under ROOT domain
+            cls.root_template = Template.register(cls.apiclient,
+                                                  cls.services["test_templates"]["kvm"],
+                                                  zoneid=cls.zone.id,
+                                                  domainid=cls.domain.id,
+                                                  hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account1.name
+            cls.template1 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account1.name,
+                                              domainid=cls.domain1.id,
+                                              hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.sub_account.name
+            cls.sub_template = Template.register(cls.apiclient,
+                                                 cls.services["test_templates"]["kvm"],
+                                                 zoneid=cls.zone.id,
+                                                 account=cls.sub_account.name,
+                                                 domainid=cls.sub_domain.id,
+                                                 hypervisor=cls.hypervisor.lower())
+            cls.services["test_templates"]["kvm"]["name"] = cls.account2.name
+            cls.template2 = Template.register(cls.apiclient,
+                                              cls.services["test_templates"]["kvm"],
+                                              zoneid=cls.zone.id,
+                                              account=cls.account2.name,
+                                              domainid=cls.domain2.id,
+                                              hypervisor=cls.hypervisor.lower())
+
+            cls._cleanup.append(cls.root_template)
+            cls._cleanup.append(cls.template1)
+            cls._cleanup.append(cls.template2)
+            cls._cleanup.append(cls.sub_template)
+        else:
+            return
+
+        cls._cleanup.append(cls.account1)
+        cls._cleanup.append(cls.account2)
+        cls._cleanup.append(cls.sub_account)
+        cls._cleanup.append(cls.sub_domain)
+        cls._cleanup.append(cls.domain1)
+        cls._cleanup.append(cls.domain2)
+
+    @classmethod
+    def tearDownClass(cls):
+        try:
+            cls.apiclient = super(
+                TestTemplateAccessAcrossDomains,
+                cls).getClsTestClient().getApiClient()
+            # Cleanup resources used
+            cleanup_resources(cls.apiclient, cls._cleanup)
+
+        except Exception as e:
+            raise Exception("Warning: Exception during cleanup : %s" % e)
+
+        return
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_01_check_cross_domain_template_access(self):
+        """
+        Verify that templates belonging to one domain should not be accessible
+        by other domains except for parent and ROOT domains
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to true
+        2. Make sure template of domain2 should not be accessible by domain1
+        3. Make sure template of domain1 should not be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("true")
+
+        self.validate_uploaded_template(self.apiclient, self.template1.id)
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, False)
+
+        self.validate_uploaded_template(self.apiclient, self.template2.id)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, False)
+
+        # Make sure root domain can still access all subdomain templates
+        # Step 4
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+
+    @attr(tags=["advanced", "basic", "sg"], required_hardware="false")
+    def test_02_create_template(self):
+        """
+        Verify that templates belonging to one domain can be accessible
+        by other domains by default
+
+        Steps:
+        1. Set global setting restrict.public.access.to.templates to false (default behavior)
+        2. Make sure template of domain2 can be accessible by domain1
+        3. Make sure template of domain1 can be accessible by domain2
+        4. Make sure parent and ROOT domain can still access above templates
+        5. Deploy virtual machine in domain1 using template from domain2
+        6. Make sure that virtual machine can be deployed and is in running state
+        :return:
+        """
+
+        # Step 1
+        self.update_configuration("false")
+
+        # Step 2
+        self.validate_template_ownership(self.template2, self.domain1, self.domain2, True)
+
+        # Step 3
+        self.validate_template_ownership(self.template1, self.domain2, self.domain1, True)
+
+        # Step 4
+        # Make sure root domain can still access all subdomain templates
+        self.validate_template_ownership(self.template1, self.domain, self.domain1, True)
+        self.validate_template_ownership(self.template2, self.domain, self.domain2, True)
+
+        # Step 5
+        # Deploy new virtual machine using template
+        virtual_machine = VirtualMachine.create(
+            self.apiclient,
+            self.services["virtual_machine"],
+            templateid=self.template2.id,
+            accountid=self.account1.name,
+            domainid=self.account1.domainid,
+            serviceofferingid=self.service_offering.id,
+        )

Review comment:
       we'll need a self.cleanup as well to add this vm to for cleanup after the test is done (see `cloudstackTestCase` for the use of `tearDownClasss()` vs `tearDown()`)




-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934285468


   > ain2's access to see the public template was restricted. It is the same for the (ii) test case.
   
   @soreana As per my understanding, when the Domain2 config "restrict.public.template.access.to.domain" is true, the public templates of Domain2 are restricted to other domains, and other domains cannot see them.


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-934310776


   @sureshanaparti Thanks for the comments and review, I will update my pr. Should I force update it and make it one commit, too?
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] sureshanaparti commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-933142077


   Hi @soreana, When the domain config setting "restrict.public.template.access.to.domain" is set to true, the public templates in that domain shouldn't be listed outside that domain scope. As per my tests, it seems the public templates are listed outside the domain when that domain config is true. Can you please check and confirm. 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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1067014688


   @blueorangutan package
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] soreana commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
soreana commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1077588898


   @blueorangutan package


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1085517681


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cloudstack] blueorangutan commented on pull request #4774: Added configuration and Integration test to restrict public template …

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4774:
URL: https://github.com/apache/cloudstack/pull/4774#issuecomment-1086100190


   <b>Trillian test result (tid-3787)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 31066 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4774-t3787-kvm-centos7.zip
   Smoke tests completed. 92 look OK, 0 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
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@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org