require backend identifier in deployer, remove redundant deploymentTy…

…pe parameter (#749)
aws-amplify · Nov 29, 2023 · cd672ba · cd672ba
1 parent e6fab21
commit cd672ba
Show file tree

Hide file tree

Showing 10 changed files with 79 additions and 133 deletions.
diff --git a/.changeset/mighty-bottles-leave.md b/.changeset/mighty-bottles-leave.md
@@ -0,0 +1,7 @@
+---
+'@aws-amplify/backend-deployer': minor
+'@aws-amplify/sandbox': patch
+'@aws-amplify/backend-cli': patch
+---
+
+require backend identifier in deployer, remove redundant deploymentType parameter
diff --git a/packages/backend-deployer/API.md b/packages/backend-deployer/API.md
@@ -5,12 +5,11 @@
 ```ts
 
 import { BackendIdentifier } from '@aws-amplify/plugin-types';
-import { DeploymentType } from '@aws-amplify/plugin-types';
 
 // @public
 export type BackendDeployer = {
-    deploy: (backendId?: BackendIdentifier, deployProps?: DeployProps) => Promise<DeployResult>;
-    destroy: (backendId?: BackendIdentifier, destroyProps?: DestroyProps) => Promise<DestroyResult>;
+    deploy: (backendId: BackendIdentifier, deployProps?: DeployProps) => Promise<DeployResult>;
+    destroy: (backendId: BackendIdentifier) => Promise<DestroyResult>;
 };
 
 // @public
@@ -26,7 +25,6 @@ export type DeploymentTimes = {
 
 // @public (undocumented)
 export type DeployProps = {
-    deploymentType?: DeploymentType;
     secretLastUpdated?: Date;
     validateAppSources?: boolean;
 };
@@ -36,11 +34,6 @@ export type DeployResult = {
     deploymentTimes: DeploymentTimes;
 };
 
-// @public (undocumented)
-export type DestroyProps = {
-    deploymentType?: DeploymentType;
-};
-
 // @public (undocumented)
 export type DestroyResult = {
     deploymentTimes: DeploymentTimes;

diff --git a/packages/backend-deployer/src/cdk_deployer.test.ts b/packages/backend-deployer/src/cdk_deployer.test.ts
@@ -7,14 +7,19 @@ import { CdkErrorMapper } from './cdk_error_mapper.js';
 import { BackendIdentifier } from '@aws-amplify/plugin-types';
 
 void describe('invokeCDKCommand', () => {
-  const backendId: BackendIdentifier = {
+  const branchBackendId: BackendIdentifier = {
     namespace: '123',
     name: 'testBranch',
     type: 'branch',
   };
 
+  const sandboxBackendId: BackendIdentifier = {
+    namespace: 'foo',
+    name: 'bar',
+    type: 'sandbox',
+  };
+
   const sandboxDeployProps: DeployProps = {
-    deploymentType: 'sandbox',
     secretLastUpdated: new Date(12345678),
   };
 
@@ -35,26 +40,10 @@ void describe('invokeCDKCommand', () => {
     execaMock.mock.restore();
   });
 
-  void it('handles no options/args', async () => {
-    await invoker.deploy();
-    assert.strictEqual(execaMock.mock.callCount(), 1);
-    assert.equal(execaMock.mock.calls[0].arguments[1]?.length, 8);
-    assert.deepStrictEqual(execaMock.mock.calls[0].arguments[1], [
-      'cdk',
-      'deploy',
-      '--ci',
-      '--app',
-      "'npx tsx amplify/backend.ts'",
-      '--all',
-      '--output',
-      '.amplify/artifacts/cdk.out',
-    ]);
-  });
-
   void it('handles options for branch deployments', async () => {
-    await invoker.deploy(backendId);
+    await invoker.deploy(branchBackendId);
     assert.strictEqual(execaMock.mock.callCount(), 1);
-    assert.equal(execaMock.mock.calls[0].arguments[1]?.length, 14);
+    assert.equal(execaMock.mock.calls[0].arguments[1]?.length, 16);
     assert.deepStrictEqual(execaMock.mock.calls[0].arguments[1], [
       'cdk',
       'deploy',
@@ -65,18 +54,20 @@ void describe('invokeCDKCommand', () => {
       '--output',
       '.amplify/artifacts/cdk.out',
       '--context',
-      `amplify-backend-namespace=123`,
+      'amplify-backend-namespace=123',
       '--context',
-      `amplify-backend-name=testBranch`,
+      'amplify-backend-name=testBranch',
       '--require-approval',
       'never',
+      '--context',
+      'amplify-backend-type=branch',
     ]);
   });
 
   void it('handles deployProps for sandbox', async () => {
-    await invoker.deploy(undefined, sandboxDeployProps);
+    await invoker.deploy(sandboxBackendId, sandboxDeployProps);
     assert.strictEqual(execaMock.mock.callCount(), 1);
-    assert.equal(execaMock.mock.calls[0].arguments[1]?.length, 14);
+    assert.equal(execaMock.mock.calls[0].arguments[1]?.length, 18);
     assert.deepStrictEqual(execaMock.mock.calls[0].arguments[1], [
       'cdk',
       'deploy',
@@ -87,7 +78,11 @@ void describe('invokeCDKCommand', () => {
       '--output',
       '.amplify/artifacts/cdk.out',
       '--context',
-      `amplify-backend-type=sandbox`,
+      'amplify-backend-namespace=foo',
+      '--context',
+      'amplify-backend-name=bar',
+      '--context',
+      'amplify-backend-type=sandbox',
       '--hotswap-fallback',
       '--method=direct',
       '--context',
@@ -98,7 +93,7 @@ void describe('invokeCDKCommand', () => {
   });
 
   void it('handles options and deployProps for sandbox', async () => {
-    await invoker.deploy(backendId, sandboxDeployProps);
+    await invoker.deploy(sandboxBackendId, sandboxDeployProps);
     assert.strictEqual(execaMock.mock.callCount(), 1);
     assert.equal(execaMock.mock.calls[0].arguments[1]?.length, 18);
     assert.deepStrictEqual(execaMock.mock.calls[0].arguments[1], [
@@ -111,9 +106,9 @@ void describe('invokeCDKCommand', () => {
       '--output',
       '.amplify/artifacts/cdk.out',
       '--context',
-      `amplify-backend-namespace=123`,
+      `amplify-backend-namespace=foo`,
       '--context',
-      `amplify-backend-name=testBranch`,
+      `amplify-backend-name=bar`,
       '--context',
       `amplify-backend-type=sandbox`,
       '--hotswap-fallback',
@@ -126,9 +121,7 @@ void describe('invokeCDKCommand', () => {
   });
 
   void it('handles destroy for sandbox', async () => {
-    await invoker.destroy(backendId, {
-      deploymentType: 'sandbox',
-    });
+    await invoker.destroy(sandboxBackendId);
     assert.strictEqual(execaMock.mock.callCount(), 1);
     assert.equal(execaMock.mock.calls[0].arguments[1]?.length, 15);
     assert.deepStrictEqual(execaMock.mock.calls[0].arguments[1], [
@@ -141,18 +134,17 @@ void describe('invokeCDKCommand', () => {
       '--output',
       '.amplify/artifacts/cdk.out',
       '--context',
-      `amplify-backend-namespace=123`,
+      'amplify-backend-namespace=foo',
       '--context',
-      `amplify-backend-name=testBranch`,
+      'amplify-backend-name=bar',
       '--context',
-      `amplify-backend-type=sandbox`,
+      'amplify-backend-type=sandbox',
       '--force',
     ]);
   });
 
   void it('enables type checking for branch deployments', async () => {
-    await invoker.deploy(backendId, {
-      deploymentType: 'branch',
+    await invoker.deploy(branchBackendId, {
       validateAppSources: true,
     });
     assert.strictEqual(execaMock.mock.callCount(), 3);
@@ -193,8 +185,7 @@ void describe('invokeCDKCommand', () => {
   });
 
   void it('enables type checking for sandbox deployments', async () => {
-    await invoker.deploy(undefined, {
-      deploymentType: 'sandbox',
+    await invoker.deploy(sandboxBackendId, {
       validateAppSources: true,
     });
     assert.strictEqual(execaMock.mock.callCount(), 3);
@@ -213,7 +204,7 @@ void describe('invokeCDKCommand', () => {
       '--project',
       'amplify',
     ]);
-    assert.equal(execaMock.mock.calls[2].arguments[1]?.length, 12);
+    assert.equal(execaMock.mock.calls[2].arguments[1]?.length, 16);
     assert.deepStrictEqual(execaMock.mock.calls[2].arguments[1], [
       'cdk',
       'deploy',
@@ -224,7 +215,11 @@ void describe('invokeCDKCommand', () => {
       '--output',
       '.amplify/artifacts/cdk.out',
       '--context',
-      `amplify-backend-type=sandbox`,
+      'amplify-backend-namespace=foo',
+      '--context',
+      'amplify-backend-name=bar',
+      '--context',
+      'amplify-backend-type=sandbox',
       '--hotswap-fallback',
       '--method=direct',
     ]);
@@ -235,8 +230,7 @@ void describe('invokeCDKCommand', () => {
     execaMock.mock.mockImplementationOnce(() =>
       Promise.reject(new Error('some error'))
     );
-    await invoker.deploy(undefined, {
-      deploymentType: 'sandbox',
+    await invoker.deploy(branchBackendId, {
       validateAppSources: true,
     });
     assert.strictEqual(execaMock.mock.callCount(), 2);
@@ -247,7 +241,7 @@ void describe('invokeCDKCommand', () => {
       '--project',
       'amplify',
     ]);
-    assert.equal(execaMock.mock.calls[1].arguments[1]?.length, 12);
+    assert.equal(execaMock.mock.calls[1].arguments[1]?.length, 16);
     assert.deepStrictEqual(execaMock.mock.calls[1].arguments[1], [
       'cdk',
       'deploy',
@@ -258,9 +252,13 @@ void describe('invokeCDKCommand', () => {
       '--output',
       '.amplify/artifacts/cdk.out',
       '--context',
-      `amplify-backend-type=sandbox`,
-      '--hotswap-fallback',
-      '--method=direct',
+      'amplify-backend-namespace=123',
+      '--context',
+      'amplify-backend-name=testBranch',
+      '--require-approval',
+      'never',
+      '--context',
+      'amplify-backend-type=branch',
     ]);
   });
 
@@ -270,7 +268,7 @@ void describe('invokeCDKCommand', () => {
     });
 
     await assert.rejects(
-      () => invoker.deploy(backendId, sandboxDeployProps),
+      () => invoker.deploy(branchBackendId, sandboxDeployProps),
       (err: Error) => {
         assert.equal(
           err.message,

diff --git a/packages/backend-deployer/src/cdk_deployer.ts b/packages/backend-deployer/src/cdk_deployer.ts
@@ -5,11 +5,10 @@ import {
   BackendDeployer,
   DeployProps,
   DeployResult,
-  DestroyProps,
   DestroyResult,
 } from './cdk_deployer_singleton_factory.js';
 import { CdkErrorMapper } from './cdk_error_mapper.js';
-import { BackendIdentifier, DeploymentType } from '@aws-amplify/plugin-types';
+import { BackendIdentifier } from '@aws-amplify/plugin-types';
 import { BackendLocator, CDKContextKey } from '@aws-amplify/platform-core';
 import { dirname } from 'path';
 
@@ -35,42 +34,29 @@ export class CDKDeployer implements BackendDeployer {
   /**
    * Invokes cdk deploy command
    */
-  deploy = async (backendId?: BackendIdentifier, deployProps?: DeployProps) => {
+  deploy = async (backendId: BackendIdentifier, deployProps?: DeployProps) => {
     await this.invokeTsc(deployProps);
 
     const cdkCommandArgs: string[] = [];
-    if (deployProps?.deploymentType === 'sandbox') {
+    if (backendId.type === 'sandbox') {
       cdkCommandArgs.push('--hotswap-fallback');
       cdkCommandArgs.push('--method=direct');
-      if (deployProps.secretLastUpdated) {
+      if (deployProps?.secretLastUpdated) {
         cdkCommandArgs.push(
           '--context',
           `secretLastUpdated=${deployProps.secretLastUpdated.getTime()}`
         );
       }
     }
 
-    return this.invokeCdk(
-      InvokableCommand.DEPLOY,
-      backendId,
-      deployProps?.deploymentType,
-      cdkCommandArgs
-    );
+    return this.invokeCdk(InvokableCommand.DEPLOY, backendId, cdkCommandArgs);
   };
 
   /**
    * Invokes cdk destroy command
    */
-  destroy = async (
-    backendId?: BackendIdentifier,
-    destroyProps?: DestroyProps
-  ) => {
-    return this.invokeCdk(
-      InvokableCommand.DESTROY,
-      backendId,
-      destroyProps?.deploymentType,
-      ['--force']
-    );
+  destroy = async (backendId: BackendIdentifier) => {
+    return this.invokeCdk(InvokableCommand.DESTROY, backendId, ['--force']);
   };
 
   private invokeTsc = async (deployProps?: DeployProps) => {
@@ -107,8 +93,7 @@ export class CDKDeployer implements BackendDeployer {
    */
   private invokeCdk = async (
     invokableCommand: InvokableCommand,
-    backendId?: BackendIdentifier,
-    deploymentType?: DeploymentType,
+    backendId: BackendIdentifier,
     additionalArguments?: string[]
   ): Promise<DeployResult | DestroyResult> => {
     // Basic args
@@ -126,25 +111,21 @@ export class CDKDeployer implements BackendDeployer {
     ];
 
     // Add context information if available
-    if (backendId) {
-      cdkCommandArgs.push(
-        '--context',
-        `${CDKContextKey.BACKEND_NAMESPACE}=${backendId.namespace}`,
-        '--context',
-        `${CDKContextKey.BACKEND_NAME}=${backendId.name}`
-      );
+    cdkCommandArgs.push(
+      '--context',
+      `${CDKContextKey.BACKEND_NAMESPACE}=${backendId.namespace}`,
+      '--context',
+      `${CDKContextKey.BACKEND_NAME}=${backendId.name}`
+    );
 
-      if (deploymentType !== 'sandbox') {
-        cdkCommandArgs.push('--require-approval', 'never');
-      }
+    if (backendId.type !== 'sandbox') {
+      cdkCommandArgs.push('--require-approval', 'never');
     }
 
-    if (deploymentType) {
-      cdkCommandArgs.push(
-        '--context',
-        `${CDKContextKey.DEPLOYMENT_TYPE}=${deploymentType}`
-      );
-    }
+    cdkCommandArgs.push(
+      '--context',
+      `${CDKContextKey.DEPLOYMENT_TYPE}=${backendId.type}`
+    );
 
     if (additionalArguments) {
       cdkCommandArgs.push(...additionalArguments);