Introducing concept of domain VPCs by GaOrtiga · Pull Request #7153 · apache/cloudstack

GaOrtiga · 2023-01-31T17:54:18Z

In ACS the creation of tiers of a VPC is restricted to the same account that owns the VPC; therefore, each account needs to have its own VPC and it is not possible to group tiers owned by different accounts in the same VPC. These tiers cannot share the same VR that is used to implement the VPC, for instance.

However, in private cloud scenarios to reduce the number of VRs, it might be interesting to have VPCs, where its tiers are owned by different accounts; thus, they (the accounts) share the same VR/VPC, but each one has their own broadcast domain and features implemented by the VPC, such as DHCP, NAT, and so on.

To address this situation, the concept of Domain VPCs has been created (only available on the API so far), where a VPC can be managed by a domain and its tiers can be created to accounts inside the domain. In the createNetwork API it will be possible to create networks (tiers) to a VPC from a different account; however, the target account must be accessible for the account that owns the VPC. The tiers will be isolated from the broadcast domain and will consume the same VR, in accordance with the current behavior. Also, if a VPN is setup in the VPC, the user will have access to all networks, in accordance with the current behavior.

Documentation: apache/cloudstack-documentation#410

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
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
Minor

Bug Severity

How Has This Been Tested?

I created VPCs with Root Admin, Domain Admin, and user accounts and tried creating tiers in these VPCs for the other accounts (should only be able to create if the VPC account has access to the account that owns the network).

#	VPC owner account	Network Owner account	Same Domain	Could Create	Expected Result
1	Root Admin	Any	Any	Y	Y
2	Domain Admin	Any	N	N	Y
3	Domain Admin	Any	Y	Y	Y
4	User	Same User	Y	Y	Y
5	User	Any Other	Any	N	Y

I also ran some basic tests like deploying VMs in the created networks and checking that the networks were working properly.

codecov · 2023-01-31T18:53:09Z

Codecov Report

Merging #7153 (32e342d) into main (5975ea8) will increase coverage by 0.13%.
Report is 53 commits behind head on main.
The diff coverage is 14.28%.

@@ Coverage Diff @@## main #7153 +/- ##
============================================
+ Coverage 13.37% 13.50% +0.13% - Complexity 9320 9731 +411 
============================================
Files 2740 2747 +7 Lines 258249 266509 +8260 Branches 40235 43764 +3529 ============================================
+ Hits 34537 35990 +1453 - Misses 219354 225914 +6560 - Partials 4358 4605 +247

Files Changed	Coverage Δ
.../src/main/java/com/cloud/vm/dao/UserVmDaoImpl.java	`0.74% <0.00%> (-0.02%)`	⬇️
...src/main/java/com/cloud/api/ApiResponseHelper.java	`3.78% <0.00%> (-0.01%)`	⬇️
...n/java/com/cloud/network/IpAddressManagerImpl.java	`2.90% <0.00%> (ø)`
...loud/network/lb/LoadBalancingRulesManagerImpl.java	`7.87% <0.00%> (ø)`
...ain/java/com/cloud/network/vpc/VpcManagerImpl.java	`9.64% <50.00%> (+0.27%)`	⬆️

... and 64 files with indirect coverage changes

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

DaanHoogland

looks good, I recommend extensive manual testing as un foreseen consequences may come from this concept. The change is simple enough and is great if it turns out it works out of the box.

sonarqubecloud · 2023-02-27T12:27:40Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

6.9% Coverage
0.0% Duplication

github-actions · 2023-04-11T09:09:24Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

sonarqubecloud · 2023-04-19T14:46:27Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
7 Code Smells

6.9% Coverage
0.0% Duplication

github-actions · 2023-06-27T09:02:07Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

kiranchavala

@GaOrtiga

what about the scenario where the network account owner is a user account in the same domain?

Ideally the network should get created since the user is present in the same domain

VPC owner account	Network Owner account	Same Domain	Could Create	Expected Result	Comments
Domain Admin	User (role)	Y	N	the network should get created	🙈 Error: (HTTP 531, error code 4365) Account [{"accountName":"<>r","id":6,"uuid":"9d9daf47-04be-4488-af71-281e255fedfe"}] does not have permission to operate with resource

