Skip to content

Commit 763ccee

Browse files
fix(core): Block unsupported changes to current config with prev success Config (#584)
* Adding vpc deletion check in compare config * Passing repo name to compare configurations * Fixing Subnet CIDR change comparision
1 parent a9fffdd commit 763ccee

File tree

4 files changed

+46
-27
lines changed

4 files changed

+46
-27
lines changed

src/core/cdk/src/initial-setup.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ export namespace InitialSetup {
172172
functionPayload: {
173173
'inputConfig.$': '$',
174174
region: cdk.Aws.REGION,
175+
configRepositoryName: props.configRepositoryName,
176+
'configFilePath.$': '$.configuration.configFilePath',
177+
'configCommitId.$': '$.configuration.configCommitId',
178+
'acceleratorVersion.$': '$.configuration.acceleratorVersion',
175179
'baseline.$': '$.configuration.baselineOutput.baseline',
176180
},
177181
resultPath: 'DISCARD',

src/core/runtime/src/compare-configurations-step.ts

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,12 @@ import { SecretsManager } from '@aws-accelerator/common/src/aws/secrets-manager'
22
import { compareAcceleratorConfig } from '@aws-accelerator/common-config/src/compare/main';
33
import { getCommitIdSecretName } from '@aws-accelerator/common-outputs/src/commitid-secret';
44

5-
export interface StepInput {
5+
export interface StepInput extends ConfigurationInput {
66
inputConfig: CompareConfigurationInput;
77
region: string;
88
}
99

1010
export interface CompareConfigurationInput {
11-
configuration: ConfigurationInput;
1211
configOverrides: { [key: string]: boolean } | undefined;
1312
overrideComparison: boolean | undefined;
1413
}
@@ -26,7 +25,7 @@ export interface CompareConfigurationsOutput {
2625
configCommitId: string;
2726
}
2827

29-
export const handler = async (input: StepInput): Promise<CompareConfigurationsOutput> => {
28+
export const handler = async (input: StepInput) => {
3029
console.log(`Loading compare configurations...`);
3130
console.log(JSON.stringify(input, null, 2));
3231

@@ -47,8 +46,7 @@ export const handler = async (input: StepInput): Promise<CompareConfigurationsOu
4746
'ov-nacl': false,
4847
};
4948

50-
const { inputConfig, region } = input;
51-
const { configFilePath, configRepositoryName, configCommitId, baseline } = inputConfig.configuration;
49+
const { inputConfig, region, baseline, configCommitId, configFilePath, configRepositoryName } = input;
5250
const commitSecretId = getCommitIdSecretName();
5351

5452
const secrets = new SecretsManager();
@@ -64,11 +62,7 @@ export const handler = async (input: StepInput): Promise<CompareConfigurationsOu
6462
console.log(
6563
'either previous git repo commitId not found or commitIds are same, so skipping validation of config file updates',
6664
);
67-
return {
68-
configRepositoryName,
69-
configFilePath,
70-
configCommitId,
71-
};
65+
return;
7266
}
7367
let configOverrides = inputConfig.configOverrides;
7468
if (baseline === 'ORGANIZATIONS') {
@@ -105,5 +99,5 @@ export const handler = async (input: StepInput): Promise<CompareConfigurationsOu
10599
throw new Error(`There were errors while comparing the configuration changes:\n${errors.join('\n')}`);
106100
}
107101

108-
return inputConfig.configuration;
102+
return;
109103
};

src/lib/common-config/src/compare/common.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,18 @@ export function matchConfigDependencyArray(diffs: Diff[], pathValues: string[],
145145
}
146146
return errors;
147147
}
148+
149+
export function deletedConfigEntry(diffs: Diff[], pathValues: string[], pathValue: string): string[] {
150+
const errors: string[] = [];
151+
for (const deletedDiff of diffs.filter(isDiffDeleted)) {
152+
if (!deletedDiff.path) {
153+
continue;
154+
}
155+
const found = pathValues.every(r => deletedDiff.path?.includes(r));
156+
const changedValue = deletedDiff.path?.[deletedDiff.path?.length - 1];
157+
if (found && changedValue === pathValue) {
158+
errors.push(`ConfigCheck: blocked deleting "${changedValue}" from config path "${deletedDiff.path?.join('/')}"`);
159+
}
160+
}
161+
return errors;
162+
}

src/lib/common-config/src/compare/validate.ts

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,21 @@ const GLOBAL_OPTIONS_CLS_REGION = ['global-options', 'central-log-services', 're
1313
/**
1414
* config path(s) for mandatory accounts vpc(s)
1515
*/
16+
const ACCOUNT_VPC = ['mandatory-account-configs', 'vpc'];
1617
const ACCOUNT_VPC_NAME = ['mandatory-account-configs', 'vpc', 'name'];
1718
const ACCOUNT_VPC_REGION = ['mandatory-account-configs', 'vpc', 'region'];
1819
const ACCOUNT_VPC_DEPLOY = ['mandatory-account-configs', 'vpc', 'deploy'];
19-
const ACCOUNT_VPC_CIDR = ['mandatory-account-configs', 'vpc', 'cidr', 'ipv4', 'value'];
20-
const ACCOUNT_VPC_CIDR2 = ['mandatory-account-configs', 'vpc', 'cidr2', 'ipv4', 'value'];
20+
const ACCOUNT_VPC_CIDR = ['mandatory-account-configs', 'vpc', 'cidr'];
21+
const ACCOUNT_VPC_CIDR2 = ['mandatory-account-configs', 'vpc', 'cidr2'];
2122

2223
/**
2324
* config path(s) for mandatory accounts vpc subnets
2425
*/
2526
const ACCOUNT_SUBNETS = ['mandatory-account-configs', 'vpc', 'subnets'];
2627
const ACCOUNT_SUBNET_NAME = ['mandatory-account-configs', 'vpc', 'subnets', 'name'];
2728
const ACCOUNT_SUBNET_AZ = ['mandatory-account-configs', 'vpc', 'subnets', 'definitions', 'az'];
28-
const ACCOUNT_SUBNET_CIDR = ['mandatory-account-configs', 'vpc', 'subnets', 'definitions', 'cidr', 'ipv4', 'value'];
29-
const ACCOUNT_SUBNET_CIDR2 = ['mandatory-account-configs', 'vpc', 'subnets', 'definitions', 'cidr2', 'ipv4', 'value'];
29+
const ACCOUNT_SUBNET_CIDR = ['mandatory-account-configs', 'vpc', 'subnets', 'definitions', 'cidr'];
30+
const ACCOUNT_SUBNET_CIDR2 = ['mandatory-account-configs', 'vpc', 'subnets', 'definitions', 'cidr2'];
3031
const ACCOUNT_SUBNET_DISABLED = ['mandatory-account-configs', 'vpc', 'subnets', 'definitions', 'disabled'];
3132

3233
/**
@@ -57,20 +58,21 @@ const VGW_ASN = ['mandatory-account-configs', 'vpc', 'vgw', 'asn'];
5758
/**
5859
* config path(s) for organizational units - vpc
5960
*/
61+
const OU_VPC = ['organizational-units', 'vpc'];
6062
const OU_VPC_NAME = ['organizational-units', 'vpc', 'name'];
6163
const OU_VPC_REGION = ['organizational-units', 'vpc', 'region'];
6264
const OU_VPC_DEPLOY = ['organizational-units', 'vpc', 'deploy'];
63-
const OU_VPC_CIDR = ['organizational-units', 'vpc', 'cidr', 'ipv4', 'value'];
64-
const OU_VPC_CIDR2 = ['organizational-units', 'vpc', 'cidr2', 'ipv4', 'value'];
65+
const OU_VPC_CIDR = ['organizational-units', 'vpc', 'cidr'];
66+
const OU_VPC_CIDR2 = ['organizational-units', 'vpc', 'cidr2'];
6567

6668
/**
6769
* config path(s) for organizational units vpc subnets
6870
*/
6971
const OU_SUBNETS = ['organizational-units', 'vpc', 'subnets'];
7072
const OU_SUBNET_NAME = ['organizational-units', 'vpc', 'subnets', 'name'];
7173
const OU_SUBNET_AZ = ['organizational-units', 'vpc', 'subnets', 'definitions', 'az'];
72-
const OU_SUBNET_CIDR = ['organizational-units', 'vpc', 'subnets', 'definitions', 'cidr', 'ipv4', 'value'];
73-
const OU_SUBNET_CIDR2 = ['organizational-units', 'vpc', 'subnets', 'definitions', 'cidr2', 'ipv4', 'value'];
74+
const OU_SUBNET_CIDR = ['organizational-units', 'vpc', 'subnets', 'definitions', 'cidr'];
75+
const OU_SUBNET_CIDR2 = ['organizational-units', 'vpc', 'subnets', 'definitions', 'cidr2'];
7476
const OU_SUBNET_DISABLED = ['organizational-units', 'vpc', 'subnets', 'definitions', 'disabled'];
7577

7678
/**
@@ -204,6 +206,8 @@ export async function validateAccountOu(differences: Diff<LHS, RHS>[], errors: s
204206
* @param errors
205207
*/
206208
export async function validateAccountVpc(differences: Diff<LHS, RHS>[], errors: string[]): Promise<void> {
209+
// the below function checks vpc deletion from Account Config
210+
errors.push(...validateConfig.deletedConfigEntry(differences, ACCOUNT_VPC, 'vpc'));
207211
// the below function checks vpc deploy of the account
208212
const accountVpcDeploy = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_VPC_DEPLOY, 5);
209213
if (accountVpcDeploy) {
@@ -217,13 +221,13 @@ export async function validateAccountVpc(differences: Diff<LHS, RHS>[], errors:
217221
}
218222

219223
// the below function checks vpc cidr of the account
220-
const accountVpcCidr = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_VPC_CIDR, 8);
224+
const accountVpcCidr = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_VPC_CIDR, 5);
221225
if (accountVpcCidr) {
222226
errors.push(...accountVpcCidr);
223227
}
224228

225229
// the below function checks vpc cidr2 of the account
226-
const accountVpcCidr2 = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_VPC_CIDR2, 8);
230+
const accountVpcCidr2 = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_VPC_CIDR2, 5);
227231
if (accountVpcCidr2) {
228232
errors.push(...accountVpcCidr2);
229233
}
@@ -259,13 +263,13 @@ export async function validateAccountSubnets(differences: Diff<LHS, RHS>[], erro
259263
}
260264

