fix: Support IService for EcsDeployAction

[ahammond]

Accept the IService interface instead of requiring BaseService

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[mergify]

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[skinny85]

Thanks for the change @ahammond , but as you've probably seen, the situation is not as simple: IService does not have information about the cluster it's part of, which is something we need in this action.

I assume you want to use existing, imported services with this Action? A proper solution would probably require changes in the ECS module. If you're interested, we can talk about what needs to be done there :).

Thanks,
Adam

[ahammond]

@skinny85 yeah, I saw it failed. I'd like to take a shot at it with some coaching.

[skinny85]

So here's the deal.

As the code shows, to be able to deploy an ECS service with the EcsDeployAction, we need two pieces of data from the service: its name, and the name of the cluster it belongs to. That's easy when the passed service is a newly created one, but becomes problematic when we want to use an imported one.

Both methods of importing existing services in ECS, for EC2 and Fargate variants, return a very anemic interface, IService, which only contains the service's ARN, not even its name! But anyway, even if name was there, we would still be missing the cluster.

But, the existing import methods, fromEc2ServiceArn and fromFargateServiceArn, have no way of providing the cluster. That's why we need new to create new ones, fromEc2ServiceAttributes and fromFargateServiceAttributes. Their return type needs to be a new interface, let's call it IBaseService, that adds cluster to the properties:

export interface IBaseService extends IService {
  readonly cluster: ICluster;
}

(we can't make cluster optional, because we want BaseService to implement IBaseService, and JSII does not allow changing the type of an overridden property)

We should also extend IService to include readonly serviceName: string (it can be parsed from the ECS service ARN).

After those changes in ECS are made, all that's left in EcsDeployAction is to change the type from BaseService to IBaseService. I think all these changes should be done in one PR (it will make it more obvious why are they needed).

Let me know if this description makes sense @ahammond , and if this sounds like something you'd be interested in implementing!

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository