Skip to content

Commit 967286f

Browse files
committed
refactor: preserve mask_value from existing entries when injecting changeset variables
1 parent 39c005a commit 967286f

2 files changed

Lines changed: 72 additions & 42 deletions

File tree

src/index.js

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,23 @@ function buildUrl(baseUrl, endpoint, environment) {
6868
function injectYamlVariables(content, changesetVariables) {
6969
const doc = yaml.load(content);
7070

71-
// Build new variable entries from the provided changeset variables
72-
const injectedVars = Object.entries(changesetVariables).map(([name, value]) => ({
73-
environment: null,
74-
mask_value: true,
75-
name,
76-
value,
77-
}));
78-
79-
// Remove existing entries for variables we're overriding
80-
const overrideNames = new Set(Object.keys(changesetVariables));
71+
// Determine mask_value for each injected variable based on existing entries
8172
const existingVars = Array.isArray(doc.variable) ? doc.variable : [];
73+
const injectedVars = Object.entries(changesetVariables).map(([name, value]) => {
74+
// Check if this variable exists in the changeset
75+
const existingEntries = existingVars.filter(v => v.name === name);
76+
// If any existing entry has mask_value: true, preserve that; otherwise use false
77+
const maskValue = existingEntries.some(v => v.mask_value === true) ? true : false;
78+
return {
79+
environment: null,
80+
mask_value: maskValue,
81+
name,
82+
value,
83+
};
84+
});
85+
86+
// Remove all existing entries for variables we're overriding
87+
const overrideNames = new Set(Object.keys(changesetVariables));
8288
const keptVars = existingVars.filter(v => !overrideNames.has(v.name));
8389

8490
doc.variable = [...keptVars, ...injectedVars];
@@ -89,17 +95,23 @@ function injectYamlVariables(content, changesetVariables) {
8995
function injectJsonVariables(content, changesetVariables) {
9096
const doc = JSON.parse(content);
9197

92-
// Build new variable entries from the provided changeset variables
93-
const injectedVars = Object.entries(changesetVariables).map(([name, value]) => ({
94-
environment: null,
95-
mask_value: true,
96-
name,
97-
value,
98-
}));
98+
// Determine mask_value for each injected variable based on existing entries
99+
const existingVars = Array.isArray(doc.variable) ? doc.variable : [];
100+
const injectedVars = Object.entries(changesetVariables).map(([name, value]) => {
101+
// Check if this variable exists in the changeset
102+
const existingEntries = existingVars.filter(v => v.name === name);
103+
// If any existing entry has mask_value: true, preserve that; otherwise use false
104+
const maskValue = existingEntries.some(v => v.mask_value === true) ? true : false;
105+
return {
106+
environment: null,
107+
mask_value: maskValue,
108+
name,
109+
value,
110+
};
111+
});
99112

100-
// Remove existing entries for variables we're overriding
113+
// Remove all existing entries for variables we're overriding
101114
const overrideNames = new Set(Object.keys(changesetVariables));
102-
const existingVars = Array.isArray(doc.variable) ? doc.variable : [];
103115
const keptVars = existingVars.filter(v => !overrideNames.has(v.name));
104116

105117
doc.variable = [...keptVars, ...injectedVars];

src/index.test.js

Lines changed: 41 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,7 +1380,7 @@ describe('run — changeset variables', () => {
13801380
expect(body).toContain('secret123');
13811381
expect(body).toContain('API_TOKEN');
13821382
expect(body).toContain('token456');
1383-
expect(body).toContain('mask_value: true');
1383+
expect(body).toContain('mask_value: false');
13841384
});
13851385

13861386
test('fails with invalid changeset_variables format', async () => {
@@ -1611,19 +1611,20 @@ action:
16111611
- action: gencloud-create
16121612
object_type: RoutingQueue`;
16131613

1614-
test('injects new variables with mask_value: true', () => {
1614+
test('injects new variables with mask_value: false by default', () => {
16151615
const result = injectYamlVariables(baseYaml, { NEW_VAR: 'new_value' });
16161616
expect(result).toContain('name: NEW_VAR');
16171617
expect(result).toContain('value: new_value');
1618-
expect(result).toContain('mask_value: true');
1618+
expect(result).toContain('mask_value: false');
16191619
});
16201620

1621-
test('replaces all entries for an existing variable name', () => {
1621+
test('when overriding variable with mask_value: true in existing entry, preserves true', () => {
16221622
const result = injectYamlVariables(baseYaml, { existing_var: 'replaced_value' });
16231623
// Should have exactly one entry for existing_var (the injected one)
16241624
const matches = result.match(/name: existing_var/g);
16251625
expect(matches).toHaveLength(1);
16261626
expect(result).toContain('value: replaced_value');
1627+
expect(result).toContain('mask_value: true');
16271628
expect(result).not.toContain('old_value');
16281629
expect(result).not.toContain('dev_value');
16291630
});
@@ -1647,14 +1648,16 @@ variable:
16471648
expect(result).not.toContain('value: old');
16481649
});
16491650

1650-
test('sets environment to null for injected variables', () => {
1651-
const result = injectYamlVariables(baseYaml, { MY_VAR: 'val' });
1651+
test('sets environment to null for injected variables and removes all others', () => {
1652+
const result = injectYamlVariables(baseYaml, { existing_var: 'val' });
16521653
// Parse back to verify structure
16531654
const yaml = require('js-yaml');
16541655
const doc = yaml.load(result);
1655-
const injected = doc.variable.find(v => v.name === 'MY_VAR');
1656+
const injected = doc.variable.find(v => v.name === 'existing_var');
16561657
expect(injected.environment).toBeNull();
1657-
expect(injected.mask_value).toBe(true);
1658+
// Should have exactly one entry for existing_var
1659+
const allExisting = doc.variable.filter(v => v.name === 'existing_var');
1660+
expect(allExisting).toHaveLength(1);
16581661
});
16591662

16601663
test('handles yaml with empty variable array', () => {
@@ -1671,14 +1674,18 @@ variable:
16711674
expect(result).toContain('value: value');
16721675
});
16731676

1674-
test('injects multiple variables at once', () => {
1675-
const result = injectYamlVariables(baseYaml, { VAR_A: 'aaa', VAR_B: 'bbb' });
1677+
test('injects multiple variables at once, preserving mask_value from existing', () => {
1678+
const result = injectYamlVariables(baseYaml, { VAR_A: 'aaa', existing_var: 'bbb' });
16761679
const yaml = require('js-yaml');
16771680
const doc = yaml.load(result);
16781681
const varA = doc.variable.find(v => v.name === 'VAR_A');
1679-
const varB = doc.variable.find(v => v.name === 'VAR_B');
1680-
expect(varA).toEqual({ environment: null, mask_value: true, name: 'VAR_A', value: 'aaa' });
1681-
expect(varB).toEqual({ environment: null, mask_value: true, name: 'VAR_B', value: 'bbb' });
1682+
const varB = doc.variable.find(v => v.name === 'existing_var');
1683+
// VAR_A is new, should have false (no existing entries with true)
1684+
expect(varA).toEqual({ environment: null, mask_value: false, name: 'VAR_A', value: 'aaa' });
1685+
// existing_var has entries with mask_value: true, should preserve it
1686+
expect(varB.environment).toBeNull();
1687+
expect(varB.mask_value).toBe(true);
1688+
expect(varB.value).toBe('bbb');
16821689
});
16831690
});
16841691

@@ -1790,10 +1797,10 @@ describe('run — JSON file format', () => {
17901797
// All existing DogsName entries should be removed, replaced with one injected entry
17911798
const dogsEntries = body.variable.filter(v => v.name === 'DogsName');
17921799
expect(dogsEntries).toHaveLength(1);
1793-
expect(dogsEntries[0]).toEqual({ environment: null, mask_value: true, name: 'DogsName', value: 'overridden' });
1800+
expect(dogsEntries[0]).toEqual({ environment: null, mask_value: false, name: 'DogsName', value: 'overridden' });
17941801
// New variable should be added
17951802
const newVarEntry = body.variable.find(v => v.name === 'NewVar');
1796-
expect(newVarEntry).toEqual({ environment: null, mask_value: true, name: 'NewVar', value: 'newval' });
1803+
expect(newVarEntry).toEqual({ environment: null, mask_value: false, name: 'NewVar', value: 'newval' });
17971804
expect(mockCore.setFailed).not.toHaveBeenCalled();
17981805
});
17991806
});
@@ -1811,17 +1818,26 @@ describe('injectJsonVariables', () => {
18111818
action: [{ action: 'gencloud-create' }]
18121819
}, null, 2);
18131820

1814-
test('injects new variables with mask_value: true', () => {
1821+
test('injects new variables with mask_value: false by default', () => {
18151822
const result = JSON.parse(injectJsonVariables(baseJson, { NEW_VAR: 'new_value' }));
18161823
const injected = result.variable.find(v => v.name === 'NEW_VAR');
1817-
expect(injected).toEqual({ environment: null, mask_value: true, name: 'NEW_VAR', value: 'new_value' });
1824+
expect(injected).toEqual({ environment: null, mask_value: false, name: 'NEW_VAR', value: 'new_value' });
18181825
});
18191826

1820-
test('replaces all entries for an existing variable name', () => {
1827+
test('when overriding variable with only mask_value: false entries, uses false', () => {
18211828
const result = JSON.parse(injectJsonVariables(baseJson, { existing_var: 'replaced_value' }));
18221829
const matches = result.variable.filter(v => v.name === 'existing_var');
18231830
expect(matches).toHaveLength(1);
18241831
expect(matches[0].value).toBe('replaced_value');
1832+
expect(matches[0].mask_value).toBe(false);
1833+
expect(matches[0].environment).toBeNull();
1834+
});
1835+
1836+
test('when overriding variable with mask_value: true in existing entry, preserves true', () => {
1837+
const result = JSON.parse(injectJsonVariables(baseJson, { keep_var: 'new_value' }));
1838+
const matches = result.variable.filter(v => v.name === 'keep_var');
1839+
expect(matches).toHaveLength(1);
1840+
expect(matches[0].value).toBe('new_value');
18251841
expect(matches[0].mask_value).toBe(true);
18261842
expect(matches[0].environment).toBeNull();
18271843
});
@@ -1846,12 +1862,14 @@ describe('injectJsonVariables', () => {
18461862
expect(result.variable[0].name).toBe('NEW_VAR');
18471863
});
18481864

1849-
test('injects multiple variables at once', () => {
1850-
const result = JSON.parse(injectJsonVariables(baseJson, { VAR_A: 'aaa', VAR_B: 'bbb' }));
1865+
test('injects multiple variables at once, preserving mask_value from existing', () => {
1866+
const result = JSON.parse(injectJsonVariables(baseJson, { VAR_A: 'aaa', keep_var: 'bbb' }));
18511867
const varA = result.variable.find(v => v.name === 'VAR_A');
1852-
const varB = result.variable.find(v => v.name === 'VAR_B');
1853-
expect(varA).toEqual({ environment: null, mask_value: true, name: 'VAR_A', value: 'aaa' });
1854-
expect(varB).toEqual({ environment: null, mask_value: true, name: 'VAR_B', value: 'bbb' });
1868+
const varB = result.variable.find(v => v.name === 'keep_var');
1869+
// VAR_A is new, should have false (no existing entries with true)
1870+
expect(varA).toEqual({ environment: null, mask_value: false, name: 'VAR_A', value: 'aaa' });
1871+
// keep_var exists with mask_value: true, should preserve it
1872+
expect(varB).toEqual({ environment: null, mask_value: true, name: 'keep_var', value: 'bbb' });
18551873
});
18561874

18571875
test('preserves other JSON fields', () => {

0 commit comments

Comments
 (0)