261265
// the below function checks subnet cidr of the account
262-
const accountSubnetCidr = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_SUBNET_CIDR, 12);
266+
const accountSubnetCidr = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_SUBNET_CIDR, 9);
263267
if (accountSubnetCidr) {
264268
errors.push(...accountSubnetCidr);
265269
}
266270

267271
// the below function checks subnet cidr of the account
268-
const accountSubnetCidr2 = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_SUBNET_CIDR2, 12);
272+
const accountSubnetCidr2 = validateConfig.matchEditedConfigDependency(differences, ACCOUNT_SUBNET_CIDR2, 9);
269273
if (accountSubnetCidr2) {
270274
errors.push(...accountSubnetCidr2);
271275
}
@@ -376,6 +380,8 @@ export async function validateVgw(differences: Diff<LHS, RHS>[], errors: string[
376380
* @param errors
377381
*/
378382
export async function validateOuVpc(differences: Diff<LHS, RHS>[], errors: string[]): Promise<void> {
383+
// the below function checks vpc deletion from Organizational Unit
384+
errors.push(...validateConfig.deletedConfigEntry(differences, OU_VPC, 'vpc'));
379385
// the below function checks vpc deploy of the account
380386
const ouVpcDeploy = validateConfig.matchEditedConfigDependency(differences, OU_VPC_DEPLOY, 5);
381387
if (ouVpcDeploy) {
@@ -389,13 +395,13 @@ export async function validateOuVpc(differences: Diff<LHS, RHS>[], errors: strin
389395
}
390396

391397
// the below function checks vpc cidr of the account
392-
const ouVpcCidr = validateConfig.matchEditedConfigDependency(differences, OU_VPC_CIDR, 8);
398+
const ouVpcCidr = validateConfig.matchEditedConfigDependency(differences, OU_VPC_CIDR, 5);
393399
if (ouVpcCidr) {
394400
errors.push(...ouVpcCidr);
395401
}
396402

397403
// the below function checks vpc cidr2 of the account
398-
const ouVpcCidr2 = validateConfig.matchEditedConfigDependency(differences, OU_VPC_CIDR2, 8);
404+
const ouVpcCidr2 = validateConfig.matchEditedConfigDependency(differences, OU_VPC_CIDR2, 5);
399405
if (ouVpcCidr2) {
400406
errors.push(...ouVpcCidr2);
401407
}
@@ -431,13 +437,13 @@ export async function validateOuSubnets(differences: Diff<LHS, RHS>[], errors: s
431437
}
432438

433439
// the below function checks subnet cidr of the account
434-
const ouSubnetCidr = validateConfig.matchEditedConfigDependency(differences, OU_SUBNET_CIDR, 12);
440+
const ouSubnetCidr = validateConfig.matchEditedConfigDependency(differences, OU_SUBNET_CIDR, 9);
435441
if (ouSubnetCidr) {
436442
errors.push(...ouSubnetCidr);
437443
}
438444

439445
// the below function checks subnet cidr of the account
440-
const ouSubnetCidr2 = validateConfig.matchEditedConfigDependency(differences, OU_SUBNET_CIDR2, 12);
446+
const ouSubnetCidr2 = validateConfig.matchEditedConfigDependency(differences, OU_SUBNET_CIDR2, 9);
441447
if (ouSubnetCidr2) {
442448
errors.push(...ouSubnetCidr2);
443449
}

0 commit comments

Comments
 (0)