Support update hook for DummyTaskDefinition (resolves #140)

[cuperman]

Resolves issue: #140

Fix: Create an onUpdate hook that calls registerTaskDefinition, as it does on the onCreate hook. This causes lets CloudFormation know that the resource was replaced so that it can clean up the old resource. This resolves the issue by having the task definition attributes on updates so that Fn::GetAtt intrinsic function continues to work.

[cuperman]

I was worried that updates to the dummy task definition would cause problems with the service since all updates require replacement. The task definition / container / task is intended to be replaced by ones deployed with CodeDeploy, so I didn't want updates to the dummy task definition to interface and start running in the tasks. But I tried various combinations of updates, and couldn't cause anything bad to happen. Therefore, I decided to always replace on update. This is more conventional, and I don't think it will cause problems with the blue/green CodeDeployed tasks.

[hupe1980]

Could we just return the physical resource id in a dummy update handler? Isn't that enough?

Maybe:

const handleUpdate = (event) => {
   return {
      PhysicalResourceId: event.PhysicalResourceId, // taskdefArn
   }
}

[cuperman]

I know that calling describeTaskDefinition works, because it returns all the attributes, allowing the getattr functions to work. I can try if returning just the physical id would do the same, but it may not.

but what is the point of deliberately suppressing updates on the resource?

[cuperman]

also, this custom resource uses AwsCustomResource which only supports AwsSdkCalls, not generic javascript functions.

[cuperman]

This is the simplest way to fix the issue and keep the existing behavior where updates are suppressed:

const taskDefinition = new AwsCustomResource(this, 'DummyTaskDefinition', {
  resourceType: 'Custom::DummyTaskDefinition',
  onCreate: {
    service: 'ECS',
    action: 'registerTaskDefinition',
    parameters: {
      requiresCompatibilities: ['FARGATE'],
      family: this.family,
      executionRoleArn: this.executionRole.roleArn,
      networkMode: NetworkMode.AWS_VPC,
      cpu: '256',
      memory: '512',
      containerDefinitions: [
        {
          name: this.containerName,
          image: props.image,
          portMappings: [
            {
              hostPort: this.containerPort,
              protocol: 'tcp',
              containerPort: this.containerPort,
            },
          ],
        },
      ],
    },
    physicalResourceId: PhysicalResourceId.fromResponse('taskDefinition.taskDefinitionArn'),
  },
  onUpdate: {
    service: 'ECS',
    action: 'describeTaskDefinition',
    parameters: {
      taskDefinition: new PhysicalResourceIdReference(),
    },
    physicalResourceId: PhysicalResourceId.fromResponse('taskDefinition.taskDefinitionArn'),
  },
  onDelete: {
    service: 'ECS',
    action: 'deregisterTaskDefinition',
    parameters: {
      taskDefinition: new PhysicalResourceIdReference(),
    },
  },
  policy: AwsCustomResourcePolicy.fromStatements([
    new PolicyStatement({
      effect: Effect.ALLOW,
      actions: ['ecs:RegisterTaskDefinition', 'ecs:DescribeTaskDefinition', 'ecs:DeregisterTaskDefinition'],
      resources: ['*'],
    }),
    new PolicyStatement({
      effect: Effect.ALLOW,
      actions: ['iam:PassRole'],
      resources: [this.executionRole.roleArn],
    }),
  ]),
});

I did some testing and couldn't find a negative impact of actually updating the resource, so I submitted the code the updates/replaces the resource instead of suppressing updates. Let me know what you would prefer. I can resubmit with changes.

Check out the initial commit on this PR and see what you think about the allowUpdates property. Or we can keep things simple and make the change above.

[hupe1980]

You've convinced me. 😃 That's okay.