test_accounts.py failure fix - fallback using the camelCase parameter name "domainId" by sureshanaparti · Pull Request #12689 · apache/cloudstack

sureshanaparti · 2026-02-23T13:08:38Z

Description

This PR fixes test_accounts.py failure - fallback using the camelCase parameter name "domainId".

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?

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

… name "domainId" (as earlier)

sureshanaparti · 2026-02-23T13:10:31Z

@blueorangutan package

blueorangutan · 2026-02-23T13:12:04Z

@sureshanaparti 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.

Copilot

Pull request overview

This PR addresses authentication failures caused by clients sending the domain identifier using the camelCase request parameter name (domainId) instead of the legacy lowercase form (domainid) during login flows.

Changes:

Added a fallback lookup for the domainId parameter when domainid is not present in the request parameter map.
Applied the same fallback behavior to both the default login authenticator and the OAuth2 login authenticator.

Reviewed changes

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

File	Description
server/src/main/java/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java	Adds fallback parameter lookup to accept `domainId` when authenticating via the default login API.
plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java	Adds the same fallback for OAuth2 login domain ID parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-23T13:13:31Z

...h2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java

        String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID);
+        if (domainIdArr == null) {
+            // Fallback to support clients using the camelCase parameter name "domainId"
+            domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID);
+        }


The new fallback to accept both "domainid" and "domainId" is duplicated here and in DefaultLoginAPIAuthenticatorCmd. Consider extracting a small shared helper (e.g., in a common auth/util class) to reduce the chance of future auth paths missing the same compatibility handling.

Copilot · 2026-02-23T13:13:31Z

...h2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java

        String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID);
+        if (domainIdArr == null) {
+            // Fallback to support clients using the camelCase parameter name "domainId"
+            domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID);
+        }


This adds behavior to accept the camelCase "domainId" parameter, but there doesn't appear to be a unit test covering this path (the existing tests use ApiConstants.DOMAIN_ID). Please add/extend a test to verify that passing ApiConstants.DOMAIN__ID is accepted and parsed correctly.

Copilot · 2026-02-23T13:13:32Z

server/src/main/java/com/cloud/api/auth/DefaultLoginAPIAuthenticatorCmd.java

+        String[] domainIdArr = (String[])params.get(ApiConstants.DOMAIN_ID);
+        if (domainIdArr == null) {
+            // Fallback to support clients using the camelCase parameter name "domainId"
+            domainIdArr = (String[])params.get(ApiConstants.DOMAIN__ID);
+        }


This same dual-parameter lookup logic is now required in multiple authenticators (e.g., OAuth2 and default login). Consider centralizing the lookup (e.g., a helper that returns the first non-null entry for DOMAIN_ID / DOMAIN__ID) to avoid future inconsistencies.

codecov · 2026-02-23T13:17:02Z

Codecov Report

❌ Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.25%. Comparing base (d054344) to head (de60cd1).
⚠️ Report is 2 commits behind head on 4.20.

Files with missing lines	Patch %	Lines
server/src/main/java/com/cloud/api/ApiServer.java	40.00%	4 Missing and 2 partials ⚠️
...loud/api/auth/DefaultLoginAPIAuthenticatorCmd.java	0.00%	4 Missing ⚠️
...th2/api/command/OauthLoginAPIAuthenticatorCmd.java	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.20   #12689   +/-   ##
=========================================
  Coverage     16.25%   16.25%           
- Complexity    13424    13427    +3     
=========================================
  Files          5662     5662           
  Lines        500149   500165   +16     
  Branches      60735    60740    +5     
=========================================
+ Hits          81309    81326   +17     
+ Misses       409755   409750    -5     
- Partials       9085     9089    +4

Flag	Coverage Δ
uitests	`4.15% <ø> (-0.01%)`	⬇️
unittests	`17.11% <33.33%> (+<0.01%)`	⬆️

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.

DaanHoogland · 2026-02-23T13:22:37Z

@sureshanaparti , can we root out camel case for API/Params completely? this is techdebt byting us.

sureshanaparti · 2026-02-23T13:57:27Z

@sureshanaparti , can we root out camel case for API/Params completely? this is techdebt byting us.

@DaanHoogland it seems camel cases (domainId, clusterId, zoneId, etc) were used in marvin tests, we can address them later in a separate pr?

blueorangutan · 2026-02-23T14:16:43Z

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

sureshanaparti · 2026-02-23T15:08:13Z

@blueorangutan package

blueorangutan · 2026-02-23T15:10:05Z

@sureshanaparti 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.

DaanHoogland · 2026-02-23T15:28:39Z

@sureshanaparti , can we root out camel case for API/Params completely? this is techdebt byting us.

@DaanHoogland it seems camel cases (domainId, clusterId, zoneId, etc) were used in marvin tests, we can address them later in a separate pr?

created an issue for it: #12691

blueorangutan · 2026-02-23T16:13:19Z

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

test_accounts.py failure fix - fallback using the camelCase parameter…

9220ffc

… name "domainId" (as earlier)

sureshanaparti mentioned this pull request Feb 23, 2026

Fix NPE during reset password #12585

Merged

14 tasks

sureshanaparti requested review from DaanHoogland and abh1sar February 23, 2026 13:09

sureshanaparti added this to the 4.20.3 milestone Feb 23, 2026

sureshanaparti requested a review from Copilot February 23, 2026 13:10

Copilot started reviewing on behalf of sureshanaparti February 23, 2026 13:10 View session

Copilot AI reviewed Feb 23, 2026

View reviewed changes

refactor and unit test

de60cd1

boring-cyborg bot added the component:api label Feb 23, 2026

DaanHoogland mentioned this pull request Feb 23, 2026

remove camel case for API/Params. #12691

Open

Comments

Conversation

sureshanaparti commented Feb 23, 2026

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!

sureshanaparti commented Feb 23, 2026

Uh oh!

blueorangutan commented Feb 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Feb 23, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland commented Feb 23, 2026

Uh oh!

sureshanaparti commented Feb 23, 2026

Uh oh!

blueorangutan commented Feb 23, 2026

Uh oh!

sureshanaparti commented Feb 23, 2026

Uh oh!

blueorangutan commented Feb 23, 2026

Uh oh!

DaanHoogland commented Feb 23, 2026

Uh oh!

blueorangutan commented Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Feb 23, 2026 •

edited

Loading