Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(codepipeline-actions): use IBaseService instead of BaseService in EcsDeployActionProps #6412

Merged
merged 18 commits into from
Mar 11, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface EcsDeployActionProps extends codepipeline.CommonAwsActionProps
/**
* The ECS Service to deploy.
*/
readonly service: ecs.BaseService;
readonly service: ecs.IBaseService;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ export = {

test.done();
},

'can be created just by serviceArn'(test: Test) {
const service = anyIService();
const artifact = new codepipeline.Artifact('Artifact');

test.doesNotThrow(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the test.doesNotThrow(), it's redundant.

new cpactions.EcsDeployAction({
actionName: 'ECS',
service,
imageFile: artifact.atPath('imageFile.json'),
});
});
skinny85 marked this conversation as resolved.
Show resolved Hide resolved

test.done();
},
},
};

Expand All @@ -98,3 +113,11 @@ function anyEcsService(): ecs.FargateService {
taskDefinition,
});
}

function anyIService(): ecs.IBaseService {
const stack = new cdk.Stack();
return ecs.FargateService.fromFargateServiceAttributes(stack, 'FargateService', {
serviceArn: 'arn:aws:ecs:us-west-2:123456789012:service/my-http-service',
clusterName: 'clusterName',
});
}
18 changes: 17 additions & 1 deletion packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ export interface IService extends IResource {
* @attribute
*/
readonly serviceArn: string;
/**
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
* The name of the service.
*
* @attribute
*/
readonly serviceName: string;
}

/**
Expand Down Expand Up @@ -245,11 +251,21 @@ class NetworkListenerConfig extends ListenerConfig {
}
}

/**
* The interface for BaseService.
*/
export interface IBaseService extends IService {
/**
* The cluster that hosts the service.
*/
readonly cluster: ICluster;
}

/**
* The base class for Ec2Service and FargateService services.
*/
export abstract class BaseService extends Resource
implements IService, elbv2.IApplicationLoadBalancerTarget, elbv2.INetworkLoadBalancerTarget, elb.ILoadBalancerTarget {
implements IBaseService, elbv2.IApplicationLoadBalancerTarget, elbv2.INetworkLoadBalancerTarget, elb.ILoadBalancerTarget {

/**
* The security groups which manage the allowed network traffic for the service.
Expand Down
35 changes: 34 additions & 1 deletion packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as ec2 from '@aws-cdk/aws-ec2';
import { Construct, Lazy, Resource, Stack } from '@aws-cdk/core';
import { BaseService, BaseServiceOptions, IService, LaunchType, PropagatedTagSource } from '../base/base-service';
import { BaseService, BaseServiceOptions, IBaseService, IService, LaunchType, PropagatedTagSource } from '../base/base-service';
import { NetworkMode, TaskDefinition } from '../base/task-definition';
import { Cluster } from '../cluster';
import { CfnService } from '../ecs.generated';
import { PlacementConstraint, PlacementStrategy } from '../placement';

Expand Down Expand Up @@ -87,6 +88,20 @@ export interface IEc2Service extends IService {

}

/**
* The properties to import from the service using the EC2 launch type.
*/
export interface Ec2ServiceAttributes {
/**
* The name of the cluster that hosts the service.
*/
readonly clusterName: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be ICluster. Not clusterName.

/**
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
* The service ARN.
*/
readonly serviceArn: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should also be serviceName, and one of serviceArn or serviceName is required.

Copy link
Contributor Author

@atsushi-ishibashi atsushi-ishibashi Feb 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added validation to prevent from setting both or neither. If both are set, it may be confusing.

}

/**
* This creates a service using the EC2 launch type on an ECS cluster.
*
Expand All @@ -100,6 +115,24 @@ export class Ec2Service extends BaseService implements IEc2Service {
public static fromEc2ServiceArn(scope: Construct, id: string, ec2ServiceArn: string): IEc2Service {
class Import extends Resource implements IEc2Service {
public readonly serviceArn = ec2ServiceArn;
public readonly serviceName = Stack.of(scope).parseArn(ec2ServiceArn).resourceName as string;
}
return new Import(scope, id);
}

/**
* Imports from the specified service attrributes.
*/
public static fromEc2ServiceAttributes(scope: Construct, id: string, attrs: Ec2ServiceAttributes): IBaseService {
const stack = Stack.of(scope);
const serviceName = stack.parseArn(attrs.serviceArn).resourceName as string;

class Import extends Resource implements IBaseService {
public readonly serviceArn = attrs.serviceArn;
public readonly serviceName = serviceName;
public readonly cluster = new Cluster(stack, serviceName, {
clusterName: attrs.clusterName,
});
}
return new Import(scope, id);
}
Expand Down
37 changes: 35 additions & 2 deletions packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import * as ec2 from '@aws-cdk/aws-ec2';
import * as cdk from '@aws-cdk/core';
import { BaseService, BaseServiceOptions, IService, LaunchType, PropagatedTagSource } from '../base/base-service';
import { BaseService, BaseServiceOptions, IBaseService, IService, LaunchType, PropagatedTagSource } from '../base/base-service';
import { TaskDefinition } from '../base/task-definition';
import { Cluster } from '../cluster';

/**
* The properties for defining a service using the Fargate launch type.
Expand Down Expand Up @@ -65,6 +66,20 @@ export interface IFargateService extends IService {

}

/**
* The properties to import from the service using the Fargate launch type.
*/
export interface FargateServiceAttributes {
/**
* The name of the cluster that hosts the service.
*/
readonly clusterName: string;
/**
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
* The service ARN.
*/
readonly serviceArn: string;
}

/**
* This creates a service using the Fargate launch type on an ECS cluster.
*
Expand All @@ -73,11 +88,29 @@ export interface IFargateService extends IService {
export class FargateService extends BaseService implements IFargateService {

/**
* Import a task definition from the specified task definition ARN.
* Imports from the specified service ARN.
*/
public static fromFargateServiceArn(scope: cdk.Construct, id: string, fargateServiceArn: string): IFargateService {
class Import extends cdk.Resource implements IFargateService {
public readonly serviceArn = fargateServiceArn;
public readonly serviceName = cdk.Stack.of(scope).parseArn(fargateServiceArn).resourceName as string;
}
return new Import(scope, id);
}

/**
* Imports from the specified service attrributes.
*/
public static fromFargateServiceAttributes(scope: cdk.Construct, id: string, attrs: FargateServiceAttributes): IBaseService {
const stack = cdk.Stack.of(scope);
const serviceName = stack.parseArn(attrs.serviceArn).resourceName as string;

class Import extends cdk.Resource implements IBaseService {
public readonly serviceArn = attrs.serviceArn;
public readonly serviceName = serviceName;
public readonly cluster = new Cluster(stack, serviceName, {
clusterName: attrs.clusterName,
});
}
return new Import(scope, id);
}
Expand Down