engine/schema: prepend algorithm to checksum during systemvm template registration #12165

weizhouapache · 2025-11-28T13:49:12Z

Description

This PR fixes #12042

No idea how to reproduce the issue. However, 2 users have faced the same issue which seems to be caused by missing {SHA-512} algorithm of checksum value.

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?

… registration

codecov · 2025-11-28T13:59:16Z

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.59%. Comparing base (b5fd39f) to head (2b8a847).

Files with missing lines	Patch %	Lines
...apache/cloudstack/utils/security/DigestHelper.java	0.00%	12 Missing ⚠️
...om/cloud/upgrade/SystemVmTemplateRegistration.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #12165      +/-   ##
============================================
- Coverage     17.59%   17.59%   -0.01%     
+ Complexity    15601    15596       -5     
============================================
  Files          5910     5910              
  Lines        529780   529792      +12     
  Branches      64729    64731       +2     
============================================
- Hits          93226    93203      -23     
- Misses       426060   426096      +36     
+ Partials      10494    10493       -1

Flag	Coverage Δ
uitests	`3.60% <ø> (ø)`
unittests	`18.66% <0.00%> (-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

clgtm

DaanHoogland · 2025-11-28T14:04:59Z

@blueorangutan package

blueorangutan · 2025-11-28T14:06:04Z

@DaanHoogland 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-11-28T15:31:05Z

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

weizhouapache · 2025-11-28T15:53:46Z

@blueorangutan test keepEnv

blueorangutan · 2025-11-28T15:56:04Z

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

blueorangutan · 2025-11-28T16:25:10Z

[SF] Trillian Build Failed (tid-14887)

weizhouapache · 2025-11-28T17:03:13Z

@blueorangutan test keepEnv

blueorangutan · 2025-11-28T17:04:03Z

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

blueorangutan · 2025-11-29T08:47:35Z

[SF] Trillian test result (tid-14889)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 53254 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr12165-t14889-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

weizhouapache · 2025-12-09T08:51:07Z

@blueorangutan package

blueorangutan · 2025-12-09T08:52:05Z

@weizhouapache 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-12-09T11:42:15Z

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

weizhouapache · 2026-01-15T10:00:38Z

@blueorangutan package

blueorangutan · 2026-01-15T10:02:34Z

@weizhouapache 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 · 2026-01-15T11:05:43Z

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

utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java

…tHelper.java

weizhouapache · 2026-01-15T16:37:43Z

@blueorangutan package

blueorangutan · 2026-01-15T16:40:35Z

@weizhouapache 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 a bug (#12042) where systemVM template registration was failing due to missing algorithm prefixes in checksum values. The fix adds a new utility method to prepend the algorithm identifier (e.g., {SHA-512}) to checksums when the algorithm is missing but can be inferred from the checksum length.

Changes:

Added DigestHelper.prependAlgorithm() method to automatically detect and prepend the hash algorithm based on checksum length
Updated template checksum registration to use the new method, ensuring checksums are stored with algorithm prefixes

Reviewed changes

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

File	Description
utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java	Adds `prependAlgorithm()` utility method to infer and prepend algorithm prefix based on checksum length
engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java	Applies `prependAlgorithm()` when setting checksums during template creation and updates

Comments suppressed due to low confidence (1)

engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java:880

The checksum comparison at line 880 compares the output of DigestHelper.calculateChecksum() (which returns a plain SHA-512 hash without prefix) against templateDetails.getChecksum() from metadata (which may or may not have a prefix). With the changes in this PR, template checksums in the database will have prefixes, but metadata checksums may not. This comparison should use DigestHelper.getHashValueFromChecksumValue() to strip any algorithm prefix before comparing, or ensure both values are in the same format.

        if (!templateChecksum.equals(templateDetails.getChecksum())) {

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

Copilot · 2026-01-15T17:05:34Z

utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java

+    public static String prependAlgorithm(String checksum) {
+        if (StringUtils.isEmpty(checksum)) {
+            return checksum;
+        }
+        int checksumLength = checksum.length();
+        Map<String, Integer> paddingLengths = getChecksumLengthsMap();
+        for (Map.Entry<String, Integer> entry : paddingLengths.entrySet()) {
+            if (entry.getValue().equals(checksumLength)) {
+                String algorithm = entry.getKey();
+                return String.format("{%s}%s", algorithm, checksum);
+            }
+        }
+        return checksum;
+    }


The prependAlgorithm method doesn't check if the algorithm prefix is already present before prepending, which could result in double-prefixing (e.g., {SHA-512}{SHA-512}checksum). Add a check using the existing isAlgorithmPresent() method at the start of the function to return the checksum unchanged if it already has the prefix.

Copilot · 2026-01-15T17:05:35Z

utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java

+        for (Map.Entry<String, Integer> entry : paddingLengths.entrySet()) {
+            if (entry.getValue().equals(checksumLength)) {
+                String algorithm = entry.getKey();
+                return String.format("{%s}%s", algorithm, checksum);
+            }
+        }


When multiple hash algorithms have different lengths but one checksum length could theoretically match multiple entries in the map, the iteration order is non-deterministic since getChecksumLengthsMap() returns a HashMap. While the current set of algorithms have unique lengths, this creates ambiguity. Consider using a LinkedHashMap with algorithms ordered by preference (most secure first), or add a comment documenting that all supported algorithms have unique digest lengths.

Suggested change

for (Map.Entry<String, Integer> entry : paddingLengths.entrySet()) {

if (entry.getValue().equals(checksumLength)) {

String algorithm = entry.getKey();

return String.format("{%s}%s", algorithm, checksum);

}

}

String selectedAlgorithm = null;

// In case multiple algorithms share the same digest length, choose deterministically

for (Map.Entry<String, Integer> entry : paddingLengths.entrySet()) {

if (entry.getValue().equals(checksumLength)) {

String algorithm = entry.getKey();

if (selectedAlgorithm == null || algorithm.compareTo(selectedAlgorithm) < 0) {

selectedAlgorithm = algorithm;

}

}

}

if (selectedAlgorithm != null) {

return String.format("{%s}%s", selectedAlgorithm, checksum);

}

Copilot · 2026-01-15T17:05:35Z

utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java

        }
    }
+
+    public static String prependAlgorithm(String checksum) {


The new prependAlgorithm method lacks test coverage. Add test cases covering: (1) checksum without prefix for each supported algorithm, (2) checksum that already has a prefix, (3) null/empty checksum, (4) checksum with invalid length that doesn't match any algorithm.

blueorangutan · 2026-01-15T17:38:54Z

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

engine/schema: prepend algorithm to checksum during systemvm template…

97424f2

… registration

weizhouapache added this to the 4.22.1 milestone Nov 28, 2025

boring-cyborg bot added the component:security label Nov 28, 2025

weizhouapache linked an issue Nov 28, 2025 that may be closed by this pull request

Cloudstack 4.22 upgrade: SystemVM template update fails #12042

Open

DaanHoogland approved these changes Nov 28, 2025

View reviewed changes

weizhouapache marked this pull request as ready for review December 9, 2025 08:50

rajujith added this to Apache CloudStack 4.22.1 Jan 5, 2026

rajujith moved this to In Progress in Apache CloudStack 4.22.1 Jan 7, 2026

Merge branch '4.22' into 4.22-prepend-sha512-algorithm-to-checksum

4f08312

weizhouapache commented Jan 15, 2026

View reviewed changes

utils/src/main/java/org/apache/cloudstack/utils/security/DigestHelper.java Show resolved Hide resolved

Update utils/src/main/java/org/apache/cloudstack/utils/security/Diges…

2b8a847

…tHelper.java

DaanHoogland requested a review from Copilot January 15, 2026 16:55

Copilot started reviewing on behalf of DaanHoogland January 15, 2026 16:56 View session

Copilot AI reviewed Jan 15, 2026

View reviewed changes

engine/schema: prepend algorithm to checksum during systemvm template registration #12165

Are you sure you want to change the base?

engine/schema: prepend algorithm to checksum during systemvm template registration #12165

Conversation

weizhouapache commented Nov 28, 2025

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 Nov 28, 2025 • 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!

DaanHoogland commented Nov 28, 2025

Uh oh!

blueorangutan commented Nov 28, 2025

Uh oh!

blueorangutan commented Nov 28, 2025

Uh oh!

weizhouapache commented Nov 28, 2025

Uh oh!

blueorangutan commented Nov 28, 2025

Uh oh!

blueorangutan commented Nov 28, 2025

Uh oh!

weizhouapache commented Nov 28, 2025

Uh oh!

blueorangutan commented Nov 28, 2025

Uh oh!

blueorangutan commented Nov 29, 2025

Uh oh!

weizhouapache commented Dec 9, 2025

Uh oh!

blueorangutan commented Dec 9, 2025

Uh oh!

blueorangutan commented Dec 9, 2025

Uh oh!

weizhouapache commented Jan 15, 2026

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

Uh oh!

weizhouapache commented Jan 15, 2026

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Jan 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Nov 28, 2025 •

edited

Loading