feat(cli): support hotswapping Lambda function tags (#17818)

offbyone · web-flow · commit e4485f4371dd · 2021-12-17T17:45:34.000Z
Fixes #17664 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
diff --git a/packages/aws-cdk/lib/api/hotswap/common.ts b/packages/aws-cdk/lib/api/hotswap/common.ts
@@ -76,7 +76,3 @@ export async function establishResourcePhysicalName(
   }
   return evaluateCfnTemplate.findPhysicalNameFor(logicalId);
 }
-
-export function assetMetadataChanged(change: HotswappableChangeCandidate): boolean {
-  return !!change.newValue?.Metadata['aws:asset:path'];
-}
diff --git a/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts b/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts
@@ -1,5 +1,5 @@
 import { ISDK } from '../aws-auth';
-import { assetMetadataChanged, ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, HotswappableChangeCandidate, establishResourcePhysicalName } from './common';
+import { ChangeHotswapImpact, ChangeHotswapResult, HotswapOperation, HotswappableChangeCandidate, establishResourcePhysicalName } from './common';
 import { EvaluateCloudFormationTemplate } from './evaluate-cloudformation-template';
 
 /**
@@ -15,22 +15,19 @@ export async function isHotswappableLambdaFunctionChange(
   if (typeof lambdaCodeChange === 'string') {
     return lambdaCodeChange;
   } else {
-    // verify that the Asset changed - otherwise,
-    // it's a Code property-only change,
-    // but not to an asset change
-    // (for example, going from Code.fromAsset() to Code.fromInline())
-    if (!assetMetadataChanged(change)) {
-      return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT;
-    }
-
     const functionName = await establishResourcePhysicalName(logicalId, change.newValue.Properties?.FunctionName, evaluateCfnTemplate);
     if (!functionName) {
       return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT;
     }
 
+    const functionArn = await evaluateCfnTemplate.evaluateCfnExpression({
+      'Fn::Sub': 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:' + functionName,
+    });
+
     return new LambdaFunctionHotswapOperation({
       physicalName: functionName,
-      code: lambdaCodeChange,
+      functionArn: functionArn,
+      resource: lambdaCodeChange,
     });
   }
 }
@@ -46,7 +43,7 @@ export async function isHotswappableLambdaFunctionChange(
  */
 async function isLambdaFunctionCodeOnlyChange(
   change: HotswappableChangeCandidate, evaluateCfnTemplate: EvaluateCloudFormationTemplate,
-): Promise<LambdaFunctionCode | ChangeHotswapImpact> {
+): Promise<LambdaFunctionChange | ChangeHotswapImpact> {
   const newResourceType = change.newValue.Type;
   if (newResourceType !== 'AWS::Lambda::Function') {
     return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT;
@@ -63,44 +60,95 @@ async function isLambdaFunctionCodeOnlyChange(
    * even if only one of them was actually changed,
    * which means we don't need the "old" values at all, and we can safely initialize these with just `''`.
    */
-  let s3Bucket = '', s3Key = '';
-  let foundCodeDifference = false;
   // Make sure only the code in the Lambda function changed
   const propertyUpdates = change.propertyUpdates;
+  let code: LambdaFunctionCode | undefined = undefined;
+  let tags: LambdaFunctionTags | undefined = undefined;
+
   for (const updatedPropName in propertyUpdates) {
     const updatedProp = propertyUpdates[updatedPropName];
-    for (const newPropName in updatedProp.newValue) {
-      switch (newPropName) {
-        case 'S3Bucket':
-          foundCodeDifference = true;
-          s3Bucket = await evaluateCfnTemplate.evaluateCfnExpression(updatedProp.newValue[newPropName]);
-          break;
-        case 'S3Key':
-          foundCodeDifference = true;
-          s3Key = await evaluateCfnTemplate.evaluateCfnExpression(updatedProp.newValue[newPropName]);
-          break;
-        default:
-          return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT;
-      }
+
+    switch (updatedPropName) {
+      case 'Code':
+        let foundCodeDifference = false;
+        let s3Bucket = '', s3Key = '';
+
+        for (const newPropName in updatedProp.newValue) {
+          switch (newPropName) {
+            case 'S3Bucket':
+              foundCodeDifference = true;
+              s3Bucket = await evaluateCfnTemplate.evaluateCfnExpression(updatedProp.newValue[newPropName]);
+              break;
+            case 'S3Key':
+              foundCodeDifference = true;
+              s3Key = await evaluateCfnTemplate.evaluateCfnExpression(updatedProp.newValue[newPropName]);
+              break;
+            default:
+              return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT;
+          }
+        }
+        if (foundCodeDifference) {
+          code = {
+            s3Bucket,
+            s3Key,
+          };
+        }
+        break;
+      case 'Tags':
+        /*
+         * Tag updates are a bit odd; they manifest as two lists, are flagged only as
+         * `isDifferent`, and we have to reconcile them.
+         */
+        const tagUpdates: { [tag: string]: string | TagDeletion } = {};
+        if (updatedProp?.isDifferent) {
+          updatedProp.newValue.forEach((tag: CfnDiffTagValue) => {
+            tagUpdates[tag.Key] = tag.Value;
+          });
+
+          updatedProp.oldValue.forEach((tag: CfnDiffTagValue) => {
+            if (tagUpdates[tag.Key] === undefined) {
+              tagUpdates[tag.Key] = TagDeletion.DELETE;
+            }
+          });
+
+          tags = { tagUpdates };
+        }
+        break;
+      default:
+        return ChangeHotswapImpact.REQUIRES_FULL_DEPLOYMENT;
     }
   }
 
-  return foundCodeDifference
-    ? {
-      s3Bucket,
-      s3Key,
-    }
-    : ChangeHotswapImpact.IRRELEVANT;
+  return code || tags ? { code, tags } : ChangeHotswapImpact.IRRELEVANT;
+}
+
+interface CfnDiffTagValue {
+  readonly Key: string;
+  readonly Value: string;
 }
 
 interface LambdaFunctionCode {
   readonly s3Bucket: string;
   readonly s3Key: string;
 }
 
+enum TagDeletion {
+  DELETE = -1,
+}
+
+interface LambdaFunctionTags {
+  readonly tagUpdates: { [tag : string] : string | TagDeletion };
+}
+
+interface LambdaFunctionChange {
+  readonly code?: LambdaFunctionCode;
+  readonly tags?: LambdaFunctionTags;
+}
+
 interface LambdaFunctionResource {
   readonly physicalName: string;
-  readonly code: LambdaFunctionCode;
+  readonly functionArn: string;
+  readonly resource: LambdaFunctionChange;
 }
 
 class LambdaFunctionHotswapOperation implements HotswapOperation {
@@ -110,10 +158,47 @@ class LambdaFunctionHotswapOperation implements HotswapOperation {
   }
 
   public async apply(sdk: ISDK): Promise<any> {
-    return sdk.lambda().updateFunctionCode({
-      FunctionName: this.lambdaFunctionResource.physicalName,
-      S3Bucket: this.lambdaFunctionResource.code.s3Bucket,
-      S3Key: this.lambdaFunctionResource.code.s3Key,
-    }).promise();
+    const lambda = sdk.lambda();
+    const resource = this.lambdaFunctionResource.resource;
+    const operations: Promise<any>[] = [];
+
+    if (resource.code !== undefined) {
+      operations.push(lambda.updateFunctionCode({
+        FunctionName: this.lambdaFunctionResource.physicalName,
+        S3Bucket: resource.code.s3Bucket,
+        S3Key: resource.code.s3Key,
+      }).promise());
+    }
+
+    if (resource.tags !== undefined) {
+      const tagsToDelete: string[] = Object.entries(resource.tags.tagUpdates)
+        .filter(([_key, val]) => val === TagDeletion.DELETE)
+        .map(([key, _val]) => key);
+
+      const tagsToSet: { [tag: string]: string } = {};
+      Object.entries(resource.tags!.tagUpdates)
+        .filter(([_key, val]) => val !== TagDeletion.DELETE)
+        .forEach(([tagName, tagValue]) => {
+          tagsToSet[tagName] = tagValue as string;
+        });
+
+
+      if (tagsToDelete.length > 0) {
+        operations.push(lambda.untagResource({
+          Resource: this.lambdaFunctionResource.functionArn,
+          TagKeys: tagsToDelete,
+        }).promise());
+      }
+
+      if (Object.keys(tagsToSet).length > 0) {
+        operations.push(lambda.tagResource({
+          Resource: this.lambdaFunctionResource.functionArn,
+          Tags: tagsToSet,
+        }).promise());
+      }
+    }
+
+    // run all of our updates in parallel
+    return Promise.all(operations);
   }
 }
diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts
@@ -11,7 +11,7 @@ beforeEach(() => {
   mockUpdateLambdaCode = jest.fn();
   mockUpdateMachineDefinition = jest.fn();
   mockGetEndpointSuffix = jest.fn(() => 'amazonaws.com');
-  hotswapMockSdkProvider.setUpdateFunctionCodeMock(mockUpdateLambdaCode);
+  hotswapMockSdkProvider.stubLambda(mockUpdateLambdaCode);
   hotswapMockSdkProvider.setUpdateStateMachineMock(mockUpdateMachineDefinition);
   hotswapMockSdkProvider.stubGetEndpointSuffix(mockGetEndpointSuffix);
 });
diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts b/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts
@@ -82,9 +82,15 @@ export class HotswapMockSdkProvider {
     });
   }
 
-  public setUpdateFunctionCodeMock(mockUpdateLambdaCode: (input: lambda.UpdateFunctionCodeRequest) => lambda.FunctionConfiguration) {
+  public stubLambda(
+    mockUpdateLambdaCode: (input: lambda.UpdateFunctionCodeRequest) => lambda.FunctionConfiguration,
+    mockTagResource?: (input: lambda.TagResourceRequest) => {},
+    mockUntagResource?: (input: lambda.UntagResourceRequest) => {},
+  ) {
     this.mockSdkProvider.stubLambda({
-      updateFunctionCode: mockUpdateLambdaCode,
+      updateFunctionCode: mockUpdateLambdaCode ?? jest.fn(),
+      tagResource: mockTagResource ?? jest.fn(),
+      untagResource: mockUntagResource ?? jest.fn(),
     });
   }
 
diff --git a/packages/aws-cdk/test/api/hotswap/lambda-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/lambda-hotswap-deployments.test.ts
@@ -2,12 +2,16 @@ import { Lambda } from 'aws-sdk';
 import * as setup from './hotswap-test-setup';
 
 let mockUpdateLambdaCode: (params: Lambda.Types.UpdateFunctionCodeRequest) => Lambda.Types.FunctionConfiguration;
+let mockTagResource: (params: Lambda.Types.TagResourceRequest) => {};
+let mockUntagResource: (params: Lambda.Types.UntagResourceRequest) => {};
 let hotswapMockSdkProvider: setup.HotswapMockSdkProvider;
 
 beforeEach(() => {
   hotswapMockSdkProvider = setup.setupHotswapTests();
   mockUpdateLambdaCode = jest.fn();
-  hotswapMockSdkProvider.setUpdateFunctionCodeMock(mockUpdateLambdaCode);
+  mockTagResource = jest.fn();
+  mockUntagResource = jest.fn();
+  hotswapMockSdkProvider.stubLambda(mockUpdateLambdaCode, mockTagResource, mockUntagResource);
 });
 
 test('returns undefined when a new Lambda function is added to the Stack', async () => {
@@ -80,6 +84,89 @@ test('calls the updateLambdaCode() API when it receives only a code difference i
   });
 });
 
+test('calls the tagResource() API when it receives only a tag difference in a Lambda function', async () => {
+  // GIVEN
+  const currentTemplate = {
+    Resources: {
+      Func: {
+        Type: 'AWS::Lambda::Function',
+        Properties: {
+          Code: {
+            S3Bucket: 'current-bucket',
+            S3Key: 'current-key',
+          },
+          FunctionName: 'my-function',
+          Tags: [
+            {
+              Key: 'to-be-deleted',
+              Value: 'a-value',
+            },
+            {
+              Key: 'to-be-changed',
+              Value: 'current-tag-value',
+            },
+          ],
+        },
+        Metadata: {
+          'aws:asset:path': 'old-path',
+        },
+      },
+    },
+  };
+
+  setup.setCurrentCfnStackTemplate(currentTemplate);
+  const cdkStackArtifact = setup.cdkStackArtifactOf({
+    template: {
+      Resources: {
+        Func: {
+          Type: 'AWS::Lambda::Function',
+          Properties: {
+            Code: {
+              S3Bucket: 'current-bucket',
+              S3Key: 'current-key',
+            },
+            FunctionName: 'my-function',
+            Tags: [
+              {
+                Key: 'to-be-changed',
+                Value: 'new-tag-value',
+              },
+              {
+                Key: 'to-be-added',
+                Value: 'added-tag-value',
+              },
+            ],
+          },
+          Metadata: {
+            'aws:asset:path': 'old-path',
+          },
+        },
+      },
+    },
+  });
+
+  // WHEN
+  const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact);
+
+  // THEN
+  expect(deployStackResult).not.toBeUndefined();
+
+  expect(mockUntagResource).toHaveBeenCalledWith({
+    Resource: 'arn:aws:lambda:here:123456789012:function:my-function',
+    TagKeys: ['to-be-deleted'],
+  });
+
+  expect(mockTagResource).toHaveBeenCalledWith({
+    Resource: 'arn:aws:lambda:here:123456789012:function:my-function',
+    Tags: {
+      'to-be-changed': 'new-tag-value',
+      'to-be-added': 'added-tag-value',
+    },
+  });
+
+  expect(mockUpdateLambdaCode).not.toHaveBeenCalled();
+});
+
 test("correctly evaluates the function's name when it references a different resource from the template", async () => {
   // GIVEN
   setup.setCurrentCfnStackTemplate({

Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,3 @@ export async function establishResourcePhysicalName(`
`76`	`76`	`}`
`77`	`77`	`return evaluateCfnTemplate.findPhysicalNameFor(logicalId);`
`78`	`78`	`}`
`79`		`-`
`80`		`-export function assetMetadataChanged(change: HotswappableChangeCandidate): boolean {`
`81`		`- return !!change.newValue?.Metadata['aws:asset:path'];`
`82`		`-}`