From b2f3bf7059ce3ca1e72cf6c451edd3e61699828a Mon Sep 17 00:00:00 2001 From: Edward Foyle Date: Wed, 5 May 2021 06:17:52 -0700 Subject: [PATCH] fix: carry existing container secret config over (#7224) When a customer selected "don't update secrets" when deploying changes to a containers resource, the existing secrets config was removed rather than retained. This change adds logic to parse the existing secret config and apply it to the updated template --- .../set-existing-secret-arns.test.ts | 60 +++++++++++++++++++ .../utils/containers-artifacts.ts | 14 ++++- .../containers/set-existing-secret-arns.ts | 42 +++++++++++++ 3 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.test.ts create mode 100644 packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.ts diff --git a/packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.test.ts b/packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.test.ts new file mode 100644 index 00000000000..63a0ccbbd41 --- /dev/null +++ b/packages/amplify-category-api/src/__tests__/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.test.ts @@ -0,0 +1,60 @@ +import { setExistingSecretArns } from '../../../../../provider-utils/awscloudformation/utils/containers/set-existing-secret-arns'; + +describe('set existing secret arns', () => { + it('does nothing if no template found', () => { + const secretMap = new Map(); + setExistingSecretArns(secretMap, {}); + expect(secretMap.size).toBe(0); + }); + + it('does nothing if template does not have secrets', () => { + const mockTemplate = { + Resources: { + TaskDefinition: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + ContainerDefinitions: [ + { + Secrets: [], + }, + ], + }, + }, + }, + }; + const secretMap = new Map(); + setExistingSecretArns(secretMap, mockTemplate); + expect(secretMap.size).toBe(0); + }); + + it('adds all secrets to secret map', () => { + const mockTemplate = { + Resources: { + TaskDefinition: { + Type: 'AWS::ECS::TaskDefinition', + Properties: { + ContainerDefinitions: [ + { + Secrets: [ + { + Name: 'SOMETHING', + ValueFrom: 'some:secretsmanager:arn', + }, + ], + }, + ], + }, + }, + }, + }; + const secretMap = new Map(); + setExistingSecretArns(secretMap, mockTemplate); + expect(secretMap.size).toBe(1); + expect(secretMap.entries().next().value).toMatchInlineSnapshot(` + Array [ + "SOMETHING", + "some:secretsmanager:arn", + ] + `); + }); +}); diff --git a/packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers-artifacts.ts b/packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers-artifacts.ts index 1aba546cae9..c33ca68fbff 100644 --- a/packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers-artifacts.ts +++ b/packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers-artifacts.ts @@ -9,8 +9,12 @@ import Container from '../docker-compose/ecs-objects/container'; import { EcsStack } from '../ecs-apigw-stack'; import { API_TYPE, ResourceDependency } from '../../../provider-utils/awscloudformation/service-walkthroughs/containers-walkthrough'; import { getGitHubOwnerRepoFromPath } from '../../../provider-utils/awscloudformation/utils/github'; -import { JSONUtilities } from 'amplify-cli-core'; +import { JSONUtilities, pathManager, readCFNTemplate } from 'amplify-cli-core'; import { DEPLOYMENT_MECHANISM } from '../base-api-stack'; +import { setExistingSecretArns } from './containers/set-existing-secret-arns'; +import { category } from '../../../category-constants'; + +export const cfnFileName = (resourceName: string) => `${resourceName}-cloudformation-template.json`; export type ApiResource = { category: string; @@ -106,8 +110,7 @@ export async function generateContainersArtifacts( const cfn = stack.toCloudFormation(); - const cfnFileName = `${resourceName}-cloudformation-template.json`; - JSONUtilities.writeJson(path.normalize(path.join(resourceDir, cfnFileName)), cfn); + JSONUtilities.writeJson(path.normalize(path.join(resourceDir, cfnFileName(resourceName))), cfn); return { exposedContainer, @@ -286,6 +289,11 @@ export async function processDockerConfig(context: any, resource: ApiResource, s secretsArns.set(secretName, secretArn); } + } else { + const { cfnTemplate } = await readCFNTemplate( + path.join(pathManager.getBackendDirPath(), category, resourceName, cfnFileName(resourceName)), + ); + setExistingSecretArns(secretsArns, cfnTemplate); } const desiredCount = service?.replicas ?? 1; // TODO: 1 should be from meta (HA setting) diff --git a/packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.ts b/packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.ts new file mode 100644 index 00000000000..e425a26da50 --- /dev/null +++ b/packages/amplify-category-api/src/provider-utils/awscloudformation/utils/containers/set-existing-secret-arns.ts @@ -0,0 +1,42 @@ +import { $TSAny } from 'amplify-cli-core'; +import _ from 'lodash'; +/** + * Check if the template contains existing secret configuration and if so, add it to the secretsMap + * The secrets configuration is stored in the template in the following format + * { + * "Resources": { + "TaskDefinition": { + "Type": "AWS::ECS::TaskDefinition", + "Properties": { + "ContainerDefinitions": [ + { + "Secrets": [ + { + "Name": "SECRETNAME", + "ValueFrom": "" + } + } + } + ] + } + } + } + */ +export const setExistingSecretArns = (secretsMap: Map, cfnObj: $TSAny) => { + if (_.isEmpty(cfnObj)) { + return; + } + const taskDef = Object.values(cfnObj?.Resources) // get all the resources + .find((value: $TSAny) => value?.Type === 'AWS::ECS::TaskDefinition') as $TSAny; // find the task definition + const containerDefs = taskDef?.Properties?.ContainerDefinitions as $TSAny[]; // pull out just the container definitions + if (!Array.isArray(containerDefs)) { + return; + } + containerDefs + .map(def => def?.Secrets) // get the secrets array + .filter(secrets => !_.isEmpty(secrets)) // filter out defs that don't contain secrets + .flat(1) // merge nested secrets array into one array + .filter(secretDef => !!secretDef?.Name) // make sure the name is defined + .filter(secretDef => !!secretDef.ValueFrom) // make sure the arn is defined + .forEach(secretDef => secretsMap.set(secretDef.Name, secretDef.ValueFrom)); // add it to the secretsMap map +};