server: consistent domainpath in api responses #11589

shwstppr · 2025-09-06T07:59:01Z

Description

Currently, some APIs return domainpath as 'ROOT/domain1/domain2' while other return it as '/domain1/domain2'. This PR makes the response consistent.

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)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Before:

(localcloud) 🐱 > list networks listall=true filter=id,name,domainpath
{
  "count": 2,
  "network": [
    {
      "domainpath": "/d1/d11/",
      "id": "5f58ee16-9140-4241-a50f-3c079b9717e1",
      "name": "d11user-network"
    },
    {
      "domainpath": "/",
      "id": "67c1bb61-1155-4b49-9288-543abd95e7ce",
      "name": "abc"
    }
  ]
}
(localcloud) 🐱 > list accounts listall=true filter=id,name,domainpath
{
  "account": [
    {
      "domainpath": "ROOT",
      "id": "b1962584-8a72-11f0-8f55-a02942fcdd70",
      "name": "admin"
    },
    {
      "domainpath": "ROOT",
      "id": "0b56a8ae-5b2d-4b15-ae7d-2a7209e25d18",
      "name": "baremetal-system-account"
    },
    {
      "domainpath": "ROOT/d1/d11",
      "id": "0a5340ac-4e5c-4cc2-9c1c-dd00014e36b5",
      "name": "d11user"
    }
  ],
  "count": 3
}

After:

(localcloud) 🐱 > list networks listall=true filter=id,name,domainpath
{
  "count": 2,
  "network": [
    {
      "domainpath": "ROOT/d1/d11",
      "id": "5f58ee16-9140-4241-a50f-3c079b9717e1",
      "name": "d11user-network"
    },
    {
      "domainpath": "ROOT",
      "id": "67c1bb61-1155-4b49-9288-543abd95e7ce",
      "name": "abc"
    }
  ]
}
(localcloud) 🐱 > list accounts listall=true filter=id,name,domainpath
{
  "account": [
    {
      "domainpath": "ROOT",
      "id": "b1962584-8a72-11f0-8f55-a02942fcdd70",
      "name": "admin"
    },
    {
      "domainpath": "ROOT",
      "id": "0b56a8ae-5b2d-4b15-ae7d-2a7209e25d18",
      "name": "baremetal-system-account"
    },
    {
      "domainpath": "ROOT/d1/d11",
      "id": "0a5340ac-4e5c-4cc2-9c1c-dd00014e36b5",
      "name": "d11user"
    }
  ],
  "count": 3
}

How did you try to break this feature and the system with this change?

Currently, some APIs return domainpath as 'ROOT/domain1/domain2' while other return it as '/domain1/domain2'. This PR makes the response consistent. Signed-off-by: Abhishek Kumar <[email protected]>

codecov · 2025-09-06T08:07:26Z

Codecov Report

❌ Patch coverage is 28.94737% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.56%. Comparing base (3ddd802) to head (64fc1f3).
⚠️ Report is 115 commits behind head on main.

Files with missing lines	Patch %	Lines
...src/main/java/com/cloud/api/ApiResponseHelper.java	35.71%	16 Missing and 2 partials ⚠️
...cloudstack/api/command/user/vpn/AddVpnUserCmd.java	0.00%	1 Missing ⚠️
...apache/cloudstack/api/response/UserVmResponse.java	0.00%	0 Missing and 1 partial ⚠️
...bernetes/cluster/KubernetesClusterManagerImpl.java	0.00%	1 Missing ⚠️
...dstack/api/response/KubernetesClusterResponse.java	0.00%	1 Missing ⚠️
...va/com/cloud/api/query/dao/AccountJoinDaoImpl.java	0.00%	1 Missing ⚠️
...a/com/cloud/api/query/dao/AsyncJobJoinDaoImpl.java	0.00%	1 Missing ⚠️
...ava/com/cloud/api/query/dao/DomainJoinDaoImpl.java	0.00%	1 Missing ⚠️
...cloud/api/query/dao/ProjectAccountJoinDaoImpl.java	0.00%	1 Missing ⚠️
...ud/api/query/dao/ProjectInvitationJoinDaoImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #11589      +/-   ##
============================================
+ Coverage     17.36%   17.56%   +0.20%     
- Complexity    15235    15491     +256     
============================================
  Files          5888     5897       +9     
  Lines        525740   528179    +2439     
  Branches      64164    64716     +552     
