From 40f3a5b73e59943a7883efe1d7693d66d14c5617 Mon Sep 17 00:00:00 2001 From: Joshua Quek Date: Wed, 29 Apr 2026 23:12:11 +0800 Subject: [PATCH] fix: migrate project settings to SonarCloud correctly (#95) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/CHANGELOG.md | 33 ++++++ .../helpers/set-project-setting.js | 4 +- .../helpers/migrate-project-settings.js | 9 +- .../helpers/project-settings-api.js | 4 +- .../helpers/migrate-project-settings.js | 10 +- .../helpers/set-project-setting.js | 4 +- .../helpers/migrate-project-settings.js | 9 +- .../helpers/project-settings.js | 4 +- .../helpers/migrate-project-settings.js | 9 +- src/shared/utils/settings-params.js | 3 +- .../project-config/helpers/verify-settings.js | 10 +- test/sonarcloud/migrators/migrators.test.js | 12 ++ test/utils/settings-params.test.js | 111 ++++++++++++++++++ 13 files changed, 205 insertions(+), 17 deletions(-) create mode 100644 test/utils/settings-params.test.js diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f2aeb984..a148f6cb 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,39 @@ All notable changes to CloudVoyager are documented in this file. Entries are ord --- +## Fix: Project Settings Migration (Issue #95) (2026-04-29) + + +Fixed project configuration settings (e.g., `sonar.exclusions`, `sonar.coverage.exclusions`) not being migrated from SonarQube Server to SonarQube Cloud. + +**Root causes identified and fixed:** + +1. **POST body format + Content-Type** — Settings were sent as URL query parameters (`/api/settings/set?key=...&values=...`) with a `null` body and `Content-Type: application/json` (from axios defaults). Changed to send parameters as a form-encoded POST body string with an explicit `Content-Type: application/x-www-form-urlencoded` header, which is the correct format for SonarQube/SonarCloud web API POST endpoints. The explicit header is required because the axios client's default `Content-Type: application/json` would not be overridden by auto-detection (axios uses `rewrite=false` for URLSearchParams). + +2. **Truthy check bug in `dispatchSettingToApi`** — Used `if (setting.value)` which is a JavaScript truthy check. This silently skipped settings with empty string values (`value: ""`). Changed to `if (setting.value !== undefined && setting.value !== null)` for correctness. + +3. **Improved logging** — Added per-setting logging showing which settings are being migrated, with a summary of applied/failed/skipped counts. Previously, failures were logged at `warn` level with no summary, making it hard to diagnose migration issues. + +4. **Verification checker fix** — The settings comparison in `verify-settings.js` had the same truthy fallback issue (`sqSetting.value || sqSetting.values`), which could produce incorrect comparisons for settings with empty string values. + +**Scope:** All 4 pipeline versions (sq-9.9, sq-10.0, sq-10.4, sq-2025) updated consistently. + +**Files changed:** +- `src/shared/utils/settings-params.js` — Fixed truthy check, added skip logging +- `src/pipelines/sq-{9.9,10.0,10.4,2025}/sonarcloud/api/project-config/helpers/set-project-setting.js` (and equivalents) — POST body format fix +- `src/pipelines/sq-{9.9,10.0,10.4,2025}/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js` — Improved logging +- `src/shared/verification/checkers/project-config/helpers/verify-settings.js` — Fixed value comparison +- `test/utils/settings-params.test.js` — New: 14 unit tests for `buildSettingsParams` and `dispatchSettingToApi` +- `test/sonarcloud/migrators/migrators.test.js` — Added empty string value dispatch test + +**Regression test (Angular Framework, SQ 2026.2 → SC-staging):** +- Created 3 project-level settings on SQ: `sonar.exclusions` (3 values), `sonar.coverage.exclusions` (2 values), `sonar.cpd.exclusions` (1 value) +- Ran `migrate --only project-settings`: 3 applied, 0 failed, 0 skipped +- Verified all 3 settings match exactly between SQ and SC via API +- No settings-related warnings in logs + +--- + ## Parallel Issue Sync with Worker Threads (2026-04-28) diff --git a/src/pipelines/sq-10.0/sonarcloud/api/project-config/helpers/set-project-setting.js b/src/pipelines/sq-10.0/sonarcloud/api/project-config/helpers/set-project-setting.js index 91c719f8..8c02c73a 100644 --- a/src/pipelines/sq-10.0/sonarcloud/api/project-config/helpers/set-project-setting.js +++ b/src/pipelines/sq-10.0/sonarcloud/api/project-config/helpers/set-project-setting.js @@ -6,5 +6,7 @@ import { buildSettingsParams } from '../../../../../../shared/utils/settings-par export async function setProjectSetting(client, key, { value, values, fieldValues } = {}, component) { logger.debug(`Setting ${key} on project ${component}`); const params = buildSettingsParams({ key, component, value, values, fieldValues }); - await client.post(`/api/settings/set?${params.toString()}`, null); + await client.post('/api/settings/set', params.toString(), { + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }); } diff --git a/src/pipelines/sq-10.0/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js b/src/pipelines/sq-10.0/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js index 6b1b3047..72280a1a 100644 --- a/src/pipelines/sq-10.0/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js +++ b/src/pipelines/sq-10.0/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js @@ -4,13 +4,18 @@ import { dispatchSettingToApi } from '../../../../../../shared/utils/settings-pa // -------- Migrate Project Settings -------- export async function migrateProjectSettings(projectKey, settings, client) { - logger.info(`Migrating ${settings.length} project settings for ${projectKey}`); + if (!settings.length) { logger.info(`No project settings to migrate for ${projectKey}`); return; } + logger.info(`Migrating ${settings.length} project setting(s) for ${projectKey}: ${settings.map(s => s.key).join(', ')}`); + let ok = 0; let fail = 0; let skip = 0; for (const setting of settings) { try { - await dispatchSettingToApi(client, setting, projectKey); + const dispatched = await dispatchSettingToApi(client, setting, projectKey); + if (dispatched) ok++; else skip++; } catch (error) { + fail++; logger.warn(`Failed to set setting ${setting.key} on ${projectKey}: ${error.message}`); } } + logger.info(`Project settings for ${projectKey}: ${ok} applied, ${fail} failed, ${skip} skipped`); } diff --git a/src/pipelines/sq-10.4/sonarcloud/api/project-config/helpers/project-settings-api.js b/src/pipelines/sq-10.4/sonarcloud/api/project-config/helpers/project-settings-api.js index 14502743..d2367ae0 100644 --- a/src/pipelines/sq-10.4/sonarcloud/api/project-config/helpers/project-settings-api.js +++ b/src/pipelines/sq-10.4/sonarcloud/api/project-config/helpers/project-settings-api.js @@ -8,7 +8,9 @@ import { buildSettingsParams } from '../../../../../../shared/utils/settings-par export async function setProjectSetting(client, key, { value, values, fieldValues } = {}, component) { logger.debug(`Setting ${key} on project ${component}`); const params = buildSettingsParams({ key, component, value, values, fieldValues }); - await client.post(`/api/settings/set?${params.toString()}`, null); + await client.post('/api/settings/set', params.toString(), { + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }); } export async function setProjectTags(client, projectKey, tags) { diff --git a/src/pipelines/sq-10.4/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js b/src/pipelines/sq-10.4/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js index 4c8ad88f..0895bf09 100644 --- a/src/pipelines/sq-10.4/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js +++ b/src/pipelines/sq-10.4/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js @@ -5,12 +5,18 @@ import { dispatchSettingToApi } from '../../../../../../shared/utils/settings-pa // Migrate project settings (non-inherited configuration values). export async function migrateProjectSettings(projectKey, settings, client) { - logger.info(`Migrating ${settings.length} project settings for ${projectKey}`); + if (!settings.length) { logger.info(`No project settings to migrate for ${projectKey}`); return; } + logger.info(`Migrating ${settings.length} project setting(s) for ${projectKey}: ${settings.map(s => s.key).join(', ')}`); + + let ok = 0; let fail = 0; let skip = 0; for (const setting of settings) { try { - await dispatchSettingToApi(client, setting, projectKey); + const dispatched = await dispatchSettingToApi(client, setting, projectKey); + if (dispatched) ok++; else skip++; } catch (error) { + fail++; logger.warn(`Failed to set setting ${setting.key} on ${projectKey}: ${error.message}`); } } + logger.info(`Project settings for ${projectKey}: ${ok} applied, ${fail} failed, ${skip} skipped`); } diff --git a/src/pipelines/sq-2025/sonarcloud/api/project-config/helpers/set-project-setting.js b/src/pipelines/sq-2025/sonarcloud/api/project-config/helpers/set-project-setting.js index e1783a94..dee50b47 100644 --- a/src/pipelines/sq-2025/sonarcloud/api/project-config/helpers/set-project-setting.js +++ b/src/pipelines/sq-2025/sonarcloud/api/project-config/helpers/set-project-setting.js @@ -6,5 +6,7 @@ import { buildSettingsParams } from '../../../../../../shared/utils/settings-par export async function setProjectSetting(client, key, { value, values, fieldValues } = {}, component) { logger.debug(`Setting ${key} on project ${component}`); const params = buildSettingsParams({ key, component, value, values, fieldValues }); - await client.post(`/api/settings/set?${params.toString()}`, null); + await client.post('/api/settings/set', params.toString(), { + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }); } diff --git a/src/pipelines/sq-2025/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js b/src/pipelines/sq-2025/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js index c88f49c2..fa7b96d7 100644 --- a/src/pipelines/sq-2025/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js +++ b/src/pipelines/sq-2025/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js @@ -5,13 +5,18 @@ import { dispatchSettingToApi } from '../../../../../../shared/utils/settings-pa /** Migrate non-inherited project settings to SonarCloud. */ export async function migrateProjectSettings(projectKey, settings, client) { - logger.info(`Migrating ${settings.length} project settings for ${projectKey}`); + if (!settings.length) { logger.info(`No project settings to migrate for ${projectKey}`); return; } + logger.info(`Migrating ${settings.length} project setting(s) for ${projectKey}: ${settings.map(s => s.key).join(', ')}`); + let ok = 0; let fail = 0; let skip = 0; for (const setting of settings) { try { - await dispatchSettingToApi(client, setting, projectKey); + const dispatched = await dispatchSettingToApi(client, setting, projectKey); + if (dispatched) ok++; else skip++; } catch (error) { + fail++; logger.warn(`Failed to set setting ${setting.key} on ${projectKey}: ${error.message}`); } } + logger.info(`Project settings for ${projectKey}: ${ok} applied, ${fail} failed, ${skip} skipped`); } diff --git a/src/pipelines/sq-9.9/sonarcloud/api/project-config/helpers/project-settings.js b/src/pipelines/sq-9.9/sonarcloud/api/project-config/helpers/project-settings.js index 1011fb08..a6a73fc9 100644 --- a/src/pipelines/sq-9.9/sonarcloud/api/project-config/helpers/project-settings.js +++ b/src/pipelines/sq-9.9/sonarcloud/api/project-config/helpers/project-settings.js @@ -6,7 +6,9 @@ import { buildSettingsParams } from '../../../../../../shared/utils/settings-par export async function setProjectSetting(client, key, { value, values, fieldValues } = {}, component) { logger.debug(`Setting ${key} on project ${component}`); const params = buildSettingsParams({ key, component, value, values, fieldValues }); - await client.post(`/api/settings/set?${params.toString()}`, null); + await client.post('/api/settings/set', params.toString(), { + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }); } export async function setProjectTags(client, projectKey, tags) { diff --git a/src/pipelines/sq-9.9/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js b/src/pipelines/sq-9.9/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js index 6b1b3047..72280a1a 100644 --- a/src/pipelines/sq-9.9/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js +++ b/src/pipelines/sq-9.9/sonarcloud/migrators/project-config/helpers/migrate-project-settings.js @@ -4,13 +4,18 @@ import { dispatchSettingToApi } from '../../../../../../shared/utils/settings-pa // -------- Migrate Project Settings -------- export async function migrateProjectSettings(projectKey, settings, client) { - logger.info(`Migrating ${settings.length} project settings for ${projectKey}`); + if (!settings.length) { logger.info(`No project settings to migrate for ${projectKey}`); return; } + logger.info(`Migrating ${settings.length} project setting(s) for ${projectKey}: ${settings.map(s => s.key).join(', ')}`); + let ok = 0; let fail = 0; let skip = 0; for (const setting of settings) { try { - await dispatchSettingToApi(client, setting, projectKey); + const dispatched = await dispatchSettingToApi(client, setting, projectKey); + if (dispatched) ok++; else skip++; } catch (error) { + fail++; logger.warn(`Failed to set setting ${setting.key} on ${projectKey}: ${error.message}`); } } + logger.info(`Project settings for ${projectKey}: ${ok} applied, ${fail} failed, ${skip} skipped`); } diff --git a/src/shared/utils/settings-params.js b/src/shared/utils/settings-params.js index 86a49cf2..ac8abae6 100644 --- a/src/shared/utils/settings-params.js +++ b/src/shared/utils/settings-params.js @@ -23,13 +23,14 @@ export function buildSettingsParams({ key, component, value, values, fieldValues * Returns true if a value was dispatched, false if the setting was empty/skipped. */ export async function dispatchSettingToApi(client, setting, projectKey) { - if (setting.value) { + if (setting.value !== undefined && setting.value !== null) { await client.setProjectSetting(setting.key, { value: setting.value }, projectKey); } else if (setting.values?.length) { await client.setProjectSetting(setting.key, { values: setting.values }, projectKey); } else if (setting.fieldValues?.length) { await client.setProjectSetting(setting.key, { fieldValues: setting.fieldValues }, projectKey); } else { + logger.debug(`Skipping setting ${setting.key} — no value, values, or fieldValues`); return false; } logger.debug(`Set setting ${setting.key} on ${projectKey}`); diff --git a/src/shared/verification/checkers/project-config/helpers/verify-settings.js b/src/shared/verification/checkers/project-config/helpers/verify-settings.js index cb4b5acf..779ea712 100644 --- a/src/shared/verification/checkers/project-config/helpers/verify-settings.js +++ b/src/shared/verification/checkers/project-config/helpers/verify-settings.js @@ -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])); + const pick = (s) => s.value !== undefined && s.value !== null ? s.value : (s.values?.length ? s.values : undefined); + for (const sqSetting of sqNonInherited) { const scSetting = scMap.get(sqSetting.key); - if (!scSetting) { result.sqOnly.push({ key: sqSetting.key, value: sqSetting.value || sqSetting.values }); continue; } - const sqVal = JSON.stringify(sqSetting.value || sqSetting.values || ''); - const scVal = JSON.stringify(scSetting.value || scSetting.values || ''); + if (!scSetting) { result.sqOnly.push({ key: sqSetting.key, value: pick(sqSetting) }); continue; } + const sqVal = JSON.stringify(pick(sqSetting) ?? ''); + const scVal = JSON.stringify(pick(scSetting) ?? ''); if (sqVal !== scVal) { - result.mismatches.push({ key: sqSetting.key, sqValue: sqSetting.value || sqSetting.values, scValue: scSetting.value || scSetting.values }); + result.mismatches.push({ key: sqSetting.key, sqValue: pick(sqSetting), scValue: pick(scSetting) }); } } diff --git a/test/sonarcloud/migrators/migrators.test.js b/test/sonarcloud/migrators/migrators.test.js index db97db27..f5fda1be 100644 --- a/test/sonarcloud/migrators/migrators.test.js +++ b/test/sonarcloud/migrators/migrators.test.js @@ -1258,6 +1258,18 @@ test('migrateProjectSettings handles empty settings', async t => { t.is(client.setProjectSetting.callCount, 0); }); +test('migrateProjectSettings dispatches empty string value', async t => { + const client = mockClient(); + const settings = [ + { key: 'sonar.test.inclusions', value: '' } + ]; + + await migrateProjectSettings('proj', settings, client); + + t.is(client.setProjectSetting.callCount, 1); + t.deepEqual(client.setProjectSetting.firstCall.args, ['sonar.test.inclusions', { value: '' }, 'proj']); +}); + // ============================================================================ // project-config.js - migrateProjectTags // ============================================================================ diff --git a/test/utils/settings-params.test.js b/test/utils/settings-params.test.js new file mode 100644 index 00000000..82dbb1c0 --- /dev/null +++ b/test/utils/settings-params.test.js @@ -0,0 +1,111 @@ +import test from 'ava'; +import sinon from 'sinon'; +import { buildSettingsParams, dispatchSettingToApi } from '../../src/shared/utils/settings-params.js'; + +test.afterEach(() => sinon.restore()); + +// ============================================================================ +// buildSettingsParams +// ============================================================================ + +test('buildSettingsParams builds scalar value params', t => { + const params = buildSettingsParams({ key: 'sonar.cpd.enabled', component: 'proj', value: 'true' }); + t.is(params.get('key'), 'sonar.cpd.enabled'); + t.is(params.get('component'), 'proj'); + t.is(params.get('value'), 'true'); + t.deepEqual(params.getAll('values'), []); + t.deepEqual(params.getAll('fieldValues'), []); +}); + +test('buildSettingsParams builds multi-value params with repeated values', t => { + const params = buildSettingsParams({ key: 'sonar.exclusions', component: 'proj', values: ['**/*.test.js', '**/*.spec.js'] }); + t.is(params.get('key'), 'sonar.exclusions'); + t.is(params.get('component'), 'proj'); + t.is(params.get('value'), null); + t.deepEqual(params.getAll('values'), ['**/*.test.js', '**/*.spec.js']); +}); + +test('buildSettingsParams builds fieldValues params as JSON', t => { + const fvs = [{ resourceKey: '**/*.js', ruleKey: 'squid:S001' }]; + const params = buildSettingsParams({ key: 'sonar.issue.enforce.multicriteria', component: 'proj', fieldValues: fvs }); + t.is(params.get('key'), 'sonar.issue.enforce.multicriteria'); + const parsed = JSON.parse(params.getAll('fieldValues')[0]); + t.deepEqual(parsed, { resourceKey: '**/*.js', ruleKey: 'squid:S001' }); +}); + +test('buildSettingsParams includes empty string value', t => { + const params = buildSettingsParams({ key: 'sonar.test.inclusions', component: 'proj', value: '' }); + t.is(params.get('value'), ''); +}); + +test('buildSettingsParams omits undefined value', t => { + const params = buildSettingsParams({ key: 'sonar.exclusions', component: 'proj', value: undefined, values: ['a'] }); + t.is(params.get('value'), null); + t.deepEqual(params.getAll('values'), ['a']); +}); + +test('buildSettingsParams omits null value', t => { + const params = buildSettingsParams({ key: 'sonar.exclusions', component: 'proj', value: null, values: ['a'] }); + t.is(params.get('value'), null); +}); + +// ============================================================================ +// dispatchSettingToApi +// ============================================================================ + +test('dispatchSettingToApi dispatches scalar value', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const result = await dispatchSettingToApi(client, { key: 'sonar.cpd.enabled', value: 'true' }, 'proj'); + t.true(result); + t.deepEqual(client.setProjectSetting.firstCall.args, ['sonar.cpd.enabled', { value: 'true' }, 'proj']); +}); + +test('dispatchSettingToApi dispatches multi-value array', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const result = await dispatchSettingToApi(client, { key: 'sonar.exclusions', values: ['a', 'b'] }, 'proj'); + t.true(result); + t.deepEqual(client.setProjectSetting.firstCall.args, ['sonar.exclusions', { values: ['a', 'b'] }, 'proj']); +}); + +test('dispatchSettingToApi dispatches fieldValues', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const fvs = [{ resourceKey: '**/*.js', ruleKey: 'squid:S001' }]; + const result = await dispatchSettingToApi(client, { key: 'sonar.multi', fieldValues: fvs }, 'proj'); + t.true(result); + t.deepEqual(client.setProjectSetting.firstCall.args, ['sonar.multi', { fieldValues: fvs }, 'proj']); +}); + +test('dispatchSettingToApi dispatches empty string value (not skipped)', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const result = await dispatchSettingToApi(client, { key: 'sonar.test.inclusions', value: '' }, 'proj'); + t.true(result); + t.deepEqual(client.setProjectSetting.firstCall.args, ['sonar.test.inclusions', { value: '' }, 'proj']); +}); + +test('dispatchSettingToApi prefers value over values', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const result = await dispatchSettingToApi(client, { key: 'k', value: 'scalar', values: ['a'] }, 'proj'); + t.true(result); + t.deepEqual(client.setProjectSetting.firstCall.args, ['k', { value: 'scalar' }, 'proj']); +}); + +test('dispatchSettingToApi returns false for empty setting', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const result = await dispatchSettingToApi(client, { key: 'sonar.empty' }, 'proj'); + t.false(result); + t.is(client.setProjectSetting.callCount, 0); +}); + +test('dispatchSettingToApi returns false for empty values array', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const result = await dispatchSettingToApi(client, { key: 'sonar.empty', values: [] }, 'proj'); + t.false(result); + t.is(client.setProjectSetting.callCount, 0); +}); + +test('dispatchSettingToApi returns false for empty fieldValues array', async t => { + const client = { setProjectSetting: sinon.stub().resolves() }; + const result = await dispatchSettingToApi(client, { key: 'sonar.empty', fieldValues: [] }, 'proj'); + t.false(result); + t.is(client.setProjectSetting.callCount, 0); +});