Skip to content

server: return the scoped value from a scoped updateConfiguration - #14641

Open
bhouse-nexthop wants to merge 1 commit into
apache:mainfrom
bhouse-nexthop:fix-updateconfiguration-scoped-response
Open

bhouse-nexthop wants to merge 1 commit into
apache:mainfrom
bhouse-nexthop:fix-updateconfiguration-scoped-response

Conversation

@bhouse-nexthop

Copy link
Copy Markdown
Collaborator

Description

updateConfiguration with a scope id returned the global value of the setting in its response, not the value that was just set. The scoped value itself was stored correctly.

ConfigurationManagerImpl.updateConfiguration(UpdateCfgCmd) returns _configDao.findByName(name), the global configuration row. Since #11770, UpdateCfgCmd.setResponseValue() takes the response value from that object rather than from the request, so a scoped update answered with the global value.

When a scope is given, the returned object now carries the value set in that scope. That value is the one the inner updateConfiguration(...) returns, after normalization. The object is only used to build the response and is not persisted. Updates without a scope are unchanged.

This only affects main. 4.22 still echoes the value from the request.

Fixes #14638

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

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

Unit tests in ConfigurationManagerImplTest:

  • updateConfigurationWithScopeReturnsValueSetInScope: a cluster-scoped update of drs.algorithm to condensed, with a global value of balanced, returns condensed. It fails without the fix (expected:<condensed> but was:<balanced>).
  • updateConfigurationWithoutScopeReturnsConfigurationRowUnchanged: a global update returns the configuration row as it was, and doesn't change its value.

ConfigurationManagerImplTest, ConfigurationManagerTest and both UpdateCfgCmdTest classes pass.

The bug was found on a lab build of main. A cluster-scoped drs.algorithm update answered with the global value, while listConfigurations with the same clusterid showed the new one. The fix hasn't been run in the lab yet.

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

  • Encrypted (Hidden or Secure) settings: ConfigurationVO.setValue() encrypts and getValue() decrypts, and UpdateCfgCmd encrypts the value again for the response, as before.
  • Boolean settings: the returned value is the lowercased one that was stored.
  • Implicit scope for a domain admin or a user calling without a scope id: they now get the value they set in their domain or account, not the global one.
  • The configuration DAO has no entity cache, so changing the value on the returned object can't leak into other lookups.

updateConfiguration returned the global configuration row, and since
the response value is now taken from that row, a zone, cluster,
storage pool, account, domain, image store or management server
scoped update answered with the global value, not the value just set.

When a scope is given, report the value set in that scope.

Signed-off-by: Brad House <[email protected]>
@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 20.78%. Comparing base (a95eb59) to head (cd54513).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../cloud/configuration/ConfigurationManagerImpl.java 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #14641   +/-   ##
=========================================
  Coverage     20.78%   20.78%           
- Complexity    21927    21936    +9     
=========================================
  Files          6496     6496           
  Lines        591657   591660    +3     
  Branches      72592    72593    +1     
=========================================
+ Hits         122955   123004   +49     
+ Misses       455356   455297   -59     
- Partials      13346    13359   +13     
Flag Coverage Δ
uitests 3.88% <ø> (ø)
unittests 22.10% <75.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

updateConfiguration with a scope id returns the global value in its response

1 participant