kiranchavala

@GaOrtiga

Another scenario where sub domains are present

Also, what will happen if an account is present under a sub domains of a domain

Ideally, the network should get created since the subdomain is also falls under the same domain

VPC owner account	Network Owner account	Same Domain	Could Create	Expected Result	Comments
Domain Admin	Account under a subdomain	Y	N	Network tier should get created	🙈 Error: (HTTP 531, error code 4365) Account <>does not have permission to operate within domain id=51427f97-6c38-4a7d-bf1c-8cdc86232cb8

GaOrtiga · 2023-07-17T11:08:54Z

@GaOrtiga
Another scenario where sub domains are present
Also, what will happen if an account is present under a sub domains of a domain
Ideally, the network should get created since the subdomain is also falls under the same domain
VPC owner account Network Owner account Same Domain Could Create Expected Result Comments
Domain Admin Account under a subdomain Y N Network tier should get created see_no_evil Error: (HTTP 531, error code 4365) Account <>does not have permission to operate within domain id=51427f97-6c38-4a7d-bf1c-8cdc86232cb8

hi @kiranchavala thanks for testing.

Which account did you use to run the API commands?

I ran both tests twice, once using the VPC owner and the other using the network owner. While running it with the VPC owner, both creations were successful. When running it with network owner I had the same results as you, however I do believe this is the correct outcome, since these accounts should not be able to access the the domain admin's resources such as the VPC.

kiranchavala · 2023-07-17T11:16:34Z

Hi @GaOrtiga

Thanks for the update

I used the network owner account to execute the api calls

Also Let me do a quick check with the VPC owner account

kiranchavala

@GaOrtiga

I have tested manually the combinations specified with the table with the VPC owner account credentials and they work fine

#	VPC owner account	Network Owner account	Same Domain	Could Create	Expected Result
1	Root Admin	Any	Any	Y	Y
2	Domain Admin	Any	N	N	Y
3	Domain Admin	Any	Y	Y	Y
4	User	Same User	Y	Y	Y
5	User	Any Other	Any	N	Y

kiranchavala

LGTM, tested manually the combinations specified with the table with the VPC owner account credentials and they work fine

#	VPC owner account	Network Owner account	Same Domain	Could Create	Expected Result
1	Root Admin	Any	Any	Y	Y
2	Domain Admin	Any	N	N	Y
3	Domain Admin	Any	Y	Y	Y
4	User	Same User	Y	Y	Y
5	User	Any Other	Any	N	Y

Also tested by launching vm in different tiers belonging to different account and the communication between them was working fine

DaanHoogland · 2023-07-28T08:22:33Z

@blueorangutan package

blueorangutan · 2023-07-28T08:24:04Z

@DaanHoogland a [SF] 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.

blueorangutan · 2023-07-28T09:30:59Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6593

DaanHoogland · 2023-07-28T11:23:01Z

@blueorangutan test

blueorangutan · 2023-07-28T11:24:03Z

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

blueorangutan · 2023-07-28T23:50:36Z

[SF] Trillian test result (tid-7197)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43295 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7153-t7197-kvm-centos7.zip
Smoke tests completed. 112 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_migrate_VM_and_root_volume	`Error`	76.63	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	50.28	test_vm_life_cycle.py

DaanHoogland

clgtm

GaOrtiga · 2024-06-28T17:24:49Z

doc PR:
apache/cloudstack-documentation#410

boring-cyborgBot added component:api component:networking component:UI component:vpc labels Jan 31, 2023

GaOrtiga changed the title ~~Implementation of Domain VPCs~~Draft: Implementation of Domain VPCsJan 31, 2023

GaOrtiga changed the title ~~Draft: Implementation of Domain VPCs~~Implementation of Domain VPCsJan 31, 2023

GaOrtiga marked this pull request as draft January 31, 2023 17:56

GaOrtiga changed the title ~~Implementation of Domain VPCs~~Introducing concept of domain VPCsJan 31, 2023

