utils: fix extra slash in Redfish default systems url path #10630

rohityadavcloud · 2025-03-28T07:37:50Z

This removes extra slash from the base system url in Redfish oobm client utility.

Fixes #10441

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

This removes extra slash from the base system url in Redfish oobm client utility. Fixes apache#10441 Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

rohityadavcloud · 2025-03-28T07:38:41Z

Requesting @kiranchavala for any quick tips, review & tests. Also cc @Pearl1594 @DaanHoogland @andrijapanicsb @steveroles

rohityadavcloud · 2025-03-28T07:44:58Z

utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java

@@ -265,12 +265,12 @@ private String getRequestPathForCommand(RedfishCmdType cmd, String resourceId) {
            if (StringUtils.isBlank(resourceId)) {
                throw new RedfishException(String.format("Command '%s' requires a valid resource ID '%s'.", cmd, resourceId));
            }
-            return String.format("%s%s", SYSTEMS_URL_PATH, resourceId);


cc @wido @GabrielBrascher Line #263 is where I worry if not having a trailing slash / may break any redfish servers/bmcs ?

so line 263 could read

return SYSTEMS_URL_PATH + "/";

o.r use File.SEPARATOR and/or String.format(). I think we should test it to make sure, but would not be a big issue

No that's not what I meant - if for the basic discovery of the endpoint, a trailing slash isn't passed some systems could fail (hypothetically, I'm just asking). I couldn't return with the / as it breaks the base case per Kiran's findings (if I understood them right)

ok, so do you mean we may have to spilt this case out into different ones?

TBH, I don't know. Usually, I don't think we should require a trailing / (most servers should handle this case).

rohityadavcloud · 2025-03-28T07:45:26Z

@blueorangutan package

blueorangutan · 2025-03-28T07:46:03Z

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

codecov · 2025-03-28T07:46:50Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 16.01%. Comparing base (5f93ce7) to head (dda8a5f).

Additional details and impacted files

@@            Coverage Diff            @@
##               4.20   #10630   +/-   ##
=========================================
  Coverage     16.01%   16.01%           
+ Complexity    13114    13113    -1     
=========================================
  Files          5651     5651           
  Lines        495858   495858           
  Branches      60048    60048           
=========================================
  Hits          79407    79407           
+ Misses       407590   407589    -1     
- Partials       8861     8862    +1

Flag	Coverage Δ
uitests	`4.00% <ø> (ø)`
unittests	`16.85% <100.00%> (ø)`

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

clgtm

Copilot

Pull Request Overview

This PR fixes an issue with an extra slash in the Redfish default systems URL path used by the OOBM client utility.

Removed the trailing slash from the base systems URL.
Updated the URL composition in both the main client code and its corresponding tests.

Reviewed Changes

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

File	Description
utils/src/test/java/org/apache/cloudstack/utils/redfish/RedfishClientTest.java	Updated test constants and expected URL strings to reflect the removal of the trailing slash.
utils/src/main/java/org/apache/cloudstack/utils/redfish/RedfishClient.java	Adjusted the URL constants and string concatenation to correctly format the request URL without the extra slash.

blueorangutan · 2025-03-28T09:00:13Z

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

rohityadavcloud · 2025-03-28T09:49:57Z

@blueorangutan test

blueorangutan · 2025-03-28T09:52:04Z

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

blueorangutan · 2025-03-28T10:17:55Z

[SF] Trillian Build Failed (tid-12841)

kiranchavala

@rohityadavcloud

getting the following exception

2025-03-28 10:48:38,052 ERROR [c.c.a.ApiAsyncJobDispatcher] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Unexpected exception while executing org.apache.cloudstack.api.command.admin.outofbandmanagement.IssueOutOfBandManagementPowerActionCmd java.lang.IllegalStateException: This is not a JSON Object.


[root@ref-trl-8237-k-Mol8-kiran-chavala-mgmt1 ~]# cat /var/log/cloudstack/management/management-server.log  |grep -i "logid:75fbfd7a"
2025-03-28 10:48:38,016 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl$5] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Executing AsyncJob {"accountId":2,"cmd":"org.apache.cloudstack.api.command.admin.outofbandmanagement.IssueOutOfBandManagementPowerActionCmd","cmdInfo":"{\"response\":\"json\",\"ctxUserId\":\"2\",\"sessionkey\":\"ACPGChmkdLjBVz9y7ogyTXjzkkU\",\"action\":\"OFF\",\"hostid\":\"f4fe4f90-1e3b-4c5f-974e-e9d48e5e4c52\",\"httpmethod\":\"GET\",\"ctxStartEventId\":\"177\",\"ctxDetails\":\"{\\\"interface com.cloud.host.Host\\\":\\\"f4fe4f90-1e3b-4c5f-974e-e9d48e5e4c52\\\"}\",\"ctxAccountId\":\"2\",\"cmdEventType\":\"HOST.OOBM.ACTION\"}","cmdVersion":0,"completeMsid":null,"created":null,"id":40,"initMsid":32989224370467,"instanceId":1,"instanceType":"Host","lastPolled":null,"lastUpdated":null,"processStatus":0,"removed":null,"result":null,"resultCode":0,"status":"IN_PROGRESS","userId":2,"uuid":"75fbfd7a-84ee-484e-8744-121d82949182"}
2025-03-28 10:48:38,041 WARN  [o.a.c.m.w.WebhookServiceImpl] (API-Job-Executor-32:[ctx-1ed229cb, job-40, ctx-291cbd2e]) (logid:75fbfd7a) Skipping delivering event Event {"description":"{\"details\":\"Host Id: 1 Action: OFF\",\"event\":\"HOST.OOBM.ACTION\",\"status\":\"Completed\"}","eventId":null,"eventType":"HOST.OOBM.ACTION","eventUuid":null,"resourceType":"Host","resourceUUID":null} to any webhook as account ID is missing
2025-03-28 10:48:38,042 WARN  [o.a.c.f.e.EventDistributorImpl] (API-Job-Executor-32:[ctx-1ed229cb, job-40, ctx-291cbd2e]) (logid:75fbfd7a) Failed to publish event [category: ActionEvent, type: HOST.OOBM.ACTION] on bus webhookEventBus
2025-03-28 10:48:38,052 ERROR [c.c.a.ApiAsyncJobDispatcher] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Unexpected exception while executing org.apache.cloudstack.api.command.admin.outofbandmanagement.IssueOutOfBandManagementPowerActionCmd java.lang.IllegalStateException: This is not a JSON Object.
2025-03-28 10:48:38,053 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Complete async job-40, jobStatus: FAILED, resultCode: 530, result: org.apache.cloudstack.api.response.ExceptionResponse/null/{"uuidList":[],"errorcode":"530","errortext":"This is not a JSON Object."}
2025-03-28 10:48:38,054 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Publish async job-40 complete on message bus
2025-03-28 10:48:38,054 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Wake up jobs related to job-40
2025-03-28 10:48:38,054 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Update db status for job-40
2025-03-28 10:48:38,055 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Wake up jobs joined with job-40 and disjoin all subjobs created from job- 40
2025-03-28 10:48:38,062 DEBUG [c.c.a.ApiServer] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Retrieved cmdEventType from job info: HOST.OOBM.ACTION
2025-03-28 10:48:38,064 DEBUG [o.a.c.f.j.i.AsyncJobManagerImpl$5] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Done executing org.apache.cloudstack.api.command.admin.outofbandmanagement.IssueOutOfBandManagementPowerActionCmd for job-40
2025-03-28 10:48:38,064 INFO  [o.a.c.f.j.i.AsyncJobMonitor] (API-Job-Executor-32:[ctx-1ed229cb, job-40]) (logid:75fbfd7a) Remove job-40 from job monitoring

wido

CLGTM

Changes look good, thanks!

utils: fix extra slash in Redfish default systems url path

dda8a5f

This removes extra slash from the base system url in Redfish oobm client utility. Fixes apache#10441 Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

rohityadavcloud added this to the 4.20.1 milestone Mar 28, 2025

rohityadavcloud requested review from kiranchavala, DaanHoogland and Pearl1594 March 28, 2025 07:37

rohityadavcloud requested review from wido and GabrielBrascher March 28, 2025 07:40

rohityadavcloud added the type:bug label Mar 28, 2025

rohityadavcloud linked an issue Mar 28, 2025 that may be closed by this pull request

Redfish OOBM integration is broken in cloudstack #10441

Open

rohityadavcloud commented Mar 28, 2025

View reviewed changes

DaanHoogland approved these changes Mar 28, 2025

View reviewed changes

DaanHoogland added Severity:Major priority: high labels Mar 28, 2025

vishesh92 requested a review from Copilot March 28, 2025 08:30

Copilot AI reviewed Mar 28, 2025

View reviewed changes

kiranchavala reviewed Mar 28, 2025

View reviewed changes

rohityadavcloud marked this pull request as draft March 29, 2025 06:01

wido approved these changes Apr 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

utils: fix extra slash in Redfish default systems url path #10630

utils: fix extra slash in Redfish default systems url path #10630

rohityadavcloud commented Mar 28, 2025

rohityadavcloud commented Mar 28, 2025

rohityadavcloud Mar 28, 2025

DaanHoogland Mar 28, 2025

rohityadavcloud Mar 28, 2025

DaanHoogland Mar 28, 2025

rohityadavcloud Mar 28, 2025

rohityadavcloud commented Mar 28, 2025

blueorangutan commented Mar 28, 2025

codecov bot commented Mar 28, 2025 •

edited

Loading

DaanHoogland left a comment

Copilot AI left a comment

blueorangutan commented Mar 28, 2025

rohityadavcloud commented Mar 28, 2025

blueorangutan commented Mar 28, 2025

blueorangutan commented Mar 28, 2025

kiranchavala left a comment

wido left a comment

utils: fix extra slash in Redfish default systems url path #10630

Are you sure you want to change the base?

utils: fix extra slash in Redfish default systems url path #10630

Conversation

rohityadavcloud commented Mar 28, 2025

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

rohityadavcloud commented Mar 28, 2025

rohityadavcloud Mar 28, 2025

Choose a reason for hiding this comment

DaanHoogland Mar 28, 2025

Choose a reason for hiding this comment

rohityadavcloud Mar 28, 2025

Choose a reason for hiding this comment

DaanHoogland Mar 28, 2025

Choose a reason for hiding this comment

rohityadavcloud Mar 28, 2025

Choose a reason for hiding this comment

rohityadavcloud commented Mar 28, 2025

blueorangutan commented Mar 28, 2025

codecov bot commented Mar 28, 2025 • edited Loading

Codecov Report

DaanHoogland left a comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

blueorangutan commented Mar 28, 2025

rohityadavcloud commented Mar 28, 2025

blueorangutan commented Mar 28, 2025

blueorangutan commented Mar 28, 2025

kiranchavala left a comment

Choose a reason for hiding this comment

wido left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 28, 2025 •

edited

Loading