Skip to content

fix: migrate project settings to SonarCloud correctly (#95)#143

Merged
joshua-quek-sonarsource merged 1 commit into
mainfrom
fix/project-settings-migration-issue-95
May 6, 2026
Merged

fix: migrate project settings to SonarCloud correctly (#95)#143
joshua-quek-sonarsource merged 1 commit into
mainfrom
fix/project-settings-migration-issue-95

Conversation

@joshua-quek-sonarsource

@joshua-quek-sonarsource joshua-quek-sonarsource commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #95 — project configuration settings (e.g., sonar.exclusions, sonar.coverage.exclusions) were silently not being applied during migration from SonarQube Server to SonarQube Cloud.

Three root causes identified and fixed:

  • POST body format + Content-Type — Settings API sent params as URL query string with null body and default Content-Type: application/json. SonarCloud's servlet framework won't parse form-encoded body when Content-Type says JSON. Fixed by sending params.toString() as POST body with explicit Content-Type: application/x-www-form-urlencoded header.
  • Truthy check bugdispatchSettingToApi used if (setting.value) which silently skipped settings with empty string values (""). Fixed to proper null/undefined check.
  • Silent failures — Added per-setting key names in log output and ok/fail/skip summary counters so failures are immediately visible.

Also fixed the same truthy fallback in verify-settings.js comparison logic. All 4 pipeline versions (sq-9.9, sq-10.0, sq-10.4, sq-2025) updated consistently.

Test plan

  • 26/26 unit tests pass (14 new in settings-params.test.js + 1 new + 11 existing in migrators.test.js)
  • Zero regressions vs main
  • Live regression test: 3 project-level settings migrated SQ 2026.2 → SC-staging with exact value match via API (sonar.exclusions 3 values, sonar.coverage.exclusions 2 values, sonar.cpd.exclusions 1 value)
  • Migration log shows: 3 applied, 0 failed, 0 skipped
  • No settings-related warnings in logs

🤖 Generated with Claude Code


Note

Medium Risk
Changes how project settings are written to SonarCloud (request encoding, value dispatch rules, and migration logging), which can affect migration outcomes across all supported pipelines. While scoped to project settings, incorrect encoding could cause settings to fail to apply or apply differently in production.

Overview
Fixes project settings migration to SonarCloud by switching /api/settings/set calls to send a form-encoded POST body with an explicit Content-Type: application/x-www-form-urlencoded header (instead of querystring params + null JSON body) across all pipeline versions.

Updates dispatchSettingToApi and project-settings verification to treat empty-string setting values as valid (null/undefined checks instead of truthy checks), adds skip/apply/fail counters and clearer logs during migration, and introduces targeted unit tests covering parameter building and empty-string dispatch behavior.

Reviewed by Cursor Bugbot for commit 40f3a5b. Bugbot is set up for automated code reviews on this repo. Configure here.

Project configuration settings (sonar.exclusions, sonar.coverage.exclusions,
etc.) were silently not being applied during migration. Three root causes:

1. POST body format — settings API sent params as URL query string with null
   body and Content-Type: application/json. SonarCloud requires form-encoded
   POST body with Content-Type: application/x-www-form-urlencoded. The axios
   client default header prevented auto-detection, so explicit header needed.

2. Truthy check — dispatchSettingToApi used `if (setting.value)` which skipped
   settings with empty string values. Fixed to null/undefined check.

3. Silent failures — no summary logging made diagnosis impossible. Added
   per-setting names and ok/fail/skip summary counters.

Also fixed the same truthy fallback in verify-settings.js comparison logic.

All 4 pipeline versions (sq-9.9, sq-10.0, sq-10.4, sq-2025) updated.
Regression tested: 3 settings migrated SQ 2026.2 → SC-staging, exact match.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonar-review-alpha

sonar-review-alpha Bot commented Apr 29, 2026

Copy link
Copy Markdown

Summary

This PR fixes silent failures in project settings migration by addressing three interconnected issues: the Settings API POST format (query params + JSON Content-Type wasn't being parsed by SonarCloud), a JavaScript truthy check that skipped empty string values, and missing diagnostic logging. All changes are applied consistently across all 4 pipeline versions (sq-9.9, sq-10.0, sq-10.4, sq-2025).

The key insight: SonarQube/SonarCloud web APIs expect form-encoded POST bodies, not query params. The axios default Content-Type: application/json prevented the servlet from parsing the body, so parameters were silently ignored. The truthy bug (if (setting.value)) made the problem worse by skipping even empty string values. Live testing verified all settings now migrate with exact value match.

What reviewers should know

Start here: src/shared/utils/settings-params.js — This is the central fix. dispatchSettingToApi now properly checks for null/undefined instead of truthy, and both the params builder and verification checker have been aligned.

Then: Any one of the 4 setProjectSetting helpers (e.g., src/pipelines/sq-10.0/sonarcloud/api/project-config/helpers/set-project-setting.js) — They all follow the same pattern: changed from query string + null body to form-encoded body + explicit header. Pick one to understand, then verify the others are consistent.

Logging improvements: The migrateProjectSettings functions across all 4 pipelines now log which settings are being processed and provide an applied/failed/skipped summary. This makes failures immediately visible in the log output.

Verification fix: src/shared/verification/checkers/project-config/helpers/verify-settings.js — Uses the same value-picking logic as dispatch to ensure settings with empty strings compare correctly.

Test coverage: test/utils/settings-params.test.js is new and tests both the params builder and dispatcher with specific focus on empty strings (the previously broken case). The existing migrators.test.js gained one test for the empty string dispatch scenario.

⚠️ Watch for: The Content-Type header is critical — form-encoded body must have that header, or SonarCloud's servlet won't parse it. The truthy-check fix is subtle but important; review how it affects each caller of dispatchSettingToApi.


  • Generate Walkthrough
  • Generate Diagram
Walkthrough

Start here: src/shared/utils/settings-params.js

This is the only shared file touched and contains both core fixes — the truthy→null-check correction in dispatchSettingToApi and the new skip-logging path. Understanding this file makes every other change in the PR self-explanatory.


File changes

File What changed
src/shared/utils/settings-params.js if (setting.value)if (setting.value !== undefined && setting.value !== null); added logger.debug on skip path; dispatchSettingToApi now returns false for skipped settings
src/pipelines/*/sonarcloud/api/**/set-project-setting.js ×4 POST body moved from query string to request body; added explicit Content-Type: application/x-www-form-urlencoded header
src/pipelines/*/sonarcloud/migrators/**/migrate-project-settings.js ×4 Early-return on empty array; per-setting key names in opening log line; ok/fail/skip counters with summary log at end
src/shared/verification/checkers/project-config/helpers/verify-settings.js Replaced ||-based value picking with a pick() helper that correctly handles empty strings; fixes mismatches and sqOnly reporting for "" values
test/utils/settings-params.test.js New file — 14 tests covering buildSettingsParams (scalar, multi-value, fieldValues, empty string, null, undefined) and dispatchSettingToApi (each branch + precedence + false returns)
test/sonarcloud/migrators/migrators.test.js One new test: migrateProjectSettings dispatches a setting with value: "" and calls setProjectSetting exactly once

Things worth extra attention

  • The Content-Type fix is load-bearing. Even if the body is correctly form-encoded, omitting the explicit header causes axios to leave its default application/json, and SonarCloud's servlet silently ignores the body. The header and the body change must travel together — verify both are present in all 4 pipeline variants.

  • dispatchSettingToApi precedence: The function falls through valuevaluesfieldValues in order. If a setting somehow carries both value and values, value wins. The test 'dispatchSettingToApi prefers value over values' covers this, but reviewers should confirm that matches SonarCloud's own API precedence expectations.

  • verify-settings.js pick() helper only resolves value and valuesfieldValues is not handled. This was the same scope as before the fix, but worth noting if fieldValues-type settings ever need verification.

  • No shared abstraction across pipelines. The 4 setProjectSetting helpers and 4 migrateProjectSettings helpers are near-identical copies. The changes are consistent here, but any future fix will need to touch all 4 again — worth flagging as tech debt if the team hasn't already tracked it.

Diagram
sequenceDiagram
    participant migrate as migrateProjectSettings
    participant dispatch as dispatchSettingToApi
    participant apiHelper as setProjectSetting
    participant SC as SonarCloud API

    migrate->>dispatch: setting {key, value/values/fieldValues}

    alt BEFORE: params as query string
        apiHelper->>SC: POST /api/settings/set?key=…&value=…<br/>Body: null, Content-Type: application/json
        SC-->>apiHelper: 200 OK (params silently ignored)
    else AFTER: params as form body
        apiHelper->>SC: POST /api/settings/set<br/>Body: key=…&value=…<br/>Content-Type: application/x-www-form-urlencoded
        SC-->>apiHelper: 200 OK (params applied ✓)
    end

    dispatch-->>migrate: true / false
    migrate->>migrate: log "N applied, N failed, N skipped"
Loading
%%{init: {'flowchart': {'curve': 'catmullRom'}}}%%
flowchart TD
    start([dispatchSettingToApi]) --> checkValue{"value !== null\n&& !== undefined?\n(was: if value)"}
    checkValue -- Yes\nincludes empty string --> sendScalar["setProjectSetting\n{ value }"]
    checkValue -- No --> checkValues{"values[]\nnon-empty?"}
    checkValues -- Yes --> sendValues["setProjectSetting\n{ values }"]
    checkValues -- No --> checkFieldValues{"fieldValues[]\nnon-empty?"}
    checkFieldValues -- Yes --> sendFieldValues["setProjectSetting\n{ fieldValues }"]
    checkFieldValues -- No --> skip["log debug skip\nreturn false ❌"]
    sendScalar & sendValues & sendFieldValues --> done["return true ✓"]
Loading

🗣️ Give feedback

@sonar-review-alpha sonar-review-alpha Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The three core fixes (POST body format, truthy check, verify-settings comparison) are all correct and the new tests cover the key cases. One logic duplication worth resolving before merge.

🗣️ Give feedback

@@ -13,13 +13,15 @@ export async function verifyProjectSettings(sqClient, scClient, sqProjectKey, sc
const sqNonInherited = sqSettings.filter(s => !s.inherited);
const scMap = new Map(scSettings.map(s => [s.key, s]));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic duplication: pick reimplements the value-selection logic from dispatchSettingToApi in src/shared/utils/settings-params.js, but with a silent divergence: it omits fieldValues. dispatchSettingToApi handles all three shapes (value → values → fieldValues); pick only handles two.

Consequence: a setting that exists as fieldValues on both sides will have pick() return undefined for both, produce '""' on both sides, and always be reported as matching — even if the SC values are wrong. The only failure mode caught is if the setting is absent from SC entirely.

Extract a shared extractSettingValue(s) helper from settings-params.js and use it here:

// in settings-params.js (exported)
export function extractSettingValue(s) {
  if (s.value !== undefined && s.value !== null) return s.value;
  if (s.values?.length) return s.values;
  if (s.fieldValues?.length) return s.fieldValues;
  return undefined;
}

Then verify-settings.js simply calls extractSettingValue(s) instead of the inline pick.

  • Mark as noise

@joshua-quek-sonarsource
joshua-quek-sonarsource merged commit 9ecf82a into main May 6, 2026
2 checks passed
@joshua-quek-sonarsource
joshua-quek-sonarsource deleted the fix/project-settings-migration-issue-95 branch May 6, 2026 13:58
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.

Some (all?) project configuration settings are not migrated

2 participants