GaOrtiga marked this pull request as ready for review January 31, 2023 20:45

DaanHoogland reviewed Feb 1, 2023
View reviewed changes

Comment threadserver/src/main/java/com/cloud/api/ApiResponseHelper.java Outdated

GaOrtiga marked this pull request as draft February 7, 2023 18:07

GaOrtigaand others added 3 commits February 20, 2023 10:57

Implement functionality
1306249

add log
8688128

some additional changes
efc4243

GaOrtiga force-pushed the domain_vpcs branch from f3242af to efc4243Compare February 22, 2023 13:48

DaanHoogland reviewed Feb 23, 2023
View reviewed changes

Comment threadserver/src/main/java/com/cloud/network/IpAddressManagerImpl.java Outdated

fix sonar bug
fc28686

github-actionsBot added the status:has-conflicts label Apr 11, 2023

Merge branch 'main' into domain_vpcs
e210f8a

github-actionsBot removed the status:has-conflicts label Apr 19, 2023

DaanHoogland reviewed Apr 28, 2023
View reviewed changes

Comment threadserver/src/main/java/com/cloud/api/ApiResponseHelper.java Outdated
Comment threadserver/src/main/java/com/cloud/network/IpAddressManagerImpl.java Outdated
Comment threadserver/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java Outdated

Extract methods
8b4c343

DaanHoogland added this to the 4.19.0.0 milestone Jun 22, 2023

github-actionsBot added the status:has-conflicts label Jun 27, 2023

Merge branch 'main' into domain_vpcs
c19da4e

DaanHoogland assigned kiranchavala Jul 12, 2023

DaanHoogland added the status:needs-testing label Jul 12, 2023

kiranchavala reviewed Jul 14, 2023
View reviewed changes

apache deleted a comment from blueorangutan Jul 14, 2023

apache deleted a comment from blueorangutan Jul 18, 2023

kiranchavala reviewed Jul 27, 2023
View reviewed changes

apache deleted a comment from blueorangutan Jul 27, 2023

DaanHoogland added the status:ready-for-merge label Jul 27, 2023

kiranchavala approved these changes Jul 28, 2023
View reviewed changes

DaanHoogland closed this Jul 28, 2023

DaanHoogland reopened this Jul 28, 2023

DaanHoogland reviewed Jul 29, 2023
View reviewed changes

DaanHoogland merged commit 0bab0db into apache:mainJul 29, 2023

weizhouapache mentioned this pull request Jun 27, 2024
server: apply rules when VR of Domain VPC is recreated #8354
Merged
11 tasks

GaOrtiga mentioned this pull request Jun 28, 2024
Add section about Domain VPCs apache/cloudstack-documentation#410
Merged

DaanHoogland removed the status:ready-for-merge label Dec 4, 2024

DaanHoogland removed the status:needs-testing label Jan 28, 2026

Uh oh!

Conversation

GaOrtiga commented Jan 31, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

How Has This Been Tested?

Uh oh!

codecovBot commented Jan 31, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sonarqubecloudBot commented Feb 27, 2023

Uh oh!

github-actionsBot commented Apr 11, 2023

Uh oh!

sonarqubecloudBot commented Apr 19, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actionsBot commented Jun 27, 2023

Uh oh!

kiranchavala left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiranchavala left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GaOrtiga commented Jul 17, 2023

Uh oh!

kiranchavala commented Jul 17, 2023

Uh oh!

kiranchavala left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jul 28, 2023

Uh oh!

blueorangutan commented Jul 28, 2023

Uh oh!

blueorangutan commented Jul 28, 2023

Uh oh!

DaanHoogland commented Jul 28, 2023

Uh oh!

blueorangutan commented Jul 28, 2023

Uh oh!

blueorangutan commented Jul 28, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

GaOrtiga commented Jun 28, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

GaOrtiga commented Jan 31, 2023•
edited
Loading

codecovBot commented Jan 31, 2023•
edited
Loading

kiranchavala left a comment •
edited
Loading

kiranchavala left a comment •
edited
Loading

kiranchavala left a comment •
edited
Loading