From 682a0877ff14ddf9e56ce6bbe09a23d379b7dba1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bence=20Nagygy=C3=B6rgy?= Date: Mon, 16 Dec 2024 13:01:38 +0100 Subject: [PATCH] fix: env merge & deploy start validation --- web/crux-ui/src/models/container-merge.ts | 24 ++++++++- .../interceptors/deploy.start.interceptor.ts | 49 ++++++++----------- web/crux/src/domain/container-merge.ts | 31 ++++++++++-- 3 files changed, 70 insertions(+), 34 deletions(-) diff --git a/web/crux-ui/src/models/container-merge.ts b/web/crux-ui/src/models/container-merge.ts index 5d7052faa..5ba8f74aa 100644 --- a/web/crux-ui/src/models/container-merge.ts +++ b/web/crux-ui/src/models/container-merge.ts @@ -5,6 +5,7 @@ import { Marker, Port, UniqueKey, + UniqueKeyValue, UniqueSecretKey, UniqueSecretKeyValue, Volume, @@ -81,6 +82,20 @@ export const mergeSecrets = (strong: UniqueSecretKeyValue[], weak: UniqueSecretK return [...missing, ...strong] } +// TODO(@robot9706): Validate +const mergeUniqueKeyValues = (strong: T[], weak: T[]): T[] => { + if (!strong) { + return weak ?? null + } + + if (!weak) { + return strong + } + + const missing = weak.filter(w => !strong.find(it => it.key === w.key)) + return [...strong, ...missing] +} + export const mergeConfigs = (strong: ContainerConfigData, weak: ContainerConfigData): ContainerConfigData => ({ // common name: strong.name ?? weak.name, @@ -122,8 +137,15 @@ export const mergeConfigs = (strong: ContainerConfigData, weak: ContainerConfigD expectedState: strong.expectedState ?? weak.expectedState, }) +// TODO(@robot9706): Validate export const squashConfigs = (configs: ContainerConfigData[]): ContainerConfigData => - configs.reduce((result, conf) => mergeConfigs(conf, result), {} as ContainerConfigData) + configs.reduce((result, conf) => { + const merged = mergeConfigs(conf, result) + return { + ...merged, + environment: mergeUniqueKeyValues(conf.environment, result.environment), + } + }, {} as ContainerConfigData) // this assumes that the concrete config takes care of any conflict between the other configs export const mergeConfigsWithConcreteConfig = ( diff --git a/web/crux/src/app/deploy/interceptors/deploy.start.interceptor.ts b/web/crux/src/app/deploy/interceptors/deploy.start.interceptor.ts index 63cb4986d..c79767d57 100644 --- a/web/crux/src/app/deploy/interceptors/deploy.start.interceptor.ts +++ b/web/crux/src/app/deploy/interceptors/deploy.start.interceptor.ts @@ -7,7 +7,7 @@ import { getConflictsForConcreteConfig } from 'src/domain/container-conflict' import { mergeConfigsWithConcreteConfig } from 'src/domain/container-merge' import { checkDeploymentDeployability } from 'src/domain/deployment' import { parseDyrectorioEnvRules } from 'src/domain/image' -import { missingSecretsOf } from 'src/domain/start-deployment' +import { deploymentConfigOf, instanceConfigOf, missingSecretsOf } from 'src/domain/start-deployment' import { createStartDeploymentSchema, nullifyUndefinedProperties, yupValidate } from 'src/domain/validation' import { CruxPreconditionFailedException } from 'src/exception/crux-exception' import PrismaService from 'src/services/prisma.service' @@ -110,25 +110,7 @@ export default class DeployStartValidationInterceptor implements NestInterceptor }) yupValidate(createStartDeploymentSchema(instanceValidations), target) - const missingSecrets = deployment.instances - .map(it => { - const imageConfig = it.image.config as any as ContainerConfigData - const instanceConfig = it.config as any as ConcreteContainerConfigData - const mergedConfig = mergeConfigsWithConcreteConfig([imageConfig], instanceConfig) - - return missingSecretsOf(it.configId, mergedConfig) - }) - .filter(it => !!it) - - if (missingSecrets.length > 0) { - throw new CruxPreconditionFailedException({ - message: 'Required secrets must have values!', - property: 'instanceSecrets', - value: missingSecrets, - }) - } - - // config bundles + // check config bundle conflicts if (deployment.configBundles.length > 0) { const configs = deployment.configBundles.map(it => it.configBundle.config as any as ContainerConfigDataWithId) const concreteConfig = deployment.config as any as ConcreteContainerConfigData @@ -140,16 +122,25 @@ export default class DeployStartValidationInterceptor implements NestInterceptor value: Object.keys(conflicts).join(', '), }) } + } - const mergedConfig = mergeConfigsWithConcreteConfig(configs, concreteConfig) - const missingInstanceSecrets = missingSecretsOf(deployment.configId, mergedConfig) - if (missingInstanceSecrets) { - throw new CruxPreconditionFailedException({ - message: 'Required secrets must have values!', - property: 'deploymentSecrets', - value: missingInstanceSecrets, - }) - } + // validate instance configs + const deploymentConfig = deploymentConfigOf(deployment) + + const missingSecrets = deployment.instances + .map(it => { + const instanceConfig = instanceConfigOf(deployment, deploymentConfig, it) + + return missingSecretsOf(it.configId, instanceConfig) + }) + .filter(it => !!it) + + if (missingSecrets.length > 0) { + throw new CruxPreconditionFailedException({ + message: 'Required secrets must have values!', + property: 'instanceSecrets', + value: missingSecrets, + }) } // node diff --git a/web/crux/src/domain/container-merge.ts b/web/crux/src/domain/container-merge.ts index a4f96f422..436c4873c 100644 --- a/web/crux/src/domain/container-merge.ts +++ b/web/crux/src/domain/container-merge.ts @@ -5,6 +5,7 @@ import { Marker, Port, UniqueKey, + UniqueKeyValue, UniqueSecretKey, UniqueSecretKeyValue, Volume, @@ -97,10 +98,11 @@ export const mergeSecrets = (strong: UniqueSecretKeyValue[], weak: UniqueSecretK weak = weak ?? [] strong = strong ?? [] - const overriddenIds: Set = new Set(strong?.map(it => it.id)) + // TODO(@robot9706): Validate this + const overriddenKeys: Set = new Set(strong?.map(it => it.key)) const missing: UniqueSecretKeyValue[] = weak - .filter(it => !overriddenIds.has(it.id)) + .filter(it => !overriddenKeys.has(it.key)) .map(it => ({ ...it, value: '', @@ -111,6 +113,20 @@ export const mergeSecrets = (strong: UniqueSecretKeyValue[], weak: UniqueSecretK return [...missing, ...strong] } +// TODO(@robot9706): Validate +const mergeUniqueKeyValues = (strong: T[], weak: T[]): T[] => { + if (!strong) { + return weak ?? null + } + + if (!weak) { + return strong + } + + const missing = weak.filter(w => !strong.find(it => it.key === w.key)) + return [...strong, ...missing] +} + export const mergeConfigs = (strong: ContainerConfigData, weak: ContainerConfigData): ContainerConfigData => ({ // common name: strong.name ?? weak.name, @@ -152,8 +168,15 @@ export const mergeConfigs = (strong: ContainerConfigData, weak: ContainerConfigD expectedState: strong.expectedState ?? weak.expectedState, }) -const squashConfigs = (configs: ContainerConfigData[]): ContainerConfigData => - configs.reduce((result, conf) => mergeConfigs(conf, result), {} as ContainerConfigData) +// TODO(@robot9706): Validate +export const squashConfigs = (configs: ContainerConfigData[]): ContainerConfigData => + configs.reduce((result, conf) => { + const merged = mergeConfigs(conf, result) + return { + ...merged, + environment: mergeUniqueKeyValues(conf.environment, result.environment), + } + }, {} as ContainerConfigData) // this assumes that the concrete config takes care of any conflict between the other configs export const mergeConfigsWithConcreteConfig = (