============================================
+ Hits          91272    92801    +1529     
- Misses       424168   424950     +782     
- Partials      10300    10428     +128

Flag	Coverage Δ
uitests	`3.59% <ø> (-0.04%)`	⬇️
unittests	`18.62% <28.94%> (+0.22%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

shwstppr · 2025-09-06T08:22:15Z

@blueorangutan package

blueorangutan · 2025-09-06T08:24:06Z

@shwstppr a [SL] 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 · 2025-09-06T10:06:25Z

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

DaanHoogland

my preference would be to remove the word ‘ROOT’ but CLGTM (also potaeto/potahto)

server/src/main/java/com/cloud/api/ApiResponseHelper.java

Signed-off-by: Abhishek Kumar <[email protected]>

shwstppr · 2025-10-08T08:12:11Z

@harikrishna-patnala I've made changes to normalize domain path across code now.
We can decide if we want ROOT/<domain_path> or /<domain_path> or <domain_path> @DaanHoogland
It will be easier to change that now.

@blueorangutan package

blueorangutan · 2025-10-08T08:14:04Z

@shwstppr a [SL] 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 · 2025-10-08T10:20:27Z

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

harikrishna-patnala · 2025-10-08T10:40:50Z

@blueorangutan test

blueorangutan · 2025-10-08T10:42:03Z

@harikrishna-patnala a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

Copilot

Pull Request Overview

This PR standardizes the format of domainpath fields in API responses across all endpoints. Previously, some APIs returned domainpath as 'ROOT/domain1/domain2' while others returned it as '/domain1/domain2'. The changes ensure all APIs consistently return the "ROOT/..." format.

Centralized domain path formatting logic in a new utility method getPrettyDomainPath()
Refactored multiple DAO implementations to use common helper methods for owner/domain population
Updated response interfaces to support the consolidated approach

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

File	Description
ApiResponseHelper.java	Added `getPrettyDomainPath()` utility method and consolidated owner/domain population logic
Multiple DAO implementations	Removed duplicated domain path logic and switched to use centralized helper methods
Response classes	Updated interfaces to support unified response handling
AddVpnUserCmd.java	Refactored to use response generator instead of manual response construction

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-08T12:46:49Z

server/src/main/java/com/cloud/api/ApiResponseHelper.java

+            return null;
+        }
+        StringBuilder domainPath = new StringBuilder("ROOT");
+        (domainPath.append(path)).deleteCharAt(domainPath.length() - 1);


The method assumes path always ends with a character that can be deleted, but if path is empty or doesn't end with a trailing character, deleteCharAt(domainPath.length() - 1) will throw a StringIndexOutOfBoundsException.

Suggested change

(domainPath.append(path)).deleteCharAt(domainPath.length() - 1);

if (path != null && !path.isEmpty()) {

domainPath.append(path);

// Remove trailing '/' if present

if (domainPath.length() > 0 && domainPath.charAt(domainPath.length() - 1) == '/') {

domainPath.deleteCharAt(domainPath.length() - 1);

}

}

Copilot · 2025-10-08T12:46:49Z

server/src/main/java/com/cloud/api/ApiResponseHelper.java

+        response.setDomainPath(getPrettyDomainPath(domain.getPath()));
+    }

+    private static void populateDomain(ControlledViewEntityResponse response, long domainId) {


This method is declared as private static but needs to be accessible from other classes. It should be public static to match the usage pattern of other helper methods in this class.

Suggested change

private static void populateDomain(ControlledViewEntityResponse response, long domainId) {

public static void populateDomain(ControlledViewEntityResponse response, long domainId) {

DaanHoogland · 2025-10-08T12:58:35Z

@shwstppr I prefer "/" but am not going to make a big issue about it. what do you think @harikrishna-patnala ?

blueorangutan · 2025-10-09T02:43:30Z

[SF] Trillian test result (tid-14576)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 54029 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11589-t14576-kvm-ol8.zip
Smoke tests completed. 149 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

harikrishna-patnala · 2025-10-09T05:09:38Z

@shwstppr I prefer "/" but am not going to make a big issue about it. what do you think @harikrishna-patnala ?

Both are fine, but ROOT/ seems better to me as it implies directly and we are showing the domain as ROOT in UI as well.

* server: consistent domainpath in api responses Currently, some APIs return domainpath as 'ROOT/domain1/domain2' while other return it as '/domain1/domain2'. This PR makes the response consistent like "ROOT/domain1/domain2" Signed-off-by: Abhishek Kumar <[email protected]> * more changes Signed-off-by: Abhishek Kumar <[email protected]> --------- Signed-off-by: Abhishek Kumar <[email protected]>

server: consistent domainpath in api responses

eaff6f0

Currently, some APIs return domainpath as 'ROOT/domain1/domain2' while other return it as '/domain1/domain2'. This PR makes the response consistent. Signed-off-by: Abhishek Kumar <[email protected]>

DaanHoogland approved these changes Sep 8, 2025

View reviewed changes

harikrishna-patnala added this to the 4.22.0 milestone Sep 10, 2025

harikrishna-patnala added this to Apache CloudStack 4.22.0 Sep 10, 2025

harikrishna-patnala moved this to In Progress in Apache CloudStack 4.22.0 Sep 10, 2025

harikrishna-patnala reviewed Sep 10, 2025

View reviewed changes

server/src/main/java/com/cloud/api/ApiResponseHelper.java Outdated Show resolved Hide resolved

more changes

64fc1f3

Signed-off-by: Abhishek Kumar <[email protected]>

boring-cyborg bot added component:api component:kubernetes labels Oct 8, 2025

DaanHoogland requested a review from Copilot October 8, 2025 12:45

Copilot AI reviewed Oct 8, 2025

View reviewed changes

harikrishna-patnala closed this Oct 9, 2025

github-project-automation bot moved this from In Progress to Done in Apache CloudStack 4.22.0 Oct 9, 2025

harikrishna-patnala reopened this Oct 9, 2025

harikrishna-patnala merged commit a6ef24d into apache:main Oct 9, 2025
51 of 52 checks passed

-        (domainPath.append(path)).deleteCharAt(domainPath.length() - 1);
+        if (path != null && !path.isEmpty()) {
+            domainPath.append(path);
+            // Remove trailing '/' if present
+            if (domainPath.length() > 0 && domainPath.charAt(domainPath.length() - 1) == '/') {
+                domainPath.deleteCharAt(domainPath.length() - 1);
+            }
+        }

	private static void populateDomain(ControlledViewEntityResponse response, long domainId) {
	public static void populateDomain(ControlledViewEntityResponse response, long domainId) {

server: consistent domainpath in api responses #11589

server: consistent domainpath in api responses #11589

Uh oh!

Conversation

shwstppr commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

codecov bot commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shwstppr commented Sep 6, 2025

Uh oh!

blueorangutan commented Sep 6, 2025

Uh oh!

blueorangutan commented Sep 6, 2025

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shwstppr commented Oct 8, 2025

Uh oh!

blueorangutan commented Oct 8, 2025

Uh oh!

blueorangutan commented Oct 8, 2025

Uh oh!

harikrishna-patnala commented Oct 8, 2025

Uh oh!

blueorangutan commented Oct 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Oct 8, 2025

Uh oh!

blueorangutan commented Oct 9, 2025

Uh oh!

harikrishna-patnala commented Oct 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shwstppr commented Sep 6, 2025 •

edited

Loading

codecov bot commented Sep 6, 2025 •

edited

Loading